Skip to content

Conversation

@kkartunov
Copy link
Contributor

…k, and MM only once, to avoid duplicate processing

…k, and MM only once, to avoid duplicate processing
@kkartunov kkartunov requested a review from jmgasper January 13, 2026 07:11
@@ -0,0 +1,2 @@
-- Add eventRaised flag to submission to prevent duplicate event publishing
ALTER TABLE "submission" ADD COLUMN "eventRaised" BOOLEAN NOT NULL DEFAULT false;

Choose a reason for hiding this comment

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

[⚠️ performance]
Consider adding an index on the eventRaised column if this column will be frequently queried for filtering purposes. This can improve query performance.

private async publishFirst2FinishEvent(
submission: SubmissionRecord,
): Promise<void> {
if (submission.eventRaised) {

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The check for submission.eventRaised is duplicated across multiple methods (publishFirst2FinishEvent, publishTopgearTaskEvent, publishMarathonMatchEvent). Consider refactoring this logic into a separate method to improve maintainability and reduce code duplication.

return (typeName ?? '').trim().toLowerCase() === 'marathon match';
}

private async markEventRaised(

Choose a reason for hiding this comment

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

[⚠️ correctness]
The markEventRaised method updates the eventRaised flag in the database. Ensure that this operation is idempotent and that the database state is consistent even if this method is called multiple times for the same submission. Consider adding a check to see if the flag is already set before attempting an update.

@kkartunov
Copy link
Contributor Author

@jmgasper can you check if this is ready for promote and if yes deploy it?

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.

3 participants