Skip to content

Conversation

@jmgasper
Copy link
Contributor

…dates

@jmgasper jmgasper merged commit 900e2da into develop Nov 11, 2025
4 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.

[💡 maintainability]
Consider adding a comment or documentation explaining the purpose and order of SUBMISSION_PHASE_PRIORITY. This will help future developers understand why these specific phases are prioritized and in this order.

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.

[⚠️ correctness]
The logic for finding the submissionPhase relies on the order of SUBMISSION_PHASE_PRIORITY. If additional submission phases are added in the future, ensure they are added to SUBMISSION_PHASE_PRIORITY in the correct order. Consider adding a test case to verify the behavior when multiple submission phases are present.


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.

[⚠️ performance]
The logic for finding the submissionPhase could be optimized by using a single _.find call with a condition that checks for both phase names in the priority order. This would avoid iterating over the phases array twice.

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