From 09ba3559dc959dc8a17774caf4692c0b74311d46 Mon Sep 17 00:00:00 2001 From: emathew Date: Wed, 18 Feb 2026 12:40:00 -0500 Subject: [PATCH 1/9] fixes for review object --- .../review-object.controller.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/controller/review-object.controller/review-object.controller.js b/src/controller/review-object.controller/review-object.controller.js index 7795f0219..d831cd61a 100644 --- a/src/controller/review-object.controller/review-object.controller.js +++ b/src/controller/review-object.controller/review-object.controller.js @@ -61,6 +61,7 @@ async function approveReviewObject (req, res, next) { const reviewRepo = req.ctx.repositories.getReviewObjectRepository() const baseOrgRepo = req.ctx.repositories.getBaseOrgRepository() const userRepo = req.ctx.repositories.getBaseUserRepository() + const isSecretariat = await baseOrgRepo.isSecretariatByShortName(req.ctx.org) const UUID = req.params.uuid const body = req.body const session = await mongoose.startSession() @@ -70,7 +71,12 @@ async function approveReviewObject (req, res, next) { try { session.startTransaction() - const reviewObject = await reviewRepo.findOneByUUID(UUID, { session }) + const bodyValidation = baseOrgRepo.validateOrg(body) + if (!bodyValidation.isValid) { + return res.status(400).json({ message: 'Invalid body parameters', errors: bodyValidation.errors }) + } + + const reviewObject = await reviewRepo.getOrgReviewObjectByOrgUUID(UUID, isSecretariat, { session }) if (!reviewObject) { await session.abortTransaction() return res.status(404).json({ message: `No review object found with UUID ${UUID}` }) @@ -97,7 +103,7 @@ async function approveReviewObject (req, res, next) { updatedOrgObj = await baseOrgRepo.findOneByUUID(reviewObject.target_object_uuid) } catch (updateErr) { await session.abortTransaction() - throw updateErr + return res.status(500).json({ message: updateErr.message || 'Failed to approve review object' }) } finally { await session.endSession() } @@ -185,7 +191,7 @@ async function rejectReviewObject (req, res, next) { await session.commitTransaction() } catch (rejectErr) { await session.abortTransaction() - throw rejectErr + return res.status(500).json({ message: rejectErr.message || 'Failed to reject review object' }) } finally { await session.endSession() } From a59eb691875c67f490bc39ce20ee23e7e1d10740 Mon Sep 17 00:00:00 2001 From: emathew Date: Wed, 18 Feb 2026 13:07:17 -0500 Subject: [PATCH 2/9] fix comment --- src/controller/org.controller/org.controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js index 05b9fc572..05e958fb5 100644 --- a/src/controller/org.controller/org.controller.js +++ b/src/controller/org.controller/org.controller.js @@ -202,7 +202,7 @@ async function getUser (req, res, next) { /** * Get details on ID quota for an org with the specified org shortname. - * Called by GET /api/registry/org/{shortname}/id_quota, GET /api/org/{shortname}/id_quota + * Called by GET /api/registry/org/{shortname}/hard_quota, GET /api/org/{shortname}/id_quota * * @param {Object} req - The request object * @param {Object} res - The response object From c08d0170aa089b52a6790e7c8dc78330941b3950 Mon Sep 17 00:00:00 2001 From: emathew Date: Mon, 2 Mar 2026 15:18:15 -0500 Subject: [PATCH 3/9] approve/reject review endpoints only allowed for pending objects and remove 'org' from their paths --- api-docs/openapi.json | 6 +- .../review-object.controller/index.js | 6 +- .../review-object.controller.js | 20 +++- src/repositories/reviewObjectRepository.js | 5 +- src/scripts/populate.js | 15 ++- .../registryOrgWithJointReviewTest.js | 6 +- .../review-object.repository.test.js | 99 +++++++++++++++++++ .../review-object/reviewObjectTest.js | 20 ++-- .../review-object.controller.test.js | 19 +++- 9 files changed, 162 insertions(+), 34 deletions(-) create mode 100644 test/integration-tests/review-object/review-object.repository.test.js diff --git a/api-docs/openapi.json b/api-docs/openapi.json index 5bcc9895f..6c00ce16a 100644 --- a/api-docs/openapi.json +++ b/api-docs/openapi.json @@ -5078,7 +5078,7 @@ } } }, - "/review/org/{uuid}": { + "/review/{uuid}": { "put": { "tags": [ "Review Object" @@ -5181,7 +5181,7 @@ } } }, - "/review/org/{uuid}/approve": { + "/review/{uuid}/approve": { "put": { "tags": [ "Review Object" @@ -5285,7 +5285,7 @@ } } }, - "/review/org/{uuid}/reject": { + "/review/{uuid}/reject": { "put": { "tags": [ "Review Object" diff --git a/src/controller/review-object.controller/index.js b/src/controller/review-object.controller/index.js index 190db6695..702da7529 100644 --- a/src/controller/review-object.controller/index.js +++ b/src/controller/review-object.controller/index.js @@ -324,7 +324,7 @@ router.get('/review/org/:identifier/reviews', ) // Update a review object -router.put('/review/org/:uuid', +router.put('/review/:uuid', /* #swagger.tags = ['Review Object'] #swagger.operationId = 'updateReviewObjectByReviewUUID' @@ -407,7 +407,7 @@ router.put('/review/org/:uuid', ) // Approve a review object -router.put('/review/org/:uuid/approve', +router.put('/review/:uuid/approve', /* #swagger.tags = ['Review Object'] #swagger.operationId = 'approveReviewObject' @@ -491,7 +491,7 @@ router.put('/review/org/:uuid/approve', ) // Reject a review object -router.put('/review/org/:uuid/reject', +router.put('/review/:uuid/reject', /* #swagger.tags = ['Review Object'] #swagger.operationId = 'rejectReviewObject' diff --git a/src/controller/review-object.controller/review-object.controller.js b/src/controller/review-object.controller/review-object.controller.js index d831cd61a..cfa167482 100644 --- a/src/controller/review-object.controller/review-object.controller.js +++ b/src/controller/review-object.controller/review-object.controller.js @@ -62,6 +62,7 @@ async function approveReviewObject (req, res, next) { const baseOrgRepo = req.ctx.repositories.getBaseOrgRepository() const userRepo = req.ctx.repositories.getBaseUserRepository() const isSecretariat = await baseOrgRepo.isSecretariatByShortName(req.ctx.org) + const isPendingReview = true const UUID = req.params.uuid const body = req.body const session = await mongoose.startSession() @@ -71,15 +72,15 @@ async function approveReviewObject (req, res, next) { try { session.startTransaction() - const bodyValidation = baseOrgRepo.validateOrg(body) + const bodyValidation = (body && Object.keys(body).length) ? baseOrgRepo.validateOrg(body) : { isValid: true } if (!bodyValidation.isValid) { return res.status(400).json({ message: 'Invalid body parameters', errors: bodyValidation.errors }) } - const reviewObject = await reviewRepo.getOrgReviewObjectByOrgUUID(UUID, isSecretariat, { session }) + const reviewObject = await reviewRepo.findOneByUUIDWithConversation(UUID, isSecretariat, isPendingReview, { session }) if (!reviewObject) { await session.abortTransaction() - return res.status(404).json({ message: `No review object found with UUID ${UUID}` }) + return res.status(404).json({ message: `No pending review object found with UUID ${UUID}` }) } const org = await baseOrgRepo.findOneByUUID(reviewObject.target_object_uuid, { session }) @@ -179,15 +180,24 @@ async function getReviewHistoryByOrgShortNamePaginated (req, res, next) { } async function rejectReviewObject (req, res, next) { - const repo = req.ctx.repositories.getReviewObjectRepository() + const reviewRepo = req.ctx.repositories.getReviewObjectRepository() + const baseOrgRepo = req.ctx.repositories.getBaseOrgRepository() const UUID = req.params.uuid + const isSecretariat = await baseOrgRepo.isSecretariatByShortName(req.ctx.org) + const isPendingReview = true const session = await mongoose.startSession() let value try { session.startTransaction() - value = await repo.rejectReviewOrgObject(UUID, { session }) + const reviewObject = await reviewRepo.findOneByUUIDWithConversation(UUID, isSecretariat, isPendingReview, { session }) + if (!reviewObject) { + await session.abortTransaction() + return res.status(404).json({ message: `No pending review object found with UUID ${UUID}` }) + } + + value = await reviewRepo.rejectReviewOrgObject(UUID, { session }) await session.commitTransaction() } catch (rejectErr) { await session.abortTransaction() diff --git a/src/repositories/reviewObjectRepository.js b/src/repositories/reviewObjectRepository.js index 24819c7d7..81d9550ba 100644 --- a/src/repositories/reviewObjectRepository.js +++ b/src/repositories/reviewObjectRepository.js @@ -24,11 +24,12 @@ class ReviewObjectRepository extends BaseRepository { return reviewObject || null } - async findOneByUUIDWithConversation (UUID, isSecretariat, options = {}) { + async findOneByUUIDWithConversation (UUID, isSecretariat, pending = false, options = {}) { const ConversationRepository = require('./conversationRepository') const conversationRepository = new ConversationRepository() let reviewObject - const reviewObjectRaw = await ReviewObjectModel.findOne({ uuid: UUID }, null, options) + const query = pending ? { uuid: UUID, status: 'pending' } : { uuid: UUID } + const reviewObjectRaw = await ReviewObjectModel.findOne(query, null, options) if (reviewObjectRaw) { reviewObject = reviewObjectRaw.toObject() const conversations = await conversationRepository.getAllByTargetUUID(reviewObject.target_object_uuid, isSecretariat, options) diff --git a/src/scripts/populate.js b/src/scripts/populate.js index 738143127..70553e1c7 100644 --- a/src/scripts/populate.js +++ b/src/scripts/populate.js @@ -132,15 +132,24 @@ db.once('open', async () => { // don't close database connection until all remaining populate // promises are resolved - Promise.all(populatePromises).then(function () { + Promise.all(populatePromises).then(async function () { logger.info('Successfully populated the database!') + const indexPromises = [] Object.keys(indexesToCreate).forEach(col => { indexesToCreate[col].forEach(index => { - db.collections[col].createIndex(index) + indexPromises.push(db.collections[col].createIndex(index)) }) }) - mongoose.connection.close() + + try { + await Promise.all(indexPromises) + logger.info('Successfully created indexes!') + } catch (err) { + logger.error('Error creating indexes:', err) + } finally { + mongoose.connection.close() + } }) } else { mongoose.connection.close() diff --git a/test/integration-tests/registry-org/registryOrgWithJointReviewTest.js b/test/integration-tests/registry-org/registryOrgWithJointReviewTest.js index e3eabff16..64d98ea9f 100644 --- a/test/integration-tests/registry-org/registryOrgWithJointReviewTest.js +++ b/test/integration-tests/registry-org/registryOrgWithJointReviewTest.js @@ -148,9 +148,9 @@ describe('Testing Joint approval', () => { }) }) it('Secretariat can approve the ORG review with body parameter', async function () { - const newBody = { short_name: 'final_non_secretariat_org', hard_quota: 20000 } + const newBody = { short_name: 'final_non_secretariat_org', hard_quota: 20000, long_name: 'Final Non Secretariat Organization' } await chai.request(app) - .put(`/api/review/org/${reviewUUID}/approve`) + .put(`/api/review/${reviewUUID}/approve`) .set(secretariatHeaders) .send(newBody) .then((res) => { @@ -306,7 +306,7 @@ describe('Testing Joint approval', () => { }) it('Secretariat can approve the ORG review', async function () { await chai.request(app) - .put(`/api/review/org/${reviewUUID}/approve`) + .put(`/api/review/${reviewUUID}/approve`) .set(secretariatHeaders) .then((res) => { expect(res).to.have.status(200) diff --git a/test/integration-tests/review-object/review-object.repository.test.js b/test/integration-tests/review-object/review-object.repository.test.js new file mode 100644 index 000000000..197d3640d --- /dev/null +++ b/test/integration-tests/review-object/review-object.repository.test.js @@ -0,0 +1,99 @@ +/* eslint-disable no-unused-expressions */ +const chai = require('chai') +const expect = chai.expect +const ReviewObjectRepository = require('../../../src/repositories/reviewObjectRepository') +const ReviewObjectModel = require('../../../src/model/reviewobject') + +describe('ReviewObjectRepository Tests', function () { + let repository + let testUUID + + before(async () => { + repository = new ReviewObjectRepository() + + // Create a test review object with pending status + const pendingReview = new ReviewObjectModel({ + uuid: 'pending-test-uuid-123', + target_object_uuid: 'target-uuid-456', + status: 'pending', + new_review_data: { short_name: 'pending_org' } + }) + await pendingReview.save() + + // Create a test review object with approved status + const approvedReview = new ReviewObjectModel({ + uuid: 'approved-test-uuid-456', + target_object_uuid: 'target-uuid-789', + status: 'approved', + new_review_data: { short_name: 'approved_org' } + }) + await approvedReview.save() + + testUUID = 'pending-test-uuid-123' + }) + + after(async () => { + // Clean up test data + await ReviewObjectModel.deleteMany({ + uuid: { $in: ['pending-test-uuid-123', 'approved-test-uuid-456'] } + }) + }) + + describe('findOneByUUIDWithConversation', function () { + it('should return the pending review object when pending=true', async () => { + const result = await repository.findOneByUUIDWithConversation(testUUID, true, true) + + expect(result).to.exist + expect(result.uuid).to.equal(testUUID) + expect(result.status).to.equal('pending') + }) + + it('should return null when review object not found with pending=true', async () => { + const nonExistentUUID = 'non-existent-uuid-999' + const result = await repository.findOneByUUIDWithConversation(nonExistentUUID, true, true) + + expect(result).to.be.null + }) + + it('should return the approved review object when pending=false', async () => { + const approvedUUID = 'approved-test-uuid-456' + const result = await repository.findOneByUUIDWithConversation(approvedUUID, true, false) + + expect(result).to.exist + expect(result.uuid).to.equal(approvedUUID) + expect(result.status).to.equal('approved') + }) + + it('should return the pending review object when pending=false', async () => { + const result = await repository.findOneByUUIDWithConversation(testUUID, true, false) + + expect(result).to.exist + expect(result.uuid).to.equal(testUUID) + expect(result.status).to.equal('pending') + }) + + it('should return the pending review object with default pending value', async () => { + const result = await repository.findOneByUUIDWithConversation(testUUID, true) + + expect(result).to.exist + expect(result.uuid).to.equal(testUUID) + expect(result.status).to.equal('pending') + }) + + it('should return the approved review object with default pending value', async () => { + const approvedUUID = 'approved-test-uuid-456' + const result = await repository.findOneByUUIDWithConversation(approvedUUID, true) + + expect(result).to.exist + expect(result.uuid).to.equal(approvedUUID) + expect(result.status).to.equal('approved') + }) + + it('should not return approved review object when pending=true', async () => { + const approvedUUID = 'approved-test-uuid-456' + const result = await repository.findOneByUUIDWithConversation(approvedUUID, true, true) + + expect(result).to.be.null + }) + }) +}) diff --git a/test/integration-tests/review-object/reviewObjectTest.js b/test/integration-tests/review-object/reviewObjectTest.js index 76d3457b5..33005ae1d 100644 --- a/test/integration-tests/review-object/reviewObjectTest.js +++ b/test/integration-tests/review-object/reviewObjectTest.js @@ -88,7 +88,7 @@ describe('Review Object Controller Integration Tests', () => { reviewObject.short_name = 'updated_org' const res = await chai .request(app) - .put(`/api/review/org/${reviewUUID}`) + .put(`/api/review/${reviewUUID}`) .set({ ...constants.headers }) .send(reviewObject) expect(res).to.have.status(200) @@ -210,7 +210,7 @@ describe('Review Object Controller Integration Tests', () => { it('Approves a review object and updates the organization', async () => { const res = await chai .request(app) - .put(`/api/review/org/${approveTestReviewUUID}/approve`) + .put(`/api/review/${approveTestReviewUUID}/approve`) .set({ ...constants.headers }) .send({}) expect(res).to.have.status(200) @@ -253,7 +253,7 @@ describe('Review Object Controller Integration Tests', () => { it('Rejects a review object', async () => { const res = await chai .request(app) - .put(`/api/review/org/${rejectTestReviewUUID}/reject`) + .put(`/api/review/${rejectTestReviewUUID}/reject`) .set({ ...constants.headers }) .send({}) expect(res).to.have.status(200) @@ -393,22 +393,22 @@ describe('Review Object Controller Integration Tests', () => { const fakeUUID = '00000000-0000-0000-0000-000000000000' const res = await chai .request(app) - .put(`/api/review/org/${fakeUUID}/approve`) + .put(`/api/review/${fakeUUID}/approve`) .set({ ...constants.headers }) .send({}) expect(res).to.have.status(404) - expect(res.body.message).to.equal(`No review object found with UUID ${fakeUUID}`) + expect(res.body.message).to.equal(`No pending review object found with UUID ${fakeUUID}`) }) it('Returns 404 when rejecting non-existent review object', async () => { const fakeUUID = '00000000-0000-0000-0000-000000000000' const res = await chai .request(app) - .put(`/api/review/org/${fakeUUID}/reject`) + .put(`/api/review/${fakeUUID}/reject`) .set({ ...constants.headers }) .send({}) expect(res).to.have.status(404) - expect(res.body.message).to.equal(`No review object found with UUID ${fakeUUID}`) + expect(res.body.message).to.equal(`No pending review object found with UUID ${fakeUUID}`) }) it('Returns 404 when updating non-existent review object', async () => { @@ -459,7 +459,7 @@ describe('Review Object Controller Integration Tests', () => { it('Non-secretariat user cannot update review object', async () => { const res = await chai .request(app) - .put(`/api/review/org/${reviewUUID}`) + .put(`/api/review/${reviewUUID}`) .set({ ...constants.nonSecretariatUserHeaders }) .send({ short_name: 'test' }) expect(res).to.have.status(403) @@ -468,7 +468,7 @@ describe('Review Object Controller Integration Tests', () => { it('Non-secretariat user cannot approve review object', async () => { const res = await chai .request(app) - .put(`/api/review/org/${reviewUUID}/approve`) + .put(`/api/review/${reviewUUID}/approve`) .set({ ...constants.nonSecretariatUserHeaders }) .send({}) expect(res).to.have.status(403) @@ -477,7 +477,7 @@ describe('Review Object Controller Integration Tests', () => { it('Non-secretariat user cannot reject review object', async () => { const res = await chai .request(app) - .put(`/api/review/org/${reviewUUID}/reject`) + .put(`/api/review/${reviewUUID}/reject`) .set({ ...constants.nonSecretariatUserHeaders }) .send({}) expect(res).to.have.status(403) diff --git a/test/unit-tests/review-object/review-object.controller.test.js b/test/unit-tests/review-object/review-object.controller.test.js index ba9a48eee..65ebb7348 100644 --- a/test/unit-tests/review-object/review-object.controller.test.js +++ b/test/unit-tests/review-object/review-object.controller.test.js @@ -233,15 +233,17 @@ describe('Review Object Controller', function () { }) it('should return 404 if review object not found', async () => { - repoStub.findOneByUUID = sinon.stub().resolves(null) + orgRepoStub.validateOrg = sinon.stub().returns({ isValid: true }) + repoStub.findOneByUUIDWithConversation = sinon.stub().resolves(null) await controller.approveReviewObject(req, res, next) expect(res.status.calledWith(404)).to.be.true - expect(res.json.calledWith({ message: `No review object found with UUID ${reviewUUID}` })).to.be.true + expect(res.json.calledWith({ message: `No pending review object found with UUID ${reviewUUID}` })).to.be.true expect(sessionStub.abortTransaction.calledOnce).to.be.true }) it('should return 404 if organization not found', async () => { - repoStub.findOneByUUID = sinon.stub().resolves(reviewObject) + orgRepoStub.validateOrg = sinon.stub().returns({ isValid: true }) + repoStub.findOneByUUIDWithConversation = sinon.stub().resolves(reviewObject) orgRepoStub.findOneByUUID = sinon.stub().resolves(null) await controller.approveReviewObject(req, res, next) expect(res.status.calledWith(404)).to.be.true @@ -250,7 +252,8 @@ describe('Review Object Controller', function () { }) it('should approve review object and update organization with review data', async () => { - repoStub.findOneByUUID = sinon.stub().resolves(reviewObject) + orgRepoStub.validateOrg = sinon.stub().returns({ isValid: true }) + repoStub.findOneByUUIDWithConversation = sinon.stub().resolves(reviewObject) orgRepoStub.findOneByUUID = sinon.stub() .onFirstCall().resolves(orgObj) .onSecondCall().resolves(updatedOrgObj) @@ -267,19 +270,25 @@ describe('Review Object Controller', function () { describe('rejectReviewObject', function () { const reviewUUID = 'review-uuid-123' + const reviewObject = { + uuid: reviewUUID, + new_review_data: { short_name: 'org' } + } beforeEach(() => { req.params.uuid = reviewUUID }) it('should return 404 if review object not found', async () => { + repoStub.findOneByUUIDWithConversation = sinon.stub().resolves(null) repoStub.rejectReviewOrgObject = sinon.stub().resolves(null) await controller.rejectReviewObject(req, res, next) expect(res.status.calledWith(404)).to.be.true - expect(res.json.calledWith({ message: `No review object found with UUID ${reviewUUID}` })).to.be.true + expect(res.json.calledWith({ message: `No pending review object found with UUID ${reviewUUID}` })).to.be.true }) it('should return 200 with rejected review object', async () => { + repoStub.findOneByUUIDWithConversation = sinon.stub().resolves(reviewObject) const rejectedObj = { uuid: reviewUUID, status: 'rejected' } repoStub.rejectReviewOrgObject = sinon.stub().resolves(rejectedObj) await controller.rejectReviewObject(req, res, next) From 8b61833e5b810b6c3c88160e8c30c8c1d0bd83e1 Mon Sep 17 00:00:00 2001 From: emathew Date: Mon, 2 Mar 2026 15:53:24 -0500 Subject: [PATCH 4/9] causal consitency approveReviewObj updated --- .../review-object.controller.js | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/controller/review-object.controller/review-object.controller.js b/src/controller/review-object.controller/review-object.controller.js index cfa167482..092c3b959 100644 --- a/src/controller/review-object.controller/review-object.controller.js +++ b/src/controller/review-object.controller/review-object.controller.js @@ -66,26 +66,21 @@ async function approveReviewObject (req, res, next) { const UUID = req.params.uuid const body = req.body const session = await mongoose.startSession() - let reviewObj let updatedOrgObj try { - session.startTransaction() - const bodyValidation = (body && Object.keys(body).length) ? baseOrgRepo.validateOrg(body) : { isValid: true } if (!bodyValidation.isValid) { return res.status(400).json({ message: 'Invalid body parameters', errors: bodyValidation.errors }) } - const reviewObject = await reviewRepo.findOneByUUIDWithConversation(UUID, isSecretariat, isPendingReview, { session }) + const reviewObject = await reviewRepo.findOneByUUIDWithConversation(UUID, isSecretariat, isPendingReview, {}) if (!reviewObject) { - await session.abortTransaction() return res.status(404).json({ message: `No pending review object found with UUID ${UUID}` }) } - const org = await baseOrgRepo.findOneByUUID(reviewObject.target_object_uuid, { session }) + const org = await baseOrgRepo.findOneByUUID(reviewObject.target_object_uuid, {}) if (!org) { - await session.abortTransaction() return res.status(404).json({ message: 'Organization not found for this review object' }) } @@ -93,27 +88,27 @@ async function approveReviewObject (req, res, next) { ? _.merge({}, org.toObject(), body) : reviewObject.new_review_data - const requestingUserUUID = await userRepo.getUserUUID(req.ctx.user, req.ctx.org, { session }) + const requestingUserUUID = await userRepo.getUserUUID(req.ctx.user, req.ctx.org, {}) - reviewObj = await reviewRepo.approveReviewOrgObject(UUID, { session }) - await baseOrgRepo.updateOrgFull(org.short_name, dataToUpdate, { session }, false, requestingUserUUID, false, true) + session.startTransaction() + const reviewObj = await reviewRepo.approveReviewOrgObject(UUID, { session }) + if (!reviewObj) { + return res.status(404).json({ message: `Review object not approved with UUID ${UUID}` }) + } + updatedOrgObj = await baseOrgRepo.updateOrgFull(org.short_name, dataToUpdate, { session }, false, requestingUserUUID, false, true) + if (!updatedOrgObj) { + return res.status(404).json({ message: `Org Object not updated with UUID ${UUID}` }) + } await session.commitTransaction() - - // Return the updated organization - updatedOrgObj = await baseOrgRepo.findOneByUUID(reviewObject.target_object_uuid) } catch (updateErr) { await session.abortTransaction() return res.status(500).json({ message: updateErr.message || 'Failed to approve review object' }) } finally { await session.endSession() } - - if (!reviewObj) { - return res.status(404).json({ message: `No review object found with UUID ${UUID}` }) - } - - return res.status(200).json(updatedOrgObj ? updatedOrgObj.toObject() : null) + _.unset(updatedOrgObj, 'joint_approval_required') + return res.status(200).json(updatedOrgObj) } async function updateReviewObjectByReviewUUID (req, res, next) { From dc7a3426161de9cb52a1b9eed2ac41f33b6d9fd6 Mon Sep 17 00:00:00 2001 From: emathew Date: Tue, 3 Mar 2026 13:33:11 -0500 Subject: [PATCH 5/9] add causal consistency fixes --- .../review-object.controller.js | 58 ++++++++++++++----- src/repositories/reviewObjectRepository.js | 4 +- 2 files changed, 47 insertions(+), 15 deletions(-) diff --git a/src/controller/review-object.controller/review-object.controller.js b/src/controller/review-object.controller/review-object.controller.js index 092c3b959..410a5e0e2 100644 --- a/src/controller/review-object.controller/review-object.controller.js +++ b/src/controller/review-object.controller/review-object.controller.js @@ -65,21 +65,23 @@ async function approveReviewObject (req, res, next) { const isPendingReview = true const UUID = req.params.uuid const body = req.body - const session = await mongoose.startSession() + const session = await mongoose.startSession({ causalConsistency: false }) let updatedOrgObj try { + session.startTransaction() + const bodyValidation = (body && Object.keys(body).length) ? baseOrgRepo.validateOrg(body) : { isValid: true } if (!bodyValidation.isValid) { return res.status(400).json({ message: 'Invalid body parameters', errors: bodyValidation.errors }) } - const reviewObject = await reviewRepo.findOneByUUIDWithConversation(UUID, isSecretariat, isPendingReview, {}) + const reviewObject = await reviewRepo.findOneByUUIDWithConversation(UUID, isSecretariat, isPendingReview, { session }) if (!reviewObject) { return res.status(404).json({ message: `No pending review object found with UUID ${UUID}` }) } - const org = await baseOrgRepo.findOneByUUID(reviewObject.target_object_uuid, {}) + const org = await baseOrgRepo.findOneByUUID(reviewObject.target_object_uuid, { session }) if (!org) { return res.status(404).json({ message: 'Organization not found for this review object' }) } @@ -88,9 +90,8 @@ async function approveReviewObject (req, res, next) { ? _.merge({}, org.toObject(), body) : reviewObject.new_review_data - const requestingUserUUID = await userRepo.getUserUUID(req.ctx.user, req.ctx.org, {}) + const requestingUserUUID = await userRepo.getUserUUID(req.ctx.user, req.ctx.org, { session }) - session.startTransaction() const reviewObj = await reviewRepo.approveReviewOrgObject(UUID, { session }) if (!reviewObj) { return res.status(404).json({ message: `Review object not approved with UUID ${UUID}` }) @@ -116,30 +117,59 @@ async function updateReviewObjectByReviewUUID (req, res, next) { const UUID = req.params.uuid const orgRepo = req.ctx.repositories.getBaseOrgRepository() const body = req.body + const session = await mongoose.startSession({ causalConsistency: false }) + let updatedReviewObj const result = orgRepo.validateOrg(body) if (!result.isValid) { return res.status(400).json({ message: 'Invalid new_review_data', errors: result.errors }) } - const value = await repo.updateReviewOrgObject(body, UUID) + try { + session.startTransaction() + const reviewObject = await repo.findOneByUUIDWithConversation(UUID, false, true, { session }) + if (!reviewObject) { + await session.abortTransaction() + return res.status(404).json({ message: `No pending review object found with UUID ${UUID}` }) + } + updatedReviewObj = await repo.updateReviewOrgObject(body, UUID, { session }) + } catch (updateErr) { + await session.abortTransaction() + return res.status(500).json({ message: updateErr.message || 'Failed to update review object' }) + } finally { + await session.endSession() + } - if (!value) { + if (!updatedReviewObj) { return res.status(404).json({ message: `No review object found with UUID ${UUID}` }) } - return res.status(200).json(value) + return res.status(200).json(updatedReviewObj) } async function createReviewObject (req, res, next) { const repo = req.ctx.repositories.getReviewObjectRepository() const body = req.body + const session = await mongoose.startSession({ causalConsistency: false }) + let createdReviewObj - const value = await repo.createReviewOrgObject(body) + try { + session.startTransaction() + const bodyValidation = (body && Object.keys(body).length) ? repo.validateReviewOrgObject(body) : { isValid: false } + if (!bodyValidation.isValid) { + return res.status(400).json({ message: 'Invalid body parameters', errors: bodyValidation.errors }) + } + createdReviewObj = await repo.createReviewOrgObject(body, { session }) + } catch (createErr) { + await session.abortTransaction() + return res.status(500).json({ message: createErr.message || 'Failed to create review object' }) + } finally { + await session.endSession() + } - if (!value) { + if (!createdReviewObj) { return res.status(500).json({ message: 'Failed to create review object' }) } - return res.status(200).json(value) + return res.status(200).json(createdReviewObj) } /** @@ -178,9 +208,11 @@ async function rejectReviewObject (req, res, next) { const reviewRepo = req.ctx.repositories.getReviewObjectRepository() const baseOrgRepo = req.ctx.repositories.getBaseOrgRepository() const UUID = req.params.uuid - const isSecretariat = await baseOrgRepo.isSecretariatByShortName(req.ctx.org) + const session = await mongoose.startSession({ causalConsistency: false }) + + const isSecretariat = await baseOrgRepo.isSecretariatByShortName(req.ctx.org, { session }) + const isPendingReview = true - const session = await mongoose.startSession() let value try { diff --git a/src/repositories/reviewObjectRepository.js b/src/repositories/reviewObjectRepository.js index 81d9550ba..8ebcda1f6 100644 --- a/src/repositories/reviewObjectRepository.js +++ b/src/repositories/reviewObjectRepository.js @@ -147,7 +147,7 @@ class ReviewObjectRepository extends BaseRepository { } const reviewObject = new ReviewObjectModel(reviewObjectRaw) - await reviewObject.save({ options }) + await reviewObject.save(options) return reviewObject.toObject() } @@ -160,7 +160,7 @@ class ReviewObjectRepository extends BaseRepository { reviewObject.new_review_data = body - const result = await reviewObject.save({ options }) + const result = await reviewObject.save(options) return result.toObject() } From 8a8c29ce450eb82503d2a1179353bff2f919faeb Mon Sep 17 00:00:00 2001 From: emathew Date: Wed, 4 Mar 2026 12:31:14 -0500 Subject: [PATCH 6/9] fix missing session.commitTransaction() --- .../review-object.controller.js | 16 +++++++++------- .../review-object/reviewObjectTest.js | 1 + 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/controller/review-object.controller/review-object.controller.js b/src/controller/review-object.controller/review-object.controller.js index 410a5e0e2..641564d03 100644 --- a/src/controller/review-object.controller/review-object.controller.js +++ b/src/controller/review-object.controller/review-object.controller.js @@ -22,9 +22,9 @@ async function getReviewObjectByOrgIdentifier (req, res, next) { let value // We may want this to be something different, but for now we are just testing if (identifierIsUUID) { - value = await repo.getOrgReviewObjectByOrgUUID(identifier, isSecretariat) + value = await repo.getOrgReviewObjectByOrgUUID(identifier, isSecretariat, {}) } else { - value = await repo.getOrgReviewObjectByOrgShortname(identifier, isSecretariat) + value = await repo.getOrgReviewObjectByOrgShortname(identifier, isSecretariat, {}) } if (!value) { return res.status(404).json({ message: 'No pending review object exists for this organization' }) @@ -65,7 +65,7 @@ async function approveReviewObject (req, res, next) { const isPendingReview = true const UUID = req.params.uuid const body = req.body - const session = await mongoose.startSession({ causalConsistency: false }) + const session = await mongoose.startSession({ causalConsistency: true }) let updatedOrgObj try { @@ -117,7 +117,7 @@ async function updateReviewObjectByReviewUUID (req, res, next) { const UUID = req.params.uuid const orgRepo = req.ctx.repositories.getBaseOrgRepository() const body = req.body - const session = await mongoose.startSession({ causalConsistency: false }) + const session = await mongoose.startSession({ causalConsistency: true }) let updatedReviewObj const result = orgRepo.validateOrg(body) @@ -147,18 +147,20 @@ async function updateReviewObjectByReviewUUID (req, res, next) { } async function createReviewObject (req, res, next) { + const baseOrgRepo = req.ctx.repositories.getBaseOrgRepository() const repo = req.ctx.repositories.getReviewObjectRepository() const body = req.body - const session = await mongoose.startSession({ causalConsistency: false }) + const session = await mongoose.startSession({ causalConsistency: false }) let createdReviewObj try { session.startTransaction() - const bodyValidation = (body && Object.keys(body).length) ? repo.validateReviewOrgObject(body) : { isValid: false } + const bodyValidation = (body && Object.keys(body).length) ? baseOrgRepo.validateOrg(body, { session }) : { isValid: false } if (!bodyValidation.isValid) { return res.status(400).json({ message: 'Invalid body parameters', errors: bodyValidation.errors }) } createdReviewObj = await repo.createReviewOrgObject(body, { session }) + await session.commitTransaction() } catch (createErr) { await session.abortTransaction() return res.status(500).json({ message: createErr.message || 'Failed to create review object' }) @@ -208,7 +210,7 @@ async function rejectReviewObject (req, res, next) { const reviewRepo = req.ctx.repositories.getReviewObjectRepository() const baseOrgRepo = req.ctx.repositories.getBaseOrgRepository() const UUID = req.params.uuid - const session = await mongoose.startSession({ causalConsistency: false }) + const session = await mongoose.startSession({ causalConsistency: true }) const isSecretariat = await baseOrgRepo.isSecretariatByShortName(req.ctx.org, { session }) diff --git a/test/integration-tests/review-object/reviewObjectTest.js b/test/integration-tests/review-object/reviewObjectTest.js index 33005ae1d..81958856f 100644 --- a/test/integration-tests/review-object/reviewObjectTest.js +++ b/test/integration-tests/review-object/reviewObjectTest.js @@ -39,6 +39,7 @@ describe('Review Object Controller Integration Tests', () => { expect(res.body).to.have.property('uuid') expect(res.body).to.have.property('target_object_uuid', orgUUID) expect(res.body).to.have.property('new_review_data') + expect(res.body.status).to.equal('pending') reviewUUID = res.body.uuid }) From c1ed4c3c209f14815fc4ece7c86f3dc13302c0ce Mon Sep 17 00:00:00 2001 From: emathew Date: Wed, 4 Mar 2026 12:59:59 -0500 Subject: [PATCH 7/9] add abortTransaction catches and fix unit tests --- .../review-object.controller/review-object.controller.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/controller/review-object.controller/review-object.controller.js b/src/controller/review-object.controller/review-object.controller.js index 641564d03..93ae1ca6f 100644 --- a/src/controller/review-object.controller/review-object.controller.js +++ b/src/controller/review-object.controller/review-object.controller.js @@ -73,16 +73,19 @@ async function approveReviewObject (req, res, next) { const bodyValidation = (body && Object.keys(body).length) ? baseOrgRepo.validateOrg(body) : { isValid: true } if (!bodyValidation.isValid) { + await session.abortTransaction() return res.status(400).json({ message: 'Invalid body parameters', errors: bodyValidation.errors }) } const reviewObject = await reviewRepo.findOneByUUIDWithConversation(UUID, isSecretariat, isPendingReview, { session }) if (!reviewObject) { + await session.abortTransaction() return res.status(404).json({ message: `No pending review object found with UUID ${UUID}` }) } const org = await baseOrgRepo.findOneByUUID(reviewObject.target_object_uuid, { session }) if (!org) { + await session.abortTransaction() return res.status(404).json({ message: 'Organization not found for this review object' }) } @@ -94,10 +97,12 @@ async function approveReviewObject (req, res, next) { const reviewObj = await reviewRepo.approveReviewOrgObject(UUID, { session }) if (!reviewObj) { + await session.abortTransaction() return res.status(404).json({ message: `Review object not approved with UUID ${UUID}` }) } updatedOrgObj = await baseOrgRepo.updateOrgFull(org.short_name, dataToUpdate, { session }, false, requestingUserUUID, false, true) if (!updatedOrgObj) { + await session.abortTransaction() return res.status(404).json({ message: `Org Object not updated with UUID ${UUID}` }) } @@ -157,6 +162,7 @@ async function createReviewObject (req, res, next) { session.startTransaction() const bodyValidation = (body && Object.keys(body).length) ? baseOrgRepo.validateOrg(body, { session }) : { isValid: false } if (!bodyValidation.isValid) { + await session.abortTransaction() return res.status(400).json({ message: 'Invalid body parameters', errors: bodyValidation.errors }) } createdReviewObj = await repo.createReviewOrgObject(body, { session }) From 2ed237d7c6c54912c07e2a8b374a56839672e538 Mon Sep 17 00:00:00 2001 From: emathew Date: Wed, 4 Mar 2026 13:00:11 -0500 Subject: [PATCH 8/9] unit-test fix --- .../review-object/review-object.controller.test.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/test/unit-tests/review-object/review-object.controller.test.js b/test/unit-tests/review-object/review-object.controller.test.js index 65ebb7348..45f4f779c 100644 --- a/test/unit-tests/review-object/review-object.controller.test.js +++ b/test/unit-tests/review-object/review-object.controller.test.js @@ -163,11 +163,11 @@ describe('Review Object Controller', function () { req.params.uuid = uuid req.body.new_review_data = { foo: 'bar' } orgRepoStub.validateOrg = sinon.stub().returns({ isValid: true }) + repoStub.findOneByUUIDWithConversation = sinon.stub().resolves(null) repoStub.updateReviewOrgObject = sinon.stub().resolves(undefined) await controller.updateReviewObjectByReviewUUID(req, res, next) - expect(repoStub.updateReviewOrgObject.calledWith(req.body, uuid)).to.be.true expect(res.status.calledWith(404)).to.be.true - expect(res.json.calledWith({ message: `No review object found with UUID ${uuid}` })).to.be.true + expect(res.json.calledWith({ message: `No pending review object found with UUID ${uuid}` })).to.be.true }) it('should return 200 with updated value', async () => { @@ -176,6 +176,7 @@ describe('Review Object Controller', function () { req.params.uuid = uuid req.body.new_review_data = { foo: 'bar' } orgRepoStub.validateOrg = sinon.stub().returns({ isValid: true }) + repoStub.findOneByUUIDWithConversation = sinon.stub().resolves(true) repoStub.updateReviewOrgObject = sinon.stub().resolves(updated) await controller.updateReviewObjectByReviewUUID(req, res, next) expect(repoStub.updateReviewOrgObject.calledWith(req.body, uuid)).to.be.true @@ -254,17 +255,15 @@ describe('Review Object Controller', function () { it('should approve review object and update organization with review data', async () => { orgRepoStub.validateOrg = sinon.stub().returns({ isValid: true }) repoStub.findOneByUUIDWithConversation = sinon.stub().resolves(reviewObject) - orgRepoStub.findOneByUUID = sinon.stub() - .onFirstCall().resolves(orgObj) - .onSecondCall().resolves(updatedOrgObj) + orgRepoStub.findOneByUUID = sinon.stub().resolves(orgObj) + userRepoStub.getUserUUID = sinon.stub().resolves('user-uuid') repoStub.approveReviewOrgObject = sinon.stub().resolves({ ...reviewObject, status: 'approved' }) orgRepoStub.updateOrgFull = sinon.stub().resolves(updatedOrgObj) - userRepoStub.getUserUUID = sinon.stub().resolves('user-uuid') await controller.approveReviewObject(req, res, next) expect(orgRepoStub.updateOrgFull.calledOnce).to.be.true expect(res.status.calledWith(200)).to.be.true - expect(res.json.calledWith({ short_name: 'updated_org' })).to.be.true + expect(res.json.calledWith(updatedOrgObj)).to.be.true }) }) From f0a810ac5e7ef323b103480d3ebc7d849e1d6be5 Mon Sep 17 00:00:00 2001 From: emathew Date: Wed, 4 Mar 2026 13:12:15 -0500 Subject: [PATCH 9/9] set causalConsistency values to false --- .../review-object.controller/review-object.controller.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/controller/review-object.controller/review-object.controller.js b/src/controller/review-object.controller/review-object.controller.js index 93ae1ca6f..2f1700d38 100644 --- a/src/controller/review-object.controller/review-object.controller.js +++ b/src/controller/review-object.controller/review-object.controller.js @@ -65,7 +65,7 @@ async function approveReviewObject (req, res, next) { const isPendingReview = true const UUID = req.params.uuid const body = req.body - const session = await mongoose.startSession({ causalConsistency: true }) + const session = await mongoose.startSession({ causalConsistency: false }) let updatedOrgObj try { @@ -122,7 +122,7 @@ async function updateReviewObjectByReviewUUID (req, res, next) { const UUID = req.params.uuid const orgRepo = req.ctx.repositories.getBaseOrgRepository() const body = req.body - const session = await mongoose.startSession({ causalConsistency: true }) + const session = await mongoose.startSession({ causalConsistency: false }) let updatedReviewObj const result = orgRepo.validateOrg(body) @@ -216,7 +216,7 @@ async function rejectReviewObject (req, res, next) { const reviewRepo = req.ctx.repositories.getReviewObjectRepository() const baseOrgRepo = req.ctx.repositories.getBaseOrgRepository() const UUID = req.params.uuid - const session = await mongoose.startSession({ causalConsistency: true }) + const session = await mongoose.startSession({ causalConsistency: false }) const isSecretariat = await baseOrgRepo.isSecretariatByShortName(req.ctx.org, { session })