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.

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

Change all clarifications in thread #2224

Merged
merged 1 commit into from
Nov 24, 2023
Merged

Conversation

vmcj
Copy link
Member

@vmcj vmcj commented Nov 23, 2023

By always finding the first in thread and changing the replies afterwards. Verified that this also works via the overview clarification page.

Partial fix for: #1785

@meisterT meisterT requested a review from thijskh November 23, 2023 09:36
@vmcj vmcj force-pushed the clarification_message branch from 1897124 to 5652cb7 Compare November 24, 2023 10:54
@@ -352,7 +352,13 @@ public function changeQueueAction(Request $request, int $clarId): Response
if ($queue === "") {
$queue = null;
}
$clarification->setQueue($queue);

// Either this is the first in a thread (of 1 or more) or this 2nd or more.
Copy link
Member

Choose a reason for hiding this comment

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

This comment is not really clear to me: or this 2nd or more.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the comment as Nicky his fix made this easy enough to read in code.

…replies

By always finding the first clarification and changing the replies
afterwards. Verified that this also works via the overview clarification
page.

In the UI this looks like a thread but this is not true in the database.
A team clarification will always be the first clarification, in case the jury
sends a clarification and the team responds this will be a new clarification (but quoted
with the earlier messages)

Verified nows also that when a team sends a clarification and jury responds and team responds
that this gives 3 clarifications where 2 are connected (team->jury)(->team).

Co-authored-by: Nicky Gerritsen <nickygerritsen@me.com>
@vmcj vmcj force-pushed the clarification_message branch from 5652cb7 to f81f88c Compare November 24, 2023 12:36
@vmcj vmcj added this pull request to the merge queue Nov 24, 2023
Merged via the queue into DOMjudge:main with commit aa4f8ba Nov 24, 2023
18 checks passed
@vmcj vmcj deleted the clarification_message branch November 24, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants