From fc4cb4884499ad07c9d1b59219a13567efdfc8bf Mon Sep 17 00:00:00 2001 From: jmgasper Date: Thu, 30 Jul 2026 20:39:02 +1000 Subject: [PATCH 1/5] PM-5761: Count every design submission concept What was broken Challenge detail and listing responses counted multiple Design concepts from one member as a single submission, so the Community App displayed an incorrect submissions total. Root cause Challenge API recomputed both final and checkpoint counters with the submitting member ID as the distinct identity for every challenge track. What was changed Use the submission ID as the distinct counter identity for Design challenges while preserving member-based counting for all other tracks. Apply the rule to both checkpoint and non-checkpoint submissions. Any added/updated tests Added regression coverage for multiple final and checkpoint Design concepts from one member across challenge detail and listing responses. Preserved the existing Development-track deduplication coverage. --- src/services/ChallengeService.ts | 33 ++++++++++++++++---- test/unit/ChallengeService.test.js | 49 ++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/src/services/ChallengeService.ts b/src/services/ChallengeService.ts index d19cf85..6f129d4 100644 --- a/src/services/ChallengeService.ts +++ b/src/services/ChallengeService.ts @@ -100,20 +100,27 @@ function isCancelledChallengeStatus(status) { /** * Loads submission counters for challenge responses from the review submission table. * - * Community app badges show the number of members with submissions, not the - * number of attempts, so repeated uploads by the same member are counted once. + * Community app badges normally show the number of members with submissions, + * so repeated uploads by the same member are counted once. Design challenges + * count every submission because each upload can be a unique concept. * * @param {Array} challengeIds challenge identifiers to count submissions for + * @param {Array} designChallengeIds Design challenge identifiers that count every upload * @returns {Promise>} * counts keyed by challenge id * @throws {Error} when the review database query fails */ -async function getLatestSubmissionCountsByChallenge(challengeIds) { +async function getLatestSubmissionCountsByChallenge(challengeIds, designChallengeIds = []) { const ids = _.uniq( (challengeIds || []) .map((challengeId) => _.toString(challengeId).trim()) .filter((challengeId) => !!challengeId), ); + const designIds = _.uniq( + (designChallengeIds || []) + .map((challengeId) => _.toString(challengeId).trim()) + .filter((challengeId) => !!challengeId), + ); const countsByChallenge = new Map(); if (!ids.length || !config.REVIEW_DB_URL) { @@ -124,16 +131,22 @@ async function getLatestSubmissionCountsByChallenge(challengeIds) { const submissionTable = reviewSchema ? Prisma.raw(`"${reviewSchema.replace(/"/g, '""')}"."submission"`) : Prisma.raw('"submission"'); + const submissionIdentity = designIds.length + ? Prisma.sql`CASE + WHEN "challengeId" IN (${Prisma.join(designIds)}) THEN "id" + ELSE "memberId" + END` + : Prisma.sql`"memberId"`; const reviewClient = getReviewClient(); const rows = await reviewClient.$queryRaw` SELECT "challengeId", COUNT(DISTINCT CASE - WHEN "type"::text = ${CHECKPOINT_SUBMISSION_TYPE} THEN "memberId" + WHEN "type"::text = ${CHECKPOINT_SUBMISSION_TYPE} THEN ${submissionIdentity} END)::int AS "numOfCheckpointSubmissions", COUNT(DISTINCT CASE - WHEN "type"::text <> ${CHECKPOINT_SUBMISSION_TYPE} THEN "memberId" + WHEN "type"::text <> ${CHECKPOINT_SUBMISSION_TYPE} THEN ${submissionIdentity} END)::int AS "numOfSubmissions" FROM ${submissionTable} WHERE "challengeId" IN (${Prisma.join(ids)}) @@ -152,7 +165,11 @@ async function getLatestSubmissionCountsByChallenge(challengeIds) { } /** - * Applies latest-member submission counts to challenge records before response conversion. + * Applies submission counts to challenge records before response conversion. + * + * Design challenges count every submission as a separate concept. Other tracks + * count distinct submitting members so replacement attempts do not inflate the + * displayed total. * * If the review query succeeds, challenges without submission rows are reset to * zero so stale stored counters are not shown. If the query cannot run, callers @@ -169,8 +186,12 @@ async function applyLatestSubmissionCounts(challenges) { let countsByChallenge; try { + const designChallengeIds = records + .filter((challenge) => phaseHelper.isDesignTrack(challenge.track)) + .map((challenge) => challenge.id); countsByChallenge = await getLatestSubmissionCountsByChallenge( records.map((challenge) => challenge.id), + designChallengeIds, ); } catch (err) { logger.warn(`Failed to load latest submission counts: ${err.message}`); diff --git a/test/unit/ChallengeService.test.js b/test/unit/ChallengeService.test.js index aa2a1ad..61ddc08 100644 --- a/test/unit/ChallengeService.test.js +++ b/test/unit/ChallengeService.test.js @@ -704,6 +704,55 @@ describe("challenge service unit tests", () => { } }); + it("counts every Design submission as a separate concept", async () => { + const challengeId = data.challenge.id; + const originalTrack = data.challengeTrack.track; + await prisma.challengeTrack.update({ + where: { id: data.challenge.trackId }, + data: { track: "DESIGN" }, + }); + + try { + await reviewClient.$executeRawUnsafe(` + INSERT INTO ${submissionTableName} + ("id", "challengeId", "memberId", "type", "status", "submittedDate") + VALUES + ('pm5761a1', '${challengeId}', 'member-1', 'CONTEST_SUBMISSION', 'ACTIVE', '2026-01-01T00:00:00Z'), + ('pm5761a2', '${challengeId}', 'member-1', 'CONTEST_SUBMISSION', 'ACTIVE', '2026-01-02T00:00:00Z'), + ('pm5761a3', '${challengeId}', 'member-1', 'CONTEST_SUBMISSION', 'ACTIVE', '2026-01-03T00:00:00Z'), + ('pm5761c1', '${challengeId}', 'member-1', 'CHECKPOINT_SUBMISSION', 'ACTIVE', '2026-01-04T00:00:00Z'), + ('pm5761c2', '${challengeId}', 'member-1', 'CHECKPOINT_SUBMISSION', 'ACTIVE', '2026-01-05T00:00:00Z') + `); + + const detail = await service.getChallenge({ isMachine: true }, challengeId); + should.equal(detail.numOfSubmissions, 3); + should.equal(detail.numOfCheckpointSubmissions, 2); + + const listing = await service.searchChallenges( + { isMachine: true }, + { + id: challengeId, + page: 1, + perPage: 10, + }, + ); + should.equal(listing.result.length, 1); + should.equal(listing.result[0].numOfSubmissions, 3); + should.equal(listing.result[0].numOfCheckpointSubmissions, 2); + } finally { + try { + await reviewClient.$executeRawUnsafe( + `DELETE FROM ${submissionTableName} WHERE "challengeId" = '${challengeId}'`, + ); + } finally { + await prisma.challengeTrack.update({ + where: { id: data.challenge.trackId }, + data: { track: originalTrack }, + }); + } + } + }); + it("get challenge preserves billing for project write users", async () => { const originalUserHasProjectWriteAccess = helper.userHasProjectWriteAccess; From 085046009baf5b81cee0a31c623aa71c65a2ef14 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Thu, 30 Jul 2026 22:08:16 +1000 Subject: [PATCH 2/5] PM-5754: update winning download metadata docs What was broken The challenge API schema described submissionsViewable as an additional prerequisite for Design winning-submission downloads. Root cause Swagger documentation still reflected the obsolete Design-specific authorization gate. What was changed Removed the obsolete submissionsViewable prerequisite from the winning-download metadata descriptions in create, update, patch, and response schemas. Any added/updated tests No tests were changed because this is a documentation-only contract correction; existing metadata validation tests continue to cover exact values and omission. --- docs/swagger.yaml | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 0890b10..1fc987f 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -2588,8 +2588,7 @@ definitions: description: >- Metadata name. Use submission_type to override the challenge submission flow. Use allowAllRegistrantsToDownloadWinningSubmissions to let all registrants download - winning submissions after the challenge ends. For Design challenges, - submissionsViewable must also be true. + winning submissions after the challenge ends. value: type: string description: >- @@ -2881,8 +2880,7 @@ definitions: description: >- Metadata name. Use submission_type to override the challenge submission flow. Use allowAllRegistrantsToDownloadWinningSubmissions to let all registrants download - winning submissions after the challenge ends. For Design challenges, - submissionsViewable must also be true. + winning submissions after the challenge ends. value: type: string description: >- @@ -3059,8 +3057,7 @@ definitions: description: >- Metadata name. Use submission_type to override the challenge submission flow. Use allowAllRegistrantsToDownloadWinningSubmissions to let all registrants download - winning submissions after the challenge ends. For Design challenges, - submissionsViewable must also be true. + winning submissions after the challenge ends. value: type: string description: >- @@ -3282,8 +3279,7 @@ definitions: description: >- Metadata name. Use submission_type to override the challenge submission flow. Use allowAllRegistrantsToDownloadWinningSubmissions to let all registrants download - winning submissions after the challenge ends. For Design challenges, - submissionsViewable must also be true. + winning submissions after the challenge ends. value: type: string description: >- From b362f1dda6c2b414f4bbdc9dd11dd19f637f38b6 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Thu, 30 Jul 2026 22:53:45 +1000 Subject: [PATCH 3/5] PM-5763: Send localized phase notifications What was broken Manual phase changes published one shared email with raw timestamps for all challenge resources, without honoring each resource's phase-notification preference or member timezone. Root cause The manual phase update path built one common template payload and did not load recipient profile locations, so the timestamp and subject could not be personalized. What was changed Filter to opted-in resources and publish one external.action.email event per unique recipient. Resolve profile locations in bounded batches, default unresolved locations to UTC, format times as MMMM DD, YYYY HH:mm z, and add localized_time and phase_change while preserving the legacy template fields. Removed member profile response logging from the lookup used by this flow. Any added/updated tests Added phase service coverage for individual delivery, preference filtering, email deduplication, local and UTC timestamps, country fallback, open and closed payloads, and reopen wording. --- package.json | 2 + pnpm-lock.yaml | 18 +++ src/common/helper.ts | 86 +++++++++++-- src/services/ChallengePhaseService.ts | 72 ++++++++--- test/unit/ChallengePhaseService.test.js | 157 ++++++++++++++++++++++++ 5 files changed, 304 insertions(+), 31 deletions(-) diff --git a/package.json b/package.json index 8723ad6..ec97a36 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,7 @@ "axios-retry": "^4.5.0", "bluebird": "^3.7.2", "body-parser": "^2.3.0", + "city-timezones": "^1.3.2", "config": "^4.1.1", "cors": "^2.8.5", "decimal.js": "^10.6.0", @@ -81,6 +82,7 @@ "jsonwebtoken": "^9.0.2", "lodash": "^4.18.1", "moment": "^2.30.1", + "moment-timezone": "^0.6.0", "node-cache": "^5.1.2", "pg": "^8.16.3", "prisma": "7.8.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1e82059..6bf9a14 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -62,6 +62,9 @@ importers: body-parser: specifier: ^2.3.0 version: 2.3.0(supports-color@5.5.0) + city-timezones: + specifier: ^1.3.2 + version: 1.3.4 config: specifier: ^4.1.1 version: 4.1.1 @@ -107,6 +110,9 @@ importers: moment: specifier: ^2.30.1 version: 2.30.1 + moment-timezone: + specifier: ^0.6.0 + version: 0.6.3 node-cache: specifier: ^5.1.2 version: 5.1.2 @@ -1796,6 +1802,9 @@ packages: resolution: {integrity: sha512-rNjApaLzuwaOTjCiT8lSDdGN1APCiqkChLMJxJPWLunPAt5fy8xgU9/jNOchV84wfIxrA0lRQB7oCT8jrn/wrQ==} engines: {node: '>=6.0'} + city-timezones@1.3.4: + resolution: {integrity: sha512-yLmtCxU4y5HLAw9XGIMcGEXO+R2KH1sb595wEQC/E9BxSDpazCD7v9ZQRySO7367IXWsWqt3z2xFVh3OkZdgEQ==} + clean-stack@2.2.0: resolution: {integrity: sha512-4diC9HaTE+KRAMWhDhrGOECgWZxoevMc5TlkObMqNSsVU62PYzXZ/SMTjzyGAFF1YusgxGcSWTEXBhp0CPwQ1A==} engines: {node: '>=6'} @@ -3050,6 +3059,9 @@ packages: engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} hasBin: true + moment-timezone@0.6.3: + resolution: {integrity: sha512-pVEPA/HCFHHbwJ130ywnzYuZpkEGcP6Daa/OwNebpA18MybeFHmQilAGGovXgWijQ8vQtmud9jZrziUBgsykfg==} + moment@2.30.1: resolution: {integrity: sha512-uEmtNhbDOrWPFS+hdjFCBfy9f2YoyzRpwcl+DqpC6taX21FzsTLQVbMV/W7PzNSX6x/bhC1zA3c2UQ5NzH6how==} @@ -6350,6 +6362,8 @@ snapshots: chrome-trace-event@1.0.4: {} + city-timezones@1.3.4: {} + clean-stack@2.2.0: {} cli-cursor@3.1.0: @@ -7611,6 +7625,10 @@ snapshots: yargs-parser: 21.1.1 yargs-unparser: 2.0.0 + moment-timezone@0.6.3: + dependencies: + moment: 2.30.1 + moment@2.30.1: {} ms@2.0.0: {} diff --git a/src/common/helper.ts b/src/common/helper.ts index cdd3881..505ed26 100644 --- a/src/common/helper.ts +++ b/src/common/helper.ts @@ -13,6 +13,8 @@ const axiosRetry = require("axios-retry").default; const busApi = require("topcoder-bus-api-wrapper"); const NodeCache = require("node-cache"); const HttpStatus = require("http-status-codes"); +const cityTimezones = require("city-timezones"); +const momentTimezone = require("moment-timezone"); const logger = require("./logger"); const projectHelper = require("./project-helper"); @@ -1736,17 +1738,16 @@ async function getReviewSummations(challengeId) { } /** - * Get member by ID + * Get a member profile by ID. * @param {String} userId the user ID - * @returns {Object} + * @returns {Promise} the matching member profile, or an empty object + * @throws {Error} when authentication or the Members API request fails */ async function getMemberById(userId) { const token = await m2mHelper.getM2MToken(); - console.log(`${config.MEMBERS_API_URL}?userId=${userId}`); const res = await axios.get(`${config.MEMBERS_API_URL}?userId=${userId}`, { headers: { Authorization: `Bearer ${token}` }, }); - console.log(res.data); if (res.data.length > 0) return res.data[0]; return {}; } @@ -1867,24 +1868,81 @@ async function sendSelfServiceNotification(type, recipients, data) { } /** - * Build payload for phase change email notification - * @param {String} challenge Id - * @param {String} challenge name - * @param {String} challenge phase name - * @param {String} operation to be performed on the phase - open | close | reopen - * @param {String|Date} at - The date/time when the phase opened/closed + * Resolve a member's IANA timezone from the city and country in their primary address. + * The first city-timezones match and profile app country/city fallback are used. + * @param {Object} memberData the member profile returned by the Members API + * @returns {String} a valid IANA timezone, or UTC when the location cannot be resolved + */ +function getMemberTimezone(memberData) { + const city = _.get(memberData, "addresses[0].city"); + if (!city) { + return "UTC"; + } + + try { + const matches = cityTimezones.lookupViaCity(String(city).trim()); + let timezone = _.get(matches, "[0].timezone"); + + if (!timezone) { + const countryCode = memberData.homeCountryCode || memberData.competitionCountryCode; + const country = countryCode + ? _.get(cityTimezones.findFromIsoCode(countryCode), "[0].country") + : null; + timezone = country ? `${country}/${String(city).trim()}` : null; + } + + return timezone && momentTimezone.tz.zone(timezone) ? timezone : "UTC"; + } catch (e) { + return "UTC"; + } +} + +/** + * Format a phase change timestamp in the member's local timezone. + * This value is displayed in phase notification emails. + * @param {String|Date} at the instant when the phase opened or closed + * @param {Object} memberData the recipient's member profile + * @returns {String} the localized timestamp in `MMMM DD, YYYY HH:mm z` format + */ +function formatLocalizedPhaseTime(at, memberData) { + return momentTimezone(at) + .tz(getMemberTimezone(memberData)) + .format("MMMM DD, YYYY HH:mm z"); +} + +/** + * Build payload for a phase change email notification. + * @param {Object} options phase notification values + * @param {String} options.challengeId the challenge ID + * @param {String} options.challengeName the challenge name + * @param {String} options.phaseName the challenge phase name + * @param {String} options.operation the phase operation: open, close, or reopen + * @param {String|Date} options.at the phase change timestamp used when no localization is supplied + * @param {String} options.localizedTime the phase change timestamp localized for the recipient + * @returns {Object} template data for the recipient's phase notification email */ -function buildPhaseChangeEmailData({ challengeId, challengeName, phaseName, operation, at }) { +function buildPhaseChangeEmailData({ + challengeId, + challengeName, + phaseName, + operation, + at, + localizedTime, +}) { const isOpen = operation === "open" || operation === "reopen"; const isClose = operation === "close"; + const displayedTime = localizedTime || at; + const phaseChange = `${phaseName} ${isClose ? "Closed" : "Open"}`; return { challengeURL: `${config.CHALLENGE_URL}/${challengeId}`, challengeName, phaseOpen: isOpen ? phaseName : null, - phaseOpenDate: isOpen ? at : null, + phaseOpenDate: isOpen ? displayedTime : null, phaseClose: isClose ? phaseName : null, - phaseCloseDate: isClose ? at : null, + phaseCloseDate: isClose ? displayedTime : null, + localized_time: displayedTime, + phase_change: phaseChange, }; } @@ -2053,6 +2111,8 @@ module.exports = { setToInternalCache, flushInternalCache, removeNullProperties, + getMemberTimezone, + formatLocalizedPhaseTime, buildPhaseChangeEmailData, sendPhaseChangeNotification, }; diff --git a/src/services/ChallengePhaseService.ts b/src/services/ChallengePhaseService.ts index 07cd85e..1bf18ee 100644 --- a/src/services/ChallengePhaseService.ts +++ b/src/services/ChallengePhaseService.ts @@ -28,6 +28,7 @@ const REVIEW_PHASE_NAMES = Object.freeze([ "approval", ]); const REVIEW_PHASE_NAME_SET = new Set(REVIEW_PHASE_NAMES.map((name) => name.toLowerCase())); +const PHASE_NOTIFICATION_BATCH_SIZE = 10; const PHASE_RESOURCE_ROLE_REQUIREMENTS = Object.freeze({ "iterative review": "Iterative Reviewer", "checkpoint screening": "Checkpoint Screener", @@ -641,7 +642,8 @@ getChallengePhase.schema = { }; /** - * Partially update challenge phase + * Partially update a challenge phase and publish individualized, localized + * notifications when the update opens, closes, or reopens the phase. * @param {Object} currentUser the user who perform operation * @param {String} challengeId the challenge id * @param {String} id the phase id @@ -1060,16 +1062,24 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data) // build recipients const resources = await helper.getChallengeResources(challengeId); - const recipients = Array.from( - new Set( - (resources || []) - .map((r) => r?.email || r?.memberEmail) - .filter(Boolean) - .map((e) => String(e).trim().toLowerCase()), - ), - ); + const recipients = new Map(); + for (const resource of resources || []) { + if (resource?.phaseChangeNotifications !== true) { + continue; + } + + const email = String(resource?.email || resource?.memberEmail || "") + .trim() + .toLowerCase(); + if (email && !recipients.has(email)) { + recipients.set(email, { + email, + memberId: resource.memberId, + }); + } + } - if (!recipients.length) { + if (!recipients.size) { logger.debug( `phase change notification skipped: no recipients for challenge ${challengeId}`, ); @@ -1079,15 +1089,41 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data) // build payload that matches the SendGrid HTML template const phaseName = result.name || data.name || challengePhase.name; - const payload = helper.buildPhaseChangeEmailData({ - challengeId, - challengeName, - phaseName, - operation, - at, - }); + for (const recipientBatch of _.chunk( + Array.from(recipients.values()), + PHASE_NOTIFICATION_BATCH_SIZE, + )) { + await Promise.all( + recipientBatch.map(async (recipient) => { + let member = {}; + if (!_.isNil(recipient.memberId)) { + try { + member = await helper.getMemberById(recipient.memberId); + } catch (e) { + logger.debug( + `phase change notification could not resolve member ${recipient.memberId}: ${e.message}`, + ); + } + } - await helper.sendPhaseChangeNotification(notificationType, recipients, payload); + const localizedTime = helper.formatLocalizedPhaseTime(at, member); + const payload = helper.buildPhaseChangeEmailData({ + challengeId, + challengeName, + phaseName, + operation, + at, + localizedTime, + }); + + await helper.sendPhaseChangeNotification( + notificationType, + [recipient.email], + payload, + ); + }), + ); + } } } catch (e) { logger.debug( diff --git a/test/unit/ChallengePhaseService.test.js b/test/unit/ChallengePhaseService.test.js index e04353f..43f46a7 100644 --- a/test/unit/ChallengePhaseService.test.js +++ b/test/unit/ChallengePhaseService.test.js @@ -413,6 +413,163 @@ describe('challenge phase service unit tests', () => { should.equal(challengePhase.duration, 7200) }) + it('sends one localized phase-open notification to each opted-in resource', async () => { + const notifications = [] + const requestedMemberIds = [] + const originalGetChallengeResources = helper.getChallengeResources + const originalGetMemberById = helper.getMemberById + const originalSendPhaseChangeNotification = helper.sendPhaseChangeNotification + const openedAt = '2026-07-29T03:35:00.000Z' + + helper.getChallengeResources = async () => [ + { + memberId: '101', + memberEmail: ' Alice@Example.com ', + phaseChangeNotifications: true + }, + { + memberId: 'duplicate', + memberEmail: 'alice@example.com', + phaseChangeNotifications: true + }, + { + memberId: '102', + email: 'bob@example.com', + phaseChangeNotifications: true + }, + { + memberId: 'opted-out', + memberEmail: 'opted-out@example.com', + phaseChangeNotifications: false + }, + { + memberId: 'not-opted-in', + memberEmail: 'not-opted-in@example.com' + } + ] + helper.getMemberById = async memberId => { + requestedMemberIds.push(memberId) + return memberId === '101' ? { addresses: [{ city: 'Hobart' }] } : {} + } + helper.sendPhaseChangeNotification = async (type, recipients, payload) => { + notifications.push({ type, recipients, payload }) + } + + try { + await service.partiallyUpdateChallengePhase( + authUser, + data.challenge.id, + data.challengePhase1Id, + { + isOpen: true, + actualStartDate: openedAt + } + ) + + notifications.should.have.length(2) + notifications[0].type.should.equal('PHASE_CHANGE') + notifications[0].recipients.should.deep.equal(['alice@example.com']) + notifications[0].payload.phase_change.should.equal('Registration Open') + notifications[0].payload.localized_time.should.equal('July 29, 2026 13:35 AEST') + notifications[0].payload.phaseOpen.should.equal('Registration') + notifications[0].payload.phaseOpenDate.should.equal('July 29, 2026 13:35 AEST') + should.equal(notifications[0].payload.phaseClose, null) + should.equal(notifications[0].payload.phaseCloseDate, null) + + notifications[1].recipients.should.deep.equal(['bob@example.com']) + notifications[1].payload.phase_change.should.equal('Registration Open') + notifications[1].payload.localized_time.should.equal('July 29, 2026 03:35 UTC') + notifications[1].payload.phaseOpenDate.should.equal('July 29, 2026 03:35 UTC') + requestedMemberIds.should.deep.equal(['101', '102']) + } finally { + helper.getChallengeResources = originalGetChallengeResources + helper.getMemberById = originalGetMemberById + helper.sendPhaseChangeNotification = originalSendPhaseChangeNotification + } + }) + + it('sends a localized phase-closed notification with the legacy close fields', async () => { + const notifications = [] + const originalGetChallengeResources = helper.getChallengeResources + const originalGetMemberById = helper.getMemberById + const originalSendPhaseChangeNotification = helper.sendPhaseChangeNotification + const closedAt = '2026-07-29T04:35:00.000Z' + + await prisma.challengePhase.update({ + where: { id: data.challengePhase1Id }, + data: { + isOpen: true, + actualStartDate: new Date('2026-07-29T03:35:00.000Z'), + actualEndDate: null + } + }) + helper.getChallengeResources = async () => [ + { + memberId: '103', + memberEmail: 'closer@example.com', + phaseChangeNotifications: true + } + ] + helper.getMemberById = async () => ({ addresses: [{ city: 'Hobart' }] }) + helper.sendPhaseChangeNotification = async (type, recipients, payload) => { + notifications.push({ type, recipients, payload }) + } + + try { + await service.partiallyUpdateChallengePhase( + authUser, + data.challenge.id, + data.challengePhase1Id, + { + isOpen: false, + actualEndDate: closedAt + } + ) + + notifications.should.have.length(1) + notifications[0].recipients.should.deep.equal(['closer@example.com']) + notifications[0].payload.phase_change.should.equal('Registration Closed') + notifications[0].payload.localized_time.should.equal('July 29, 2026 14:35 AEST') + notifications[0].payload.phaseClose.should.equal('Registration') + notifications[0].payload.phaseCloseDate.should.equal('July 29, 2026 14:35 AEST') + should.equal(notifications[0].payload.phaseOpen, null) + should.equal(notifications[0].payload.phaseOpenDate, null) + } finally { + helper.getChallengeResources = originalGetChallengeResources + helper.getMemberById = originalGetMemberById + helper.sendPhaseChangeNotification = originalSendPhaseChangeNotification + } + }) + + it('resolves member timezones with UTC fallback and maps reopen to Open', () => { + helper + .getMemberTimezone({ addresses: [{ city: 'Hobart' }] }) + .should.equal('Australia/Hobart') + helper + .getMemberTimezone({ + addresses: [{ city: 'Lindeman' }], + homeCountryCode: 'AUS' + }) + .should.equal('Australia/Lindeman') + helper.getMemberTimezone({ addresses: [{ city: 'not-a-real-city' }] }).should.equal('UTC') + helper.getMemberTimezone({}).should.equal('UTC') + helper + .formatLocalizedPhaseTime('2026-07-29T03:35:00.000Z', {}) + .should.equal('July 29, 2026 03:35 UTC') + + const payload = helper.buildPhaseChangeEmailData({ + challengeId: data.challenge.id, + challengeName: data.challenge.name, + phaseName: 'Checkpoint Submission', + operation: 'reopen', + at: '2026-07-29T03:35:00.000Z', + localizedTime: 'July 29, 2026 13:35 AEST' + }) + payload.phase_change.should.equal('Checkpoint Submission Open') + payload.localized_time.should.equal('July 29, 2026 13:35 AEST') + payload.phaseOpenDate.should.equal('July 29, 2026 13:35 AEST') + }) + it('partially update challenge phase - closing sets actual end date', async () => { await prisma.challengePhase.update({ where: { id: data.challengePhase1Id }, From b220490ea309d1d7bf0430a5623fb63281a856c7 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Fri, 31 Jul 2026 00:57:21 +1000 Subject: [PATCH 4/5] PM-5775: expose active checkpoint winners What was broken The Challenge API hid assigned checkpoint winners until the entire challenge was completed, so the review app could not identify winners after Checkpoint Review. Root cause Winner response sanitization removed both final-placement and checkpoint winner data for every non-completed challenge. What was changed - Expose checkpoint winners for active challenges only after Checkpoint Review has opened and closed. - Continue hiding final-placement winners until challenge completion. - Document when checkpoint winners are returned. Any added/updated tests - Added detail and listing coverage for open and closed Checkpoint Review states. - Verified placement winners remain hidden while checkpoint winners become visible. --- docs/swagger.yaml | 1 + src/services/ChallengeService.ts | 42 ++++++++++- test/unit/ChallengeService.test.js | 109 +++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 3 deletions(-) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 0890b10..28d73f9 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -2681,6 +2681,7 @@ definitions: - placement checkpointWinners: type: array + description: Checkpoint winners are returned after Checkpoint Review closes, including while the challenge is active. items: properties: userId: diff --git a/src/services/ChallengeService.ts b/src/services/ChallengeService.ts index d19cf85..dd0a6d5 100644 --- a/src/services/ChallengeService.ts +++ b/src/services/ChallengeService.ts @@ -845,8 +845,9 @@ const challengeDomain = { const phaseAdvancer = new PhaseAdvancer(challengeDomain); const REVIEW_STATUS_BLOCKING = Object.freeze(["IN_PROGRESS", "COMPLETED"]); +const CHECKPOINT_REVIEW_PHASE_NAME = "checkpoint review"; const REVIEW_PHASE_NAMES = Object.freeze([ - "checkpoint review", + CHECKPOINT_REVIEW_PHASE_NAME, "checkpoint screening", "screening", "review", @@ -861,6 +862,33 @@ function normalizePhaseNameForComparison(phaseName) { return _.toString(phaseName).replace(/-/g, " ").trim().toLowerCase(); } +/** + * Determines whether checkpoint winners are ready to be included in challenge responses. + * Detail and search response sanitization use this after Checkpoint Review has closed, + * while completed challenges preserve their existing winner visibility. + * + * @param {Object} challenge challenge data containing status and phase state + * @returns {Boolean} true when assigned checkpoint winners may be returned + * @throws {Error} this function does not throw + */ +function shouldExposeCheckpointWinners(challenge) { + if (challenge.status === ChallengeStatusEnum.COMPLETED) { + return true; + } + if (challenge.status !== ChallengeStatusEnum.ACTIVE) { + return false; + } + + return _.some( + challenge.phases, + (phase) => + normalizePhaseNameForComparison(phase.name) === CHECKPOINT_REVIEW_PHASE_NAME && + phase.isOpen !== true && + !_.isNil(phase.actualStartDate) && + !_.isNil(phase.actualEndDate), + ); +} + function extractSubmissionId(submission) { const candidate = _.get(submission, "id") || @@ -2316,6 +2344,8 @@ async function searchChallenges(currentUser, criteria) { result.forEach((challenge) => { if (challenge.status !== ChallengeStatusEnum.COMPLETED) { _.unset(challenge, "winners"); + } + if (!shouldExposeCheckpointWinners(challenge)) { _.unset(challenge, "checkpointWinners"); } if (!_hasAdminRole && !_.get(currentUser, "isMachine", false)) { @@ -3041,8 +3071,14 @@ async function getChallenge(currentUser, id, checkIfExists?: any) { } if (challenge.status !== ChallengeStatusEnum.COMPLETED) { - _.unset(challenge, "winners"); - _.unset(challenge, "checkpointWinners"); + if (shouldExposeCheckpointWinners(challenge)) { + challenge.winners = _.filter( + challenge.winners, + (winner) => winner.type === PrizeSetTypeEnum.CHECKPOINT, + ); + } else { + _.unset(challenge, "winners"); + } } // TODO: in the long run we wanna do a finer grained filtering of the payments diff --git a/test/unit/ChallengeService.test.js b/test/unit/ChallengeService.test.js index aa2a1ad..be30590 100644 --- a/test/unit/ChallengeService.test.js +++ b/test/unit/ChallengeService.test.js @@ -652,6 +652,115 @@ describe("challenge service unit tests", () => { should.equal(result.numOfRegistrants, 0); }); + it("returns checkpoint winners after checkpoint review closes while keeping placement winners hidden", async () => { + const challengeId = data.challenge.id; + const checkpointPhase = await prisma.challengePhase.findFirstOrThrow({ + where: { challengeId }, + }); + const originalPhase = _.pick(checkpointPhase, [ + "name", + "isOpen", + "actualStartDate", + "actualEndDate", + ]); + const phaseStartDate = new Date(Date.now() - 60_000); + await prisma.challenge.update({ + where: { id: challengeId }, + data: { status: ChallengeStatusEnum.ACTIVE }, + }); + await prisma.challengePhase.update({ + where: { id: checkpointPhase.id }, + data: { + name: "Checkpoint Review", + isOpen: true, + actualStartDate: phaseStartDate, + actualEndDate: null, + }, + }); + await prisma.challengeWinner.createMany({ + data: [ + { + challengeId, + userId: 123, + handle: "checkpoint-winner", + placement: 1, + type: PrizeSetTypeEnum.CHECKPOINT, + createdBy: "test", + updatedBy: "test", + }, + { + challengeId, + userId: 456, + handle: "placement-winner", + placement: 1, + type: PrizeSetTypeEnum.PLACEMENT, + createdBy: "test", + updatedBy: "test", + }, + ], + }); + + try { + const openPhaseDetail = await service.getChallenge({ isMachine: true }, challengeId); + should.equal(_.isUndefined(openPhaseDetail.checkpointWinners), true); + openPhaseDetail.winners.should.deep.equal([]); + + const openPhaseListing = await service.searchChallenges( + { isMachine: true }, + { + id: challengeId, + page: 1, + perPage: 10, + }, + ); + should.equal(openPhaseListing.result.length, 1); + should.equal(_.isUndefined(openPhaseListing.result[0].checkpointWinners), true); + should.equal(_.isUndefined(openPhaseListing.result[0].winners), true); + + await prisma.challengePhase.update({ + where: { id: checkpointPhase.id }, + data: { + isOpen: false, + actualEndDate: new Date(), + }, + }); + + const closedPhaseDetail = await service.getChallenge({ isMachine: true }, challengeId); + closedPhaseDetail.checkpointWinners.should.deep.equal([ + { + userId: 123, + handle: "checkpoint-winner", + placement: 1, + }, + ]); + closedPhaseDetail.winners.should.deep.equal([]); + + const closedPhaseListing = await service.searchChallenges( + { isMachine: true }, + { + id: challengeId, + page: 1, + perPage: 10, + }, + ); + should.equal(closedPhaseListing.result.length, 1); + closedPhaseListing.result[0].checkpointWinners.should.deep.equal( + closedPhaseDetail.checkpointWinners, + ); + should.equal(_.isUndefined(closedPhaseListing.result[0].winners), true); + } finally { + await prisma.challengeWinner.deleteMany({ where: { challengeId } }); + await prisma.challengePhase.update({ + where: { id: checkpointPhase.id }, + data: originalPhase, + }); + await prisma.challenge.update({ + where: { id: challengeId }, + data: { status: ChallengeStatusEnum.COMPLETED }, + }); + } + }); + it("returns latest-member submission counters for challenge detail and listing", async () => { const challengeId = data.challenge.id; await prisma.challenge.update({ From cb5f0dddad3483480c086fca7cd23a4d219d5793 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Mon, 3 Aug 2026 17:31:32 +1000 Subject: [PATCH 5/5] Test challenge flag PM-5802 --- README.md | 7 + app-constants.ts | 1 + docs/swagger.yaml | 35 +- src/common/challenge-helper.ts | 65 +++ ...fill-completed-point-challenge-results.sql | 317 ++++++++++ src/services/ChallengeService.ts | 113 +++- test/unit/ChallengeService.test.js | 549 ++++++++++++++++++ test/unit/challenge-helper.test.js | 73 +++ 8 files changed, 1143 insertions(+), 17 deletions(-) create mode 100644 src/scripts/backfill-completed-point-challenge-results.sql diff --git a/README.md b/README.md index 6e2ec1b..c9fe8de 100644 --- a/README.md +++ b/README.md @@ -303,6 +303,13 @@ Refer to the verification document `Verification.md` - Challenge `metadata` may include `submission_type` to override the community-app submission flow: `zip` shows the standard Topcoder zip upload page, and `url` shows the Topgear URL upload page. When omitted, consumers should keep their existing default behavior. +- Challenge `metadata` uses the exact string values `true` and `false` for `is_test_challenge`. + Challenge creation adds `is_test_challenge: false` when it is omitted. `NEW` challenges retain + their existing deletion behavior. A `COMPLETED` or `CANCELLED*` challenge can be deleted when this + metadata value is exactly `true`; `DRAFT`, `APPROVED`, and `ACTIVE` challenges cannot use this + bypass. Any update that starts in or transitions to a completed or cancelled status cannot change + the effective `is_test_challenge` value; omitting metadata preserves it. Normal authorization + checks still apply. - API base configuration points to v6 in dev/local and v5 in prod (for compatibility): - Dev: `work-manager/config/constants/development.js`. - Local: `work-manager/config/constants/local.js`. diff --git a/app-constants.ts b/app-constants.ts index a03315f..608bef7 100644 --- a/app-constants.ts +++ b/app-constants.ts @@ -21,6 +21,7 @@ const prizeTypes = { const ChallengeMetadataNames = { ALLOW_ALL_REGISTRANTS_TO_DOWNLOAD_WINNING_SUBMISSIONS: "allowAllRegistrantsToDownloadWinningSubmissions", + IS_TEST_CHALLENGE: "is_test_challenge", }; const BOOLEAN_METADATA_VALUES = ["true", "false"]; diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 0890b10..ccc832c 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -647,7 +647,10 @@ paths: tags: - Challenges description: Delete the challenge with the provided id. - Only challenges with status of "NEW" can be deleted. + Challenges with status "NEW" retain their existing deletion behavior. Challenges with a + "COMPLETED" or "CANCELLED*" status can also be deleted when their is_test_challenge metadata + value is the exact string "true". "DRAFT", "APPROVED", and "ACTIVE" challenges cannot use + this bypass. Normal deletion authorization checks still apply. security: - bearer: [] produces: @@ -2589,13 +2592,16 @@ definitions: Metadata name. Use submission_type to override the challenge submission flow. Use allowAllRegistrantsToDownloadWinningSubmissions to let all registrants download winning submissions after the challenge ends. For Design challenges, - submissionsViewable must also be true. + submissionsViewable must also be true. Use is_test_challenge to mark production + test data that may be deleted after testing. value: type: string description: >- Metadata value. For submission_type, supported values are zip and url. For allowAllRegistrantsToDownloadWinningSubmissions, only the exact strings true and - false are accepted; a missing entry behaves as false. + false are accepted; a missing entry behaves as false. For is_test_challenge, only + the exact strings true and false are accepted, and create requests that omit it + persist false. required: - name - value @@ -2882,13 +2888,16 @@ definitions: Metadata name. Use submission_type to override the challenge submission flow. Use allowAllRegistrantsToDownloadWinningSubmissions to let all registrants download winning submissions after the challenge ends. For Design challenges, - submissionsViewable must also be true. + submissionsViewable must also be true. Use is_test_challenge to mark production + test data that may be deleted after testing. value: type: string description: >- Metadata value. For submission_type, supported values are zip and url. For allowAllRegistrantsToDownloadWinningSubmissions, only the exact strings true and - false are accepted; a missing entry behaves as false. + false are accepted; a missing entry behaves as false. For is_test_challenge, only + the exact strings true and false are accepted, and create requests that omit it + persist false. required: - name - value @@ -3060,13 +3069,17 @@ definitions: Metadata name. Use submission_type to override the challenge submission flow. Use allowAllRegistrantsToDownloadWinningSubmissions to let all registrants download winning submissions after the challenge ends. For Design challenges, - submissionsViewable must also be true. + submissionsViewable must also be true. Use is_test_challenge to mark production + test data that may be deleted after testing. value: type: string description: >- Metadata value. For submission_type, supported values are zip and url. For allowAllRegistrantsToDownloadWinningSubmissions, only the exact strings true and - false are accepted; a missing entry behaves as false. + false are accepted; a missing entry behaves as false. For is_test_challenge, only + the exact strings true and false are accepted; omitted values behave as false. Its + effective value cannot change in an update that starts in or transitions to + COMPLETED or CANCELLED status; omitting metadata preserves the existing value. required: - name - value @@ -3283,13 +3296,17 @@ definitions: Metadata name. Use submission_type to override the challenge submission flow. Use allowAllRegistrantsToDownloadWinningSubmissions to let all registrants download winning submissions after the challenge ends. For Design challenges, - submissionsViewable must also be true. + submissionsViewable must also be true. Use is_test_challenge to mark production + test data that may be deleted after testing. value: type: string description: >- Metadata value. For submission_type, supported values are zip and url. For allowAllRegistrantsToDownloadWinningSubmissions, only the exact strings true and - false are accepted; a missing entry behaves as false. + false are accepted; a missing entry behaves as false. For is_test_challenge, only + the exact strings true and false are accepted; omitted values behave as false. Its + effective value cannot change in an update that starts in or transitions to + COMPLETED or CANCELLED status; omitting metadata preserves the existing value. required: - name - value diff --git a/src/common/challenge-helper.ts b/src/common/challenge-helper.ts index 9e6fa15..3234404 100644 --- a/src/common/challenge-helper.ts +++ b/src/common/challenge-helper.ts @@ -168,6 +168,69 @@ class ChallengeHelper { } } + /** + * Add the explicit false default for the metadata-backed test challenge flag. + * Challenge creation uses this before persistence so all newly created challenges have a + * deterministic `is_test_challenge` value. An existing entry is preserved unchanged so the + * subsequent validator can reject invalid values instead of silently replacing them. + * + * @param {Array|undefined|null} metadata challenge metadata entries + * @returns {Array} the original metadata entries plus the default flag when absent + * @throws {BadRequestError} if metadata is supplied with a non-array value + */ + applyTestChallengeMetadataDefault(metadata) { + if (!_.isNil(metadata) && !_.isArray(metadata)) { + throw new errors.BadRequestError("metadata must be an array"); + } + + const resolvedMetadata = metadata || []; + const testChallengeEntry = _.find(resolvedMetadata, { + name: ChallengeMetadataNames.IS_TEST_CHALLENGE, + }); + if (!_.isNil(testChallengeEntry)) { + return resolvedMetadata; + } + + return [ + ...resolvedMetadata, + { + name: ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "false", + }, + ]; + } + + /** + * Validate the metadata-backed test challenge flag. + * Create and update request validation call this before metadata is persisted. The exact string + * representation keeps Challenge API responses and downstream payment checks consistent. + * + * @param {Array|undefined|null} metadata challenge metadata entries + * @returns {void} + * @throws {BadRequestError} if `is_test_challenge` is not the string `true` or `false` + */ + validateTestChallengeMetadata(metadata) { + if (_.isNil(metadata)) { + return; + } + + const testChallengeEntry = _.find(metadata, { + name: ChallengeMetadataNames.IS_TEST_CHALLENGE, + }); + if (_.isNil(testChallengeEntry)) { + return; + } + + if ( + typeof testChallengeEntry.value !== "string" || + !_.includes(BOOLEAN_METADATA_VALUES, testChallengeEntry.value) + ) { + throw new errors.BadRequestError( + "metadata is_test_challenge must be either true or false as a string" + ); + } + } + validatePrizeSetsAndGetPrizeType(prizeSets) { if (_.isEmpty(prizeSets)) return null; @@ -266,6 +329,7 @@ class ChallengeHelper { // helper.ensureNoDuplicateOrNullElements(challenge.events, 'events') this.validateSubmissionTypeMetadata(challenge.metadata); this.validateRegisteredMemberWinningSubmissionDownloadMetadata(challenge.metadata); + this.validateTestChallengeMetadata(challenge.metadata); // check groups authorization if (challenge.groups && challenge.groups.length > 0) { @@ -743,6 +807,7 @@ class ChallengeHelper { helper.ensureNoDuplicateOrNullElements(data.groups, "groups"); this.validateSubmissionTypeMetadata(data.metadata); this.validateRegisteredMemberWinningSubmissionDownloadMetadata(data.metadata); + this.validateTestChallengeMetadata(data.metadata); if (data.projectId) { await ChallengeHelper.ensureProjectExist(data.projectId, currentUser); diff --git a/src/scripts/backfill-completed-point-challenge-results.sql b/src/scripts/backfill-completed-point-challenge-results.sql new file mode 100644 index 0000000..4eab888 --- /dev/null +++ b/src/scripts/backfill-completed-point-challenge-results.sql @@ -0,0 +1,317 @@ +/* + * Backfill member profile point awards for completed point-prize challenges. + * + * Run this against the PostgreSQL database that contains both the `challenges` + * and `members` schemas. The source mapping matches Autopilot's completion flow: + * + * - placement prizes are ordered by value descending; + * - a placement winner receives the prize at the same ordinal; + * - only prizes whose normalized type is POINT are copied; + * - fractional point values are truncated; and + * - duplicate winner rows retain the member's lowest placement. + * + * The script is safe to rerun. It inserts missing memberChallengePoints rows, + * updates differing rows, leaves matching rows unchanged, and never deletes + * rows. Challenges with ambiguous source data are reported and skipped. + * + * Usage: + * psql "$DATABASE_URL" \ + * -f src/scripts/backfill-completed-point-challenge-results.sql + * + * To preview without retaining changes, replace the final COMMIT with ROLLBACK. + */ + +BEGIN; + +CREATE TEMP TABLE "_point_challenge_ranked_prizes" ON COMMIT DROP AS +WITH placement_sets AS ( + SELECT + cps."id" AS "prizeSetId", + cps."challengeId", + COUNT(*) OVER (PARTITION BY cps."challengeId") AS "placementSetCount" + FROM "challenges"."ChallengePrizeSet" cps + WHERE cps."type"::text = 'PLACEMENT' +), +prize_value_groups AS ( + SELECT + p."prizeSetId", + p."value", + COUNT(DISTINCT UPPER(BTRIM(p."type"))) AS "currencyTypeCount" + FROM "challenges"."Prize" p + INNER JOIN placement_sets ps + ON ps."prizeSetId" = p."prizeSetId" + GROUP BY p."prizeSetId", p."value" +) +SELECT + ps."challengeId", + ps."prizeSetId", + ps."placementSetCount", + p."id" AS "prizeId", + UPPER(BTRIM(p."type")) AS "prizeType", + p."value" AS "prizeValue", + ROW_NUMBER() OVER ( + PARTITION BY ps."prizeSetId" + ORDER BY p."value" DESC, p."id" ASC + )::integer AS "prizePlacement", + value_groups."currencyTypeCount" > 1 AS "hasMixedCurrencyTie" +FROM placement_sets ps +INNER JOIN "challenges"."Prize" p + ON p."prizeSetId" = ps."prizeSetId" +INNER JOIN prize_value_groups value_groups + ON value_groups."prizeSetId" = p."prizeSetId" + AND value_groups."value" = p."value"; + +CREATE TEMP TABLE "_point_challenge_ranked_winners" ON COMMIT DROP AS +SELECT + cw."id" AS "winnerId", + cw."challengeId", + cw."userId"::bigint AS "userId", + cw."placement", + ROW_NUMBER() OVER ( + PARTITION BY cw."challengeId", cw."userId" + ORDER BY cw."placement" ASC, cw."createdAt" ASC, cw."id" ASC + )::integer AS "winnerRank" +FROM "challenges"."ChallengeWinner" cw +WHERE cw."type"::text = 'PLACEMENT'; + +CREATE TEMP TABLE "_point_challenge_award_candidates" ON COMMIT DROP AS +WITH matched_awards AS ( + SELECT + c."id" AS "challengeId", + c."name" AS "challengeName", + winners."userId", + winners."placement", + prizes."prizeId", + prizes."prizeValue", + prizes."hasMixedCurrencyTie", + member_row."userId" IS NOT NULL AS "memberExists", + ROW_NUMBER() OVER ( + PARTITION BY c."id", winners."userId" + ORDER BY winners."placement" ASC, winners."winnerId" ASC + )::integer AS "awardRank" + FROM "challenges"."Challenge" c + INNER JOIN "_point_challenge_ranked_prizes" prizes + ON prizes."challengeId" = c."id" + AND prizes."placementSetCount" = 1 + AND prizes."prizeType" = 'POINT' + INNER JOIN "_point_challenge_ranked_winners" winners + ON winners."challengeId" = c."id" + AND winners."placement" = prizes."prizePlacement" + LEFT JOIN "members"."member" member_row + ON member_row."userId" = winners."userId" + WHERE c."status"::text = 'COMPLETED' +) +SELECT + matched."challengeId", + matched."challengeName", + matched."userId", + matched."placement", + matched."prizeId", + matched."prizeValue", + CASE + WHEN matched."prizeValue" > 0 + AND matched."prizeValue" <= 2147483647 + THEN TRUNC(matched."prizeValue")::integer + ELSE NULL + END AS "points", + matched."hasMixedCurrencyTie", + matched."memberExists" +FROM matched_awards matched +WHERE matched."awardRank" = 1; + +-- Preflight summary. `eligibleRows` is the maximum number of rows this run can +-- insert or update after excluding source ambiguities and missing members. +SELECT + COUNT(DISTINCT candidates."challengeId") AS "challengesWithMappedPointAwards", + COUNT(*) AS "mappedPointAwards", + COUNT(*) FILTER ( + WHERE candidates."points" IS NOT NULL + AND candidates."points" > 0 + AND NOT candidates."hasMixedCurrencyTie" + AND candidates."memberExists" + ) AS "eligibleRows", + COUNT(*) FILTER (WHERE NOT candidates."memberExists") AS "missingMemberRows", + COUNT(*) FILTER (WHERE candidates."points" IS NULL OR candidates."points" <= 0) + AS "invalidPointValueRows", + COUNT(*) FILTER (WHERE candidates."hasMixedCurrencyTie") AS "ambiguousPrizeRows", + COUNT(*) FILTER ( + WHERE existing."id" IS NULL + AND candidates."points" > 0 + AND NOT candidates."hasMixedCurrencyTie" + AND candidates."memberExists" + ) AS "rowsToInsert", + COUNT(*) FILTER ( + WHERE existing."id" IS NOT NULL + AND candidates."points" > 0 + AND NOT candidates."hasMixedCurrencyTie" + AND candidates."memberExists" + AND ( + existing."challengeName" IS DISTINCT FROM candidates."challengeName" + OR existing."placement" IS DISTINCT FROM candidates."placement" + OR existing."points" IS DISTINCT FROM candidates."points" + ) + ) AS "rowsToUpdate" +FROM "_point_challenge_award_candidates" candidates +LEFT JOIN "members"."memberChallengePoints" existing + ON existing."challengeId" = candidates."challengeId" + AND existing."userId" = candidates."userId"; + +-- Completed point challenges with more than one placement prize set are +-- ambiguous because the application expects one placement set. They are not +-- included in the backfill. +SELECT DISTINCT + c."id" AS "challengeId", + c."name" AS "challengeName", + prizes."placementSetCount" +FROM "challenges"."Challenge" c +INNER JOIN "_point_challenge_ranked_prizes" prizes + ON prizes."challengeId" = c."id" +WHERE c."status"::text = 'COMPLETED' + AND prizes."prizeType" = 'POINT' + AND prizes."placementSetCount" > 1 +ORDER BY c."id"; + +-- A completed point challenge without placement winners has no authoritative +-- member result to copy and requires separate winner-data investigation. +SELECT DISTINCT + c."id" AS "challengeId", + c."name" AS "challengeName" +FROM "challenges"."Challenge" c +INNER JOIN "_point_challenge_ranked_prizes" prizes + ON prizes."challengeId" = c."id" +WHERE c."status"::text = 'COMPLETED' + AND prizes."prizeType" = 'POINT' + AND NOT EXISTS ( + SELECT 1 + FROM "_point_challenge_ranked_winners" winners + WHERE winners."challengeId" = c."id" + ) +ORDER BY c."id"; + +-- Equal-valued prizes with different currencies have no reliable placement +-- ordering. These mapped awards are reported and skipped. +SELECT + candidates."challengeId", + candidates."challengeName", + candidates."userId", + candidates."placement", + candidates."prizeValue" +FROM "_point_challenge_award_candidates" candidates +WHERE candidates."hasMixedCurrencyTie" +ORDER BY candidates."challengeId", candidates."placement", candidates."userId"; + +-- Invalid or non-positive point amounts are not accepted by the member API and +-- are omitted from the write. +SELECT + candidates."challengeId", + candidates."challengeName", + candidates."userId", + candidates."placement", + candidates."prizeValue" +FROM "_point_challenge_award_candidates" candidates +WHERE candidates."points" IS NULL OR candidates."points" <= 0 +ORDER BY candidates."challengeId", candidates."placement", candidates."userId"; + +-- Missing member rows would violate the memberChallengePoints foreign key. +SELECT + candidates."challengeId", + candidates."challengeName", + candidates."userId", + candidates."placement", + candidates."points" +FROM "_point_challenge_award_candidates" candidates +WHERE NOT candidates."memberExists" +ORDER BY candidates."challengeId", candidates."placement", candidates."userId"; + +-- Duplicate placement-winner rows are reduced to the member's lowest +-- placement, matching the completion flow. They are shown for investigation. +SELECT + winners."challengeId", + winners."userId", + winners."placement", + winners."winnerId" +FROM "_point_challenge_ranked_winners" winners +INNER JOIN "challenges"."Challenge" c + ON c."id" = winners."challengeId" +WHERE c."status"::text = 'COMPLETED' + AND winners."winnerRank" > 1 + AND EXISTS ( + SELECT 1 + FROM "_point_challenge_ranked_prizes" prizes + WHERE prizes."challengeId" = winners."challengeId" + AND prizes."prizeType" = 'POINT' + ) +ORDER BY winners."challengeId", winners."userId", winners."placement"; + +WITH eligible_awards AS ( + SELECT + candidates."challengeId", + candidates."challengeName", + candidates."userId", + candidates."placement", + candidates."points" + FROM "_point_challenge_award_candidates" candidates + WHERE candidates."points" > 0 + AND NOT candidates."hasMixedCurrencyTie" + AND candidates."memberExists" +), +upserted AS ( + INSERT INTO "members"."memberChallengePoints" AS stored_points ( + "challengeId", + "challengeName", + "userId", + "placement", + "points", + "createdAt", + "createdBy", + "updatedAt", + "updatedBy" + ) + SELECT + awards."challengeId", + awards."challengeName", + awards."userId", + awards."placement", + awards."points", + CURRENT_TIMESTAMP, + 'challenge-points-backfill', + CURRENT_TIMESTAMP, + 'challenge-points-backfill' + FROM eligible_awards awards + ON CONFLICT ("challengeId", "userId") DO UPDATE + SET + "challengeName" = EXCLUDED."challengeName", + "placement" = EXCLUDED."placement", + "points" = EXCLUDED."points", + "updatedAt" = CURRENT_TIMESTAMP, + "updatedBy" = 'challenge-points-backfill' + WHERE stored_points."challengeName" IS DISTINCT FROM EXCLUDED."challengeName" + OR stored_points."placement" IS DISTINCT FROM EXCLUDED."placement" + OR stored_points."points" IS DISTINCT FROM EXCLUDED."points" + RETURNING "challengeId", "userId" +) +SELECT + COUNT(*) AS "rowsInsertedOrUpdated", + COUNT(DISTINCT "challengeId") AS "challengesAffected" +FROM upserted; + +-- Post-check: both counts should be zero. +SELECT + COUNT(*) FILTER (WHERE stored."id" IS NULL) AS "eligibleRowsStillMissing", + COUNT(*) FILTER ( + WHERE stored."id" IS NOT NULL + AND ( + stored."challengeName" IS DISTINCT FROM candidates."challengeName" + OR stored."placement" IS DISTINCT FROM candidates."placement" + OR stored."points" IS DISTINCT FROM candidates."points" + ) + ) AS "eligibleRowsStillDifferent" +FROM "_point_challenge_award_candidates" candidates +LEFT JOIN "members"."memberChallengePoints" stored + ON stored."challengeId" = candidates."challengeId" + AND stored."userId" = candidates."userId" +WHERE candidates."points" > 0 + AND NOT candidates."hasMixedCurrencyTie" + AND candidates."memberExists"; + +COMMIT; diff --git a/src/services/ChallengeService.ts b/src/services/ChallengeService.ts index d19cf85..e33a53d 100644 --- a/src/services/ChallengeService.ts +++ b/src/services/ChallengeService.ts @@ -88,6 +88,11 @@ const CANCELLED_CHALLENGE_STATUSES = new Set([ ChallengeStatusEnum.CANCELLED_ZERO_REGISTRATIONS, ]); +const TERMINAL_CHALLENGE_STATUSES = new Set([ + ChallengeStatusEnum.COMPLETED, + ...CANCELLED_CHALLENGE_STATUSES, +]); + /** * Determines whether a challenge status is one of the terminal cancelled states. * @param {String} status challenge status from the update payload or stored challenge @@ -97,6 +102,64 @@ function isCancelledChallengeStatus(status) { return CANCELLED_CHALLENGE_STATUSES.has(status); } +/** + * Determines whether a challenge has reached a terminal status for test-data cleanup rules. + * Completed and every explicit cancelled status are terminal; draft, approved, active, deleted, + * and new challenges are not. + * + * @param {String} status challenge status from persistence + * @returns {Boolean} true for COMPLETED and CANCELLED* statuses + */ +function isTerminalChallengeStatus(status) { + return TERMINAL_CHALLENGE_STATUSES.has(status); +} + +/** + * Reads the effective test-challenge flag from metadata using strict enabled semantics. + * Only the exact metadata pair `is_test_challenge: "true"` is enabled. Missing, false, and + * malformed values are disabled. Update protection and deletion eligibility use this method. + * + * @param {Array|undefined|null} metadata challenge metadata entries + * @returns {Boolean} true only when an exact enabled metadata entry exists + */ +function isTestChallengeMetadataEnabled(metadata) { + return _.some(metadata, { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }); +} + +/** + * Prevents changing test-data classification in updates that start or finish terminal. + * Metadata arrays replace the stored array on update, so supplying an array without the flag has + * an effective false value. Omitting the metadata property entirely preserves the stored value. + * This guard runs before project lookups or persistence in updateChallenge. + * + * @param {Object} challenge current persisted challenge response + * @param {Object} data raw validated update payload + * @returns {void} + * @throws {BadRequestError} when a terminal update changes the effective test-challenge flag + */ +function ensureTerminalTestChallengeMetadataIsUnchanged(challenge, data) { + const currentStatus = _.get(challenge, "status"); + const finalStatus = _.isNil(_.get(data, "status")) ? currentStatus : _.get(data, "status"); + if ( + _.isNil(data) || + (!isTerminalChallengeStatus(currentStatus) && !isTerminalChallengeStatus(finalStatus)) || + !Object.prototype.hasOwnProperty.call(data, "metadata") + ) { + return; + } + + const currentFlag = isTestChallengeMetadataEnabled(_.get(challenge, "metadata")); + const requestedFlag = isTestChallengeMetadataEnabled(data.metadata); + if (currentFlag !== requestedFlag) { + throw new errors.BadRequestError( + "is_test_challenge metadata cannot be changed when a challenge is or becomes COMPLETED or CANCELLED", + ); + } +} + /** * Loads submission counters for challenge responses from the review submission table. * @@ -2418,11 +2481,13 @@ searchChallenges.schema = { * Create challenge. * Challenges billed to configured Topgear accounts skip manual budget approval and are auto-approved. * @param {Object} currentUser the user who perform operation - * @param {Object} challenge the challenge to created + * @param {Object} challenge the challenge to create; omitted `is_test_challenge` metadata defaults + * to the exact string `false` * @param {String} userToken the user token * @returns {Object} the created challenge */ async function createChallenge(currentUser, challenge, userToken) { + challenge.metadata = challengeHelper.applyTestChallengeMetadataDefault(challenge.metadata); const buildLogContext = () => JSON.stringify({ challengeName: challenge.name, @@ -2849,8 +2914,11 @@ createChallenge.schema = { Joi.object().keys({ name: Joi.string().required(), value: Joi.when("name", { - is: constants.ChallengeMetadataNames - .ALLOW_ALL_REGISTRANTS_TO_DOWNLOAD_WINNING_SUBMISSIONS, + is: Joi.valid( + constants.ChallengeMetadataNames + .ALLOW_ALL_REGISTRANTS_TO_DOWNLOAD_WINNING_SUBMISSIONS, + constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + ), then: Joi.string() .valid(...constants.BOOLEAN_METADATA_VALUES) .strict() @@ -3530,10 +3598,13 @@ function prepareTaskCompletionData(challenge, challengeResources, data) { * When a challenge transitions to completed task status or a cancelled status, * payment generation is requested after the database update commits. * Challenges billed to configured Topgear accounts skip manual budget approval and remain approved. + * Updates that start in or transition to a completed/cancelled status may not change the effective + * `is_test_challenge` metadata value. * @param {Object} currentUser the user who perform operation * @param {String} challengeId the challenge id * @param {Object} data the challenge data to be updated * @returns {Object} the updated challenge + * @throws {BadRequestError} if an update starting or finishing terminal changes the test flag */ // Note: `options` may be a boolean for backward compatibility (emitEvent flag), // or an object { emitEvent?: boolean }. @@ -3555,6 +3626,7 @@ async function updateChallenge(currentUser, challengeId, data, options: any = {} await helper.ensureChallengeWhitelistAccess(currentUser, challenge.id); enrichChallengeForResponse(challenge); prismaHelper.convertModelToResponse(challenge); + ensureTerminalTestChallengeMetadataIsUnchanged(challenge, data); const originalChallengePhases = _.cloneDeep(challenge.phases || []); const auditUserId = _.toString(currentUser.userId); const payloadIncludesTerms = @@ -4616,8 +4688,11 @@ updateChallenge.schema = { .keys({ name: Joi.string().required(), value: Joi.when("name", { - is: constants.ChallengeMetadataNames - .ALLOW_ALL_REGISTRANTS_TO_DOWNLOAD_WINNING_SUBMISSIONS, + is: Joi.valid( + constants.ChallengeMetadataNames + .ALLOW_ALL_REGISTRANTS_TO_DOWNLOAD_WINNING_SUBMISSIONS, + constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + ), then: Joi.string() .valid(...constants.BOOLEAN_METADATA_VALUES) .strict() @@ -5369,19 +5444,41 @@ function sanitizeData(data, challenge) { } /** - * Delete challenge. + * Delete a challenge in NEW status or terminal test data after completion/cancellation. + * The terminal-status bypass requires both a COMPLETED/CANCELLED* status and the exact metadata + * pair `is_test_challenge: "true"`. Draft, approved, and active challenges cannot use the bypass. + * Missing, false, or malformed values are disabled. Existing modification authorization checks + * are applied before deletion. + * * @param {Object} currentUser the user who perform operation * @param {String} challengeId the challenge id * @returns {Object} the deleted challenge + * @throws {NotFoundError} if the challenge does not exist or is not eligible for deletion + * @throws {ForbiddenError} if the caller cannot modify the challenge */ async function deleteChallenge(currentUser, challengeId) { // Use findFirst for compound filters; findUnique only supports unique fields const challenge = await prisma.challenge.findFirst({ - where: { id: challengeId, status: ChallengeStatusEnum.NEW }, + where: { + id: challengeId, + OR: [ + { status: ChallengeStatusEnum.NEW }, + { + status: { in: Array.from(TERMINAL_CHALLENGE_STATUSES) }, + metadata: { + some: { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }, + }, + }, + ], + }, + include: { metadata: true }, }); if (_.isNil(challenge) || _.isNil(challenge.id)) { throw new errors.NotFoundError( - `Challenge with id: ${challengeId} doesn't exist or is not in New status`, + `Challenge with id: ${challengeId} doesn't exist or is not eligible for deletion; deletion requires NEW status or a COMPLETED/CANCELLED status with is_test_challenge set to the exact string true`, ); } // ensure user can modify challenge diff --git a/test/unit/ChallengeService.test.js b/test/unit/ChallengeService.test.js index aa2a1ad..432f9a6 100644 --- a/test/unit/ChallengeService.test.js +++ b/test/unit/ChallengeService.test.js @@ -293,6 +293,52 @@ describe("challenge service unit tests", () => { should.equal(result.numOfRegistrants, 0); }); + it("persists false is_test_challenge metadata when create omits the flag", async () => { + const challengeData = _.cloneDeep(testChallengeData); + challengeData.discussions[0].type = "CHALLENGE"; + challengeData.prizeSets[0].type = PrizeSetTypeEnum.PLACEMENT; + challengeData.status = ChallengeStatusEnum.NEW; + const originalGetProject = projectHelper.getProject; + const originalGetProjectBillingInformation = projectHelper.getProjectBillingInformation; + const originalPostBusEvent = helper.postBusEvent; + let createdChallengeId; + + projectHelper.getProject = async () => ({ directProjectId: 33541 }); + projectHelper.getProjectBillingInformation = async () => ({ + billingAccountId: null, + markup: 0, + }); + helper.postBusEvent = async () => {}; + + try { + const result = await service.createChallenge( + { isMachine: true, sub: "sub", userId: "testuser" }, + challengeData, + config.M2M_FULL_ACCESS_TOKEN || "test-token", + ); + createdChallengeId = result.id; + + _.find(result.metadata, { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + }).value.should.equal("false"); + + const persistedMetadata = await prisma.challengeMetadata.findFirst({ + where: { + challengeId: result.id, + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + }, + }); + persistedMetadata.value.should.equal("false"); + } finally { + projectHelper.getProject = originalGetProject; + projectHelper.getProjectBillingInformation = originalGetProjectBillingInformation; + helper.postBusEvent = originalPostBusEvent; + if (createdChallengeId) { + await prisma.challenge.deleteMany({ where: { id: createdChallengeId } }); + } + } + }); + it("locks draft challenge budget when the challenge is saved", async () => { const challengeData = _.cloneDeep(testChallengeData); challengeData.status = ChallengeStatusEnum.DRAFT; @@ -3030,6 +3076,509 @@ describe("challenge service unit tests", () => { }); }); + describe("delete challenge tests", () => { + const challengeIds = []; + let originalEnsureUserCanModifyChallenge; + let originalPostBusEvent; + + const createDeletionChallenge = async ({ status, testMetadataValue }) => { + const challengeId = uuid(); + challengeIds.push(challengeId); + const metadata = _.isUndefined(testMetadataValue) + ? undefined + : { + create: { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: testMetadataValue, + createdBy: "delete-test", + updatedBy: "delete-test", + }, + }; + + return prisma.challenge.create({ + data: { + id: challengeId, + name: `Deletion coverage ${challengeId}`, + typeId: data.challenge.typeId, + trackId: data.challenge.trackId, + status, + tags: [], + groups: [], + currentPhaseNames: [], + createdBy: "delete-test", + updatedBy: "delete-test", + ...(_.isUndefined(metadata) ? {} : { metadata }), + }, + }); + }; + + beforeEach(() => { + originalEnsureUserCanModifyChallenge = helper.ensureUserCanModifyChallenge; + originalPostBusEvent = helper.postBusEvent; + helper.ensureUserCanModifyChallenge = async () => {}; + helper.postBusEvent = async () => {}; + }); + + afterEach(async () => { + helper.ensureUserCanModifyChallenge = originalEnsureUserCanModifyChallenge; + helper.postBusEvent = originalPostBusEvent; + await prisma.challenge.deleteMany({ where: { id: { in: challengeIds.splice(0) } } }); + }); + + it("deletes a completed challenge with exact true test metadata", async () => { + const challenge = await createDeletionChallenge({ + status: ChallengeStatusEnum.COMPLETED, + testMetadataValue: "true", + }); + + const result = await service.deleteChallenge( + { isMachine: true, userId: "delete-test" }, + challenge.id, + ); + + should.equal(result.id, challenge.id); + _.find(result.metadata, { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + }).value.should.equal("true"); + should.equal(await prisma.challenge.count({ where: { id: challenge.id } }), 0); + }); + + it("deletes exact-true test challenges in every cancelled terminal status", async () => { + const cancelledStatuses = Object.values(ChallengeStatusEnum).filter((status) => + status.startsWith("CANCELLED"), + ); + + for (const status of cancelledStatuses) { + const challenge = await createDeletionChallenge({ + status, + testMetadataValue: "true", + }); + + await service.deleteChallenge( + { isMachine: true, userId: "delete-test" }, + challenge.id, + ); + + should.equal(await prisma.challenge.count({ where: { id: challenge.id } }), 0); + } + }); + + it("preserves NEW challenge deletion regardless of test metadata", async () => { + const challenge = await createDeletionChallenge({ + status: ChallengeStatusEnum.NEW, + testMetadataValue: "false", + }); + + await service.deleteChallenge({ isMachine: true, userId: "delete-test" }, challenge.id); + + should.equal(await prisma.challenge.count({ where: { id: challenge.id } }), 0); + }); + + for (const testMetadataValue of [undefined, "false", "TRUE"]) { + it(`rejects non-NEW deletion with ${ + _.isUndefined(testMetadataValue) ? "missing" : testMetadataValue + } test metadata`, async () => { + const challenge = await createDeletionChallenge({ + status: ChallengeStatusEnum.COMPLETED, + testMetadataValue, + }); + + try { + await service.deleteChallenge( + { isMachine: true, userId: "delete-test" }, + challenge.id, + ); + } catch (error) { + should.equal(error.name, "NotFoundError"); + should.equal( + error.message, + `Challenge with id: ${challenge.id} doesn't exist or is not eligible for deletion; deletion requires NEW status or a COMPLETED/CANCELLED status with is_test_challenge set to the exact string true`, + ); + should.equal(await prisma.challenge.count({ where: { id: challenge.id } }), 1); + return; + } + + throw new Error("should not reach here"); + }); + } + + for (const status of [ + ChallengeStatusEnum.DRAFT, + ChallengeStatusEnum.APPROVED, + ChallengeStatusEnum.ACTIVE, + ]) { + it(`rejects exact-true deletion while the challenge is ${status}`, async () => { + const challenge = await createDeletionChallenge({ + status, + testMetadataValue: "true", + }); + + try { + await service.deleteChallenge( + { isMachine: true, userId: "delete-test" }, + challenge.id, + ); + } catch (error) { + should.equal(error.name, "NotFoundError"); + should.equal(await prisma.challenge.count({ where: { id: challenge.id } }), 1); + return; + } + + throw new Error("should not reach here"); + }); + } + }); + + describe("test challenge metadata update tests", () => { + const challengeIds = []; + let originalEnsureUserCanModifyChallenge; + let originalGenerateChallengePayments; + let originalGetChallengeResources; + let originalGetProjectBillingInformation; + let originalPostBusEvent; + + const createMetadataUpdateChallenge = async ({ status, testMetadataValue }) => { + const challengeId = uuid(); + challengeIds.push(challengeId); + const metadata = _.isUndefined(testMetadataValue) + ? undefined + : { + create: { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: testMetadataValue, + createdBy: "metadata-update-test", + updatedBy: "metadata-update-test", + }, + }; + + return prisma.challenge.create({ + data: { + id: challengeId, + name: `Metadata update coverage ${challengeId}`, + typeId: data.challenge.typeId, + trackId: data.challenge.trackId, + status, + tags: [], + groups: [], + currentPhaseNames: [], + createdBy: "metadata-update-test", + updatedBy: "metadata-update-test", + ...(_.isUndefined(metadata) ? {} : { metadata }), + }, + }); + }; + + beforeEach(() => { + originalEnsureUserCanModifyChallenge = helper.ensureUserCanModifyChallenge; + originalGenerateChallengePayments = helper.generateChallengePayments; + originalGetChallengeResources = helper.getChallengeResources; + originalGetProjectBillingInformation = projectHelper.getProjectBillingInformation; + originalPostBusEvent = helper.postBusEvent; + projectHelper.getProjectBillingInformation = async () => ({ + billingAccountId: null, + markup: 0, + }); + helper.ensureUserCanModifyChallenge = async () => {}; + helper.generateChallengePayments = async () => true; + helper.getChallengeResources = async () => []; + helper.postBusEvent = async () => {}; + }); + + afterEach(async () => { + projectHelper.getProjectBillingInformation = originalGetProjectBillingInformation; + helper.ensureUserCanModifyChallenge = originalEnsureUserCanModifyChallenge; + helper.generateChallengePayments = originalGenerateChallengePayments; + helper.getChallengeResources = originalGetChallengeResources; + helper.postBusEvent = originalPostBusEvent; + await prisma.challenge.deleteMany({ where: { id: { in: challengeIds.splice(0) } } }); + }); + + it("allows a non-terminal challenge to enable the test flag", async () => { + const challenge = await createMetadataUpdateChallenge({ + status: ChallengeStatusEnum.DRAFT, + }); + + const result = await service.updateChallenge( + { isMachine: true, userId: "metadata-update-test" }, + challenge.id, + { + metadata: [ + { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }, + ], + }, + ); + + _.find(result.metadata, { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + }).value.should.equal("true"); + }); + + const terminalTransitionMutationCases = [ + { + name: "rejects missing-to-true while transitioning ACTIVE to COMPLETED", + initialStatus: ChallengeStatusEnum.ACTIVE, + initialValue: undefined, + finalStatus: ChallengeStatusEnum.COMPLETED, + requestedMetadata: [ + { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }, + ], + }, + { + name: "rejects false-to-true while transitioning DRAFT to CANCELLED", + initialStatus: ChallengeStatusEnum.DRAFT, + initialValue: "false", + finalStatus: ChallengeStatusEnum.CANCELLED_CLIENT_REQUEST, + requestedMetadata: [ + { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }, + ], + }, + { + name: "rejects true-to-missing while transitioning ACTIVE to COMPLETED", + initialStatus: ChallengeStatusEnum.ACTIVE, + initialValue: "true", + finalStatus: ChallengeStatusEnum.COMPLETED, + requestedMetadata: [], + }, + { + name: "rejects true-to-false while transitioning DRAFT to CANCELLED", + initialStatus: ChallengeStatusEnum.DRAFT, + initialValue: "true", + finalStatus: ChallengeStatusEnum.CANCELLED_CLIENT_REQUEST, + requestedMetadata: [ + { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "false", + }, + ], + }, + ]; + + for (const testCase of terminalTransitionMutationCases) { + it(testCase.name, async () => { + const challenge = await createMetadataUpdateChallenge({ + status: testCase.initialStatus, + testMetadataValue: testCase.initialValue, + }); + + try { + await service.updateChallenge( + { isMachine: true, userId: "metadata-update-test" }, + challenge.id, + { + status: testCase.finalStatus, + metadata: testCase.requestedMetadata, + }, + ); + } catch (error) { + should.equal(error.name, "BadRequestError"); + should.equal( + error.message, + "is_test_challenge metadata cannot be changed when a challenge is or becomes COMPLETED or CANCELLED", + ); + const persistedChallenge = await prisma.challenge.findUnique({ + where: { id: challenge.id }, + include: { metadata: true }, + }); + should.equal(persistedChallenge.status, testCase.initialStatus); + should.equal( + _.some(persistedChallenge.metadata, { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }), + testCase.initialValue === "true", + ); + return; + } + + throw new Error("should not reach here"); + }); + } + + const terminalTransitionPreservationCases = [ + { + name: "allows explicit true preservation while transitioning ACTIVE to COMPLETED", + initialStatus: ChallengeStatusEnum.ACTIVE, + initialValue: "true", + finalStatus: ChallengeStatusEnum.COMPLETED, + requestedMetadata: [ + { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }, + ], + }, + { + name: "allows omitted metadata while transitioning DRAFT test data to CANCELLED", + initialStatus: ChallengeStatusEnum.DRAFT, + initialValue: "true", + finalStatus: ChallengeStatusEnum.CANCELLED_CLIENT_REQUEST, + }, + { + name: "allows omitted metadata while transitioning ACTIVE ordinary data to COMPLETED", + initialStatus: ChallengeStatusEnum.ACTIVE, + initialValue: undefined, + finalStatus: ChallengeStatusEnum.COMPLETED, + }, + { + name: "allows explicit false preservation while transitioning DRAFT to CANCELLED", + initialStatus: ChallengeStatusEnum.DRAFT, + initialValue: "false", + finalStatus: ChallengeStatusEnum.CANCELLED_CLIENT_REQUEST, + requestedMetadata: [ + { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "false", + }, + ], + }, + ]; + + for (const testCase of terminalTransitionPreservationCases) { + it(testCase.name, async () => { + const challenge = await createMetadataUpdateChallenge({ + status: testCase.initialStatus, + testMetadataValue: testCase.initialValue, + }); + const updateData = { status: testCase.finalStatus }; + if (!_.isUndefined(testCase.requestedMetadata)) { + updateData.metadata = testCase.requestedMetadata; + } + + const result = await service.updateChallenge( + { isMachine: true, userId: "metadata-update-test" }, + challenge.id, + updateData, + ); + + should.equal(result.status, testCase.finalStatus); + should.equal( + _.some(result.metadata, { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }), + testCase.initialValue === "true", + ); + }); + } + + it("rejects enabling the test flag on a completed ordinary challenge", async () => { + const challenge = await createMetadataUpdateChallenge({ + status: ChallengeStatusEnum.COMPLETED, + }); + + try { + await service.updateChallenge( + { isMachine: true, userId: "metadata-update-test" }, + challenge.id, + { + metadata: [ + { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }, + ], + }, + ); + } catch (error) { + should.equal(error.name, "BadRequestError"); + should.equal( + error.message, + "is_test_challenge metadata cannot be changed when a challenge is or becomes COMPLETED or CANCELLED", + ); + should.equal( + await prisma.challengeMetadata.count({ where: { challengeId: challenge.id } }), + 0, + ); + return; + } + + throw new Error("should not reach here"); + }); + + it("rejects removing the test flag from a cancelled test challenge", async () => { + const challenge = await createMetadataUpdateChallenge({ + status: ChallengeStatusEnum.CANCELLED_CLIENT_REQUEST, + testMetadataValue: "true", + }); + + try { + await service.updateChallenge( + { isMachine: true, userId: "metadata-update-test" }, + challenge.id, + { metadata: [] }, + ); + } catch (error) { + should.equal(error.name, "BadRequestError"); + should.equal( + await prisma.challengeMetadata.count({ + where: { + challengeId: challenge.id, + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }, + }), + 1, + ); + return; + } + + throw new Error("should not reach here"); + }); + + it("allows terminal metadata updates that keep an enabled test flag unchanged", async () => { + const challenge = await createMetadataUpdateChallenge({ + status: ChallengeStatusEnum.COMPLETED, + testMetadataValue: "true", + }); + + const result = await service.updateChallenge( + { isMachine: true, userId: "metadata-update-test" }, + challenge.id, + { + metadata: [ + { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }, + { name: "lifecycle-note", value: "updated" }, + ], + }, + ); + + _.find(result.metadata, { name: "lifecycle-note" }).value.should.equal("updated"); + }); + + it("allows terminal metadata updates that keep a disabled test flag unchanged", async () => { + const challenge = await createMetadataUpdateChallenge({ + status: ChallengeStatusEnum.CANCELLED_ZERO_SUBMISSIONS, + }); + + const result = await service.updateChallenge( + { isMachine: true, userId: "metadata-update-test" }, + challenge.id, + { metadata: [{ name: "lifecycle-note", value: "updated" }] }, + ); + + should.equal( + _.some(result.metadata, { + name: constants.ChallengeMetadataNames.IS_TEST_CHALLENGE, + value: "true", + }), + false, + ); + _.find(result.metadata, { name: "lifecycle-note" }).value.should.equal("updated"); + }); + }); + describe("close marathon match tests", () => { const adminUser = { isMachine: false, roles: [constants.UserRoles.Admin], userId: "admin" }; const m2mUser = { isMachine: true }; diff --git a/test/unit/challenge-helper.test.js b/test/unit/challenge-helper.test.js index d0331fa..ace9d51 100644 --- a/test/unit/challenge-helper.test.js +++ b/test/unit/challenge-helper.test.js @@ -226,4 +226,77 @@ describe("challenge metadata validation", () => { ); } }); + + it("adds an explicit false default when is_test_challenge is omitted", () => { + challengeHelper.applyTestChallengeMetadataDefault(undefined).should.deep.equal([ + { + name: "is_test_challenge", + value: "false", + }, + ]); + + challengeHelper.applyTestChallengeMetadataDefault([ + { + name: "submission_type", + value: "zip", + }, + ]).should.deep.equal([ + { + name: "submission_type", + value: "zip", + }, + { + name: "is_test_challenge", + value: "false", + }, + ]); + }); + + it("preserves an explicit is_test_challenge value when applying the default", () => { + challengeHelper.applyTestChallengeMetadataDefault([ + { + name: "is_test_challenge", + value: "true", + }, + ]).should.deep.equal([ + { + name: "is_test_challenge", + value: "true", + }, + ]); + }); + + it("allows exact string boolean values for is_test_challenge", () => { + for (const value of ["true", "false"]) { + expect(() => challengeHelper.validateTestChallengeMetadata([ + { + name: "is_test_challenge", + value, + }, + ])).not.to.throw(); + } + }); + + it("allows is_test_challenge to be omitted from update metadata", () => { + expect(() => challengeHelper.validateTestChallengeMetadata(undefined)).not.to.throw(); + expect(() => challengeHelper.validateTestChallengeMetadata([ + { + name: "submission_type", + value: "zip", + }, + ])).not.to.throw(); + }); + + it("rejects non-string or non-boolean is_test_challenge values", () => { + for (const value of [true, false, "TRUE", "yes", " true "]) { + expect(() => challengeHelper.validateTestChallengeMetadata([ + { + name: "is_test_challenge", + value, + }, + ])).to.throw( + "metadata is_test_challenge must be either true or false as a string" + ); + } + }); });