Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9279a19
Performance indices
jmgasper Oct 23, 2025
2449c2e
Expose phaseId for defaultReviewer
vas3a Oct 23, 2025
d269ee5
Merge pull request #24 from topcoder-platform/default-reviewers-phaseid
vas3a Oct 23, 2025
aa77c21
expose fixedAmount
vas3a Oct 23, 2025
a215324
Merge pull request #25 from topcoder-platform/expose-fixedamount
vas3a Oct 23, 2025
80170e9
Review guard when changing scorecards, and make sure we don't wipe te…
jmgasper Oct 24, 2025
3f96faa
Enrich skills data for PUT / PATCH response and add blocks on review …
jmgasper Oct 24, 2025
c003e03
Minor tweak for review status checking.
jmgasper Oct 24, 2025
41b0ca8
Add check before manually closing an Appeals Response phase to make s…
jmgasper Oct 24, 2025
362af3f
fix: added timeout for prisma client
hentrymartin Oct 27, 2025
21e760d
fix: added timeout for prisma client
hentrymartin Oct 27, 2025
6bd3690
Add Trivy scanner workflow for vulnerability scanning
kkartunov Oct 28, 2025
ec0656f
Sort prizes
rishabhtc Oct 28, 2025
76506ca
Merge pull request #27 from topcoder-platform/PM-2436-sort-prizes
rishabhtc Oct 28, 2025
fd82edf
fix: used CHALLENGE_SERVICE_PRISMA_TIMEOUT instead of PRISMA_TRANSACT…
hentrymartin Oct 28, 2025
4a20ab7
fix: lint
hentrymartin Oct 28, 2025
d34e20b
Allow for management of defaultchallengereviewers by admins
jmgasper Oct 29, 2025
6486a94
Merge pull request #26 from topcoder-platform/pm-2539
kkartunov Oct 29, 2025
1c69c38
Incremental update tweaks
jmgasper Oct 30, 2025
0b8dc13
Fix validation to not require GUIDs
jmgasper Oct 30, 2025
9fe2350
Additional indices for performance issues noted
jmgasper Oct 31, 2025
c80586d
Merge pull request #29 from topcoder-platform/performance
jmgasper Oct 31, 2025
0e0aa70
Fix up extension creation
jmgasper Oct 31, 2025
73a547b
Merge pull request #30 from topcoder-platform/performance
jmgasper Oct 31, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ workflows:
- feature/top-262-projectid-non-mandatory
- TOP-2364
- PM-2097
- pm-2456
- pm-2539

- "build-qa":
context: org-global
Expand Down
34 changes: 34 additions & 0 deletions .github/workflows/trivy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
name: Trivy Scanner

permissions:
contents: read
security-events: write
on:
push:
branches:
- main
- dev
pull_request:
jobs:
trivy-scan:
name: Use Trivy
runs-on: ubuntu-24.04

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using ubuntu-latest instead of a specific version like ubuntu-24.04 to ensure the workflow uses the most up-to-date and supported version of the runner. This can help avoid issues with deprecated or unsupported versions in the future.

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Run Trivy scanner in repo mode
uses: aquasecurity/trivy-action@0.33.1
with:
scan-type: "fs"
ignore-unfixed: true
format: "sarif"
output: "trivy-results.sarif"
severity: "CRITICAL,HIGH,UNKNOWN"
scanners: vuln,secret,misconfig,license
github-pat: ${{ secrets.GITHUB_TOKEN }}

Choose a reason for hiding this comment

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

[⚠️ security]
Ensure that the GITHUB_TOKEN secret has the necessary permissions for the actions being performed. While the default permissions should suffice, it's important to verify this to avoid unexpected permission issues.


- name: Upload Trivy scan results to GitHub Security tab
uses: github/codeql-action/upload-sarif@v3
with:
sarif_file: "trivy-results.sarif"
6 changes: 6 additions & 0 deletions app-constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ const Topics = {
ChallengeTimelineTemplateDeleted: "challenge.action.challenge.timeline.deleted",
ChallengePhaseUpdated: "challenge.action.phase.updated",
ChallengePhaseDeleted: "challenge.action.phase.deleted",
DefaultChallengeReviewerCreated: "challenge.action.default.reviewer.created",
DefaultChallengeReviewerUpdated: "challenge.action.default.reviewer.updated",
DefaultChallengeReviewerDeleted: "challenge.action.default.reviewer.deleted",
// Self Service topics
Notifications: "notifications.action.create",
};
Expand Down Expand Up @@ -95,6 +98,9 @@ const DisabledTopics = [
Topics.ChallengeTimelineTemplateDeleted,
Topics.ChallengePhaseUpdated,
Topics.ChallengePhaseDeleted,
Topics.DefaultChallengeReviewerCreated,
Topics.DefaultChallengeReviewerUpdated,
Topics.DefaultChallengeReviewerDeleted,
];

