From 9279a1930aa6e7e943069576de25a23734bb75af Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Thu, 23 Oct 2025 21:24:13 +1100 Subject: [PATCH 01/18] Performance indices --- .../migration.sql | 29 +++++++++++++++++++ prisma/schema.prisma | 10 +++++++ 2 files changed, 39 insertions(+) create mode 100644 prisma/migrations/20251023101420_performance_indices/migration.sql diff --git a/prisma/migrations/20251023101420_performance_indices/migration.sql b/prisma/migrations/20251023101420_performance_indices/migration.sql new file mode 100644 index 0000000..c57fe43 --- /dev/null +++ b/prisma/migrations/20251023101420_performance_indices/migration.sql @@ -0,0 +1,29 @@ +-- CreateIndex +CREATE INDEX "Challenge_status_startDate_idx" ON "Challenge"("status", "startDate"); + +-- CreateIndex +CREATE INDEX "Challenge_trackId_typeId_status_idx" ON "Challenge"("trackId", "typeId", "status"); + +-- CreateIndex +CREATE INDEX "Challenge_legacyId_idx" ON "Challenge"("legacyId"); + +-- CreateIndex +CREATE INDEX "Challenge_projectId_status_idx" ON "Challenge"("projectId", "status"); + +-- CreateIndex +CREATE INDEX "ChallengePhase_challengeId_isOpen_idx" ON "ChallengePhase"("challengeId", "isOpen"); + +-- 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"); + +-- CreateIndex +CREATE INDEX "TimelineTemplatePhase_timelineTemplateId_phaseId_idx" ON "TimelineTemplatePhase"("timelineTemplateId", "phaseId"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index e69440b..602163f 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -153,6 +153,10 @@ model Challenge { @@index([registrationEndDate]) @@index([startDate]) @@index([endDate]) + @@index([status, startDate]) + @@index([trackId, typeId, status]) + @@index([legacyId]) + @@index([projectId, status]) } ////////////////////////////////////////// @@ -329,6 +333,7 @@ model ChallengeWinner { updatedBy String @@index([challengeId]) + @@index([challengeId, type, placement]) } ////////////////////////////////////////// @@ -533,6 +538,8 @@ model ChallengePhase { updatedBy String @@index([challengeId]) + @@index([challengeId, isOpen]) + @@index([challengeId, name]) } ////////////////////////////////////////// @@ -576,6 +583,7 @@ model ChallengePrizeSet { updatedBy String @@index([challengeId]) + @@index([challengeId, type]) } ////////////////////////////////////////// @@ -611,6 +619,7 @@ model ChallengeReviewer { @@index([challengeId]) @@index([phaseId]) + @@index([challengeId, phaseId]) } ////////////////////////////////////////// @@ -697,4 +706,5 @@ model TimelineTemplatePhase { timelineTemplate TimelineTemplate @relation(fields: [timelineTemplateId], references: [id], onDelete: Cascade) @@index([timelineTemplateId]) + @@index([timelineTemplateId, phaseId]) } From 2449c2e0782ebb8ae1a14a9497929c4c712726bd Mon Sep 17 00:00:00 2001 From: Vasilica Olariu Date: Thu, 23 Oct 2025 17:03:11 +0300 Subject: [PATCH 02/18] Expose phaseId for defaultReviewer --- src/services/ChallengeService.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index a4d0904..a39f775 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -258,6 +258,7 @@ async function getDefaultReviewers(currentUser, criteria) { isMemberReview: r.isMemberReview, memberReviewerCount: r.memberReviewerCount, phaseName: r.phaseName, + phaseId: r.phaseId, fixedAmount: r.fixedAmount, baseCoefficient: r.baseCoefficient, incrementalCoefficient: r.incrementalCoefficient, From aa77c21d5cd4cae3182f9b985593961e0c6878a7 Mon Sep 17 00:00:00 2001 From: Vasilica Olariu Date: Thu, 23 Oct 2025 21:39:42 +0300 Subject: [PATCH 03/18] expose fixedAmount --- src/common/prisma-helper.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/common/prisma-helper.js b/src/common/prisma-helper.js index 559e5bd..b97d774 100644 --- a/src/common/prisma-helper.js +++ b/src/common/prisma-helper.js @@ -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 @@ -370,6 +371,7 @@ function convertModelToResponse(ret) { "isMemberReview", "memberReviewerCount", "phaseId", + "fixedAmount", "baseCoefficient", "incrementalCoefficient", "type", From 80170e92e0d1c2088f66c42fc4ebbaf2ae92e4dd Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 24 Oct 2025 11:31:49 +1100 Subject: [PATCH 04/18] Review guard when changing scorecards, and make sure we don't wipe terms in a PUT / PATCH --- src/services/ChallengeService.js | 187 ++++++++++++++++------------- test/unit/ChallengeService.test.js | 50 ++++++++ 2 files changed, 154 insertions(+), 83 deletions(-) diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index 62365c6..88c7bec 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -2064,6 +2064,9 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) { const originalChallengePhases = _.cloneDeep(challenge.phases || []); const auditUserId = _.toString(currentUser.userId); const existingPrizeType = challengeHelper.validatePrizeSetsAndGetPrizeType(challenge.prizeSets); + const payloadIncludesTerms = + !_.isNil(data) && Object.prototype.hasOwnProperty.call(data, "terms"); + const originalTermsValue = payloadIncludesTerms ? data.terms : undefined; // No conversion needed - values are already in dollars in the database @@ -2111,6 +2114,9 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) { // Remove fields from data that are not allowed to be updated and that match the existing challenge data = sanitizeData(sanitizeChallenge(data), challenge); + const sanitizedIncludesTerms = Object.prototype.hasOwnProperty.call(data, "terms"); + const shouldReplaceTerms = + sanitizedIncludesTerms || (payloadIncludesTerms && originalTermsValue === null); logger.debug(`Sanitized Data: ${JSON.stringify(data)}`); logger.debug(`updateChallenge(${challengeId}): fetching challenge resources`); @@ -2639,7 +2645,7 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) { if (_.isNil(updateData.attachment)) { await tx.attachment.deleteMany({ where: { challengeId } }); } - if (_.isNil(updateData.terms)) { + if (shouldReplaceTerms) { await tx.challengeTerm.deleteMany({ where: { challengeId } }); } // if (_.isNil(updateData.skills)) { @@ -2987,13 +2993,6 @@ async function ensureScorecardChangeDoesNotConflict({ updatedReviewers = [], originalChallengePhases = [], }) { - if (!config.REVIEW_DB_URL) { - logger.debug( - `Skipping scorecard change guard for challenge ${challengeId} because REVIEW_DB_URL is not configured` - ); - return; - } - if (!Array.isArray(originalReviewers) || originalReviewers.length === 0) { return; } @@ -3077,15 +3076,31 @@ async function ensureScorecardChangeDoesNotConflict({ phaseIdToChallengePhaseIds.get(phaseKey).add(String(phase.id)); } - const reviewClient = getReviewClient(); - const hasReviewModel = - reviewClient && - typeof reviewClient.review === "object" && - typeof reviewClient.review?.count === "function"; - - if (!hasReviewModel) { + if (!config.REVIEW_DB_URL) { logger.debug( - `Prisma review model is unavailable; falling back to raw query guard for challenge ${challengeId}` + `Skipping scorecard change guard for challenge ${challengeId} because REVIEW_DB_URL is not configured` + ); + return; + } + + let reviewClient; + try { + reviewClient = getReviewClient(); + } catch (error) { + logger.warn( + `Unable to initialize review Prisma client for challenge ${challengeId}: ${error.message}` + ); + throw new errors.ServiceUnavailableError( + "Cannot change the scorecard because review status could not be verified. Please try again later." + ); + } + + if (!reviewClient || typeof reviewClient.$queryRaw !== "function") { + logger.warn( + `Prisma review client does not support raw queries for challenge ${challengeId}` + ); + throw new errors.ServiceUnavailableError( + "Cannot change the scorecard because review status could not be verified. Please try again later." ); } @@ -3113,6 +3128,73 @@ async function ensureScorecardChangeDoesNotConflict({ openReviewPhaseNameByKey.has(phaseKey) ); + const reviewPhaseKeysSet = new Set(removedScorecardsByPhase.keys()); + const challengePhaseIdsToInspect = new Set(); + for (const phaseKey of reviewPhaseKeysSet) { + const challengePhaseIds = phaseIdToChallengePhaseIds.get(phaseKey); + if (!challengePhaseIds) { + continue; + } + for (const challengePhaseId of challengePhaseIds) { + if (!_.isNil(challengePhaseId)) { + challengePhaseIdsToInspect.add(String(challengePhaseId)); + } + } + } + + const blockingCountByPhase = new Map(); + const blockingCountByPhaseAndScorecard = new Map(); + + const challengePhaseIdList = Array.from(challengePhaseIdsToInspect); + if (challengePhaseIdList.length > 0) { + const reviewSchema = String(config.REVIEW_DB_SCHEMA || "").trim(); + const reviewTableIdentifier = Prisma.raw( + reviewSchema + ? `"${reviewSchema.replace(/"/g, '""')}"."review"` + : '"review"' + ); + + let blockingReviewRows = []; + try { + blockingReviewRows = await reviewClient.$queryRaw` + SELECT "phaseId", "scorecardId", COUNT(*)::int AS "count" + FROM ${reviewTableIdentifier} + WHERE "phaseId" IN (${Prisma.join(challengePhaseIdList)}) + AND "status" IN (${Prisma.join(REVIEW_STATUS_BLOCKING)}) + GROUP BY "phaseId", "scorecardId" + `; + } catch (error) { + logger.warn( + `Failed to query the review database for challenge ${challengeId}: ${error.message}` + ); + throw new errors.ServiceUnavailableError( + "Cannot change the scorecard because review status could not be verified. Please try again later." + ); + } + + for (const row of blockingReviewRows || []) { + const phaseId = _.isNil(row?.phaseId) ? null : String(row.phaseId); + if (!phaseId) { + continue; + } + const countValue = Number(_.get(row, "count", 0)); + if (!Number.isFinite(countValue) || countValue <= 0) { + continue; + } + + blockingCountByPhase.set(phaseId, (blockingCountByPhase.get(phaseId) || 0) + countValue); + + const scorecardId = _.isNil(row?.scorecardId) ? null : String(row.scorecardId); + if (scorecardId) { + const scorecardKey = `${phaseId}|${scorecardId}`; + blockingCountByPhaseAndScorecard.set( + scorecardKey, + (blockingCountByPhaseAndScorecard.get(scorecardKey) || 0) + countValue + ); + } + } + } + if (reviewPhaseKeysToCheck.length > 0) { for (const phaseKey of reviewPhaseKeysToCheck) { const challengePhaseIds = Array.from(phaseIdToChallengePhaseIds.get(phaseKey) || []); @@ -3124,38 +3206,8 @@ async function ensureScorecardChangeDoesNotConflict({ } let activePhaseReviewCount = 0; - - if (hasReviewModel) { - activePhaseReviewCount = await reviewClient.review.count({ - where: { - phaseId: { in: challengePhaseIds }, - status: { in: REVIEW_STATUS_BLOCKING }, - }, - }); - } else if (typeof reviewClient?.$queryRaw === "function") { - try { - const queryResult = await reviewClient.$queryRaw` - SELECT COUNT(*)::int AS count - FROM "review" - WHERE "phaseId" IN (${Prisma.join(challengePhaseIds)}) - AND "status" IN (${Prisma.join(REVIEW_STATUS_BLOCKING)}) - `; - const rawCount = - Array.isArray(queryResult) && queryResult.length > 0 - ? Number(queryResult[0]?.count || 0) - : 0; - activePhaseReviewCount = Number.isFinite(rawCount) ? rawCount : 0; - } catch (error) { - logger.warn( - `Skipping active phase scorecard guard for challenge ${challengeId} phase ${phaseKey} due to review DB query failure: ${error.message}` - ); - continue; - } - } else { - logger.warn( - `Skipping active phase scorecard guard for challenge ${challengeId} phase ${phaseKey} because review Prisma client does not support raw queries` - ); - continue; + for (const challengePhaseId of challengePhaseIds) { + activePhaseReviewCount += blockingCountByPhase.get(challengePhaseId) || 0; } if (activePhaseReviewCount > 0) { @@ -3182,42 +3234,11 @@ async function ensureScorecardChangeDoesNotConflict({ continue; } + const normalizedScorecardId = String(scorecardId); let conflictingReviews = 0; - - if (hasReviewModel) { - conflictingReviews = await reviewClient.review.count({ - where: { - phaseId: { in: challengePhaseIds }, - scorecardId, - status: { in: REVIEW_STATUS_BLOCKING }, - }, - }); - } else if (typeof reviewClient?.$queryRaw === "function") { - try { - const queryResult = await reviewClient.$queryRaw` - SELECT COUNT(*)::int AS count - FROM "review" - WHERE "phaseId" IN (${Prisma.join(challengePhaseIds)}) - AND "scorecardId" = ${scorecardId} - AND "status" IN (${Prisma.join(REVIEW_STATUS_BLOCKING)}) - `; - - const rawCount = - Array.isArray(queryResult) && queryResult.length > 0 - ? Number(queryResult[0]?.count || 0) - : 0; - conflictingReviews = Number.isFinite(rawCount) ? rawCount : 0; - } catch (error) { - logger.warn( - `Skipping scorecard change guard for challenge ${challengeId} phase ${phaseKey} due to review DB query failure: ${error.message}` - ); - continue; - } - } else { - logger.warn( - `Skipping scorecard change guard for challenge ${challengeId} phase ${phaseKey} because review Prisma client does not support raw queries` - ); - continue; + for (const challengePhaseId of challengePhaseIds) { + const scorecardKey = `${challengePhaseId}|${normalizedScorecardId}`; + conflictingReviews += blockingCountByPhaseAndScorecard.get(scorecardKey) || 0; } if (conflictingReviews > 0) { diff --git a/test/unit/ChallengeService.test.js b/test/unit/ChallengeService.test.js index 9fd49be..a27c06d 100644 --- a/test/unit/ChallengeService.test.js +++ b/test/unit/ChallengeService.test.js @@ -737,6 +737,56 @@ describe('challenge service unit tests', () => { should.equal(testHelper.getDatesDiff(result.startDate, testChallengeData.startDate), 0) }) + it('preserves existing terms when update payload omits the terms field', async () => { + const challengeData = _.cloneDeep(testChallengeData) + challengeData.name = `${challengeData.name} Terms ${Date.now()}` + challengeData.legacyId = Math.floor(Math.random() * 1000000) + const challengeWithTerms = await service.createChallenge( + { isMachine: true, sub: 'sub-terms', userId: 22838965 }, + challengeData, + config.M2M_FULL_ACCESS_TOKEN + ) + + const termRecords = [ + { + challengeId: challengeWithTerms.id, + termId: uuid(), + roleId: uuid(), + createdBy: 'unit-test', + updatedBy: 'unit-test' + }, + { + challengeId: challengeWithTerms.id, + termId: uuid(), + roleId: uuid(), + createdBy: 'unit-test', + updatedBy: 'unit-test' + } + ] + await prisma.challengeTerm.createMany({ data: termRecords }) + + try { + const updated = await service.updateChallenge( + { isMachine: true, sub: 'sub-terms', userId: 22838965 }, + challengeWithTerms.id, + { + description: 'Updated description to ensure persistence of terms' + } + ) + + should.exist(updated.terms) + should.equal(updated.terms.length, termRecords.length) + const sortedTerms = _.sortBy(updated.terms, ['id', 'roleId']) + const expectedTerms = _.sortBy( + termRecords.map((t) => ({ id: t.termId, roleId: t.roleId })), + ['id', 'roleId'] + ) + sortedTerms.should.deep.equal(expectedTerms) + } finally { + await prisma.challenge.delete({ where: { id: challengeWithTerms.id } }) + } + }).timeout(5000) + it('update challenge successfully with winners', async () => { const result = await service.updateChallenge({ isMachine: true, sub: 'sub3', userId: 22838965 }, data.challenge.id, { winners: [{ From 3f96faaf7a47dc8b7c6d9fc688c55c187bcade11 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 24 Oct 2025 11:48:21 +1100 Subject: [PATCH 05/18] Enrich skills data for PUT / PATCH response and add blocks on review type phases to ensure they can't be manually opened without at least 1 resource assigned to the review type role associated with the phase type. --- src/services/ChallengePhaseService.js | 54 +++++++++++++ src/services/ChallengeService.js | 2 + test/unit/ChallengePhaseService.test.js | 102 ++++++++++++++++++++++++ 3 files changed, 158 insertions(+) diff --git a/src/services/ChallengePhaseService.js b/src/services/ChallengePhaseService.js index fc15045..61db582 100644 --- a/src/services/ChallengePhaseService.js +++ b/src/services/ChallengePhaseService.js @@ -24,6 +24,13 @@ const REVIEW_PHASE_NAMES = Object.freeze([ "approval", ]); const REVIEW_PHASE_NAME_SET = new Set(REVIEW_PHASE_NAMES.map((name) => name.toLowerCase())); +const PHASE_RESOURCE_ROLE_REQUIREMENTS = Object.freeze({ + "iterative review": "Iterative Reviewer", + "checkpoint screening": "Checkpoint Screener", + screening: "Screener", + review: "Reviewer", + "checkpoint review": "Checkpoint Reviewer", +}); async function hasPendingScorecardsForPhase(challengePhaseId) { if (!config.REVIEW_DB_URL) { @@ -201,6 +208,48 @@ async function postChallengeUpdatedNotification(challengeId) { } } +async function ensureRequiredResourcesBeforeOpeningPhase(challengeId, phaseName) { + const normalizedPhaseName = _.toLower(_.trim(phaseName || "")); + const requiredRoleName = PHASE_RESOURCE_ROLE_REQUIREMENTS[normalizedPhaseName]; + if (!requiredRoleName) { + return; + } + + const challengeResources = await helper.getChallengeResources(challengeId); + const requiredRoleNameLower = _.toLower(requiredRoleName); + const hasRequiredRoleByName = (challengeResources || []).some((resource) => { + const roleName = + resource.roleName || + resource.role || + _.get(resource, "role.name") || + _.get(resource, "resourceRoleName") || + _.get(resource, "resourceRole.name"); + return roleName && _.toLower(roleName) === requiredRoleNameLower; + }); + + let hasRequiredRole = hasRequiredRoleByName; + + if (!hasRequiredRole) { + const resourceRoles = await helper.getResourceRoles(); + const requiredRoleIds = (resourceRoles || []) + .filter((role) => role.name && _.toLower(role.name) === requiredRoleNameLower) + .map((role) => _.toString(role.id)); + if (requiredRoleIds.length > 0) { + const requiredRoleIdSet = new Set(requiredRoleIds); + hasRequiredRole = (challengeResources || []).some( + (resource) => resource.roleId && requiredRoleIdSet.has(_.toString(resource.roleId)) + ); + } + } + + if (!hasRequiredRole) { + const displayPhaseName = phaseName || "phase"; + throw new errors.BadRequestError( + `Cannot open ${displayPhaseName} phase because the challenge does not have any resource with the ${requiredRoleName} role` + ); + } +} + /** * Get all phase information for that challenge * @param {String} challengeId the challenge id @@ -330,6 +379,11 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data) } } + if (isOpeningPhase) { + const phaseName = data.name || challengePhase.name; + await ensureRequiredResourcesBeforeOpeningPhase(challengeId, phaseName); + } + if (data["scheduledStartDate"] || data["scheduledEndDate"]) { const startDate = data["scheduledStartDate"] || challengePhase.scheduledStartDate; const endDate = data["scheduledEndDate"] || challengePhase.scheduledEndDate; diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index 88c7bec..306799a 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -1515,6 +1515,7 @@ async function createChallenge(currentUser, challenge, userToken) { // No conversion needed - values are already in dollars in the database prismaHelper.convertModelToResponse(ret); + await enrichSkillsData(ret); enrichChallengeForResponse(ret, track, type); // If the challenge is self-service, add the creating user as the "client manager", *not* the manager @@ -2683,6 +2684,7 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) { // Convert to response shape before any business-logic checks that expect it prismaHelper.convertModelToResponse(updatedChallenge); + await enrichSkillsData(updatedChallenge); enrichChallengeForResponse(updatedChallenge); if (_.get(updatedChallenge, "legacy.selfService")) { diff --git a/test/unit/ChallengePhaseService.test.js b/test/unit/ChallengePhaseService.test.js index 2391d08..0fc9bfb 100644 --- a/test/unit/ChallengePhaseService.test.js +++ b/test/unit/ChallengePhaseService.test.js @@ -13,6 +13,7 @@ const uuid = require('uuid/v4') const { getReviewClient } = require('../../src/common/review-prisma') const prisma = require('../../src/common/prisma').getClient() const service = require('../../src/services/ChallengePhaseService') +const helper = require('../../src/common/helper') const testHelper = require('../testHelper') const should = chai.should() @@ -802,6 +803,107 @@ describe('challenge phase service unit tests', () => { const challengePhase = await service.partiallyUpdateChallengePhase(authUser, data.challenge.id, data.challengePhase1Id, { isOpen: true }) should.equal(challengePhase.isOpen, true) }) + + it('partially update challenge phase - cannot open review phase without reviewer resource', async () => { + const reviewPhase = await prisma.phase.create({ + data: { + id: uuid(), + name: 'Review', + description: 'desc', + isOpen: false, + duration: 86400, + createdBy: 'admin', + updatedBy: 'admin' + } + }) + const reviewChallengePhaseId = uuid() + await prisma.challengePhase.create({ + data: { + id: reviewChallengePhaseId, + challengeId: data.challenge.id, + phaseId: reviewPhase.id, + name: 'Review', + isOpen: false, + createdBy: 'admin', + updatedBy: 'admin' + } + }) + + const originalGetChallengeResources = helper.getChallengeResources + const originalGetResourceRoles = helper.getResourceRoles + helper.getChallengeResources = async () => ([ + { roleId: 'some-other-role-id' } + ]) + helper.getResourceRoles = async () => ([ + { id: 'reviewer-role-id', name: 'Reviewer' } + ]) + + try { + await service.partiallyUpdateChallengePhase(authUser, data.challenge.id, reviewChallengePhaseId, { isOpen: true }) + } catch (e) { + should.equal(e.httpStatus || e.statusCode, 400) + should.equal( + e.message, + 'Cannot open Review phase because the challenge does not have any resource with the Reviewer role' + ) + return + } finally { + helper.getChallengeResources = originalGetChallengeResources + helper.getResourceRoles = originalGetResourceRoles + await prisma.challengePhase.delete({ where: { id: reviewChallengePhaseId } }) + await prisma.phase.delete({ where: { id: reviewPhase.id } }) + } + + throw new Error('should not reach here') + }) + + it('partially update challenge phase - opens review phase when reviewer resource exists', async () => { + const reviewPhase = await prisma.phase.create({ + data: { + id: uuid(), + name: 'Review', + description: 'desc', + isOpen: false, + duration: 86400, + createdBy: 'admin', + updatedBy: 'admin' + } + }) + const reviewChallengePhaseId = uuid() + await prisma.challengePhase.create({ + data: { + id: reviewChallengePhaseId, + challengeId: data.challenge.id, + phaseId: reviewPhase.id, + name: 'Review', + isOpen: false, + createdBy: 'admin', + updatedBy: 'admin' + } + }) + + const originalGetChallengeResources = helper.getChallengeResources + const originalGetResourceRoles = helper.getResourceRoles + helper.getChallengeResources = async () => ([ + { + roleId: 'reviewer-role-id', + resourceRole: { name: 'Reviewer' } + } + ]) + helper.getResourceRoles = async () => ([ + { id: 'reviewer-role-id', name: 'Reviewer' } + ]) + + try { + const challengePhase = await service.partiallyUpdateChallengePhase(authUser, data.challenge.id, reviewChallengePhaseId, { isOpen: true }) + should.equal(challengePhase.isOpen, true) + } finally { + helper.getChallengeResources = originalGetChallengeResources + helper.getResourceRoles = originalGetResourceRoles + await prisma.challengePhase.delete({ where: { id: reviewChallengePhaseId } }) + await prisma.phase.delete({ where: { id: reviewPhase.id } }) + } + }) }) describe('delete challenge phase tests', () => { From c003e03c2728cd74e6b1d52762c39fdee3476c53 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 24 Oct 2025 11:56:42 +1100 Subject: [PATCH 06/18] Minor tweak for review status checking. --- src/services/ChallengeService.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index 306799a..7234221 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -3162,7 +3162,7 @@ async function ensureScorecardChangeDoesNotConflict({ SELECT "phaseId", "scorecardId", COUNT(*)::int AS "count" FROM ${reviewTableIdentifier} WHERE "phaseId" IN (${Prisma.join(challengePhaseIdList)}) - AND "status" IN (${Prisma.join(REVIEW_STATUS_BLOCKING)}) + AND "status"::text IN (${Prisma.join(REVIEW_STATUS_BLOCKING)}) GROUP BY "phaseId", "scorecardId" `; } catch (error) { From 41b0ca8a4ac28d9e81af17b954f63798f2f7e813 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Sat, 25 Oct 2025 07:01:31 +1100 Subject: [PATCH 07/18] Add check before manually closing an Appeals Response phase to make sure no appeals are pending --- src/services/ChallengePhaseService.js | 62 +++++++++++- test/unit/ChallengePhaseService.test.js | 127 ++++++++++++++++++++++++ 2 files changed, 188 insertions(+), 1 deletion(-) diff --git a/src/services/ChallengePhaseService.js b/src/services/ChallengePhaseService.js index 61db582..380fb29 100644 --- a/src/services/ChallengePhaseService.js +++ b/src/services/ChallengePhaseService.js @@ -180,6 +180,57 @@ async function hasSubmittedAppealsForChallenge(challengeId) { return Number(count) > 0; } +async function hasPendingAppealResponsesForChallenge(challengeId) { + if (!config.REVIEW_DB_URL) { + logger.debug( + `Skipping pending appeal response check for challenge ${challengeId} because REVIEW_DB_URL is not configured` + ); + return false; + } + + const reviewPrisma = getReviewClient(); + const reviewSchema = config.REVIEW_DB_SCHEMA; + const appealTable = Prisma.raw(`"${reviewSchema}"."appeal"`); + const appealResponseTable = Prisma.raw(`"${reviewSchema}"."appealResponse"`); + const reviewItemCommentTable = Prisma.raw(`"${reviewSchema}"."reviewItemComment"`); + const reviewItemTable = Prisma.raw(`"${reviewSchema}"."reviewItem"`); + const reviewTable = Prisma.raw(`"${reviewSchema}"."review"`); + const submissionTable = Prisma.raw(`"${reviewSchema}"."submission"`); + + let rows; + try { + rows = await reviewPrisma.$queryRaw( + Prisma.sql` + SELECT COUNT(DISTINCT a."id")::int AS count + FROM ${appealTable} a + LEFT JOIN ${appealResponseTable} ar ON ar."appealId" = a."id" + INNER JOIN ${reviewItemCommentTable} ric ON ric."id" = a."reviewItemCommentId" + INNER JOIN ${reviewItemTable} ri ON ri."id" = ric."reviewItemId" + INNER JOIN ${reviewTable} r ON r."id" = ri."reviewId" + WHERE ar."id" IS NULL + AND EXISTS ( + SELECT 1 + FROM ${submissionTable} s + WHERE s."challengeId" = ${challengeId} + AND ( + (r."submissionId" IS NOT NULL AND s."id" = r."submissionId") + OR (r."legacySubmissionId" IS NOT NULL AND s."legacySubmissionId" = r."legacySubmissionId") + ) + ) + ` + ); + } catch (err) { + logger.error( + `Failed to check pending appeal responses for challenge ${challengeId}: ${err.message}`, + err + ); + throw err; + } + + const [{ count = 0 } = {}] = rows || []; + return Number(count) > 0; +} + async function checkChallengeExists(challengeId) { const challenge = await prisma.challenge.findUnique({ where: { id: challengeId } }); if (!challenge) { @@ -419,13 +470,22 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data) const isReopeningPhase = "isOpen" in data && data["isOpen"] === true && !challengePhase.isOpen; if (isClosingPhase) { + const closingPhaseName = data.name || challengePhase.name; const pendingScorecards = await hasPendingScorecardsForPhase(challengePhase.id); if (pendingScorecards) { - const phaseName = challengePhase.name || "phase"; + const phaseName = closingPhaseName || "phase"; throw new errors.ForbiddenError( `Cannot close ${phaseName} because there are still pending scorecards` ); } + if ( + String(closingPhaseName || "").toLowerCase() === "appeals response" && + (await hasPendingAppealResponsesForChallenge(challengePhase.challengeId)) + ) { + throw new errors.BadRequestError( + "Appeals Response phase can't be closed because there are still appeals that haven't been responded to" + ); + } if (!("actualEndDate" in data) || _.isNil(data.actualEndDate)) { data.actualEndDate = new Date(); } diff --git a/test/unit/ChallengePhaseService.test.js b/test/unit/ChallengePhaseService.test.js index 0fc9bfb..0c0ac5c 100644 --- a/test/unit/ChallengePhaseService.test.js +++ b/test/unit/ChallengePhaseService.test.js @@ -81,10 +81,17 @@ describe('challenge phase service unit tests', () => { "reviewItemCommentId" varchar(14) NOT NULL ) `) + await reviewClient.$executeRawUnsafe(` + CREATE TABLE IF NOT EXISTS "${reviewSchema}"."appealResponse" ( + "id" varchar(14) PRIMARY KEY, + "appealId" varchar(14) UNIQUE NOT NULL + ) + `) }) after(async () => { if (reviewClient) { + await reviewClient.$executeRawUnsafe(`TRUNCATE TABLE "${reviewSchema}"."appealResponse"`) await reviewClient.$executeRawUnsafe(`TRUNCATE TABLE "${reviewSchema}"."appeal"`) await reviewClient.$executeRawUnsafe(`TRUNCATE TABLE "${reviewSchema}"."reviewItemComment"`) await reviewClient.$executeRawUnsafe(`TRUNCATE TABLE "${reviewSchema}"."reviewItem"`) @@ -696,6 +703,126 @@ describe('challenge phase service unit tests', () => { } }) + it('partially update challenge phase - cannot close Appeals Response when appeals lack responses', async function () { + this.timeout(50000) + const appealsPhaseId = uuid() + const appealsChallengePhaseId = uuid() + const submissionId = shortId() + const reviewId = uuid() + const reviewItemId = shortId() + const reviewItemCommentId = shortId() + const appealId = shortId() + + await prisma.phase.create({ + data: { + id: appealsPhaseId, + name: 'Appeals Response', + description: 'Appeals Response phase', + isOpen: true, + duration: 123, + createdBy: 'testuser', + updatedBy: 'testuser' + } + }) + + await prisma.challengePhase.create({ + data: { + id: appealsChallengePhaseId, + challengeId: data.challenge.id, + phaseId: appealsPhaseId, + name: 'Appeals Response', + isOpen: true, + duration: 1000, + actualStartDate: new Date(), + createdBy: 'testuser', + updatedBy: 'testuser' + } + }) + + try { + await reviewClient.$executeRaw( + Prisma.sql` + INSERT INTO ${submissionTable} ("id", "challengeId") + VALUES (${submissionId}, ${data.challenge.id}) + ON CONFLICT ("id") DO NOTHING + ` + ) + + await reviewClient.$executeRaw( + Prisma.sql` + INSERT INTO ${reviewTable} ("id", "phaseId", "submissionId", "status") + VALUES (${reviewId}, ${appealsChallengePhaseId}, ${submissionId}, ${'COMPLETED'}) + ON CONFLICT ("id") DO NOTHING + ` + ) + + await reviewClient.$executeRaw( + Prisma.sql` + INSERT INTO ${reviewItemTable} ("id", "reviewId") + VALUES (${reviewItemId}, ${reviewId}) + ON CONFLICT ("id") DO NOTHING + ` + ) + + await reviewClient.$executeRaw( + Prisma.sql` + INSERT INTO ${reviewItemCommentTable} ("id", "reviewItemId") + VALUES (${reviewItemCommentId}, ${reviewItemId}) + ON CONFLICT ("id") DO NOTHING + ` + ) + + await reviewClient.$executeRaw( + Prisma.sql` + INSERT INTO ${appealTable} ("id", "reviewItemCommentId") + VALUES (${appealId}, ${reviewItemCommentId}) + ON CONFLICT ("id") DO NOTHING + ` + ) + + let caughtError + try { + await service.partiallyUpdateChallengePhase( + authUser, + data.challenge.id, + appealsChallengePhaseId, + { isOpen: false } + ) + } catch (e) { + caughtError = e + } + + should.exist(caughtError) + should.equal(caughtError.httpStatus || caughtError.statusCode, 400) + should.equal( + caughtError.message, + "Appeals Response phase can't be closed because there are still appeals that haven't been responded to" + ) + } finally { + await reviewClient.$executeRaw( + Prisma.sql`DELETE FROM ${appealTable} WHERE "id" = ${appealId}` + ) + await reviewClient.$executeRaw( + Prisma.sql`DELETE FROM ${reviewItemCommentTable} WHERE "id" = ${reviewItemCommentId}` + ) + await reviewClient.$executeRaw( + Prisma.sql`DELETE FROM ${reviewItemTable} WHERE "id" = ${reviewItemId}` + ) + await reviewClient.$executeRaw( + Prisma.sql`DELETE FROM ${reviewTable} WHERE "id" = ${reviewId}` + ) + await reviewClient.$executeRaw( + Prisma.sql`DELETE FROM ${submissionTable} WHERE "id" = ${submissionId}` + ) + await prisma.challengePhase.delete({ + where: { id: appealsChallengePhaseId } + }) + await prisma.phase.delete({ + where: { id: appealsPhaseId } + }) + } + }) + it('partially update challenge phase - unexpected field', async () => { try { await service.partiallyUpdateChallengePhase(authUser, data.challenge.id, data.challengePhase1Id, { name: 'xx', other: 'xx' }) From 362af3fed1363153a4d14901b7fc25136440bf3c Mon Sep 17 00:00:00 2001 From: Hentry Martin Date: Mon, 27 Oct 2025 12:58:00 +0100 Subject: [PATCH 08/18] fix: added timeout for prisma client --- config/default.js | 1 + src/common/prisma.js | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/config/default.js b/config/default.js index b270dd8..ef8a2ef 100644 --- a/config/default.js +++ b/config/default.js @@ -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, }; diff --git a/src/common/prisma.js b/src/common/prisma.js index 69bdb54..19b05de 100644 --- a/src/common/prisma.js +++ b/src/common/prisma.js @@ -8,8 +8,12 @@ const { ReviewOpportunityTypeEnum, } = require("@prisma/client"); const logger = require("./logger"); +const config = require("config"); const prismaClient = new PrismaClient({ + transactionOptions: { + timeout: config.CHALLENGE_SERVICE_PRISMA_TIMEOUT, + }, log: [ { level: "query", emit: "event" }, { level: "info", emit: "event" }, From 21e760d25e41fac8f44b77279af45aa256034b4c Mon Sep 17 00:00:00 2001 From: Hentry Martin Date: Mon, 27 Oct 2025 12:58:18 +0100 Subject: [PATCH 09/18] fix: added timeout for prisma client --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 64c0915..5477ddb 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -103,7 +103,7 @@ workflows: - feature/top-262-projectid-non-mandatory - TOP-2364 - PM-2097 - - pm-2456 + - pm-2539 - "build-qa": context: org-global From 6bd3690e8e3f8d62f547fa2f78f0e66a27f0803e Mon Sep 17 00:00:00 2001 From: Kiril Kartunov Date: Tue, 28 Oct 2025 15:33:45 +0200 Subject: [PATCH 10/18] Add Trivy scanner workflow for vulnerability scanning --- .github/workflows/trivy.yaml | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 .github/workflows/trivy.yaml diff --git a/.github/workflows/trivy.yaml b/.github/workflows/trivy.yaml new file mode 100644 index 0000000..7b9fa48 --- /dev/null +++ b/.github/workflows/trivy.yaml @@ -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 + 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 }} + + - name: Upload Trivy scan results to GitHub Security tab + uses: github/codeql-action/upload-sarif@v3 + with: + sarif_file: "trivy-results.sarif" From ec0656f4510d4db2f88df5552ac426fb11f6d8af Mon Sep 17 00:00:00 2001 From: rishabhtc Date: Tue, 28 Oct 2025 20:58:15 +0530 Subject: [PATCH 11/18] Sort prizes --- src/services/ChallengeService.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index 7234221..c4bde00 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -217,7 +217,11 @@ const includeReturnFields = { }, events: true, prizeSets: { - include: { prizes: true }, + include: { + prizes: { + orderBy: { value: "desc" }, + }, + } }, reviewers: { orderBy: { createdAt: "asc" }, From fd82edf47654ea757ed2922808da1eb46066b25c Mon Sep 17 00:00:00 2001 From: Hentry Martin Date: Tue, 28 Oct 2025 18:35:27 +0100 Subject: [PATCH 12/18] fix: used CHALLENGE_SERVICE_PRISMA_TIMEOUT instead of PRISMA_TRANSACTION_TIMEOUT_MS --- src/common/prisma.js | 2 +- src/common/review-prisma.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/prisma.js b/src/common/prisma.js index 19b05de..1f49d85 100644 --- a/src/common/prisma.js +++ b/src/common/prisma.js @@ -25,7 +25,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 }, }); diff --git a/src/common/review-prisma.js b/src/common/review-prisma.js index efa9df1..12d0a36 100644 --- a/src/common/review-prisma.js +++ b/src/common/review-prisma.js @@ -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, }, }); From 4a20ab7d32e33499911844b58c9ee8da727f98b0 Mon Sep 17 00:00:00 2001 From: Hentry Martin Date: Tue, 28 Oct 2025 20:36:48 +0100 Subject: [PATCH 13/18] fix: lint --- src/common/prisma.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/common/prisma.js b/src/common/prisma.js index 1f49d85..7d302ea 100644 --- a/src/common/prisma.js +++ b/src/common/prisma.js @@ -11,9 +11,6 @@ const logger = require("./logger"); const config = require("config"); const prismaClient = new PrismaClient({ - transactionOptions: { - timeout: config.CHALLENGE_SERVICE_PRISMA_TIMEOUT, - }, log: [ { level: "query", emit: "event" }, { level: "info", emit: "event" }, From d34e20b5cbbd7f9aa788da1f830f434c01917656 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Wed, 29 Oct 2025 17:09:16 +1100 Subject: [PATCH 14/18] Allow for management of defaultchallengereviewers by admins --- app-constants.js | 6 + .../DefaultChallengeReviewerController.js | 95 +++ src/routes.js | 46 ++ .../DefaultChallengeReviewerService.js | 558 ++++++++++++++++++ 4 files changed, 705 insertions(+) create mode 100644 src/controllers/DefaultChallengeReviewerController.js create mode 100644 src/services/DefaultChallengeReviewerService.js diff --git a/app-constants.js b/app-constants.js index e86c05b..0c6f361 100644 --- a/app-constants.js +++ b/app-constants.js @@ -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", }; @@ -95,6 +98,9 @@ const DisabledTopics = [ Topics.ChallengeTimelineTemplateDeleted, Topics.ChallengePhaseUpdated, Topics.ChallengePhaseDeleted, + Topics.DefaultChallengeReviewerCreated, + Topics.DefaultChallengeReviewerUpdated, + Topics.DefaultChallengeReviewerDeleted, ]; const challengeTextSortField = { diff --git a/src/controllers/DefaultChallengeReviewerController.js b/src/controllers/DefaultChallengeReviewerController.js new file mode 100644 index 0000000..1c141ee --- /dev/null +++ b/src/controllers/DefaultChallengeReviewerController.js @@ -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); + 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); + 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( + 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( + 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( + 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( + req.params.defaultChallengeReviewerId + ); + res.send(result); +} + +module.exports = { + searchDefaultChallengeReviewers, + createDefaultChallengeReviewer, + getDefaultChallengeReviewer, + fullyUpdateDefaultChallengeReviewer, + partiallyUpdateDefaultChallengeReviewer, + deleteDefaultChallengeReviewer, +}; + diff --git a/src/routes.js b/src/routes.js index 09f941b..9178920 100644 --- a/src/routes.js +++ b/src/routes.js @@ -316,6 +316,52 @@ module.exports = { scopes: [DELETE, ALL], }, }, + "/default-challenge-reviewers": { + get: { + controller: "DefaultChallengeReviewerController", + method: "searchDefaultChallengeReviewers", + auth: "jwt", + access: [constants.UserRoles.Admin], + 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], + 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", diff --git a/src/services/DefaultChallengeReviewerService.js b/src/services/DefaultChallengeReviewerService.js new file mode 100644 index 0000000..d9439a7 --- /dev/null +++ b/src/services/DefaultChallengeReviewerService.js @@ -0,0 +1,558 @@ +/** + * This service provides operations for default challenge reviewers. + */ +const _ = require("lodash"); +const Joi = require("joi"); +const helper = require("../common/helper"); +const logger = require("../common/logger"); +const constants = require("../../app-constants"); +const errors = require("../common/errors"); + +const prismaModule = require("../common/prisma"); +const prisma = prismaModule.getClient(); +const { ReviewOpportunityTypeEnum } = prismaModule; + +const defaultInclude = { + challengeType: true, + challengeTrack: true, + timelineTemplate: true, + phase: true, +}; + +/** + * Normalize record by removing audit fields + * + * @param {Object} record the record to sanitize + * @returns {Object} sanitized record + */ +function sanitize(record) { + if (!record) { + return record; + } + const result = _.omit(record, constants.auditFields); + + if (record.challengeType) { + result.challengeType = _.omit(record.challengeType, constants.auditFields); + } + if (record.challengeTrack) { + result.challengeTrack = _.omit(record.challengeTrack, constants.auditFields); + } + if (record.timelineTemplate) { + result.timelineTemplate = _.omit(record.timelineTemplate, constants.auditFields); + } + if (record.phase) { + result.phase = _.omit(record.phase, constants.auditFields); + } + return result; +} + +/** + * Build search filter for prisma query. + * + * @param {Object} criteria search criteria + * @returns {Object} prisma filter + */ +function getSearchFilter(criteria = {}) { + const filter = {}; + + if (!_.isEmpty(criteria.typeId)) { + filter.typeId = { equals: criteria.typeId }; + } + if (!_.isEmpty(criteria.trackId)) { + filter.trackId = { equals: criteria.trackId }; + } + if (!_.isUndefined(criteria.timelineTemplateId)) { + filter.timelineTemplateId = _.isNil(criteria.timelineTemplateId) + ? { equals: null } + : { equals: criteria.timelineTemplateId }; + } + if (!_.isEmpty(criteria.phaseName)) { + filter.phaseName = { equals: criteria.phaseName }; + } + if (!_.isEmpty(criteria.scorecardId)) { + filter.scorecardId = { equals: criteria.scorecardId }; + } + + return filter; +} + +/** + * Search default challenge reviewers. + * + * @param {Object} criteria search criteria + * @returns {Promise} paginated result + */ +async function searchDefaultChallengeReviewers(criteria = {}) { + const searchFilter = getSearchFilter(_.omit(criteria, ["page", "perPage"])); + + const page = criteria.page || 1; + const perPage = criteria.perPage || 50; + + const [total, rows] = await Promise.all([ + prisma.defaultChallengeReviewer.count({ where: searchFilter }), + prisma.defaultChallengeReviewer.findMany({ + where: searchFilter, + orderBy: { createdAt: "asc" }, + skip: (page - 1) * perPage, + take: perPage, + include: defaultInclude, + }), + ]); + + return { + total, + page, + perPage, + result: _.map(rows, sanitize), + }; +} + +searchDefaultChallengeReviewers.schema = { + criteria: Joi.object().keys({ + page: Joi.page(), + perPage: Joi.perPage().default(50), + typeId: Joi.optionalId(), + trackId: Joi.optionalId(), + timelineTemplateId: Joi.optionalId().allow(null), + phaseName: Joi.string(), + scorecardId: Joi.string(), + }), +}; + +/** + * Ensure related entities exist. + * + * @param {Object} data payload + * @param {Boolean} isPartial indicates partial update + */ +async function validateRelatedEntities(data = {}, isPartial = false) { + const validations = []; + + const shouldValidate = (value) => (!isPartial || !_.isUndefined(value)); + + if (shouldValidate(data.typeId)) { + validations.push( + prisma.challengeType.findUnique({ where: { id: data.typeId } }).then((res) => { + if (!res) { + throw new errors.NotFoundError(`ChallengeType with id: ${data.typeId} doesn't exist`); + } + }) + ); + } + + if (shouldValidate(data.trackId)) { + validations.push( + prisma.challengeTrack.findUnique({ where: { id: data.trackId } }).then((res) => { + if (!res) { + throw new errors.NotFoundError(`ChallengeTrack with id: ${data.trackId} doesn't exist`); + } + }) + ); + } + + if (shouldValidate(data.timelineTemplateId) && !_.isNil(data.timelineTemplateId)) { + validations.push( + prisma.timelineTemplate.findUnique({ where: { id: data.timelineTemplateId } }).then((res) => { + if (!res) { + throw new errors.NotFoundError( + `TimelineTemplate with id: ${data.timelineTemplateId} doesn't exist` + ); + } + }) + ); + } + + let phaseByName; + if (shouldValidate(data.phaseName) && !_.isEmpty(data.phaseName)) { + validations.push( + prisma.phase.findUnique({ where: { name: data.phaseName } }).then((res) => { + if (!res) { + throw new errors.BadRequestError(`Invalid phaseName: ${data.phaseName}`); + } + phaseByName = res; + }) + ); + } + + let phaseById; + if (shouldValidate(data.phaseId) && !_.isNil(data.phaseId)) { + validations.push( + prisma.phase.findUnique({ where: { id: data.phaseId } }).then((res) => { + if (!res) { + throw new errors.NotFoundError(`Phase with id: ${data.phaseId} doesn't exist`); + } + phaseById = res; + }) + ); + } + + await Promise.all(validations); + + if (phaseByName && phaseById && phaseByName.id !== phaseById.id) { + throw new errors.BadRequestError( + `phaseId ${phaseById.id} does not match phaseName ${phaseByName.name}` + ); + } + + return { phaseByName, phaseById }; +} + +/** + * Normalize payload values for persistence. + * + * @param {Object} data incoming data + * @param {Boolean} isPartial whether payload is partial + * @returns {Object} normalized data + */ +function normalizePayload(data = {}, isPartial = false) { + const normalized = {}; + + const shouldAssign = (value) => (!isPartial || !_.isUndefined(value)); + + const toNullableId = (value) => (_.isNil(value) ? null : value); + const toNullableInteger = (value) => (_.isNil(value) ? null : Number(value)); + const toNullableNumber = (value) => (_.isNil(value) ? null : Number(value)); + const toOpportunityType = (value) => (_.isNil(value) ? null : _.toUpper(value)); + + if (shouldAssign(data.typeId)) { + normalized.typeId = data.typeId; + } + if (shouldAssign(data.trackId)) { + normalized.trackId = data.trackId; + } + if (shouldAssign(data.timelineTemplateId)) { + normalized.timelineTemplateId = toNullableId(data.timelineTemplateId); + } + if (shouldAssign(data.scorecardId)) { + normalized.scorecardId = _.isNil(data.scorecardId) ? null : String(data.scorecardId); + } + if (shouldAssign(data.isMemberReview)) { + normalized.isMemberReview = data.isMemberReview; + } + if (shouldAssign(data.memberReviewerCount)) { + normalized.memberReviewerCount = toNullableInteger(data.memberReviewerCount); + } else if (!isPartial && _.isNil(data.memberReviewerCount)) { + normalized.memberReviewerCount = null; + } + if (shouldAssign(data.phaseName)) { + normalized.phaseName = data.phaseName; + } + if (shouldAssign(data.phaseId)) { + normalized.phaseId = toNullableId(data.phaseId); + } + if (shouldAssign(data.fixedAmount)) { + normalized.fixedAmount = toNullableNumber(data.fixedAmount); + } else if (!isPartial && _.isNil(data.fixedAmount)) { + normalized.fixedAmount = null; + } + if (shouldAssign(data.baseCoefficient)) { + normalized.baseCoefficient = toNullableNumber(data.baseCoefficient); + } else if (!isPartial && _.isNil(data.baseCoefficient)) { + normalized.baseCoefficient = null; + } + if (shouldAssign(data.incrementalCoefficient)) { + normalized.incrementalCoefficient = toNullableNumber(data.incrementalCoefficient); + } else if (!isPartial && _.isNil(data.incrementalCoefficient)) { + normalized.incrementalCoefficient = null; + } + if (shouldAssign(data.opportunityType)) { + normalized.opportunityType = toOpportunityType(data.opportunityType); + } else if (!isPartial && _.isNil(data.opportunityType)) { + normalized.opportunityType = null; + } + if (shouldAssign(data.isAIReviewer)) { + normalized.isAIReviewer = data.isAIReviewer; + } + if (shouldAssign(data.shouldOpenOpportunity)) { + normalized.shouldOpenOpportunity = data.shouldOpenOpportunity; + } + + return normalized; +} + +/** + * Create a default challenge reviewer. + * + * @param {Object} authUser authenticated user + * @param {Object} data payload + * @returns {Promise} created record + */ +async function createDefaultChallengeReviewer(authUser, data) { + const references = await validateRelatedEntities(data); + + const userId = _.toString(authUser && authUser.userId ? authUser.userId : "system"); + const payload = normalizePayload(data); + if (references.phaseById) { + payload.phaseName = references.phaseById.name; + } else if (references.phaseByName) { + payload.phaseName = references.phaseByName.name; + } + payload.createdBy = userId; + payload.updatedBy = userId; + + const duplicate = await prisma.defaultChallengeReviewer.findFirst({ + where: { + typeId: payload.typeId, + trackId: payload.trackId, + timelineTemplateId: payload.timelineTemplateId, + phaseName: payload.phaseName, + scorecardId: payload.scorecardId, + isMemberReview: payload.isMemberReview, + }, + }); + if (duplicate) { + throw new errors.ConflictError( + "A default challenge reviewer already exists for the specified combination" + ); + } + + let ret = await prisma.defaultChallengeReviewer.create({ + data: payload, + include: defaultInclude, + }); + + ret = sanitize(ret); + await helper.postBusEvent(constants.Topics.DefaultChallengeReviewerCreated, ret); + return ret; +} + +createDefaultChallengeReviewer.schema = { + authUser: Joi.any(), + data: Joi.object() + .keys({ + typeId: Joi.id().required(), + trackId: Joi.id().required(), + timelineTemplateId: Joi.optionalId().allow(null), + scorecardId: Joi.string().required(), + isMemberReview: Joi.boolean().required(), + memberReviewerCount: Joi.when("isMemberReview", { + is: true, + then: Joi.number().integer().min(1).required(), + otherwise: Joi.valid(null), + }), + phaseName: Joi.string().required(), + phaseId: Joi.optionalId().allow(null), + fixedAmount: Joi.number().min(0).allow(null), + baseCoefficient: Joi.number().min(0).max(1).allow(null), + incrementalCoefficient: Joi.number().min(0).max(1).allow(null), + opportunityType: Joi.string().valid(..._.values(ReviewOpportunityTypeEnum)).insensitive(), + isAIReviewer: Joi.boolean().required(), + shouldOpenOpportunity: Joi.boolean().required(), + }) + .required(), +}; + +/** + * Retrieve a default challenge reviewer by id. + * + * @param {String} id record id + * @returns {Promise} default challenge reviewer + */ +async function getDefaultChallengeReviewer(id) { + const ret = await prisma.defaultChallengeReviewer.findUnique({ + where: { id }, + include: defaultInclude, + }); + if (!ret || _.isUndefined(ret.id)) { + throw new errors.NotFoundError(`DefaultChallengeReviewer with id: ${id} doesn't exist`); + } + return sanitize(ret); +} + +getDefaultChallengeReviewer.schema = { + id: Joi.id(), +}; + +/** + * Fully update a default challenge reviewer. + * + * @param {Object} authUser authenticated user + * @param {String} id record id + * @param {Object} data payload + * @returns {Promise} updated record + */ +async function fullyUpdateDefaultChallengeReviewer(authUser, id, data) { + await getDefaultChallengeReviewer(id); + const references = await validateRelatedEntities(data); + + const payload = normalizePayload(data); + if (references.phaseById) { + payload.phaseName = references.phaseById.name; + } else if (references.phaseByName) { + payload.phaseName = references.phaseByName.name; + } + payload.updatedBy = _.toString(authUser && authUser.userId ? authUser.userId : "system"); + + let ret = await prisma.defaultChallengeReviewer.update({ + where: { id }, + data: payload, + include: defaultInclude, + }); + + ret = sanitize(ret); + await helper.postBusEvent(constants.Topics.DefaultChallengeReviewerUpdated, ret); + return ret; +} + +fullyUpdateDefaultChallengeReviewer.schema = { + authUser: Joi.any(), + id: Joi.id(), + data: Joi.object() + .keys({ + typeId: Joi.id().required(), + trackId: Joi.id().required(), + timelineTemplateId: Joi.optionalId().allow(null), + scorecardId: Joi.string().required(), + isMemberReview: Joi.boolean().required(), + memberReviewerCount: Joi.when("isMemberReview", { + is: true, + then: Joi.number().integer().min(1).required(), + otherwise: Joi.valid(null), + }), + phaseName: Joi.string().required(), + phaseId: Joi.optionalId().allow(null), + fixedAmount: Joi.number().min(0).allow(null), + baseCoefficient: Joi.number().min(0).max(1).allow(null), + incrementalCoefficient: Joi.number().min(0).max(1).allow(null), + opportunityType: Joi.string().valid(..._.values(ReviewOpportunityTypeEnum)).insensitive(), + isAIReviewer: Joi.boolean().required(), + shouldOpenOpportunity: Joi.boolean().required(), + }) + .required(), +}; + +/** + * Partially update a default challenge reviewer. + * + * @param {Object} authUser authenticated user + * @param {String} id record id + * @param {Object} data payload + * @returns {Promise} updated record + */ +async function partiallyUpdateDefaultChallengeReviewer(authUser, id, data) { + const existing = await getDefaultChallengeReviewer(id); + const references = await validateRelatedEntities(data, true); + + const payload = normalizePayload(data, true); + if (_.isUndefined(data.phaseName) && !_.isUndefined(data.phaseId) && references.phaseById) { + payload.phaseName = references.phaseById.name; + } + + const targetIsMemberReview = !_.isUndefined(data.isMemberReview) + ? data.isMemberReview + : existing.isMemberReview; + const memberCountProvided = _.has(data, "memberReviewerCount"); + const opportunityTypeProvided = _.has(data, "opportunityType"); + + if (targetIsMemberReview) { + if (memberCountProvided) { + if (_.isNil(data.memberReviewerCount)) { + throw new errors.BadRequestError( + "memberReviewerCount cannot be null when isMemberReview is true" + ); + } + } else if (_.isNil(existing.memberReviewerCount)) { + throw new errors.BadRequestError( + "memberReviewerCount must be provided when isMemberReview is true" + ); + } + } else { + if (memberCountProvided && !_.isNil(data.memberReviewerCount)) { + throw new errors.BadRequestError( + "memberReviewerCount is only allowed when isMemberReview is true" + ); + } + if (opportunityTypeProvided && !_.isNil(data.opportunityType)) { + throw new errors.BadRequestError( + "opportunityType is only allowed when isMemberReview is true" + ); + } + } + + if (!targetIsMemberReview) { + if (memberCountProvided || data.isMemberReview === false) { + payload.memberReviewerCount = null; + } + if (opportunityTypeProvided || data.isMemberReview === false) { + payload.opportunityType = null; + } + } + + payload.updatedBy = _.toString(authUser && authUser.userId ? authUser.userId : "system"); + + let ret = await prisma.defaultChallengeReviewer.update({ + where: { id }, + data: payload, + include: defaultInclude, + }); + + ret = sanitize(ret); + await helper.postBusEvent( + constants.Topics.DefaultChallengeReviewerUpdated, + _.assignIn({ id }, _.omit(payload, ["updatedBy"])) + ); + return ret; +} + +partiallyUpdateDefaultChallengeReviewer.schema = { + authUser: Joi.any(), + id: Joi.id(), + data: Joi.object() + .keys({ + typeId: Joi.optionalId(), + trackId: Joi.optionalId(), + timelineTemplateId: Joi.optionalId().allow(null), + scorecardId: Joi.string(), + isMemberReview: Joi.boolean(), + memberReviewerCount: Joi.number().integer().min(1).allow(null), + phaseName: Joi.string(), + phaseId: Joi.optionalId().allow(null), + fixedAmount: Joi.number().min(0).allow(null), + baseCoefficient: Joi.number().min(0).max(1).allow(null), + incrementalCoefficient: Joi.number().min(0).max(1).allow(null), + opportunityType: Joi.string() + .valid(..._.values(ReviewOpportunityTypeEnum)) + .insensitive() + .allow(null), + isAIReviewer: Joi.boolean(), + shouldOpenOpportunity: Joi.boolean(), + }) + .required(), +}; + +/** + * Delete a default challenge reviewer. + * + * @param {String} id record id + * @returns {Promise} deleted record + */ +async function deleteDefaultChallengeReviewer(id) { + const existing = await prisma.defaultChallengeReviewer.findUnique({ + where: { id }, + include: defaultInclude, + }); + if (!existing || _.isUndefined(existing.id)) { + throw new errors.NotFoundError(`DefaultChallengeReviewer with id: ${id} doesn't exist`); + } + + await prisma.defaultChallengeReviewer.delete({ where: { id } }); + + const ret = sanitize(existing); + await helper.postBusEvent(constants.Topics.DefaultChallengeReviewerDeleted, ret); + return ret; +} + +deleteDefaultChallengeReviewer.schema = { + id: Joi.id(), +}; + +module.exports = { + searchDefaultChallengeReviewers, + createDefaultChallengeReviewer, + getDefaultChallengeReviewer, + fullyUpdateDefaultChallengeReviewer, + partiallyUpdateDefaultChallengeReviewer, + deleteDefaultChallengeReviewer, +}; + +logger.buildService(module.exports); From 1c69c3874f48224f07c3f7b60df8376fc68ebf44 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Thu, 30 Oct 2025 11:16:29 +1100 Subject: [PATCH 15/18] Incremental update tweaks --- data-migration/src/config.js | 97 +++++- data-migration/src/migrators/_baseMigrator.js | 191 ++++++++++- data-migration/src/utils/dataLoader.js | 308 +++++++++++++++--- 3 files changed, 526 insertions(+), 70 deletions(-) diff --git a/data-migration/src/config.js b/data-migration/src/config.js index 60f92a6..729c4fd 100644 --- a/data-migration/src/config.js +++ b/data-migration/src/config.js @@ -14,8 +14,81 @@ const parseMigrationMode = value => { return ALLOWED_MIGRATION_MODES.has(normalized) ? normalized : 'full'; }; -// Default configuration with fallbacks -module.exports = { +const RECOMMENDED_INCREMENTAL_FIELDS = ['updatedAt', 'updatedBy']; + +const collectKnownSchemaFields = migratorConfig => { + const knownFields = new Set(); + if (!migratorConfig) { + return knownFields; + } + + Object.values(migratorConfig).forEach(definition => { + if (!definition || typeof definition !== 'object') { + return; + } + (definition.requiredFields || []).forEach(field => knownFields.add(field)); + (definition.optionalFields || []).forEach(field => knownFields.add(field)); + const defaults = definition.hasDefaults || []; + defaults.forEach(field => knownFields.add(field)); + }); + + return knownFields; +}; + +const validateIncrementalFieldConfiguration = (config) => { + const incrementalFields = Array.isArray(config.INCREMENTAL_FIELDS) ? config.INCREMENTAL_FIELDS : []; + const isIncrementalMode = config.MIGRATION_MODE === 'incremental'; + const knownFields = collectKnownSchemaFields(config.migrator); + + const result = { + warnings: [], + errors: [] + }; + + if (isIncrementalMode && incrementalFields.length === 0) { + result.errors.push('INCREMENTAL_FIELDS must be configured when running incremental migrations.'); + } + + incrementalFields.forEach(field => { + if (!knownFields.has(field)) { + result.warnings.push(`Incremental field "${field}" is not present in any migrator schema and will be ignored.`); + } + }); + + const recommendedMissing = RECOMMENDED_INCREMENTAL_FIELDS.filter(field => incrementalFields.length > 0 && !incrementalFields.includes(field)); + if (recommendedMissing.length) { + result.warnings.push(`Consider including ${recommendedMissing.join(', ')} in INCREMENTAL_FIELDS to preserve audit columns during updates.`); + } + + if (incrementalFields.length) { + console.info(`[config] Incremental fields configured: ${incrementalFields.join(', ')}`); + } + + result.warnings.forEach(message => console.warn(`[config] ${message}`)); + result.errors.forEach(message => console.error(`[config] ${message}`)); + + return result; +}; + +/** + * Migration configuration with environment-driven overrides. + * + * Environment variables: + * - DATABASE_URL: Postgres connection string. + * - DATA_DIRECTORY: Root directory where migration payloads are stored. + * - MIGRATION_MODE: 'full' or 'incremental' (defaults to 'full'). + * - INCREMENTAL_SINCE_DATE: ISO-8601 date used to filter source records. + * - INCREMENTAL_FIELDS: Comma-separated list of fields updated during incremental runs. + * - INCREMENTAL_FIELDS examples: + * INCREMENTAL_FIELDS=updatedAt,updatedBy,status + * INCREMENTAL_FIELDS=updatedAt,updatedBy,status,currentPhaseNames + * Fields not listed remain unchanged during incremental updates. + * - INCREMENTAL_DATE_FIELDS: Ordered list of timestamp fields to evaluate for incremental filtering (default: updatedAt,updated). + * - MISSING_DATE_FIELD_BEHAVIOR: Behaviour when timestamps are missing (skip, include, warn-and-skip, warn-and-include). + * - INVALID_DATE_FIELD_BEHAVIOR: Behaviour when timestamps are invalid or suspicious (same options as above). + * - CREATED_BY / UPDATED_BY: Attribution columns written during migration. + */ +const config = { // Database connection DATABASE_URL: process.env.DATABASE_URL, @@ -24,6 +97,9 @@ module.exports = { BATCH_SIZE: parseInt(process.env.BATCH_SIZE || '100', 10), CONCURRENCY_LIMIT: parseInt(process.env.CONCURRENCY_LIMIT || '10', 10), LOG_LEVEL: process.env.LOG_LEVEL || 'info', + LOG_VERBOSITY: (process.env.LOG_VERBOSITY || 'normal').toLowerCase(), + SUMMARY_LOG_LIMIT: parseInt(process.env.SUMMARY_LOG_LIMIT || '5', 10), + COLLECT_UPSERT_STATS: process.env.COLLECT_UPSERT_STATS === 'true', // Migration behavior SKIP_MISSING_REQUIRED: process.env.SKIP_MISSING_REQUIRED === 'true', @@ -34,7 +110,15 @@ module.exports = { // Incremental migration settings MIGRATION_MODE: parseMigrationMode(process.env.MIGRATION_MODE), INCREMENTAL_SINCE_DATE: process.env.INCREMENTAL_SINCE_DATE || null, + /** + * Fields that should be mutated when MIGRATION_MODE=incremental. + * Only columns listed here are included in UPDATE operations; omitted columns retain their existing values. + * Works in tandem with INCREMENTAL_SINCE_DATE to scope the incremental window. + */ INCREMENTAL_FIELDS: parseListEnv(process.env.INCREMENTAL_FIELDS), + INCREMENTAL_DATE_FIELDS: parseListEnv(process.env.INCREMENTAL_DATE_FIELDS) || ['updatedAt', 'updated'], + MISSING_DATE_FIELD_BEHAVIOR: (process.env.MISSING_DATE_FIELD_BEHAVIOR || 'warn-and-skip').toLowerCase(), + INVALID_DATE_FIELD_BEHAVIOR: (process.env.INVALID_DATE_FIELD_BEHAVIOR || 'warn-and-skip').toLowerCase(), // Migration attribution CREATED_BY: process.env.CREATED_BY || 'migration', @@ -424,7 +508,16 @@ module.exports = { }, }; +config.incrementalFieldValidation = validateIncrementalFieldConfiguration(config); + +module.exports = config; + Object.defineProperty(module.exports, 'parseMigrationMode', { value: parseMigrationMode, enumerable: false }); + +Object.defineProperty(module.exports, 'validateIncrementalFieldConfiguration', { + value: validateIncrementalFieldConfiguration, + enumerable: false +}); diff --git a/data-migration/src/migrators/_baseMigrator.js b/data-migration/src/migrators/_baseMigrator.js index d080f4b..d68e4cc 100644 --- a/data-migration/src/migrators/_baseMigrator.js +++ b/data-migration/src/migrators/_baseMigrator.js @@ -18,6 +18,7 @@ class BaseMigrator { this.requiredFields = this.manager.config.migrator?.[this.modelName]?.requiredFields || []; this.optionalFields = this.manager.config.migrator?.[this.modelName]?.optionalFields || []; this.validIds = new Set(); + this.collectUpsertStats = this.manager.config.COLLECT_UPSERT_STATS === true; } /** @@ -69,6 +70,28 @@ class BaseMigrator { const incrementalFields = Array.isArray(this.manager.config.INCREMENTAL_FIELDS) ? this.manager.config.INCREMENTAL_FIELDS : []; + this._incrementalFieldStats = null; + this._upsertStats = null; + + if (isIncremental) { + if (!sinceDate || sinceDate === 'unspecified date') { + this.manager.logger.warn(`${this.modelName} incremental run has no INCREMENTAL_SINCE_DATE; entire dataset will be considered.`); + } else { + const sinceDateObj = new Date(sinceDate); + if (Number.isNaN(sinceDateObj.getTime())) { + this.manager.logger.error(`${this.modelName} incremental run received invalid INCREMENTAL_SINCE_DATE "${sinceDate}".`); + } else { + const now = new Date(); + if (sinceDateObj > now) { + this.manager.logger.error(`${this.modelName} incremental run date ${sinceDateObj.toISOString()} is in the future; this may yield zero records.`); + } + const oneYearMs = 365 * 24 * 60 * 60 * 1000; + if ((now - sinceDateObj) > oneYearMs) { + this.manager.logger.warn(`${this.modelName} incremental run uses a date older than one year (${sinceDateObj.toISOString()}); verify this is intentional.`); + } + } + } + } if (isIncremental) { this.manager.logger.info(`Running in INCREMENTAL mode since ${sinceDate}`); @@ -80,6 +103,11 @@ class BaseMigrator { } const data = await this.loadData(); + if (isIncremental) { + this.manager.logger.info(`Incremental filter retained ${data.length} ${this.modelName} records for processing.`); + } else { + this.manager.logger.info(`Loaded ${data.length} ${this.modelName} records for full migration.`); + } // Allow subclasses to perform pre-processing const processedData = await this.beforeMigration(data); @@ -92,6 +120,9 @@ class BaseMigrator { // Allow subclasses to perform post-processing await this.afterMigration(result); + + this.reportIncrementalFieldStats(); + this.reportUpsertStats(); // Return statistics for this migrator return { @@ -304,13 +335,19 @@ class BaseMigrator { const incrementalFields = Array.isArray(this.manager.config.INCREMENTAL_FIELDS) ? this.manager.config.INCREMENTAL_FIELDS : []; + const stats = this._ensureIncrementalFieldStats(); + stats.totalProcessed += 1; if (!incrementalFields.length) { + if (!stats.emptyWarningLogged) { + stats.emptyWarningLogged = true; + this.manager.logger.warn(`${this.modelName} incremental migration has no INCREMENTAL_FIELDS configured; falling back to full updates.`); + } + stats.fallbackToFull = true; return this.createUpsertData(record, idField); } const updateData = {}; - const missingFields = []; for (const field of incrementalFields) { if (field === idField) { @@ -319,25 +356,28 @@ class BaseMigrator { if (record[field] !== undefined) { updateData[field] = record[field]; + stats.updatedCounts.set(field, (stats.updatedCounts.get(field) || 0) + 1); } else { updateData[field] = Prisma.skip; - missingFields.push(field); + stats.missingCounts.set(field, (stats.missingCounts.get(field) || 0) + 1); + + if (!stats.missingExamples.has(field)) { + const recordIdentifier = record[idField] ?? '[unknown id]'; + stats.missingExamples.set(field, recordIdentifier !== undefined ? [recordIdentifier] : []); + // TODO: expose warnAlways flag in configuration when more granular logging is required. + this.manager.logger.warn(`${this.modelName} record ${recordIdentifier} is missing incremental field "${field}"; skipping updates for this field on this record.`); + } else { + const examples = stats.missingExamples.get(field); + if (examples.length < 5 && record[idField] !== undefined && !examples.includes(record[idField])) { + examples.push(record[idField]); + } + } } } updateData.updatedAt = record.updatedAt ? new Date(record.updatedAt) : new Date(); updateData.updatedBy = record.updatedBy; - if (missingFields.length) { - this._missingIncrementalFieldWarnings = this._missingIncrementalFieldWarnings || new Set(); - for (const field of missingFields) { - if (!this._missingIncrementalFieldWarnings.has(field)) { - this._missingIncrementalFieldWarnings.add(field); - this.manager.logger.warn(`Configured incremental field "${field}" is missing on some ${this.modelName} records; skipping updates for this field`); - } - } - } - const createData = { ...record }; createData.createdAt = record.createdAt ? new Date(record.createdAt) : new Date(); @@ -348,6 +388,20 @@ class BaseMigrator { }; } + _ensureIncrementalFieldStats() { + if (!this._incrementalFieldStats) { + this._incrementalFieldStats = { + totalProcessed: 0, + fallbackToFull: false, + emptyWarningLogged: false, + updatedCounts: new Map(), + missingCounts: new Map(), + missingExamples: new Map() + }; + } + return this._incrementalFieldStats; + } + /** * Initialize unique trackers for this model */ @@ -450,6 +504,112 @@ class BaseMigrator { customizeUpsertData(upsertData, _record) { return upsertData; // Default implementation returns unmodified data } + + _extractUpdatedFields(upsertData) { + const updateSection = upsertData?.update || {}; + const idField = this.getIdField(); + return Object.entries(updateSection) + .filter(([field, value]) => field !== idField && value !== Prisma.skip) + .map(([field]) => field); + } + + _recordUpsertStats(operation, durationMs, upsertData) { + if (!this._upsertStats) { + this._upsertStats = { + creates: 0, + updates: 0, + totalDurationMs: 0, + totalOperations: 0, + fieldUpdateCounts: new Map() + }; + } + + if (operation === 'create') { + this._upsertStats.creates += 1; + } else { + this._upsertStats.updates += 1; + } + this._upsertStats.totalDurationMs += durationMs; + this._upsertStats.totalOperations += 1; + + const fields = this._extractUpdatedFields(upsertData); + fields.forEach(field => { + this._upsertStats.fieldUpdateCounts.set(field, (this._upsertStats.fieldUpdateCounts.get(field) || 0) + 1); + }); + } + + reportIncrementalFieldStats() { + if (!this.manager.isIncrementalMode() || !this._incrementalFieldStats) { + return; + } + + const stats = this._incrementalFieldStats; + if (stats.fallbackToFull) { + this.manager.logger.warn(`${this.modelName} incremental run defaulted to full updates because INCREMENTAL_FIELDS was empty.`); + } + + if (stats.updatedCounts.size) { + const updatedSummary = Array.from(stats.updatedCounts.entries()) + .sort((a, b) => b[1] - a[1]) + .map(([field, count]) => `${field}=${count}`) + .join(', '); + this.manager.logger.info(`${this.modelName} incremental field coverage: ${updatedSummary}`); + } + + if (stats.missingCounts.size) { + stats.missingCounts.forEach((count, field) => { + const examples = stats.missingExamples.get(field) || []; + this.manager.logger.warn(`${this.modelName} missing incremental field "${field}" on ${count} record(s). Example IDs: ${examples.join(', ') || 'none captured'}`); + }); + } + } + + reportUpsertStats() { + if (!this.collectUpsertStats || !this._upsertStats || !this._upsertStats.totalOperations) { + return; + } + + const { creates = 0, updates = 0, totalDurationMs, totalOperations, fieldUpdateCounts } = this._upsertStats; + const averageDuration = (totalDurationMs / totalOperations).toFixed(2); + this.manager.logger.info(`${this.modelName} upsert summary: ${creates} creates, ${updates} updates, avg ${averageDuration}ms per operation.`); + + if (fieldUpdateCounts && fieldUpdateCounts.size) { + const topFields = Array.from(fieldUpdateCounts.entries()) + .sort((a, b) => b[1] - a[1]) + .slice(0, 10) + .map(([field, count]) => `${field}=${count}`) + .join(', '); + this.manager.logger.debug(`${this.modelName} most frequently updated fields: ${topFields}`); + } + } + + async _executeUpsertWithStats(prisma, upsertData) { + const start = Date.now(); + let existing = null; + if (this.collectUpsertStats) { + try { + existing = await prisma[this.queryName].findUnique({ + where: upsertData.where + }); + } catch (lookupError) { + this.manager.logger.debug(`Unable to pre-fetch ${this.modelName} ${this.describeRecord(upsertData)} before upsert: ${lookupError.message}`); + } + } + const data = await prisma[this.queryName].upsert(upsertData); + const durationMs = Date.now() - start; + if (this.collectUpsertStats) { + const operation = existing ? 'update' : 'create'; + const updatedFields = this._extractUpdatedFields(upsertData); + const updatedFieldSummary = updatedFields.length ? updatedFields.join(', ') : 'none'; + + this.manager.logger.debug(`${this.modelName} ${operation} completed in ${durationMs}ms for ${this.describeRecord(upsertData)} (updated fields: ${updatedFieldSummary}).`); + this._recordUpsertStats(operation, durationMs, upsertData); + } else { + this.manager.logger.debug(`${this.modelName} upsert completed in ${durationMs}ms for ${this.describeRecord(upsertData)}.`); + } + + return { data }; + } /** * Perform the upsert operation @@ -459,15 +619,13 @@ class BaseMigrator { */ async performUpsert(prisma, upsertData) { try { - const data = await prisma[this.queryName].upsert(upsertData); - return { data }; + return await this._executeUpsertWithStats(prisma, upsertData); } catch (error) { const resolution = await this.handleUpsertError(error, upsertData); if (resolution?.retryData) { try { - const data = await prisma[this.queryName].upsert(resolution.retryData); - return { data }; + return await this._executeUpsertWithStats(prisma, resolution.retryData); } catch (retryError) { this.manager.logger.error(`Retry upsert failed for ${this.modelName} ${this.describeRecord(upsertData)}:`, retryError); return { skip: true }; @@ -475,6 +633,7 @@ class BaseMigrator { } if (resolution?.skip) { + this.manager.logger.warn(`Skipping ${this.modelName} ${this.describeRecord(upsertData)} due to unresolved upsert error: ${error.message}`); return { skip: true }; } diff --git a/data-migration/src/utils/dataLoader.js b/data-migration/src/utils/dataLoader.js index 4ed5677..fda7628 100644 --- a/data-migration/src/utils/dataLoader.js +++ b/data-migration/src/utils/dataLoader.js @@ -2,6 +2,37 @@ const fs = require('fs'); const path = require('path'); const JSONStream = require('JSONStream'); const es = require('event-stream'); +const config = require('../config'); +const isDebugLog = (config.LOG_VERBOSITY || '').toLowerCase() === 'debug'; +const summaryLimit = Number.isFinite(config.SUMMARY_LOG_LIMIT) + ? Math.max(0, config.SUMMARY_LOG_LIMIT) + : 5; +const exampleLimit = summaryLimit || 5; + +const BEHAVIOR_MAP = { + skip: { warn: false, include: false, strategy: 'skip' }, + include: { warn: false, include: true, strategy: 'include' }, + 'warn-and-skip': { warn: true, include: false, strategy: 'warn-and-skip' }, + 'warn-and-include': { warn: true, include: true, strategy: 'warn-and-include' } +}; + +const parseBehavior = (behavior) => { + const normalized = (behavior || '').toLowerCase().trim(); + return BEHAVIOR_MAP[normalized] || BEHAVIOR_MAP.skip; +}; + +const getRecordIdentifier = (record) => { + if (!record || typeof record !== 'object') { + return 'unknown'; + } + const candidateKeys = ['id', 'challengeId', 'legacyId', 'name', 'slug', 'referenceId']; + for (const key of candidateKeys) { + if (record[key] !== undefined && record[key] !== null) { + return record[key]; + } + } + return 'unknown'; +}; /** * Load and parse JSON data from a file * @param {string} dataDir Directory containing data files @@ -25,73 +56,266 @@ async function loadData(dataDir, fileName, isElasticsearch = false, sinceDate = } } + const dateFieldPriority = Array.isArray(config.INCREMENTAL_DATE_FIELDS) && config.INCREMENTAL_DATE_FIELDS.length + ? config.INCREMENTAL_DATE_FIELDS + : ['updatedAt', 'updated']; + + const missingDateBehavior = parseBehavior(config.MISSING_DATE_FIELD_BEHAVIOR); + const invalidDateBehavior = parseBehavior(config.INVALID_DATE_FIELD_BEHAVIOR); + const hasValidSinceDate = hasSinceDate && sinceDateTimestamp !== null; + let missingDateFieldCount = 0; + let missingDateIncludedCount = 0; + let missingDateSkippedCount = 0; + const missingDateExamples = []; + let invalidDateFieldCount = 0; + let invalidDateIncludedCount = 0; + let invalidDateSkippedCount = 0; + const invalidDateExamples = []; + + let futureDateCount = 0; + let ancientDateCount = 0; + const futureDateExamples = []; + const ancientDateExamples = []; + let parseErrorCount = 0; - const shouldIncludeRecord = (record, sinceDateValue) => { - if (!sinceDateValue || sinceDateTimestamp === null) { - return true; - } + const dateFieldUsageAll = new Map(); + const dateFieldUsageIncluded = new Map(); + const dateHistogram = new Map(); - if (!record || typeof record !== 'object') { - missingDateFieldCount += 1; - return false; + let minTimestamp = null; + let maxTimestamp = null; + + let recordsWithAllDateFields = 0; + let recordsWithSingleDateField = 0; + let recordsWithNoDateFields = 0; + + const evaluateRecord = (record) => { + const outcome = { include: true, usedField: null, parsedDate: null, reason: null }; + const recordIdentifier = getRecordIdentifier(record); + + const availableFields = []; + if (record && typeof record === 'object') { + for (const field of dateFieldPriority) { + if (record[field] !== undefined && record[field] !== null) { + availableFields.push(field); + } + } } - let recordDateValue = null; - if (record.updatedAt !== undefined && record.updatedAt !== null) { - recordDateValue = record.updatedAt; - } else if (record.updated !== undefined && record.updated !== null) { - recordDateValue = record.updated; + if (availableFields.length === dateFieldPriority.length && availableFields.length > 0) { + recordsWithAllDateFields += 1; + } else if (availableFields.length > 0) { + recordsWithSingleDateField += 1; + } else { + recordsWithNoDateFields += 1; } - if (recordDateValue === null || recordDateValue === undefined) { + const usedField = availableFields[0] || null; + if (!usedField) { missingDateFieldCount += 1; - return false; + if (missingDateExamples.length < exampleLimit) { + missingDateExamples.push(recordIdentifier); + } + if (missingDateBehavior.warn) { + console.warn(`${fileName}: record ${recordIdentifier} missing date fields (${dateFieldPriority.join(', ')}); strategy=${missingDateBehavior.strategy}`); + } + if (missingDateBehavior.include) { + missingDateIncludedCount += 1; + return outcome; + } + missingDateSkippedCount += 1; + outcome.include = false; + outcome.reason = 'missing-date'; + return outcome; } - const parsedRecordDate = new Date(recordDateValue); - if (Number.isNaN(parsedRecordDate.getTime())) { + outcome.usedField = usedField; + const recordDateValue = record[usedField]; + const parsedDate = new Date(recordDateValue); + if (Number.isNaN(parsedDate.getTime())) { invalidDateFieldCount += 1; - return false; + if (invalidDateExamples.length < exampleLimit) { + invalidDateExamples.push({ id: recordIdentifier, value: recordDateValue }); + } + if (invalidDateBehavior.warn) { + console.warn(`${fileName}: record ${recordIdentifier} has invalid date "${recordDateValue}" in field ${usedField}; strategy=${invalidDateBehavior.strategy}`); + } + if (invalidDateBehavior.include) { + invalidDateIncludedCount += 1; + return outcome; + } + invalidDateSkippedCount += 1; + outcome.include = false; + outcome.reason = 'invalid-date'; + return outcome; } - return parsedRecordDate.getTime() >= sinceDateTimestamp; + outcome.parsedDate = parsedDate; + const nowTimestamp = Date.now(); + let suspiciousReason = null; + + if (parsedDate.getTime() > nowTimestamp) { + futureDateCount += 1; + suspiciousReason = 'future-date'; + if (futureDateExamples.length < exampleLimit) { + futureDateExamples.push({ id: recordIdentifier, value: parsedDate.toISOString() }); + } + } + + if (parsedDate.getFullYear() < 2000) { + ancientDateCount += 1; + suspiciousReason = suspiciousReason ? `${suspiciousReason}|ancient-date` : 'ancient-date'; + if (ancientDateExamples.length < exampleLimit) { + ancientDateExamples.push({ id: recordIdentifier, value: parsedDate.toISOString() }); + } + } + + if (suspiciousReason) { + if (invalidDateBehavior.warn) { + console.warn(`${fileName}: record ${recordIdentifier} has ${suspiciousReason.replace('|', ' & ')} (${parsedDate.toISOString()}); strategy=${invalidDateBehavior.strategy}`); + } + if (!invalidDateBehavior.include) { + invalidDateSkippedCount += 1; + outcome.include = false; + outcome.reason = suspiciousReason; + return outcome; + } + invalidDateIncludedCount += 1; + } + + if (hasValidSinceDate && parsedDate.getTime() < sinceDateTimestamp) { + outcome.include = false; + outcome.reason = 'out-of-window'; + } + + return outcome; }; try { let totalRecordsEncountered = 0; let recordsAfterFilter = 0; + const processRecord = (record, resultsArray) => { + totalRecordsEncountered += 1; + const evaluation = evaluateRecord(record); + + if (evaluation.usedField) { + dateFieldUsageAll.set(evaluation.usedField, (dateFieldUsageAll.get(evaluation.usedField) || 0) + 1); + } + + if (evaluation.include) { + resultsArray.push(record); + recordsAfterFilter += 1; + + if (evaluation.usedField) { + dateFieldUsageIncluded.set(evaluation.usedField, (dateFieldUsageIncluded.get(evaluation.usedField) || 0) + 1); + } + + if (evaluation.parsedDate) { + const timestamp = evaluation.parsedDate.getTime(); + if (minTimestamp === null || timestamp < minTimestamp) { + minTimestamp = timestamp; + } + if (maxTimestamp === null || timestamp > maxTimestamp) { + maxTimestamp = timestamp; + } + const dayKey = evaluation.parsedDate.toISOString().slice(0, 10); + dateHistogram.set(dayKey, (dateHistogram.get(dayKey) || 0) + 1); + } + } + }; + const logFilteringSummary = () => { const filteredOut = totalRecordsEncountered - recordsAfterFilter; - if (hasSinceDate && sinceDateTimestamp !== null) { + if (hasValidSinceDate) { console.info(`Filtered ${fileName}: ${recordsAfterFilter}/${totalRecordsEncountered} records (${filteredOut} filtered out) since ${sinceDate}`); - if (missingDateFieldCount > 0) { - console.warn(`${fileName}: ${missingDateFieldCount} records skipped due to missing updatedAt/updated fields while applying sinceDate ${sinceDate}`); - } - if (invalidDateFieldCount > 0) { - console.warn(`${fileName}: ${invalidDateFieldCount} records skipped due to invalid date values while applying sinceDate ${sinceDate}`); - } - } else if (hasSinceDate && sinceDateTimestamp === null) { + } else if (hasSinceDate && !hasValidSinceDate) { console.info(`Loaded ${fileName}: ${recordsAfterFilter}/${totalRecordsEncountered} records (invalid sinceDate provided, no filtering applied)`); } else { console.info(`Loaded ${fileName}: ${totalRecordsEncountered} records (no date filter)`); } + if (recordsAfterFilter === 0) { + console.warn(`${fileName}: no records matched the provided date filter.`); + } else if (hasValidSinceDate && filteredOut === 0) { + console.warn(`${fileName}: 100% of records matched the incremental filter; validate INCREMENTAL_SINCE_DATE (${sinceDate}).`); + } + + if (missingDateFieldCount > 0) { + const examples = exampleLimit > 0 ? missingDateExamples.slice(0, exampleLimit).join(', ') : 'none'; + console.warn(`${fileName}: ${missingDateFieldCount} records missing date fields (${missingDateBehavior.strategy}); included=${missingDateIncludedCount}, skipped=${missingDateSkippedCount}. Examples: ${examples}`); + } + + if (invalidDateFieldCount > 0 || invalidDateSkippedCount > 0) { + const invalidExamples = exampleLimit > 0 + ? invalidDateExamples.slice(0, exampleLimit).map(example => `${example.id}:${example.value}`).join(', ') + : 'none'; + console.warn(`${fileName}: ${invalidDateFieldCount} records with invalid date values; included=${invalidDateIncludedCount}, skipped=${invalidDateSkippedCount}. Examples: ${invalidExamples}`); + } + + if (futureDateCount > 0) { + const futureExamples = exampleLimit > 0 + ? futureDateExamples.slice(0, exampleLimit).map(example => `${example.id}:${example.value}`).join(', ') + : 'none'; + console.warn(`${fileName}: ${futureDateCount} records have future timestamps. Examples: ${futureExamples}`); + } + if (ancientDateCount > 0) { + const ancientExamples = exampleLimit > 0 + ? ancientDateExamples.slice(0, exampleLimit).map(example => `${example.id}:${example.value}`).join(', ') + : 'none'; + console.warn(`${fileName}: ${ancientDateCount} records have timestamps before the year 2000. Examples: ${ancientExamples}`); + } + + if (dateFieldUsageIncluded.size) { + let includedSummaryEntries = Array.from(dateFieldUsageIncluded.entries()) + .map(([field, count]) => `${field}=${count}`); + let includedSummaryNote = ''; + if (!isDebugLog && summaryLimit > 0 && includedSummaryEntries.length > summaryLimit) { + includedSummaryEntries = includedSummaryEntries.slice(0, summaryLimit); + includedSummaryNote = ' (truncated)'; + } + if (isDebugLog || summaryLimit > 0) { + console.info(`${fileName}: date fields used for included records -> ${includedSummaryEntries.join(', ') || 'none'}${includedSummaryNote}`); + } + } + + if (dateFieldUsageAll.size && isDebugLog) { + const allSummary = Array.from(dateFieldUsageAll.entries()) + .map(([field, count]) => `${field}=${count}`) + .join(', '); + console.info(`${fileName}: date fields present in source data -> ${allSummary}`); + } + + console.info(`${fileName}: records with all date fields=${recordsWithAllDateFields}, partial=${recordsWithSingleDateField}, none=${recordsWithNoDateFields}`); + + if (minTimestamp !== null && maxTimestamp !== null) { + console.info(`${fileName}: processed date range ${new Date(minTimestamp).toISOString()} to ${new Date(maxTimestamp).toISOString()}`); + } + + if (dateHistogram.size && (isDebugLog || summaryLimit > 0)) { + const histogramSummary = Array.from(dateHistogram.entries()) + .sort((a, b) => b[1] - a[1]) + .slice(0, summaryLimit || 7) + .map(([day, count]) => `${day}=${count}`) + .join(', '); + const histogramNote = !isDebugLog && dateHistogram.size > summaryLimit ? ' (truncated)' : ''; + console.info(`${fileName}: daily distribution${summaryLimit ? ` (top ${summaryLimit})` : ''} -> ${histogramSummary}${histogramNote}`); + } + if (parseErrorCount > 0) { console.warn(`${fileName}: ${parseErrorCount} lines skipped due to JSON parse errors.`); } }; if (isElasticsearch) { - // For Elasticsearch format (line-delimited JSON) const results = []; const fileStream = fs.createReadStream(filePath, { encoding: 'utf8', - highWaterMark: 1024 * 1024 // 1MB chunks + highWaterMark: 1024 * 1024 }); return new Promise((resolve, reject) => { @@ -106,7 +330,6 @@ async function loadData(dataDir, fileName, isElasticsearch = false, sinceDate = isFirstChunk = false; } const lines = buffer.split('\n'); - // Keep the last line in the buffer as it might be incomplete buffer = lines.pop(); for (const line of lines) { @@ -129,18 +352,11 @@ async function loadData(dataDir, fileName, isElasticsearch = false, sinceDate = continue; } - const record = parsedLine._source; - totalRecordsEncountered += 1; - - if (shouldIncludeRecord(record, sinceDate)) { - results.push(record); - recordsAfterFilter += 1; - } + processRecord(parsedLine._source, results); } }); fileStream.on('end', () => { - // Process the last line if needed if (buffer) { lineNumber += 1; const trimmed = buffer.trim(); @@ -155,13 +371,7 @@ async function loadData(dataDir, fileName, isElasticsearch = false, sinceDate = } if (parsedLine && parsedLine._source !== undefined) { - const record = parsedLine._source; - totalRecordsEncountered += 1; - - if (shouldIncludeRecord(record, sinceDate)) { - results.push(record); - recordsAfterFilter += 1; - } + processRecord(parsedLine._source, results); } } } @@ -176,18 +386,12 @@ async function loadData(dataDir, fileName, isElasticsearch = false, sinceDate = }); } - // For regular JSON files, use JSONStream return new Promise((resolve, reject) => { const results = []; const stream = fs.createReadStream(filePath, { encoding: 'utf8' }) - .pipe(JSONStream.parse('*')) // Parse all items in the array + .pipe(JSONStream.parse('*')) .pipe(es.through(function(data) { - totalRecordsEncountered += 1; - - if (shouldIncludeRecord(data, sinceDate)) { - results.push(data); - recordsAfterFilter += 1; - } + processRecord(data, results); })); stream.on('end', () => { From 0b8dc13c01fc1f97e1298834fca54e08dcb19796 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Thu, 30 Oct 2025 11:25:48 +1100 Subject: [PATCH 16/18] Fix validation to not require GUIDs --- src/services/DefaultChallengeReviewerService.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/services/DefaultChallengeReviewerService.js b/src/services/DefaultChallengeReviewerService.js index d9439a7..3a82a70 100644 --- a/src/services/DefaultChallengeReviewerService.js +++ b/src/services/DefaultChallengeReviewerService.js @@ -19,6 +19,8 @@ const defaultInclude = { phase: true, }; +const reviewerIdSchema = Joi.string().trim().required(); + /** * Normalize record by removing audit fields * @@ -360,7 +362,7 @@ async function getDefaultChallengeReviewer(id) { } getDefaultChallengeReviewer.schema = { - id: Joi.id(), + id: reviewerIdSchema, }; /** @@ -396,7 +398,7 @@ async function fullyUpdateDefaultChallengeReviewer(authUser, id, data) { fullyUpdateDefaultChallengeReviewer.schema = { authUser: Joi.any(), - id: Joi.id(), + id: reviewerIdSchema, data: Joi.object() .keys({ typeId: Joi.id().required(), @@ -496,7 +498,7 @@ async function partiallyUpdateDefaultChallengeReviewer(authUser, id, data) { partiallyUpdateDefaultChallengeReviewer.schema = { authUser: Joi.any(), - id: Joi.id(), + id: reviewerIdSchema, data: Joi.object() .keys({ typeId: Joi.optionalId(), @@ -543,7 +545,7 @@ async function deleteDefaultChallengeReviewer(id) { } deleteDefaultChallengeReviewer.schema = { - id: Joi.id(), + id: reviewerIdSchema, }; module.exports = { From 9fe235051ef413f6889dc56fafaccf676593bf4d Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 31 Oct 2025 15:33:40 +1100 Subject: [PATCH 17/18] Additional indices for performance issues noted --- .../migration.sql | 13 +++++++++++++ prisma/schema.prisma | 3 +++ 2 files changed, 16 insertions(+) create mode 100644 prisma/migrations/20251107090000_add_my_reviews_indexes/migration.sql diff --git a/prisma/migrations/20251107090000_add_my_reviews_indexes/migration.sql b/prisma/migrations/20251107090000_add_my_reviews_indexes/migration.sql new file mode 100644 index 0000000..89c16fd --- /dev/null +++ b/prisma/migrations/20251107090000_add_my_reviews_indexes/migration.sql @@ -0,0 +1,13 @@ +-- Add indexes to support faster `/v6/my-reviews` queries. + +CREATE EXTENSION IF NOT EXISTS pg_trgm; + +CREATE INDEX IF NOT EXISTS "challenge_status_type_track_created_at_idx" + ON "Challenge" ("status", "typeId", "trackId", "createdAt" DESC); + +CREATE INDEX IF NOT EXISTS "challenge_phase_challenge_open_end_idx" + ON "ChallengePhase" ("challengeId", "isOpen", "scheduledEndDate", "actualEndDate"); + +CREATE INDEX IF NOT EXISTS "challenge_name_trgm_idx" + ON "Challenge" + USING gin ("name" gin_trgm_ops); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 602163f..836aa6d 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -155,6 +155,8 @@ model Challenge { @@index([endDate]) @@index([status, startDate]) @@index([trackId, typeId, status]) + @@index([status, typeId, trackId, createdAt(sort: Desc)], map: "challenge_status_type_track_created_at_idx") + @@index([name], type: Gin, ops: [gin_trgm_ops], map: "challenge_name_trgm_idx") @@index([legacyId]) @@index([projectId, status]) } @@ -540,6 +542,7 @@ model ChallengePhase { @@index([challengeId]) @@index([challengeId, isOpen]) @@index([challengeId, name]) + @@index([challengeId, isOpen, scheduledEndDate, actualEndDate], map: "challenge_phase_challenge_open_end_idx") } ////////////////////////////////////////// From 0e0aa70d7ae0a27f6cce69aeb769001cd1943368 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Sat, 1 Nov 2025 09:30:43 +1100 Subject: [PATCH 18/18] Fix up extension creation --- .../20251107090000_add_my_reviews_indexes/migration.sql | 2 +- prisma/schema.prisma | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/prisma/migrations/20251107090000_add_my_reviews_indexes/migration.sql b/prisma/migrations/20251107090000_add_my_reviews_indexes/migration.sql index 89c16fd..c71d518 100644 --- a/prisma/migrations/20251107090000_add_my_reviews_indexes/migration.sql +++ b/prisma/migrations/20251107090000_add_my_reviews_indexes/migration.sql @@ -10,4 +10,4 @@ CREATE INDEX IF NOT EXISTS "challenge_phase_challenge_open_end_idx" CREATE INDEX IF NOT EXISTS "challenge_name_trgm_idx" ON "Challenge" - USING gin ("name" gin_trgm_ops); + USING gin ("name" pg_catalog.gin_trgm_ops); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 836aa6d..19235cf 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -5,7 +5,7 @@ datasource db { generator client { provider = "prisma-client-js" - previewFeatures = ["fullTextSearchPostgres"] + previewFeatures = ["fullTextSearchPostgres", "postgresqlExtensions"] } // Enum for allowed challenge track values (matches app-constants) @@ -156,7 +156,7 @@ model Challenge { @@index([status, startDate]) @@index([trackId, typeId, status]) @@index([status, typeId, trackId, createdAt(sort: Desc)], map: "challenge_status_type_track_created_at_idx") - @@index([name], type: Gin, ops: [gin_trgm_ops], map: "challenge_name_trgm_idx") + @@index([name(ops: raw("pg_catalog.gin_trgm_ops"))], type: Gin, map: "challenge_name_trgm_idx") @@index([legacyId]) @@index([projectId, status]) }