Skip to content

Issue 246 - support v5 scorecard id #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,15 @@ module.exports = {
AUTH0_PROXY_SERVER_URL: process.env.AUTH0_PROXY_SERVER_URL,
FETCH_CREATED_DATE_START: process.env.FETCH_CREATED_DATE_START || '2021-01-01',
FETCH_PAGE_SIZE: process.env.FETCH_PAGE_SIZE || 500,
MIGRATE_CHALLENGES: process.env.MIGRATE_CHALLENGES || []
MIGRATE_CHALLENGES: process.env.MIGRATE_CHALLENGES || [],

V5TOLEGACYSCORECARDMAPPING: {
'c56a4180-65aa-42ec-a945-5fd21dec0501': 30001363,
'c56a4180-65aa-42ec-a945-5fd21dec0502': 123456789,
'c56a4180-65aa-42ec-a945-5fd21dec0503': 30001031,
'c56a4180-65aa-42ec-a945-5fd21dec0504': 987654321,
'c56a4180-65aa-42ec-a945-5fd21dec0505': 987123456,
'9ecc88e5-a4ee-44a4-8ec1-70bd98022510': 123789456,
'd6d31f34-8ee5-4589-ae65-45652fcc01a6': 30000720
}
Comment on lines +47 to +55
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would like to add here that these mapping are random for now. This needs to be replaced with the scorecard api, when that gets ready

}
12 changes: 9 additions & 3 deletions docs/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1757,7 +1757,9 @@ components:
description: The scoreCardId filter of the reviews associated with the submission.
required: false
schema:
type: integer
oneOf:
- type: integer
- type: string
filterSubmissionReviewSubmissionId:
in: query
name: review.submissionId
Expand Down Expand Up @@ -1871,7 +1873,9 @@ components:
description: The score card id filter for reviews.
required: false
schema:
type: integer
oneOf:
- type: integer
- type: string
filterReviewSubmissionId:
in: query
name: submissionId
Expand Down Expand Up @@ -2147,7 +2151,9 @@ components:
example: a12bc280-65ab-42ec-a945-5fd21dec1567
description: The review reviewer id.
scoreCardId:
type: integer
oneOf:
- type: integer
- type: string
description: The review score card id.
example: 123456789
submissionId:
Expand Down
18 changes: 17 additions & 1 deletion src/common/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,21 @@ function * getLatestChallenges (page) {
}
}

