From 665e532b0dd09072f6bd3753ed92c8896b52b5fe Mon Sep 17 00:00:00 2001 From: liuliquan Date: Thu, 5 Oct 2023 08:09:18 -0500 Subject: [PATCH 1/9] PLAT-3453 Stop copilots from paying themselves (#666) * PLAT-3453 Stop copilots from paying themselves 1. Stop copilots from paying themselves, thus copilots will need to contact manager to launch/complete the task 2. When updateChallenge, the helper.getChallengeResources function is called multiple times, now it's called only once * Only restrict launch/complete self-assigned Task Also validate winners is present in request payload when complete a Task * Task status change (#665) --------- Co-authored-by: Rakib Ansary --- src/common/challenge-helper.js | 4 +-- src/common/helper.js | 12 ++++--- src/services/ChallengeService.js | 62 ++++++++++++++++++++++++++++---- 3 files changed, 66 insertions(+), 12 deletions(-) diff --git a/src/common/challenge-helper.js b/src/common/challenge-helper.js index aef2bd1a..ac691cdb 100644 --- a/src/common/challenge-helper.js +++ b/src/common/challenge-helper.js @@ -105,9 +105,9 @@ class ChallengeHelper { } } - async validateChallengeUpdateRequest(currentUser, challenge, data) { + async validateChallengeUpdateRequest(currentUser, challenge, data, challengeResources) { if (process.env.LOCAL != "true") { - await helper.ensureUserCanModifyChallenge(currentUser, challenge); + await helper.ensureUserCanModifyChallenge(currentUser, challenge, challengeResources); } helper.ensureNoDuplicateOrNullElements(data.tags, "tags"); diff --git a/src/common/helper.js b/src/common/helper.js index f39800f0..46db3d85 100644 --- a/src/common/helper.js +++ b/src/common/helper.js @@ -528,14 +528,17 @@ async function getResourceRoles() { * Check if a user has full access on a challenge * @param {String} challengeId the challenge UUID * @param {String} userId the user ID + * @param {Array} challengeResources the challenge resources */ -async function userHasFullAccess(challengeId, userId) { +async function userHasFullAccess(challengeId, userId, challengeResources) { const resourceRoles = await getResourceRoles(); const rolesWithFullAccess = _.map( _.filter(resourceRoles, (r) => r.fullWriteAccess), "id" ); - const challengeResources = await getChallengeResources(challengeId); + if (!challengeResources) { + challengeResources = await getChallengeResources(challengeId); + } return ( _.filter( challengeResources, @@ -982,13 +985,14 @@ async function ensureUserCanViewChallenge(currentUser, challenge) { * * @param {Object} currentUser the user who perform operation * @param {Object} challenge the challenge to check + * @param {Array} challengeResources the challenge resources * @returns {Promise} */ -async function ensureUserCanModifyChallenge(currentUser, challenge) { +async function ensureUserCanModifyChallenge(currentUser, challenge, challengeResources) { // check groups authorization await ensureAccessibleByGroupsAccess(currentUser, challenge); // check full access - const isUserHasFullAccess = await userHasFullAccess(challenge.id, currentUser.userId); + const isUserHasFullAccess = await userHasFullAccess(challenge.id, currentUser.userId, challengeResources); if ( !currentUser.isMachine && !hasAdminRole(currentUser) && diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index 9b0fd8da..4b1e9e2d 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -1364,10 +1364,9 @@ function isDifferentPrizeSets(prizeSets = [], otherPrizeSets = []) { /** * Validate the winners array. * @param {Array} winners the Winner Array - * @param {String} winchallengeIdners the challenge ID + * @param {Array} challengeResources the challenge resources */ -async function validateWinners(winners, challengeId) { - const challengeResources = await helper.getChallengeResources(challengeId); +async function validateWinners(winners, challengeResources) { const registrants = _.filter(challengeResources, (r) => r.roleId === config.SUBMITTER_ROLE_ID); for (const prizeType of _.values(constants.prizeSetTypes)) { const filteredWinners = _.filter(winners, (w) => w.type === prizeType); @@ -1410,6 +1409,55 @@ async function validateWinners(winners, challengeId) { } } +/** + * Task shouldn't be launched/completed when it is assigned to the current user self. + * E.g: stop copilots from paying themselves, thus copilots will need to contact manager to launch/complete the task. + * @param {Object} currentUser the user who perform operation + * @param {Object} challenge the existing challenge + * @param {Object} data the new input challenge data + * @param {Array} challengeResources the challenge resources + */ +function validateTask(currentUser, challenge, data, challengeResources) { + if (!_.get(challenge, "legacy.pureV5Task")) { + // Not a Task + return; + } + + // Status changed to Active, indicating launch a Task + const isLaunchTask = + data.status === constants.challengeStatuses.Active && + challenge.status !== constants.challengeStatuses.Active; + + // Status changed to Completed, indicating complete a Task + const isCompleteTask = + data.status === constants.challengeStatuses.Completed && + challenge.status !== constants.challengeStatuses.Completed; + + // When complete a Task, input data should have winners + if (isCompleteTask && (!data.winners || !data.winners.length)) { + throw new errors.BadRequestError("The winners is required to complete a Task"); + } + + if (!currentUser.isMachine && (isLaunchTask || isCompleteTask)) { + // Whether task is assigned to current user + const assignedToCurrentUser = + _.filter( + challengeResources, + (r) => + r.roleId === config.SUBMITTER_ROLE_ID && + _.toString(r.memberId) === _.toString(currentUser.userId) + ).length > 0; + + if (assignedToCurrentUser) { + throw new errors.ForbiddenError( + `You are not allowed to ${ + data.status === constants.challengeStatuses.Active ? "lanuch" : "complete" + } task assigned to yourself. Please contact manager to operate.` + ); + } + } +} + /** * Update challenge. * @param {Object} currentUser the user who perform operation @@ -1441,7 +1489,10 @@ async function updateChallenge(currentUser, challengeId, data) { data = sanitizeData(sanitizeChallenge(data), challenge); logger.debug(`Sanitized Data: ${JSON.stringify(data)}`); - await validateChallengeUpdateRequest(currentUser, challenge, data); + const challengeResources = await helper.getChallengeResources(challengeId); + + await validateChallengeUpdateRequest(currentUser, challenge, data, challengeResources); + validateTask(currentUser, challenge, data, challengeResources); let sendActivationEmail = false; let sendSubmittedEmail = false; @@ -1716,7 +1767,7 @@ async function updateChallenge(currentUser, challengeId, data) { } if (data.winners && data.winners.length && data.winners.length > 0) { - await validateWinners(data.winners, challengeId); + await validateWinners(data.winners, challengeResources); } // Only m2m tokens are allowed to modify the `task.*` information on a challenge @@ -1777,7 +1828,6 @@ async function updateChallenge(currentUser, challengeId, data) { if (_.get(type, "isTask")) { if (!_.isEmpty(_.get(data, "task.memberId"))) { - const challengeResources = await helper.getChallengeResources(challengeId); const registrants = _.filter( challengeResources, (r) => r.roleId === config.SUBMITTER_ROLE_ID From f976de1e9cb8078ad3119ec525ecfc78af598817 Mon Sep 17 00:00:00 2001 From: sarojbehera1 Date: Thu, 5 Oct 2023 03:01:36 +0530 Subject: [PATCH 2/9] Validate Group Ids are valid or not --- src/common/helper.js | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/common/helper.js b/src/common/helper.js index f39800f0..33085b1d 100644 --- a/src/common/helper.js +++ b/src/common/helper.js @@ -898,7 +898,27 @@ async function _filterChallengesByGroupsAccess(currentUser, challenges) { const needToCheckForGroupAccess = !currentUser ? true : !currentUser.isMachine && !hasAdminRole(currentUser); - if (!needToCheckForGroupAccess) return challenges; + if(!needToCheckForGroupAccess) + { + for (const challenge of challenges) { + if(challenge && challenge.groups && challenge.groups.length>0) { + const promises = []; + _.each(challenge.groups, (g) => { + promises.push( + (async () => { + const group = await getGroupById(g); + if ( !group || !group.status==='active') { + throw new errors.BadRequestError("The groups provided are invalid "+g); + } + })() + ); + }); + await Promise.all(promises); + res.push(challenge); + } + } + return res; + } let userGroups; From 3357c683b9f5b4d1429c5339cfaabae693cf49c9 Mon Sep 17 00:00:00 2001 From: sarojbehera1 Date: Fri, 6 Oct 2023 03:40:01 +0530 Subject: [PATCH 3/9] Creating function for validate groups by getGroupById --- src/common/helper.js | 22 +------------------ src/services/ChallengeService.js | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/common/helper.js b/src/common/helper.js index 33085b1d..f39800f0 100644 --- a/src/common/helper.js +++ b/src/common/helper.js @@ -898,27 +898,7 @@ async function _filterChallengesByGroupsAccess(currentUser, challenges) { const needToCheckForGroupAccess = !currentUser ? true : !currentUser.isMachine && !hasAdminRole(currentUser); - if(!needToCheckForGroupAccess) - { - for (const challenge of challenges) { - if(challenge && challenge.groups && challenge.groups.length>0) { - const promises = []; - _.each(challenge.groups, (g) => { - promises.push( - (async () => { - const group = await getGroupById(g); - if ( !group || !group.status==='active') { - throw new errors.BadRequestError("The groups provided are invalid "+g); - } - })() - ); - }); - await Promise.all(promises); - res.push(challenge); - } - } - return res; - } + if (!needToCheckForGroupAccess) return challenges; let userGroups; diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index 9b0fd8da..eed62efb 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -910,6 +910,24 @@ searchChallenges.schema = { }) .unknown(true), }; +/** + * Validate Challenge groups. + * @param {Object} groups the group of a challenge + */ +async function validateGroups(groups) { + const promises = []; + _.each(groups, (g) => { + promises.push( + (async () => { + const group = await helper.getGroupById(g); + if (!group || group.status !== "active") { + throw new errors.BadRequestError("The groups provided are invalid " + g); + } + })() + ); + }); + await Promise.all(promises); +} /** * Create challenge. @@ -921,6 +939,15 @@ searchChallenges.schema = { async function createChallenge(currentUser, challenge, userToken) { await challengeHelper.validateCreateChallengeRequest(currentUser, challenge); + //Validate the groups if Valid or Not + if ( + challenge.groups && + challenge.groups.length > 0 && + (currentUser.isMachine || hasAdminRole(currentUser)) + ) { + await validateGroups(challenge.groups); + } + if (challenge.legacy.selfService) { // if self-service, create a new project (what about if projectId is provided in the payload? confirm with business!) if (!challenge.projectId) { @@ -1443,6 +1470,15 @@ async function updateChallenge(currentUser, challengeId, data) { await validateChallengeUpdateRequest(currentUser, challenge, data); + //Validate the groups if Valid or Not + if ( + data.groups && + data.groups.length > 0 && + (currentUser.isMachine || hasAdminRole(currentUser)) + ) { + await validateGroups(data.groups); + } + let sendActivationEmail = false; let sendSubmittedEmail = false; let sendCompletedEmail = false; From 2cf3744507d9872ed1c084d0da4f7eca9af2f43f Mon Sep 17 00:00:00 2001 From: sarojbehera1 Date: Wed, 11 Oct 2023 13:59:01 +0530 Subject: [PATCH 4/9] Moving code to challenge-helper --- src/common/challenge-helper.js | 35 ++++++++++++++++++++++++++++--- src/services/ChallengeService.js | 36 -------------------------------- 2 files changed, 32 insertions(+), 39 deletions(-) diff --git a/src/common/challenge-helper.js b/src/common/challenge-helper.js index aef2bd1a..485bc179 100644 --- a/src/common/challenge-helper.js +++ b/src/common/challenge-helper.js @@ -80,6 +80,25 @@ class ChallengeHelper { } } + /** + * Validate Challenge groups. + * @param {Object} groups the group of a challenge + */ + async validateGroups(groups) { + const promises = []; + _.each(groups, (g) => { + promises.push( + (async () => { + const group = await helper.getGroupById(g); + if (!group || group.status !== "active") { + throw new errors.BadRequestError("The groups provided are invalid " + g); + } + })() + ); + }); + await Promise.all(promises); + } + async validateCreateChallengeRequest(currentUser, challenge) { // projectId is required for non self-service challenges if (challenge.legacy.selfService == null && challenge.projectId == null) { @@ -98,7 +117,13 @@ class ChallengeHelper { // helper.ensureNoDuplicateOrNullElements(challenge.events, 'events') // check groups authorization - await helper.ensureAccessibleByGroupsAccess(currentUser, challenge); + if (challenge.groups && challenge.groups.length > 0) { + if (currentUser.isMachine || hasAdminRole(currentUser)) { + await this.validateGroups(challenge.groups); + } else { + await helper.ensureAccessibleByGroupsAccess(currentUser, challenge); + } + } if (challenge.constraints) { await ChallengeHelper.validateChallengeConstraints(challenge.constraints); @@ -118,8 +143,12 @@ class ChallengeHelper { } // check groups access to be updated group values - if (data.groups) { - await ensureAcessibilityToModifiedGroups(currentUser, data, challenge); + if (data.groups && data.groups.length > 0) { + if (currentUser.isMachine || hasAdminRole(currentUser)) { + await this.validateGroups(data.groups); + } else { + await ensureAcessibilityToModifiedGroups(currentUser, data, challenge); + } } // Ensure descriptionFormat is either 'markdown' or 'html' diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index eed62efb..9b0fd8da 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -910,24 +910,6 @@ searchChallenges.schema = { }) .unknown(true), }; -/** - * Validate Challenge groups. - * @param {Object} groups the group of a challenge - */ -async function validateGroups(groups) { - const promises = []; - _.each(groups, (g) => { - promises.push( - (async () => { - const group = await helper.getGroupById(g); - if (!group || group.status !== "active") { - throw new errors.BadRequestError("The groups provided are invalid " + g); - } - })() - ); - }); - await Promise.all(promises); -} /** * Create challenge. @@ -939,15 +921,6 @@ async function validateGroups(groups) { async function createChallenge(currentUser, challenge, userToken) { await challengeHelper.validateCreateChallengeRequest(currentUser, challenge); - //Validate the groups if Valid or Not - if ( - challenge.groups && - challenge.groups.length > 0 && - (currentUser.isMachine || hasAdminRole(currentUser)) - ) { - await validateGroups(challenge.groups); - } - if (challenge.legacy.selfService) { // if self-service, create a new project (what about if projectId is provided in the payload? confirm with business!) if (!challenge.projectId) { @@ -1470,15 +1443,6 @@ async function updateChallenge(currentUser, challengeId, data) { await validateChallengeUpdateRequest(currentUser, challenge, data); - //Validate the groups if Valid or Not - if ( - data.groups && - data.groups.length > 0 && - (currentUser.isMachine || hasAdminRole(currentUser)) - ) { - await validateGroups(data.groups); - } - let sendActivationEmail = false; let sendSubmittedEmail = false; let sendCompletedEmail = false; From d3f8b5d64035845c6fc8aad8272533a1bf43abee Mon Sep 17 00:00:00 2001 From: sarojbehera1 <137802881+sarojbehera1@users.noreply.github.com> Date: Thu, 12 Oct 2023 15:15:09 +0530 Subject: [PATCH 5/9] Correcting error: removing this (#667) --- src/common/challenge-helper.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/challenge-helper.js b/src/common/challenge-helper.js index 6ee91781..81764aeb 100644 --- a/src/common/challenge-helper.js +++ b/src/common/challenge-helper.js @@ -119,7 +119,7 @@ class ChallengeHelper { // check groups authorization if (challenge.groups && challenge.groups.length > 0) { if (currentUser.isMachine || hasAdminRole(currentUser)) { - await this.validateGroups(challenge.groups); + await validateGroups(challenge.groups); } else { await helper.ensureAccessibleByGroupsAccess(currentUser, challenge); } @@ -145,7 +145,7 @@ class ChallengeHelper { // check groups access to be updated group values if (data.groups && data.groups.length > 0) { if (currentUser.isMachine || hasAdminRole(currentUser)) { - await this.validateGroups(data.groups); + await validateGroups(data.groups); } else { await ensureAcessibilityToModifiedGroups(currentUser, data, challenge); } From 427dde2738f52e2645246e02ea09221b94f7100c Mon Sep 17 00:00:00 2001 From: sarojbehera1 <137802881+sarojbehera1@users.noreply.github.com> Date: Thu, 12 Oct 2023 17:35:50 +0530 Subject: [PATCH 6/9] Putting back this (#669) * Correcting error: removing this * Putting back *this* * Update challenge-helper.js to use the this keyword --------- Co-authored-by: Thomas Kranitsas --- src/common/challenge-helper.js | 4 ++-- src/services/ChallengeService.js | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/common/challenge-helper.js b/src/common/challenge-helper.js index 81764aeb..6ee91781 100644 --- a/src/common/challenge-helper.js +++ b/src/common/challenge-helper.js @@ -119,7 +119,7 @@ class ChallengeHelper { // check groups authorization if (challenge.groups && challenge.groups.length > 0) { if (currentUser.isMachine || hasAdminRole(currentUser)) { - await validateGroups(challenge.groups); + await this.validateGroups(challenge.groups); } else { await helper.ensureAccessibleByGroupsAccess(currentUser, challenge); } @@ -145,7 +145,7 @@ class ChallengeHelper { // check groups access to be updated group values if (data.groups && data.groups.length > 0) { if (currentUser.isMachine || hasAdminRole(currentUser)) { - await validateGroups(data.groups); + await this.validateGroups(data.groups); } else { await ensureAcessibilityToModifiedGroups(currentUser, data, challenge); } diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js index 4b1e9e2d..469d8806 100644 --- a/src/services/ChallengeService.js +++ b/src/services/ChallengeService.js @@ -36,7 +36,6 @@ const { ChallengeDomain } = require("@topcoder-framework/domain-challenge"); const { hasAdminRole } = require("../common/role-helper"); const { - validateChallengeUpdateRequest, enrichChallengeForResponse, sanitizeRepeatedFieldsInUpdateRequest, convertPrizeSetValuesToCents, @@ -1491,7 +1490,7 @@ async function updateChallenge(currentUser, challengeId, data) { const challengeResources = await helper.getChallengeResources(challengeId); - await validateChallengeUpdateRequest(currentUser, challenge, data, challengeResources); + await challengeHelper.validateChallengeUpdateRequest(currentUser, challenge, data, challengeResources); validateTask(currentUser, challenge, data, challengeResources); let sendActivationEmail = false; From 6dde37d3814642ef856fa500b05c1f3fe8ca2606 Mon Sep 17 00:00:00 2001 From: sarojbehera1 <137802881+sarojbehera1@users.noreply.github.com> Date: Sat, 14 Oct 2023 03:11:38 +0530 Subject: [PATCH 7/9] Plat 2528 (#676) merge commit From 6940a12c210b11ada3ade7d944da77557839a303 Mon Sep 17 00:00:00 2001 From: liuliquan Date: Wed, 18 Oct 2023 22:13:05 +0800 Subject: [PATCH 8/9] feat: versioned responses via transformer --- package.json | 1 + src/common/transformer.js | 81 ++++++++++++++++++++++++++ src/controllers/ChallengeController.js | 19 +++--- yarn.lock | 5 ++ 4 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 src/common/transformer.js diff --git a/package.json b/package.json index 6a8db892..1b97984b 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "axios-retry": "^3.4.0", "bluebird": "^3.5.1", "body-parser": "^1.15.1", + "compare-versions": "^6.1.0", "config": "^3.0.1", "cors": "^2.8.5", "decimal.js": "^10.4.3", diff --git a/src/common/transformer.js b/src/common/transformer.js new file mode 100644 index 00000000..9469b4d3 --- /dev/null +++ b/src/common/transformer.js @@ -0,0 +1,81 @@ +const _ = require("lodash"); +const { compareVersions } = require("compare-versions"); +const challengeService = require("../services/ChallengeService"); + +function transformData(data, fieldsToDelete) { + if (!fieldsToDelete || !fieldsToDelete.length) { + return data; + } + + if (_.isArray(data)) { + return data.map((item) => transformData(item, fieldsToDelete)); + } else if (_.isObject(data)) { + const clonedData = { ...data }; + for (const field of fieldsToDelete) { + delete clonedData[field]; + } + if (clonedData.result) { + clonedData.result = transformData(clonedData.result, fieldsToDelete); + } + return clonedData; + } + + return data; +} + +function transformServices() { + _.each(services, (service, serviceName) => { + const serviceConfig = servicesConfig[serviceName]; + if (!serviceConfig) { + return; + } + + _.each(service, (method, methodName) => { + service[methodName] = async function () { + const args = Array.prototype.slice.call(arguments); + const data = await method.apply(this, args.slice(1)); + + // No transform need for this method + if (!serviceConfig.methods.includes(methodName)) { + return data; + } + + // args[0] is request, get version header + const apiVersion = args[0].headers["challenge-api-version"] || "1.0.0"; + + const fieldsToDelete = []; + _.each(serviceConfig.fieldsVersion, (version, field) => { + // If input version less than required version, delete fields from response + if (compareVersions(apiVersion, version) < 0) { + fieldsToDelete.push(field); + } + }); + + // Transform response data by deleting fields + return transformData(data, fieldsToDelete); + }; + service[methodName].params = ["req", ...method.params]; + }); + }); +} + +// Define the version config for services +const servicesConfig = { + challengeService: { + methods: ["searchChallenges", "getChallenge", "createChallenge", "updateChallenge"], + fieldsVersion: { + skills: "1.1.0", + payments: "2.0.0", + }, + }, +}; + +// Define the services to export +const services = { + challengeService, +}; + +// Transform services before export +transformServices(); + +module.exports = services; diff --git a/src/controllers/ChallengeController.js b/src/controllers/ChallengeController.js index fcf61e56..18c61937 100644 --- a/src/controllers/ChallengeController.js +++ b/src/controllers/ChallengeController.js @@ -2,7 +2,7 @@ * Controller for challenge endpoints */ const HttpStatus = require("http-status-codes"); -const service = require("../services/ChallengeService"); +const { challengeService: service } = require("../common/transformer"); const helper = require("../common/helper"); const logger = require("../common/logger"); @@ -12,7 +12,7 @@ const logger = require("../common/logger"); * @param {Object} res the response */ async function searchChallenges(req, res) { - let result = await service.searchChallenges(req.authUser, { + let result = await service.searchChallenges(req, req.authUser, { ...req.query, ...req.body, }); @@ -23,7 +23,7 @@ async function searchChallenges(req, res) { logger.debug(`Staring to get mm challengeId`); const legacyId = await helper.getProjectIdByRoundId(req.query.legacyId); logger.debug(`Get mm challengeId successfully ${legacyId}`); - result = await service.searchChallenges(req.authUser, { + result = await service.searchChallenges(req, req.authUser, { ...req.query, ...req.body, legacyId, @@ -50,7 +50,7 @@ async function createChallenge(req, res) { logger.debug( `createChallenge User: ${JSON.stringify(req.authUser)} - Body: ${JSON.stringify(req.body)}` ); - const result = await service.createChallenge(req.authUser, req.body, req.userToken); + const result = await service.createChallenge(req, req.authUser, req.body, req.userToken); res.status(HttpStatus.CREATED).send(result); } @@ -60,7 +60,7 @@ async function createChallenge(req, res) { * @param {Object} res the response */ async function sendNotifications(req, res) { - const result = await service.sendNotifications(req.authUser, req.params.challengeId); + const result = await service.sendNotifications(req, req.authUser, req.params.challengeId); res.status(HttpStatus.CREATED).send(result); } @@ -71,6 +71,7 @@ async function sendNotifications(req, res) { */ async function getChallenge(req, res) { const result = await service.getChallenge( + req, req.authUser, req.params.challengeId, req.query.checkIfExists @@ -84,7 +85,7 @@ async function getChallenge(req, res) { * @param {Object} res the response */ async function getChallengeStatistics(req, res) { - const result = await service.getChallengeStatistics(req.authUser, req.params.challengeId); + const result = await service.getChallengeStatistics(req, req.authUser, req.params.challengeId); res.send(result); } @@ -99,7 +100,7 @@ async function updateChallenge(req, res) { req.params.challengeId } - Body: ${JSON.stringify(req.body)}` ); - const result = await service.updateChallenge(req.authUser, req.params.challengeId, req.body); + const result = await service.updateChallenge(req, req.authUser, req.params.challengeId, req.body); res.send(result); } @@ -112,7 +113,7 @@ async function deleteChallenge(req, res) { logger.debug( `deleteChallenge User: ${JSON.stringify(req.authUser)} - ChallengeID: ${req.params.challengeId}` ); - const result = await service.deleteChallenge(req.authUser, req.params.challengeId); + const result = await service.deleteChallenge(req, req.authUser, req.params.challengeId); res.send(result); } @@ -122,7 +123,7 @@ async function deleteChallenge(req, res) { * @param {Object} res the response */ async function advancePhase(req, res) { - res.send(await service.advancePhase(req.authUser, req.params.challengeId, req.body)); + res.send(await service.advancePhase(req, req.authUser, req.params.challengeId, req.body)); } module.exports = { diff --git a/yarn.lock b/yarn.lock index a52ead2b..ae8800e4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -978,6 +978,11 @@ commondir@^1.0.1: resolved "https://registry.yarnpkg.com/commondir/-/commondir-1.0.1.tgz#ddd800da0c66127393cca5950ea968a3aaf1253b" integrity sha512-W9pAhw0ja1Edb5GVdIF1mjZw/ASI0AlShXM83UUGe2DVr5TdAPEA1OA8m/g8zWp9x6On7gqufY+FatDbC3MDQg== +compare-versions@^6.1.0: + version "6.1.0" + resolved "https://registry.yarnpkg.com/compare-versions/-/compare-versions-6.1.0.tgz#3f2131e3ae93577df111dba133e6db876ffe127a" + integrity sha512-LNZQXhqUvqUTotpZ00qLSaify3b4VFD588aRr8MKFw4CMUr98ytzCW5wDH5qx/DEY5kCDXcbcRuCqL0szEf2tg== + component-emitter@^1.2.0, component-emitter@^1.3.0: version "1.3.0" resolved "https://registry.yarnpkg.com/component-emitter/-/component-emitter-1.3.0.tgz#16e4070fba8ae29b679f2215853ee181ab2eabc0" From 8aa77b2a81e81d01594e974a07599fcc7f05e98e Mon Sep 17 00:00:00 2001 From: liuliquan Date: Wed, 18 Oct 2023 22:44:15 +0800 Subject: [PATCH 9/9] feat: remove route transformer --- app-routes.js | 45 ----------------------------------- src/common/transformations.js | 24 ------------------- src/routes.js | 2 -- 3 files changed, 71 deletions(-) delete mode 100644 src/common/transformations.js diff --git a/app-routes.js b/app-routes.js index 0b3bc880..4a943233 100644 --- a/app-routes.js +++ b/app-routes.js @@ -10,7 +10,6 @@ const helper = require("./src/common/helper"); const errors = require("./src/common/errors"); const logger = require("./src/common/logger"); const routes = require("./src/routes"); -const transformations = require("./src/common/transformations"); const authenticator = require("tc-core-library-js").middleware.jwtAuthenticator; /** @@ -18,14 +17,6 @@ const authenticator = require("tc-core-library-js").middleware.jwtAuthenticator; * @param app the express app */ module.exports = (app) => { - app.use((req, res, next) => { - req.appVersion = req.headers["app-version"] || "1.0.0"; - if (!transformations[req.appVersion]) { - req.appVersion = "1.0.0"; // default to 1.0.0 if provided version doesn't match any transformation - } - next(); - }); - // Load all routes _.each(routes, (verbs, path) => { _.each(verbs, (def, verb) => { @@ -45,42 +36,6 @@ module.exports = (app) => { next(); }); - if (def.versioned) { - actions.push((req, res, next) => { - // TODO: Overriding res.send is a temporary solution to inject version-based transformations. - // TODO: A more conventional approach in Express would be to use res.locals to pass data through middleware, - // TODO: and then send the response in a centralized middleware after all transformations are applied. - // TODO: This would require a refactor of the current controllers' response handling. - // TODO: Consider revisiting this implementation in the future for a more maintainable architecture. - - const originalSend = res.send; - const originalStatus = res.status; - let currentStatusCode = 200; // Default status code for Express - - // Override res.status to capture the status code - res.status = function (code) { - currentStatusCode = code; - return originalStatus.apply(this, arguments); - }; - - res.send = (data) => { - // If the status code indicates a successful response, apply the transformation - if (currentStatusCode >= 200 && currentStatusCode < 300) { - const transformer = transformations[req.appVersion] || transformations["1.0.0"]; - data = transformer(data); - } - - // Reset the send function to its original behavior - res.send = originalSend; - - // Call the original send function with the transformed (or original) data - originalSend.call(res, data); - }; - - next(); - }); - } - actions.push((req, res, next) => { if (_.get(req, "query.token")) { _.set(req, "headers.authorization", `Bearer ${_.trim(req.query.token)}`); diff --git a/src/common/transformations.js b/src/common/transformations.js deleted file mode 100644 index 81850d48..00000000 --- a/src/common/transformations.js +++ /dev/null @@ -1,24 +0,0 @@ -// Transformations for version 1.0.0 -function transformV1(data) { - if (Array.isArray(data)) { - return data.map((item) => { - const clonedItem = { ...item }; - delete clonedItem.payments; - return clonedItem; - }); - } else { - const clonedData = { ...data }; - delete clonedData.payments; - return clonedData; - } -} - -// Transformations for version 2.0.0 -function transformV2(data) { - return data; -} - -module.exports = { - "1.0.0": transformV1, - "2.0.0": transformV2, -}; diff --git a/src/routes.js b/src/routes.js index fc425246..74e571c8 100644 --- a/src/routes.js +++ b/src/routes.js @@ -20,7 +20,6 @@ module.exports = { constants.UserRoles.User, ], scopes: [READ, ALL], - versioned: true, }, post: { controller: "ChallengeController", @@ -52,7 +51,6 @@ module.exports = { controller: "ChallengeController", method: "getChallenge", scopes: [READ, ALL], - versioned: true, }, // @deprecated put: {