search

DOMjudge / domjudge

DOMjudge programming contest jury system
https://www.domjudge.org
GNU General Public License v2.0
739 stars 259 forks source link

Add a warning log when clarifications are claimed #2766

Closed as6325400 closed 5 days ago

as6325400 commented 1 month ago

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

issue

https://github.com/DOMjudge/domjudge/issues/2481

display

379348192-4b890df0-8ad7-49a7-8dfc-d47e2131f825

meisterT commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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.