const challengeTextSortField = {
Expand Down
1 change: 1 addition & 0 deletions config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,5 @@ module.exports = {
// Database schemas for direct counts (shared DB)
RESOURCES_DB_SCHEMA: process.env.RESOURCES_DB_SCHEMA || "resources",
REVIEW_DB_SCHEMA: process.env.REVIEW_DB_SCHEMA || "reviews",
CHALLENGE_SERVICE_PRISMA_TIMEOUT: process.env.CHALLENGE_SERVICE_PRISMA_TIMEOUT ? parseInt(process.env.CHALLENGE_SERVICE_PRISMA_TIMEOUT, 10) : 10000,

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider using Number() instead of parseInt() for parsing CHALLENGE_SERVICE_PRISMA_TIMEOUT. Number() will convert the entire string to a number, while parseInt() stops at the first non-numeric character, which might lead to unexpected results if the environment variable contains non-numeric characters.

};
29 changes: 29 additions & 0 deletions prisma/migrations/20251023101420_performance_indices/migration.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
-- CreateIndex
CREATE INDEX "Challenge_status_startDate_idx" ON "Challenge"("status", "startDate");

Choose a reason for hiding this comment

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

[⚠️ performance]
Consider the selectivity of the status and startDate columns when creating this index. If status has low cardinality, this index might not be as effective as expected.


-- CreateIndex
CREATE INDEX "Challenge_trackId_typeId_status_idx" ON "Challenge"("trackId", "typeId", "status");

Choose a reason for hiding this comment

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

[⚠️ performance]
Ensure that the order of columns in this index (trackId, typeId, status) aligns with the most common query patterns. The order can significantly impact the index's effectiveness.


-- CreateIndex
CREATE INDEX "Challenge_legacyId_idx" ON "Challenge"("legacyId");

-- CreateIndex
CREATE INDEX "Challenge_projectId_status_idx" ON "Challenge"("projectId", "status");

Choose a reason for hiding this comment

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

[⚠️ performance]
Verify that the projectId and status columns are frequently queried together. Otherwise, this index might not provide the expected performance benefits.


-- CreateIndex
CREATE INDEX "ChallengePhase_challengeId_isOpen_idx" ON "ChallengePhase"("challengeId", "isOpen");

Choose a reason for hiding this comment

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

[⚠️ performance]
Consider the impact of indexing the isOpen column if it has low cardinality. This might not be beneficial if the column has few distinct values.


-- CreateIndex
CREATE INDEX "ChallengePhase_challengeId_name_idx" ON "ChallengePhase"("challengeId", "name");

-- CreateIndex
CREATE INDEX "ChallengePrizeSet_challengeId_type_idx" ON "ChallengePrizeSet"("challengeId", "type");

-- CreateIndex
CREATE INDEX "ChallengeReviewer_challengeId_phaseId_idx" ON "ChallengeReviewer"("challengeId", "phaseId");

-- CreateIndex
CREATE INDEX "ChallengeWinner_challengeId_type_placement_idx" ON "ChallengeWinner"("challengeId", "type", "placement");

Choose a reason for hiding this comment

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

[⚠️ performance]
Check if the placement column is used in range queries or sorting operations. If not, its inclusion in the index might not be necessary.


-- CreateIndex
CREATE INDEX "TimelineTemplatePhase_timelineTemplateId_phaseId_idx" ON "TimelineTemplatePhase"("timelineTemplateId", "phaseId");
10 changes: 10 additions & 0 deletions prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ model Challenge {
@@index([registrationEndDate])
@@index([startDate])
@@index([endDate])
@@index([status, startDate])
@@index([trackId, typeId, status])
@@index([legacyId])
@@index([projectId, status])
}

//////////////////////////////////////////
Expand Down Expand Up @@ -329,6 +333,7 @@ model ChallengeWinner {
updatedBy String

@@index([challengeId])
@@index([challengeId, type, placement])
}

//////////////////////////////////////////
Expand Down Expand Up @@ -533,6 +538,8 @@ model ChallengePhase {
updatedBy String

@@index([challengeId])
@@index([challengeId, isOpen])
@@index([challengeId, name])
}

//////////////////////////////////////////
Expand Down Expand Up @@ -576,6 +583,7 @@ model ChallengePrizeSet {
updatedBy String

@@index([challengeId])
@@index([challengeId, type])
}

//////////////////////////////////////////
Expand Down Expand Up @@ -611,6 +619,7 @@ model ChallengeReviewer {

@@index([challengeId])
@@index([phaseId])
@@index([challengeId, phaseId])
}

//////////////////////////////////////////
Expand Down Expand Up @@ -697,4 +706,5 @@ model TimelineTemplatePhase {
timelineTemplate TimelineTemplate @relation(fields: [timelineTemplateId], references: [id], onDelete: Cascade)

@@index([timelineTemplateId])
@@index([timelineTemplateId, phaseId])
}
2 changes: 2 additions & 0 deletions src/common/prisma-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ function convertChallengeSchemaToPrisma(currentUser, challenge) {
memberReviewerCount: _.isNil(r.memberReviewerCount)
? null
: Number(r.memberReviewerCount),
fixedAmount: _.isNil(r.fixedAmount) ? null : Number(r.fixedAmount),
baseCoefficient: _.isNil(r.baseCoefficient) ? null : Number(r.baseCoefficient),
incrementalCoefficient: _.isNil(r.incrementalCoefficient)
? null
Expand Down Expand Up @@ -391,6 +392,7 @@ function convertModelToResponse(ret) {
"isMemberReview",
"memberReviewerCount",
"phaseId",
"fixedAmount",
"baseCoefficient",
"incrementalCoefficient",
"type",
Expand Down
3 changes: 2 additions & 1 deletion src/common/prisma.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const {
ReviewOpportunityTypeEnum,
} = require("@prisma/client");
const logger = require("./logger");
const config = require("config");

const prismaClient = new PrismaClient({
log: [
Expand All @@ -21,7 +22,7 @@ const prismaClient = new PrismaClient({
// Allow overriding via environment variables if needed.
transactionOptions: {
maxWait: Number(process.env.PRISMA_TRANSACTION_MAX_WAIT_MS || 10000), // wait up to 10s to start
timeout: Number(process.env.PRISMA_TRANSACTION_TIMEOUT_MS || 10000), // allow up to 30s per transaction
timeout: config.CHALLENGE_SERVICE_PRISMA_TIMEOUT, // allow up to 30s per transaction

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The change from an environment variable to a config value for timeout means that the timeout is now controlled by the config module rather than environment variables. Ensure that config.CHALLENGE_SERVICE_PRISMA_TIMEOUT is correctly set and documented, as this could impact transaction behavior if not properly configured.

},
});

Expand Down
2 changes: 1 addition & 1 deletion src/common/review-prisma.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const createClient = () =>
],
transactionOptions: {
maxWait: Number(process.env.PRISMA_TRANSACTION_MAX_WAIT_MS || 10000),
timeout: Number(process.env.PRISMA_TRANSACTION_TIMEOUT_MS || 10000),
timeout: config.CHALLENGE_SERVICE_PRISMA_TIMEOUT,

Choose a reason for hiding this comment

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

[❗❗ correctness]
The timeout value is now being set from config.CHALLENGE_SERVICE_PRISMA_TIMEOUT. Ensure that this configuration value is validated and defaults are handled appropriately elsewhere in the codebase. This change assumes that config.CHALLENGE_SERVICE_PRISMA_TIMEOUT is always set and valid, which could lead to runtime errors if not properly managed.

},
});

Expand Down
95 changes: 95 additions & 0 deletions src/controllers/DefaultChallengeReviewerController.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/**
* Controller for default challenge reviewer endpoints.
*/
const HttpStatus = require("http-status-codes");
const service = require("../services/DefaultChallengeReviewerService");
const helper = require("../common/helper");

/**
* Search default challenge reviewers.
*
* @param {Object} req the request
* @param {Object} res the response
*/
async function searchDefaultChallengeReviewers(req, res) {
const result = await service.searchDefaultChallengeReviewers(req.query);

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.searchDefaultChallengeReviewers call to manage potential exceptions and provide meaningful error responses.

helper.setResHeaders(req, res, result);
res.send(result.result);
}

/**
* Create default challenge reviewer.
*
* @param {Object} req the request
* @param {Object} res the response
*/
async function createDefaultChallengeReviewer(req, res) {
const result = await service.createDefaultChallengeReviewer(req.authUser, req.body);

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.createDefaultChallengeReviewer call to manage potential exceptions and provide meaningful error responses.

res.status(HttpStatus.CREATED).send(result);
}

/**
* Get default challenge reviewer.
*
* @param {Object} req the request
* @param {Object} res the response
*/
async function getDefaultChallengeReviewer(req, res) {
const result = await service.getDefaultChallengeReviewer(

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.getDefaultChallengeReviewer call to manage potential exceptions and provide meaningful error responses.

req.params.defaultChallengeReviewerId
);
res.send(result);
}

/**
* Fully update default challenge reviewer.
*
* @param {Object} req the request
* @param {Object} res the response
*/
async function fullyUpdateDefaultChallengeReviewer(req, res) {
const result = await service.fullyUpdateDefaultChallengeReviewer(

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.fullyUpdateDefaultChallengeReviewer call to manage potential exceptions and provide meaningful error responses.

req.authUser,
req.params.defaultChallengeReviewerId,
req.body
);
res.send(result);
}

/**
* Partially update default challenge reviewer.
*
* @param {Object} req the request
* @param {Object} res the response
*/
async function partiallyUpdateDefaultChallengeReviewer(req, res) {
const result = await service.partiallyUpdateDefaultChallengeReviewer(

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.partiallyUpdateDefaultChallengeReviewer call to manage potential exceptions and provide meaningful error responses.

req.authUser,
req.params.defaultChallengeReviewerId,
req.body
);
res.send(result);
}

/**
* Delete default challenge reviewer.
*
* @param {Object} req the request
* @param {Object} res the response
*/
async function deleteDefaultChallengeReviewer(req, res) {
const result = await service.deleteDefaultChallengeReviewer(

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.deleteDefaultChallengeReviewer call to manage potential exceptions and provide meaningful error responses.

req.params.defaultChallengeReviewerId
);
res.send(result);
}

module.exports = {
searchDefaultChallengeReviewers,
createDefaultChallengeReviewer,
getDefaultChallengeReviewer,
fullyUpdateDefaultChallengeReviewer,
partiallyUpdateDefaultChallengeReviewer,
deleteDefaultChallengeReviewer,
};

46 changes: 46 additions & 0 deletions src/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,52 @@ module.exports = {
scopes: [DELETE, ALL],
},
},
"/default-challenge-reviewers": {
get: {
controller: "DefaultChallengeReviewerController",
method: "searchDefaultChallengeReviewers",
auth: "jwt",
access: [constants.UserRoles.Admin],

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider adding more roles to the access list for the /default-challenge-reviewers endpoint if other roles besides Admin should have access. This will enhance flexibility and maintainability if access requirements change in the future.

scopes: [READ, ALL],
},
post: {
controller: "DefaultChallengeReviewerController",
method: "createDefaultChallengeReviewer",
auth: "jwt",
access: [constants.UserRoles.Admin],
scopes: [CREATE, ALL],
},
},
"/default-challenge-reviewers/:defaultChallengeReviewerId": {
get: {
controller: "DefaultChallengeReviewerController",
method: "getDefaultChallengeReviewer",
auth: "jwt",
access: [constants.UserRoles.Admin],

Choose a reason for hiding this comment

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

[⚠️ security]
The access list for the /default-challenge-reviewers/:defaultChallengeReviewerId endpoint is limited to Admin. Ensure this aligns with the intended access control policy, as it might restrict legitimate users from accessing this resource.

scopes: [READ, ALL],
},
put: {
controller: "DefaultChallengeReviewerController",
method: "fullyUpdateDefaultChallengeReviewer",
auth: "jwt",
access: [constants.UserRoles.Admin],
scopes: [UPDATE, ALL],
},
patch: {
controller: "DefaultChallengeReviewerController",
method: "partiallyUpdateDefaultChallengeReviewer",
auth: "jwt",
access: [constants.UserRoles.Admin],
scopes: [UPDATE, ALL],
},
delete: {
controller: "DefaultChallengeReviewerController",
method: "deleteDefaultChallengeReviewer",
auth: "jwt",
access: [constants.UserRoles.Admin],
scopes: [DELETE, ALL],
},
},
"/timeline-templates": {
get: {
controller: "TimelineTemplateController",
Expand Down
Loading