Skip to content

Conversation

@jmgasper
Copy link
Contributor

@jmgasper jmgasper merged commit be54289 into master Nov 11, 2025
8 checks passed
const { ensureAcessibilityToModifiedGroups } = require("./group-helper");
const { ChallengeStatusEnum } = require("@prisma/client");

const SUBMISSION_PHASE_PRIORITY = ["Topcoder Submission", "Submission"];

Choose a reason for hiding this comment

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

[⚠️ performance]
Consider using a Set for SUBMISSION_PHASE_PRIORITY to improve lookup performance when checking phase names. This change can enhance performance, especially if the list grows or is checked frequently.

const registrationPhase = _.find(challenge.phases, (p) => p.name === "Registration");
const submissionPhase = _.find(challenge.phases, (p) => p.name === "Submission");
const submissionPhase =
_.find(challenge.phases, (p) => p.name === SUBMISSION_PHASE_PRIORITY[0]) ||

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The current logic assumes that only the first two elements in SUBMISSION_PHASE_PRIORITY will be checked. If more submission types are added in the future, this logic will need to be updated. Consider iterating over SUBMISSION_PHASE_PRIORITY to make the code more maintainable and adaptable to future changes.


const registrationPhase = _.find(phases, (p) => p.name === "Registration");
const submissionPhase =
_.find(phases, (p) => p.name === SUBMISSION_PHASE_PRIORITY[0]) ||

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The logic for finding the submissionPhase relies on the order of SUBMISSION_PHASE_PRIORITY. If more submission types are added in the future, this logic will need to be updated. Consider using a more flexible approach, such as iterating through SUBMISSION_PHASE_PRIORITY and finding the first match.

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.

2 participants