From 1bfc5d5ab6b3b74c95561b6b228bf6b3970222da Mon Sep 17 00:00:00 2001 From: PE39806 <185931318+PE39806@users.noreply.github.com> Date: Wed, 30 Oct 2024 16:31:00 +0000 Subject: [PATCH] BAI-1464 fix hard -> soft delete and optimise patch script --- ...te_reviews_with_removed_access_requests.ts | 14 +- backend/src/services/review.ts | 26 +- .../__snapshots__/review.spec.ts.snap | 10 + backend/test/services/review.spec.ts | 14 +- package-lock.json | 222 +++++++++++++++++- package.json | 3 + 6 files changed, 257 insertions(+), 32 deletions(-) diff --git a/backend/src/migrations/008_delete_reviews_with_removed_access_requests.ts b/backend/src/migrations/008_delete_reviews_with_removed_access_requests.ts index c636f22e9..dc2c31506 100644 --- a/backend/src/migrations/008_delete_reviews_with_removed_access_requests.ts +++ b/backend/src/migrations/008_delete_reviews_with_removed_access_requests.ts @@ -1,18 +1,10 @@ import AccessRequestModel from '../models/AccessRequest.js' -import ReviewModel from '../models/Review.js' import { removeAccessRequestReviews } from '../services/review.js' export async function up() { - const accessRequests = await AccessRequestModel.find({ deleted: false }) - const reviews = await ReviewModel.find({}) - for (const review of reviews) { - const reviewAccessRequestId = review.get('accessRequestId') - if ( - reviewAccessRequestId && - !accessRequests.some((accessRequest) => accessRequest.get('id') == reviewAccessRequestId) - ) { - await removeAccessRequestReviews(reviewAccessRequestId) - } + const deletedAccessRequests = await (AccessRequestModel as any).findDeleted() + for (const accessRequest of deletedAccessRequests) { + await removeAccessRequestReviews(accessRequest.get('id')) } } diff --git a/backend/src/services/review.ts b/backend/src/services/review.ts index af4d72e01..6329cd12f 100644 --- a/backend/src/services/review.ts +++ b/backend/src/services/review.ts @@ -1,5 +1,3 @@ -import { DeleteResult } from 'mongodb' - import authentication from '../connectors/authentication/index.js' import { ModelAction } from '../connectors/authorisation/actions.js' import authorisation from '../connectors/authorisation/index.js' @@ -91,13 +89,21 @@ export async function createAccessRequestReviews(model: ModelDoc, accessRequest: await Promise.all(createReviews) } -export async function removeAccessRequestReviews(accessRequestId: string): Promise { - const deletions = await Review.deleteMany({ accessRequestId: accessRequestId }) - - if (deletions.acknowledged === false) { - throw InternalError('The requested access request reviews could not be deleted.', { - accessRequestId, - }) +export async function removeAccessRequestReviews(accessRequestId: string) { + // finding and then calling potentially multiple deletes is inefficient but the mongoose-softdelete + // plugin doesn't cover bulkDelete + const accessRequestReviews = await findReviewsForAccessRequests([accessRequestId]) + + const deletions: ReviewDoc[] = [] + for (const accessRequestReview of accessRequestReviews) { + try { + deletions.push(await accessRequestReview.delete()) + } catch (error) { + throw InternalError('The requested access request review could not be deleted.', { + accessRequestId, + error, + }) + } } return deletions @@ -149,7 +155,7 @@ export async function findReviewForResponse( //TODO This won't work for response refactor export async function findReviewsForAccessRequests(accessRequestIds: string[]) { - const reviews = await Review.find({ + const reviews: ReviewDoc[] = await Review.find({ accessRequestId: accessRequestIds, }) return reviews.filter((review) => requiredRoles.accessRequest.includes(review.role)) diff --git a/backend/test/services/__snapshots__/review.spec.ts.snap b/backend/test/services/__snapshots__/review.spec.ts.snap index d8bad95c1..59e5d3f21 100644 --- a/backend/test/services/__snapshots__/review.spec.ts.snap +++ b/backend/test/services/__snapshots__/review.spec.ts.snap @@ -67,3 +67,13 @@ exports[`services > review > findReviewsForAccessRequests > success 1`] = ` }, ] `; + +exports[`services > review > removeAccessRequestReviews > successful 1`] = ` +[ + { + "accessRequestId": [ + "Hello", + ], + }, +] +`; diff --git a/backend/test/services/review.spec.ts b/backend/test/services/review.spec.ts index ac0c95e74..3201cde22 100644 --- a/backend/test/services/review.spec.ts +++ b/backend/test/services/review.spec.ts @@ -31,7 +31,6 @@ const reviewModelMock = vi.hoisted(() => { obj.updateOne = vi.fn(() => obj) obj.save = vi.fn(() => obj) obj.delete = vi.fn(() => obj) - obj.deleteMany = vi.fn(() => obj) obj.limit = vi.fn(() => obj) obj.unwind = vi.fn(() => obj) obj.at = vi.fn(() => obj) @@ -127,20 +126,19 @@ describe('services > review', () => { }) test('removeAccessRequestReviews > successful', async () => { - const deleteResultsMock: any = { acknowledged: true } - reviewModelMock.deleteMany.mockResolvedValue(deleteResultsMock) - await removeAccessRequestReviews(reviewModelMock.accessRequestId) - expect(reviewModelMock.deleteMany).toBeCalled() + expect(reviewModelMock.find.mock.calls.at(0)).toMatchSnapshot() + expect(reviewModelMock.delete).toBeCalled() }) test('removeAccessRequestReviews > could not delete failure', async () => { - const deleteResultsMock: any = { acknowledged: false } - reviewModelMock.deleteMany.mockResolvedValue(deleteResultsMock) + reviewModelMock.delete.mockImplementationOnce(() => { + throw Error('Error deleting object') + }) expect(() => removeAccessRequestReviews('')).rejects.toThrowError( - /^The requested access request reviews could not be deleted./, + /^The requested access request review could not be deleted./, ) }) }) diff --git a/package-lock.json b/package-lock.json index 3dee526d1..125716105 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,12 +8,53 @@ "name": "bailo", "version": "0.4.0", "hasInstallScript": true, + "dependencies": { + "@types/mongoose-delete": "^1.0.6" + }, "devDependencies": { "husky": "^9.1.6", "lint-staged": "^15.2.10", "prettier": "^3.3.3" } }, + "node_modules/@mongodb-js/saslprep": { + "version": "1.1.9", + "resolved": "https://registry.npmjs.org/@mongodb-js/saslprep/-/saslprep-1.1.9.tgz", + "integrity": "sha512-tVkljjeEaAhCqTzajSdgbQ6gE6f3oneVwa3iXR6csiEwXXOFsiC6Uh9iAjAhXPtqa/XMDHWjjeNH/77m/Yq2dw==", + "dependencies": { + "sparse-bitfield": "^3.0.3" + } + }, + "node_modules/@types/mongoose-delete": { + "version": "1.0.6", + "resolved": "https://registry.npmjs.org/@types/mongoose-delete/-/mongoose-delete-1.0.6.tgz", + "integrity": "sha512-8q5uYEZ68sd6Ray+DK7Ifs7vJnoMmnk6Ld5YhmXIDIEFbZNqpbWI4TTksUeLy4w4szDS/KiTTp/5eeyp4ILNfA==", + "dependencies": { + "@types/node": "*", + "mongoose": "^5.11 || ^6 || ^7 || ^8" + } + }, + "node_modules/@types/node": { + "version": "22.8.4", + "resolved": "https://registry.npmjs.org/@types/node/-/node-22.8.4.tgz", + "integrity": "sha512-SpNNxkftTJOPk0oN+y2bIqurEXHTA2AOZ3EJDDKeJ5VzkvvORSvmQXGQarcOzWV1ac7DCaPBEdMDxBsM+d8jWw==", + "dependencies": { + "undici-types": "~6.19.8" + } + }, + "node_modules/@types/webidl-conversions": { + "version": "7.0.3", + "resolved": "https://registry.npmjs.org/@types/webidl-conversions/-/webidl-conversions-7.0.3.tgz", + "integrity": "sha512-CiJJvcRtIgzadHCYXw7dqEnMNRjhGZlYK05Mj9OyktqV8uVT8fD2BFOB7S1uwBE3Kj2Z+4UyPmFw/Ixgw/LAlA==" + }, + "node_modules/@types/whatwg-url": { + "version": "11.0.5", + "resolved": "https://registry.npmjs.org/@types/whatwg-url/-/whatwg-url-11.0.5.tgz", + "integrity": "sha512-coYR071JRaHa+xoEvvYqvnIHaVqaYrLPbsufM9BF63HkwI5Lgmy2QR8Q5K/lYDYo5AK82wOvSOS0UsLTpTG7uQ==", + "dependencies": { + "@types/webidl-conversions": "*" + } + }, "node_modules/ansi-escapes": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/ansi-escapes/-/ansi-escapes-7.0.0.tgz", @@ -65,6 +106,14 @@ "node": ">=8" } }, + "node_modules/bson": { + "version": "6.9.0", + "resolved": "https://registry.npmjs.org/bson/-/bson-6.9.0.tgz", + "integrity": "sha512-X9hJeyeM0//Fus+0pc5dSUMhhrrmWwQUtdavaQeF3Ta6m69matZkGWV/MrBcnwUeLC8W9kwwc2hfkZgUuCX3Ig==", + "engines": { + "node": ">=16.20.1" + } + }, "node_modules/chalk": { "version": "5.3.0", "resolved": "https://registry.npmjs.org/chalk/-/chalk-5.3.0.tgz", @@ -141,7 +190,6 @@ "version": "4.3.6", "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.6.tgz", "integrity": "sha512-O/09Bd4Z1fBrU4VzkhFqVgpPzaGbw6Sm9FEkBT1A/YBXQFGuuSxa1dN2nxgxS34JmKXqYx8CZAwEVoJFImUXIg==", - "dev": true, "dependencies": { "ms": "2.1.2" }, @@ -300,6 +348,14 @@ "integrity": "sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==", "dev": true }, + "node_modules/kareem": { + "version": "2.6.3", + "resolved": "https://registry.npmjs.org/kareem/-/kareem-2.6.3.tgz", + "integrity": "sha512-C3iHfuGUXK2u8/ipq9LfjFfXFxAZMQJJq7vLS45r3D9Y2xQ/m4S8zaR4zMLFWh9AsNPXmcFfUDhTEO8UIC/V6Q==", + "engines": { + "node": ">=12.0.0" + } + }, "node_modules/lilconfig": { "version": "3.1.2", "resolved": "https://registry.npmjs.org/lilconfig/-/lilconfig-3.1.2.tgz", @@ -406,6 +462,11 @@ "url": "https://github.com/chalk/slice-ansi?sponsor=1" } }, + "node_modules/memory-pager": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/memory-pager/-/memory-pager-1.5.0.tgz", + "integrity": "sha512-ZS4Bp4r/Zoeq6+NLJpP+0Zzm0pR8whtGPf1XExKLJBAczGMnSi3It14OiNCStjQjM6NU1okjQGSxgEZN8eBYKg==" + }, "node_modules/merge-stream": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/merge-stream/-/merge-stream-2.0.0.tgz", @@ -449,11 +510,109 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/mongodb": { + "version": "6.9.0", + "resolved": "https://registry.npmjs.org/mongodb/-/mongodb-6.9.0.tgz", + "integrity": "sha512-UMopBVx1LmEUbW/QE0Hw18u583PEDVQmUmVzzBRH0o/xtE9DBRA5ZYLOjpLIa03i8FXjzvQECJcqoMvCXftTUA==", + "dependencies": { + "@mongodb-js/saslprep": "^1.1.5", + "bson": "^6.7.0", + "mongodb-connection-string-url": "^3.0.0" + }, + "engines": { + "node": ">=16.20.1" + }, + "peerDependencies": { + "@aws-sdk/credential-providers": "^3.188.0", + "@mongodb-js/zstd": "^1.1.0", + "gcp-metadata": "^5.2.0", + "kerberos": "^2.0.1", + "mongodb-client-encryption": ">=6.0.0 <7", + "snappy": "^7.2.2", + "socks": "^2.7.1" + }, + "peerDependenciesMeta": { + "@aws-sdk/credential-providers": { + "optional": true + }, + "@mongodb-js/zstd": { + "optional": true + }, + "gcp-metadata": { + "optional": true + }, + "kerberos": { + "optional": true + }, + "mongodb-client-encryption": { + "optional": true + }, + "snappy": { + "optional": true + }, + "socks": { + "optional": true + } + } + }, + "node_modules/mongodb-connection-string-url": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/mongodb-connection-string-url/-/mongodb-connection-string-url-3.0.1.tgz", + "integrity": "sha512-XqMGwRX0Lgn05TDB4PyG2h2kKO/FfWJyCzYQbIhXUxz7ETt0I/FqHjUeqj37irJ+Dl1ZtU82uYyj14u2XsZKfg==", + "dependencies": { + "@types/whatwg-url": "^11.0.2", + "whatwg-url": "^13.0.0" + } + }, + "node_modules/mongoose": { + "version": "8.7.3", + "resolved": "https://registry.npmjs.org/mongoose/-/mongoose-8.7.3.tgz", + "integrity": "sha512-Xl6+dzU5ZpEcDoJ8/AyrIdAwTY099QwpolvV73PIytpK13XqwllLq/9XeVzzLEQgmyvwBVGVgjmMrKbuezxrIA==", + "dependencies": { + "bson": "^6.7.0", + "kareem": "2.6.3", + "mongodb": "6.9.0", + "mpath": "0.9.0", + "mquery": "5.0.0", + "ms": "2.1.3", + "sift": "17.1.3" + }, + "engines": { + "node": ">=16.20.1" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/mongoose" + } + }, + "node_modules/mongoose/node_modules/ms": { + "version": "2.1.3", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", + "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==" + }, + "node_modules/mpath": { + "version": "0.9.0", + "resolved": "https://registry.npmjs.org/mpath/-/mpath-0.9.0.tgz", + "integrity": "sha512-ikJRQTk8hw5DEoFVxHG1Gn9T/xcjtdnOKIU1JTmGjZZlg9LST2mBLmcX3/ICIbgJydT2GOc15RnNy5mHmzfSew==", + "engines": { + "node": ">=4.0.0" + } + }, + "node_modules/mquery": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/mquery/-/mquery-5.0.0.tgz", + "integrity": "sha512-iQMncpmEK8R8ncT8HJGsGc9Dsp8xcgYMVSbs5jgnm1lFHTZqMJTUWTDx1LBO8+mK3tPNZWFLBghQEIOULSTHZg==", + "dependencies": { + "debug": "4.x" + }, + "engines": { + "node": ">=14.0.0" + } + }, "node_modules/ms": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", - "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==", - "dev": true + "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" }, "node_modules/npm-run-path": { "version": "5.1.0", @@ -545,6 +704,14 @@ "url": "https://github.com/prettier/prettier?sponsor=1" } }, + "node_modules/punycode": { + "version": "2.3.1", + "resolved": "https://registry.npmjs.org/punycode/-/punycode-2.3.1.tgz", + "integrity": "sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg==", + "engines": { + "node": ">=6" + } + }, "node_modules/restore-cursor": { "version": "5.1.0", "resolved": "https://registry.npmjs.org/restore-cursor/-/restore-cursor-5.1.0.tgz", @@ -603,6 +770,11 @@ "node": ">=8" } }, + "node_modules/sift": { + "version": "17.1.3", + "resolved": "https://registry.npmjs.org/sift/-/sift-17.1.3.tgz", + "integrity": "sha512-Rtlj66/b0ICeFzYTuNvX/EF1igRbbnGSvEyT79McoZa/DeGhMyC5pWKOEsZKnpkqtSeovd5FL/bjHWC3CIIvCQ==" + }, "node_modules/signal-exit": { "version": "4.1.0", "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-4.1.0.tgz", @@ -631,6 +803,14 @@ "url": "https://github.com/chalk/slice-ansi?sponsor=1" } }, + "node_modules/sparse-bitfield": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/sparse-bitfield/-/sparse-bitfield-3.0.3.tgz", + "integrity": "sha512-kvzhi7vqKTfkh0PZU+2D2PIllw2ymqJKujUcyPMd9Y75Nv4nPbGJZXNhxsgdQab2BmlDct1YnfQCguEvHr7VsQ==", + "dependencies": { + "memory-pager": "^1.0.2" + } + }, "node_modules/string-argv": { "version": "0.3.2", "resolved": "https://registry.npmjs.org/string-argv/-/string-argv-0.3.2.tgz", @@ -696,6 +876,42 @@ "node": ">=8.0" } }, + "node_modules/tr46": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/tr46/-/tr46-4.1.1.tgz", + "integrity": "sha512-2lv/66T7e5yNyhAAC4NaKe5nVavzuGJQVVtRYLyQ2OI8tsJ61PMLlelehb0wi2Hx6+hT/OJUWZcw8MjlSRnxvw==", + "dependencies": { + "punycode": "^2.3.0" + }, + "engines": { + "node": ">=14" + } + }, + "node_modules/undici-types": { + "version": "6.19.8", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.19.8.tgz", + "integrity": "sha512-ve2KP6f/JnbPBFyobGHuerC9g1FYGn/F8n1LWTwNxCEzd6IfqTwUQcNXgEtmmQ6DlRrC1hrSrBnCZPokRrDHjw==" + }, + "node_modules/webidl-conversions": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-7.0.0.tgz", + "integrity": "sha512-VwddBukDzu71offAQR975unBIGqfKZpM+8ZX6ySk8nYhVoo5CYaZyzt3YBvYtRtO+aoGlqxPg/B87NGVZ/fu6g==", + "engines": { + "node": ">=12" + } + }, + "node_modules/whatwg-url": { + "version": "13.0.0", + "resolved": "https://registry.npmjs.org/whatwg-url/-/whatwg-url-13.0.0.tgz", + "integrity": "sha512-9WWbymnqj57+XEuqADHrCJ2eSXzn8WXIW/YSGaZtb2WKAInQ6CHfaUUcTyyver0p8BDg5StLQq8h1vtZuwmOig==", + "dependencies": { + "tr46": "^4.1.1", + "webidl-conversions": "^7.0.0" + }, + "engines": { + "node": ">=16" + } + }, "node_modules/which": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz", diff --git a/package.json b/package.json index 577888043..44d1c2b73 100644 --- a/package.json +++ b/package.json @@ -17,5 +17,8 @@ "husky": "^9.1.6", "lint-staged": "^15.2.10", "prettier": "^3.3.3" + }, + "dependencies": { + "@types/mongoose-delete": "^1.0.6" } }