Add a warning log when clarifications are claimed

[as6325400]

I accidentally closed the PR. I'll open a new one.

issue

#2481

display

[meisterT]

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.

[meisterT]

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

[as6325400]

In which API should this be added?

[meisterT]

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.

[meisterT]

This should also work, doesn't it?

Suggested change

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

[as6325400]

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

[vmcj]

Suggested change

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

[Kevinjil]

Like with the previous PR, this front-end check does not properly cover concurrent users. The check for clarification claim should happen in the same request as the submission of a clarification.

I think the best way to handle it, would be to ask for an additional form submission with confirmation if the clarification has been claimed in while typing the message.

[meisterT]

Like with the previous PR, this front-end check does not properly cover concurrent users. The check for clarification claim should happen in the same request as the submission of a clarification.

I think the best way to handle it, would be to ask for an additional form submission with confirmation if the clarification has been claimed in while typing the message.

It does happen not happen in the "same request" but on clicking submit: $body.on('submit', 'form[name=jury_clarification]'...

[Kevinjil]

It does happen not happen in the "same request" but on clicking submit: $body.on('submit', 'form[name=jury_clarification]'...

That is exactly the problem I point out, concurrent jury members handling clarifications can have interleaved requests and double replies.

[meisterT]

If one of the jury members uses claim, then this should not happen in practice.

Also, we likely don't want to forbid double posting but make it intentional.

[Kevinjil]

If one of the jury members uses claim, then this should not happen in practice.

I prefer to stay away from "should not happen in practice". My solution does not strictly guarantee it as well, but is as close as you can get with minimal changes. Also removing the need of an additional API endpoint is worth it IMHO.

Also, we likely don't want to forbid double posting but make it intentional.

Hence my suggestion to force a form resubmit, thus keeping form contents and adding an explicit warning of the claim in the meantime and asking additional confirmation for the double posting by resubmission of the form.

[meisterT]

If one of the jury members uses claim, then this should not happen in practice.

I prefer to stay away from "should not happen in practice". My solution does not strictly guarantee it as well, but is as close as you can get with minimal changes. Also removing the need of an additional API endpoint is worth it IMHO.

See my comment above which does make the additional API endpoint unnecessary by using the existing one.

Also, we likely don't want to forbid double posting but make it intentional.

Hence my suggestion to force a form resubmit, thus keeping form contents and adding an explicit warning of the claim in the meantime and asking additional confirmation for the double posting by resubmission of the form.

I see, that works too, although it's a little bit more complex.