/**
* Get legacy scorecard id if the scorecard id is uuid form
* @param {String} scoreCardId Scorecard ID
* @returns {String} Legacy scorecard ID of the given challengeId
*/
function getLegacyScoreCardId (scoreCardId) {
if (/^[0-9a-f]{8}-[0-9a-f]{4}-[0-5][0-9a-f]{3}-[089ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(scoreCardId)) {
logger.debug(`${scoreCardId} detected as uuid. Converting to legacy scorecard id`)

return config.get('V5TOLEGACYSCORECARDMAPPING')[scoreCardId]
}

return scoreCardId
}

module.exports = {
wrapExpress,
autoWrapExpress,
Expand All @@ -856,5 +871,6 @@ module.exports = {
getRoleIdToRoleNameMap,
getV5ChallengeId,
adjustSubmissionChallengeId,
getLatestChallenges
getLatestChallenges,
getLegacyScoreCardId
}
63 changes: 58 additions & 5 deletions src/services/ReviewService.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ getReview.schema = {
* @return {Object} Data fetched from ES
*/
function * listReviews (query) {
if (query.scoreCardId) {
// Always use legacy scorecard id since v5 isn't stored in db
query.scoreCardId = helper.getLegacyScoreCardId(query.scoreCardId)

if (!query.scoreCardId) {
throw new errors.HttpStatusError(400, 'Legacy scorecard id not found for the provided v5 scorecard id')
}
}

return yield helper.fetchFromES(query, helper.camelize(table))
}

Expand Down Expand Up @@ -134,6 +143,20 @@ function * createReview (authUser, entity) {

const currDate = new Date().toISOString()

const possibleV5ScoreCardId = entity.scoreCardId

// Always use legacy id instead of v5 legacy id
entity.scoreCardId = helper.getLegacyScoreCardId(entity.scoreCardId)

if (!entity.scoreCardId) {
throw new errors.HttpStatusError(400, 'Legacy scorecard id not found for the provided v5 scorecard id')
}

if (entity.scoreCardId !== possibleV5ScoreCardId) {
// Remember the v5 score card id too that was passed
entity.v5ScoreCardId = possibleV5ScoreCardId
}

const item = _.extend(
{
id: uuid(),
Expand Down Expand Up @@ -240,6 +263,28 @@ function * _updateReview (authUser, reviewId, entity) {

const currDate = new Date().toISOString()

let scoreCardId = exist.scoreCardId
let v5ScoreCardId = exist.v5ScoreCardId

if (entity.scoreCardId) {
scoreCardId = helper.getLegacyScoreCardId(entity.scoreCardId)

if (!scoreCardId) {
throw new errors.HttpStatusError(400, 'Legacy scorecard id not found for the provided v5 scorecard id')
}

if (entity.scoreCardId !== scoreCardId) {
v5ScoreCardId = entity.scoreCardId
} else if (v5ScoreCardId) {
// Since we currently have a static mapping b/w legacy and v5 scorecard ids,
// there is no way for us to fetch the v5 scorecard id for a legacy scorecard id
// In this scenario, the review already had a v5 scorecard id, so if it's legacy
// scorecard id gets updated, we would also need to update the v5 scorecard id
// which we cannot - hence the error. Ideally, in this case, a new review needs to be created
throw new errors.HttpStatusError(400, `Cannot update legacy scorecard id since review with id ${reviewId} already has a v5 scorecard id`)
}
}

// Record used for updating in Database
const record = {
TableName: table,
Expand All @@ -248,17 +293,19 @@ function * _updateReview (authUser, reviewId, entity) {
},
UpdateExpression: `set score = :s, scoreCardId = :sc, submissionId = :su,
typeId = :t, reviewerId = :r, #st = :st,
updated = :ua, updatedBy = :ub, reviewedDate = :rd`,
updated = :ua, updatedBy = :ub, reviewedDate = :rd
${v5ScoreCardId ? ', v5ScoreCardId = :v5s' : ''}`,
ExpressionAttributeValues: {
':s': entity.score || exist.score,
':sc': entity.scoreCardId || exist.scoreCardId,
':sc': scoreCardId,
':su': entity.submissionId || exist.submissionId,
':t': entity.typeId || exist.typeId,
':r': entity.reviewerId || exist.reviewerId,
':st': entity.status || exist.status || 'completed',
':ua': currDate,
':ub': authUser.handle || authUser.sub,
':rd': entity.reviewedDate || exist.reviewedDate || exist.created
':rd': entity.reviewedDate || exist.reviewedDate || exist.created,
...(v5ScoreCardId ? { ':v5s': v5ScoreCardId } : {})
},
ExpressionAttributeNames: {
'#st': 'status'
Expand Down Expand Up @@ -293,7 +340,11 @@ function * _updateReview (authUser, reviewId, entity) {
updatedBy: authUser.handle || authUser.sub,
reviewedDate: entity.reviewedDate || exist.reviewedDate || exist.created
},
entity
entity,
{
scoreCardId,
v5ScoreCardId
}
)
}

Expand All @@ -305,7 +356,9 @@ function * _updateReview (authUser, reviewId, entity) {
return _.extend(exist, entity, {
updated: currDate,
updatedBy: authUser.handle || authUser.sub,
reviewedDate: entity.reviewedDate || exist.reviewedDate || exist.created
reviewedDate: entity.reviewedDate || exist.reviewedDate || exist.created,
scoreCardId,
v5ScoreCardId
})
}

Expand Down
11 changes: 10 additions & 1 deletion src/services/SubmissionService.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,15 @@ function * listSubmissions (authUser, query) {
query.challengeId = yield helper.getV5ChallengeId(query.challengeId)
}

// TODO - support v5 for review scorecardid
if (query['review.scoreCardId']) {
query['review.scoreCardId'] = helper.getLegacyScoreCardId(query['review.scoreCardId'])

if (!query['review.scoreCardId']) {
throw new errors.HttpStatusError(400, 'Legacy scorecard id not found for the provided v5 scorecard id')
}
}

const data = yield helper.fetchFromES(query, helper.camelize(table))
logger.info(`Data returned from ES: ${JSON.stringify(data, null, 4)}`)
logger.info(`listSubmissions: returning ${data.length} submissions for query: ${JSON.stringify(query)}`)
Expand Down Expand Up @@ -216,7 +225,7 @@ const listSubmissionsQuerySchema = {
'review.score': joi.score(),
'review.typeId': joi.string().uuid(),
'review.reviewerId': joi.string().uuid(),
'review.scoreCardId': joi.id(),
'review.scoreCardId': joi.alternatives().try(joi.id(), joi.string().uuid()),
'review.submissionId': joi.string().uuid(),
'review.status': joi.reviewStatus(),
'reviewSummation.scoreCardId': joi.id(),
Expand Down