Skip to content

Commit

Permalink
fix: update userId and ts only when review evaluation fields are pres…
Browse files Browse the repository at this point in the history
…ent (#1226)

* fix: update userId and ts only when review fields change

* store UTC_TIMESTAMP() value in variable to prevent timestamp creep during query execution

* debug test failure: log timestamp value

* remove debug code

* debug test failure: query for @utcTimestamp, no logging

* debug test failure: putting console logging back

* remove debug code; fix newman expected object properties

* patching resultEngine requires result

* update userId/ts if request contains eval fields

* squashed merge of main

* Added checks for timestamp updates and resultEngine patches

* fixed issue with new test statements

---------

Co-authored-by: cd-rite <61710958+cd-rite@users.noreply.github.com>
  • Loading branch information
csmig and cd-rite authored Mar 11, 2024
1 parent ed4c889 commit aa45a70
Show file tree
Hide file tree
Showing 4 changed files with 434 additions and 37 deletions.
3 changes: 3 additions & 0 deletions api/source/controllers/Review.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ module.exports.putReviewByAssetRule = async function (req, res, next) {

module.exports.patchReviewByAssetRule = async function (req, res, next) {
try {
if (Object.hasOwn(req.body, 'resultEngine') && !Object.hasOwn(req.body, 'result')) {
throw new SmError.UnprocessableError('Request body with resultEngine must include a result')
}
const collectionId = Collection.getCollectionIdAndCheckPermission(req, Security.ACCESS_LEVEL.Restricted)
const {assetId, ruleId} = {...req.params}
const currentReviews = await ReviewService.getReviews([], { assetId, ruleId }, req.userObject)
Expand Down
21 changes: 17 additions & 4 deletions api/source/service/ReviewService.js
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,7 @@ select
or review.reviewId is null -- no existing review
or (cteCollectionSetting.resetCriteria = 'result' and rChangedResult.reviewId is not null) -- status meets criteria for resetting
or (cteCollectionSetting.resetCriteria = 'any' and rChangedAny.reviewId is not null) -- status meets criteria for resetting
THEN UTC_TIMESTAMP() -- now
THEN @utcTimestamp -- now
ELSE review.statusTs -- saved time
END as statusTs,
Expand All @@ -1069,8 +1069,21 @@ select
ELSE review.statusUserId -- saved user
END as statusUserId,
@userId as userId,
UTC_TIMESTAMP() as ts
CASE WHEN review.reviewId is null -- no existing review
or cteIncoming.resultId is not null -- patch request contains result
or cteIncoming.detail is not null -- patch request contains detail
or cteIncoming.comment is not null -- patch request contains comment
THEN @userId -- this user
ELSE review.userId -- saved user
END as userId,
CASE WHEN review.reviewId is null -- no existing review
or cteIncoming.resultId is not null -- patch request contains result
or cteIncoming.detail is not null -- patch request contains detail
or cteIncoming.comment is not null -- patch request contains comment
THEN @utcTimestamp -- now
ELSE review.ts -- saved time
END as ts
from
cteIncoming
LEFT JOIN cteGrant on cteIncoming.ruleId = cteGrant.ruleId
Expand Down Expand Up @@ -1267,7 +1280,7 @@ where
try {
connection = await dbUtils.pool.getConnection()

const sqlSetVariables = `set @collectionId = ?, @assetId = ?, @userId = ?, @reviews = ?`
const sqlSetVariables = `set @collectionId = ?, @assetId = ?, @userId = ?, @reviews = ?, @utcTimestamp = UTC_TIMESTAMP()`
await connection.query(sqlSetVariables, [parseInt(collectionId), parseInt(assetId), parseInt(userId), JSON.stringify(reviews)])
const [settings] = await connection.query(`select c.settings->>"$.history.maxReviews" as maxReviews FROM collection c where collectionId = @collectionId`)
const historyMaxReviews = settings[0].maxReviews
Expand Down
3 changes: 2 additions & 1 deletion api/source/specification/stig-manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,8 @@ paths:
description: |
The request MUST target an existing Review and the request body MUST contain one or more Review properties. The overall request is validated as follows:
- conformance with the OAS schema for `ReviewAssetRulePatch`
- the request body conforms with the OAS schema for `ReviewAssetRulePatch`
- a request body that includes `resultEngine` must also include `result`
- the requesting user has been granted access to the {collectionId}
- a Review already exists for the {assetId} and {ruleId}
Expand Down
Loading

0 comments on commit aa45a70

Please sign in to comment.