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 74cb26c..15837ed 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: @@ -2594,7 +2597,9 @@ definitions: 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 @@ -2881,13 +2886,17 @@ 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. + winning submissions after the challenge ends. For Design challenges, + 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 @@ -3058,13 +3067,18 @@ 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. + winning submissions after the challenge ends. For Design challenges, + 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 @@ -3280,13 +3294,18 @@ 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. + winning submissions after the challenge ends. For Design challenges, + 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 24270ea..4192db3 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. * @@ -2469,11 +2532,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, @@ -2900,8 +2965,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() @@ -3587,10 +3655,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 }. @@ -3612,6 +3683,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 = @@ -4673,8 +4745,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() @@ -5426,19 +5501,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 da60dca..2325db9 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; @@ -3188,6 +3234,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" + ); + } + }); });