-
Notifications
You must be signed in to change notification settings - Fork 6
Additional checks for reviewer setup before allowing activation (PM-3… #55
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
Conversation
| RESOURCE_ROLES_API_URL: | ||
| process.env.RESOURCE_ROLES_API_URL || "http://api.topcoder-dev.com/v5/resource-roles", | ||
| GROUPS_API_URL: process.env.GROUPS_API_URL || "http://localhost:4000/v5/groups", | ||
| GROUPS_API_URL: process.env.GROUPS_API_URL || "http://localhost:4000/v6/groups", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[❗❗ correctness]
The change from /v5/groups to /v6/groups in the GROUPS_API_URL suggests an API version update. Ensure that the new version is backward compatible or that all necessary changes are made to accommodate any breaking changes in the new API version.
|
|
||
| describe('update challenge tests', () => { | ||
| const ensureSkipTimelineTemplate = async () => { | ||
| const templateId = config.SKIP_PROJECT_ID_BY_TIMLINE_TEMPLATE_ID |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[correctness]
The variable templateId is assigned using config.SKIP_PROJECT_ID_BY_TIMLINE_TEMPLATE_ID. Ensure that this configuration value is correctly set and validated before use to prevent potential runtime errors.
| const existingPhases = await prisma.timelineTemplatePhase.findMany({ | ||
| where: { timelineTemplateId: templateId } | ||
| }) | ||
| const existingPhaseIds = new Set(existingPhases.map(p => p.phaseId)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[correctness]
The use of new Set(existingPhases.map(p => p.phaseId)) is efficient for checking existence, but ensure that data.phase.id and data.phase2.id are defined and valid before this check to avoid potential errors.
| } | ||
|
|
||
| const createChallengeWithRequiredReviewPhases = async () => { | ||
| await ensureSkipTimelineTemplate() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[correctness]
The function createChallengeWithRequiredReviewPhases assumes that data.challenge.typeId and data.challenge.trackId are available. Ensure these values are properly initialized and validated to prevent potential issues.
| } | ||
| } | ||
|
|
||
| const createActivationChallenge = async (status = ChallengeStatusEnum.NEW) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[correctness]
The function createActivationChallenge uses data.challenge.typeId and data.challenge.trackId. Ensure these values are initialized and validated to prevent potential issues.
| status: ChallengeStatusEnum.ACTIVE | ||
| }) | ||
| } catch (e) { | ||
| should.equal(e.message.indexOf('reviewer configured') >= 0, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[maintainability]
The error message check e.message.indexOf('reviewer configured') >= 0 is fragile and could break if the error message changes. Consider using a more robust error handling strategy.
| } | ||
| ) | ||
| } catch (e) { | ||
| should.equal(e.message.indexOf('missing reviewers for phase(s): Review') >= 0, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[maintainability]
The error message check e.message.indexOf('missing reviewers for phase(s): Review') >= 0 is fragile and could break if the error message changes. Consider using a more robust error handling strategy.
PM-3062