Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a warning log when clarifications are claimed #2766

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add a warning log when clarifications are claimed #2766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions webapp/src/Controller/Jury/ClarificationController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,25 @@ public function changeQueueAction(Request $request, int $clarId): Response
return $this->redirectToRoute('jury_clarification', ['id' => $clarId]);
}

#[Route('/check-claimed', name: 'check_if_claimed', methods: ['GET'])]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[Route('/check-claimed', name: 'check_if_claimed', methods: ['GET'])]
#[Route('/check-claimed', name: 'check_if_claimed')]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding another endpoint here, let's just re-use the existing API endpoint for clarifications.

It currently doesn't have the info on whether a clarification has been claimed, but that can be added easily.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we do this, we also have the info on whether it has been answered before and can use that as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which API should this be added?

Copy link
Member

@meisterT meisterT Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am talking about the default API clarification controller:

$ http http://localhost/domjudge/api/contests/demo/clarifications/1
HTTP/1.1 200 OK
Cache-Control: max-age=0, must-revalidate, private
Connection: Keep-Alive
Content-Security-Policy: default-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; img-src 'self' data:
Content-Type: application/json
Date: Fri, 01 Nov 2024 17:01:08 GMT
Expires: Fri, 01 Nov 2024 17:01:08 GMT
Keep-Alive: timeout=5, max=100
Referrer-Policy: same-origin
Server: Apache/2.4.62 (Debian)
Transfer-Encoding: chunked
X-Content-Type-Options: nosniff
X-Current-Contest: 
X-Debug-Token: 55677e
X-Debug-Token-Link: http://localhost/domjudge/_profiler/55677e
X-Frame-Options: DENY
X-Robots-Tag: noindex
X-XSS-Protection: 1; mode=block

{
    "answered": false,
    "clarid": 1,
    "contest_time": "233:54:09.605",
    "from_team_id": "domjudge",
    "id": "1",
    "problem_id": "boolfind",
    "reply_to_id": null,
    "text": "fsdfsdfdf",
    "time": "2024-11-01T10:47:16.605+00:00",
    "to_team_id": null
}

As you can see, it doesn't output jury_member as of now. The implementation of it is really simple: https://github.com/DOMjudge/domjudge/blob/main/webapp/src/Controller/API/ClarificationController.php#L81

That means all the logic is in the entity definition. If we check there, you can see the following: https://github.com/DOMjudge/domjudge/blob/main/webapp/src/Entity/Clarification.php#L67

This means it is excluded from serialization in the API. We can change it to

 #[Serializer\Groups([ARC::GROUP_RESTRICTED_NONSTRICT])]

and it should appear.

public function checkIfClaimed(Request $request): Response
{
$clarid = $request->query->get('clarid');
$currentUserName = $this->getUser()->getUserIdentifier();

$queryBuilder = $this->em->createQueryBuilder()
->select('clar.jury_member')
->from(Clarification::class, 'clar')
->where('clar.clarid = :clarid')
->setParameter('clarid', $clarid);
$claimedJuryMember = $queryBuilder->getQuery()->getSingleResult();

return $this->json([
'isClaimedByOther' => $claimedJuryMember['jury_member'] !== null && $claimedJuryMember['jury_member'] !== $currentUserName,
'ClaimedBy' => $claimedJuryMember['jury_member'],
]);
}

protected function processSubmittedClarification(
FormInterface $form,
?Clarification $inReplTo = null
Expand Down
27 changes: 27 additions & 0 deletions webapp/templates/jury/partials/clarification_form.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,30 @@

</div>
{{ form_end(form) }}

<script>
$(function () {
var $body = $('body');

$body.on('submit', 'form[name=jury_clarification]', function () {
var clarid = {{ origclar | json_encode(constant('JSON_HEX_TAG')) | raw }}.clarid;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also work, doesn't it?

Suggested change
var clarid = {{ origclar | json_encode(constant('JSON_HEX_TAG')) | raw }}.clarid;
var clarid = {{ origclar.clarid }};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give it a try. I remember it didn’t work initially, which is why I wrote it this way.


fetch(`/jury/clarifications/check-claimed?clarid=${clarid}`, {
method: 'GET',
headers: {
'Content-Type': 'application/json',
}
})
.then(response => response.json())
.then(data => {
if (data.isClaimedByOther === true) {
confirm(`This clarification has been claimed by ${data.ClaimedBy}. Are you sure you want to send this message?`);
}
})
.catch(error => {
console.error('Error:', error);
alert('An error occurred while checking claim status.');
});
});
});
</script>
Loading