Handle pre-contest clarifications #3174

vmcj · 2025-10-26T18:02:40Z

I looked into if we can let both API & UI get the data via a service (to remove having to handle everything twice) but that would require a lot of rewrites which need a bit of further thought.

The first 2 commits are unrelated, I'll open another PR for those but they broke my local build.

sentry-io · 2025-10-26T18:02:57Z

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: webapp/src/Controller/API/ClarificationController.php

Function	Unhandled Issue
`App\Controller\API\ClarificationController::addAction`	Symfony\Component\HttpKernel\Exception\BadRequestHttpException: Problem 'A' not found. /src/Controller/API/ClarificationController.php in App\Controller... `Event Count:` 3

meisterT · 2025-10-26T19:05:21Z

I would only do the warning, but include implications for the API in the message.

vmcj · 2025-10-26T19:30:52Z

I would only do the warning, but include implications for the API in the message.

So drop all other commits? I personally think keeping the UI/API uniform is an improvement.

There are good reasons to already send the clarification, so we don't disable the feature but warn the user.

In the Jury UI we can warn with a flash message, for API we can't easily prevent automated tools from unintentionally disclosing the information. We provide both relevant times, either contest start or now to keep this easy to copy/paste. The format is already in the timezone of the contest to make sure validation passes. In theory this can still give problems in case you submit a clarification and afterwards the contest starts later, it's up to the contest admin to change the database in such cases as we don't have a reltime for clarifications.

We already disclose those in the API, so keep this consistent. This does result in the `contest time` of that clarification to be negative but thats factually true.

We allow via the API, this just makes the behaviour uniform.

It looks ugly in some cases and less professional in others where printing happened much earlier. The exact time is not relevant and could lead to discussions with teams.

…start

We do for the jury so they can still see the interface as a time would see it.

meisterT · 2025-10-27T19:14:34Z

webapp/src/Controller/Jury/ClarificationController.php

+        $cc = $this->dj->getCurrentContest();
+        $message = "Generic clarifications are visible before contest start.";
+        if ($cc && $cc->getStartTime() > Utils::now()) {
+            $this->addFlash('warning', $message);


Does this flash message show on the current page visit or the next one?

You're loading the request for the page, in gathering all data for the request you add the flash. When we render the twig template there is a function which collects all flash messages and displays them.

So the answer would be "The next rendered twig template" which if you only have 1 tab open is the current page (there is an issue open where it displays on the wrong page).

If you mean that we would only provide the flash when the clarification already happened, no, not that I'm aware of.

meisterT · 2025-10-27T19:17:41Z

webapp/src/Controller/Jury/ClarificationController.php

+    private function warnClarificationBeforeContestStart(): void
+    {
+        $cc = $this->dj->getCurrentContest();
+        $message = "Generic clarifications are visible before contest start.";


not just generic, right?

In the later commits I don't disclose clarifications about problems, I know you guys had opinions on this but I think we should try the referential integrity. If we don't like the extra code path for maintainability we can discuss it now and change this message.

meisterT · 2025-10-27T19:18:09Z

webapp/src/Controller/Jury/ClarificationController.php

        #[MapQueryParameter]
        ?string $teamto = null,
    ): Response {
+        $this->warnClarificationBeforeContestStart();


in addition, I was thinking whether we should add a modal dialog on submission warning of the implications

I don't like this for 2 reasons... modals are more work and in case you know what you're doing this feels a little patronizing. You have more experience with judges so if you think it's smarter I can take a look how much work it is.

meisterT · 2025-10-27T19:21:35Z

webapp/templates/team/partials/index_content.html.twig

    <h2 id="contestnotstarted" class="text-center">
        Contest {{ contest | printContestStart }}
    </h2>
+    {% if clarifications is not empty %}


Thinking a little bit more: when do we send out "static events" in the event feed? Do we do that on state changes? Then hiding them in the API is perhaps not too much code complexity, but one would have to implement it to really see.

I don't think I understand.

meisterT · 2025-10-27T19:22:57Z

webapp/templates/team/partials/clarifications_team.html.twig

I would rather reject clar requests before contest start through the API

So instead of letting teams ask them via UI (because they can via API) disallow both via UI & API?

I think I'm fine with that, I want the jury to still be able to send them via API because of imports and suspected we didn't want the extra branch. I did not really like having to add this to the UI but found consistency important.

meisterT · 2025-10-27T19:24:14Z

webapp/src/Twig/TwigExtension.php

        if ($contest !== null && $this->config->get('show_relative_time')) {
            $relativeTime = $contest->getContestTime((float)$datetime);
+            if ($relativeTime < 0 && $squash) {
+                return "Before contest";


wonder if this makes sense in general and not just here?

This should just display it in the table, I didn't find other locations yet (also didn't really search) but in general I agree. Probably best to move this commit out and do this in a separate PR to find the other locations.

meisterT · 2025-10-27T19:25:27Z

I would only do the warning, but include implications for the API in the message.

So drop all other commits? I personally think keeping the UI/API uniform is an improvement.

I misunderstood what was happening, and left now individual comments on the code

vmcj added 7 commits October 27, 2025 13:34

Warn jury for clarifications before contest start

0071234

There are good reasons to already send the clarification, so we don't disable the feature but warn the user.

Display pre-contest clarifications

4706e75

We already disclose those in the API, so keep this consistent. This does result in the `contest time` of that clarification to be negative but thats factually true.

Allow teams to request clarifications before the contest

1f77c9a

We allow via the API, this just makes the behaviour uniform.

Don't display clarification time from before contest

8342cf2

It looks ugly in some cases and less professional in others where printing happened much earlier. The exact time is not relevant and could lead to discussions with teams.

Don't reference undisclosed problems in clarifications before contest…

c8c97ec

…start

Don't display clarifications about non-disclosed problems yet

3a60f88

We do for the jury so they can still see the interface as a time would see it.

vmcj force-pushed the jury_clarification_warning branch from 9b278b2 to 3a60f88 Compare October 27, 2025 12:36

meisterT reviewed Oct 27, 2025

View reviewed changes

Uh oh!

Handle pre-contest clarifications #3174

Are you sure you want to change the base?

Handle pre-contest clarifications #3174

Conversation

vmcj commented Oct 26, 2025

Uh oh!

sentry-io bot commented Oct 26, 2025

🔍 Existing Issues For Review

Uh oh!

meisterT commented Oct 26, 2025

Uh oh!

vmcj commented Oct 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

meisterT commented Oct 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants