BAI-1464 replace removeAccessRequestReview (singular) with removeAcce…

…ssRequestReviews (multiple)

backend/src/migrations/008_delete_reviews_with_removed_access_requests.ts

@@ -1,6 +1,6 @@
 import AccessRequestModel from '../models/AccessRequest.js'
 import ReviewModel from '../models/Review.js'
-import { removeAccessRequestReview } from '../services/review.js'
+import { removeAccessRequestReviews } from '../services/review.js'
 
 export async function up() {
   const accessRequests = await AccessRequestModel.find({ deleted: false })
@@ -11,7 +11,7 @@ export async function up() {
       reviewAccessRequestId &&
       !accessRequests.some((accessRequest) => accessRequest.get('id') == reviewAccessRequestId)
     ) {
-      removeAccessRequestReview(reviewAccessRequestId)
+      removeAccessRequestReviews(reviewAccessRequestId)
     }
   }
 }

backend/src/services/accessRequest.ts

@@ -17,7 +17,7 @@ import { convertStringToId } from '../utils/id.js'
 import { authResponseToUserPermission } from '../utils/permissions.js'
 import log from './log.js'
 import { getModelById } from './model.js'
-import { createAccessRequestReviews, removeAccessRequestReview } from './review.js'
+import { createAccessRequestReviews, removeAccessRequestReviews } from './review.js'
 import { getSchemaById } from './schema.js'
 import { sendWebhooks } from './webhook.js'
 
@@ -91,7 +91,7 @@ export async function removeAccessRequest(user: UserInterface, accessRequestId:
 
   await accessRequest.delete()
 
-  await removeAccessRequestReview(accessRequestId)
+  await removeAccessRequestReviews(accessRequestId)
 
   return { accessRequestId }
 }

backend/src/services/review.ts

@@ -1,3 +1,5 @@
+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'
@@ -7,7 +9,7 @@ import { ReleaseDoc } from '../models/Release.js'
 import Review, { ReviewDoc, ReviewInterface } from '../models/Review.js'
 import { UserInterface } from '../models/User.js'
 import { ReviewKind, ReviewKindKeys } from '../types/enums.js'
-import { BadReq, Forbidden, NotFound } from '../utils/error.js'
+import { BadReq, InternalError, NotFound } from '../utils/error.js'
 import log from './log.js'
 import { getModelById } from './model.js'
 import { requestReviewForAccessRequest, requestReviewForRelease } from './smtp/smtp.js'
@@ -89,12 +91,16 @@ export async function createAccessRequestReviews(model: ModelDoc, accessRequest:
   await Promise.all(createReviews)
 }
 
-export async function removeAccessRequestReview(accessRequestId: string) {
-  const accessRequestReview = await findReviewForAccessRequest(accessRequestId)
+export async function removeAccessRequestReviews(accessRequestId: string): Promise<DeleteResult> {
+  const deletions = await Review.deleteMany({ accessRequestId: accessRequestId })
 
-  await accessRequestReview.delete()
+  if (deletions.deletedCount === 0) {
+    throw InternalError('The requested access request reviews could not be deleted.', {
+      accessRequestId,
+    })
+  }
 
-  return { accessRequestId }
+  return deletions
 }
 
 export async function findReviewForResponse(
@@ -149,20 +155,6 @@ export async function findReviewsForAccessRequests(accessRequestIds: string[]) {
   return reviews.filter((review) => requiredRoles.accessRequest.includes(review.role))
 }
 
-export async function findReviewForAccessRequest(accessRequestId: string) {
-  const review = await Review.findOne({ accessRequestId: accessRequestId })
-
-  if (!review) {
-    throw NotFound('The requested access request review was not found.', { accessRequestId })
-  }
-
-  if (!requiredRoles.accessRequest.includes(review.role)) {
-    throw Forbidden('Permission error when sending getting review for Access Request.', { accessRequestId })
-  }
-
-  return review
-}
-
 function getRoleEntities(roles: string[], collaborators: CollaboratorEntry[]) {
   return roles.map((role) => {
     const entities = collaborators

backend/test/services/__snapshots__/review.spec.ts.snap

@@ -1,7 +1,5 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
-exports[`services > review > findReviewForAccessRequest > success 1`] = `undefined`;
-
 exports[`services > review > findReviews > active reviews for a specific model 1`] = `
 [
   {
@@ -59,3 +57,13 @@ exports[`services > review > findReviews > all reviews for user 2`] = `
   },
 ]
 `;
+
+exports[`services > review > findReviewsForAccessRequests > success 1`] = `
+[
+  {
+    "accessRequestId": [
+      "Hello",
+    ],
+  },
+]
+`;

backend/test/services/accessRequest.spec.ts

@@ -42,7 +42,7 @@ vi.mock('../../src/models/AccessRequest.js', () => ({ default: accessRequestMode
 const mockReviewService = vi.hoisted(() => {
   return {
     createAccessRequestReviews: vi.fn(),
-    removeAccessRequestReview: vi.fn(),
+    removeAccessRequestReviews: vi.fn(),
   }
 })
 vi.mock('../../src/services/review.js', () => mockReviewService)

backend/test/services/review.spec.ts

@@ -6,9 +6,9 @@ import Release from '../../src/models/Release.js'
 import {
   createAccessRequestReviews,
   createReleaseReviews,
-  findReviewForAccessRequest,
   findReviews,
-  removeAccessRequestReview,
+  findReviewsForAccessRequests,
+  removeAccessRequestReviews,
 } from '../../src/services/review.js'
 
 vi.mock('../../src/connectors/authorisation/index.js')
@@ -24,13 +24,14 @@ const reviewModelMock = vi.hoisted(() => {
   obj.sort = vi.fn(() => obj)
   obj.lookup = vi.fn(() => obj)
   obj.append = vi.fn(() => obj)
-  obj.find = vi.fn(() => obj)
+  obj.find = vi.fn(() => [obj])
   obj.findOne = vi.fn(() => obj)
   obj.findOneAndUpdate = vi.fn(() => obj)
   obj.findByIdAndUpdate = vi.fn(() => obj)
   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)
@@ -91,27 +92,10 @@ describe('services > review', () => {
     expect(reviewModelMock.match.mock.calls.at(1)).toMatchSnapshot()
   })
 
-  test('findReviewForAccessRequest > success', async () => {
-    await findReviewForAccessRequest(reviewModelMock.accessRequestId)
+  test('findReviewsForAccessRequests > success', async () => {
+    await findReviewsForAccessRequests([reviewModelMock.accessRequestId])
 
-    expect(reviewModelMock.match.mock.calls.at(0)).toMatchSnapshot()
-  })
-
-  test('findReviewForAccessRequest > NotFound failure', async () => {
-    reviewModelMock.findOne.mockResolvedValue(undefined)
-
-    expect(() => findReviewForAccessRequest('')).rejects.toThrowError(
-      /^The requested access request review was not found./,
-    )
-  })
-
-  test('findReviewForAccessRequest > Forbidden failure', async () => {
-    reviewModelMock.role = 'this-does-not-exist'
-    reviewModelMock.findOne.mockResolvedValue(reviewModelMock)
-
-    expect(() => findReviewForAccessRequest(reviewModelMock.accessRequestId)).rejects.toThrowError(
-      /^Permission error when sending getting review for Access Request./,
-    )
+    expect(reviewModelMock.find.mock.calls.at(0)).toMatchSnapshot()
   })
 
   test('createReleaseReviews > No entities found for required roles', async () => {
@@ -142,17 +126,21 @@ describe('services > review', () => {
     expect(smtpMock.requestReviewForAccessRequest).toBeCalled()
   })
 
-  test('removeAccessRequestReview > successful', async () => {
-    await removeAccessRequestReview(reviewModelMock.accessRequestId)
+  test('removeAccessRequestReviews > successful', async () => {
+    const deleteResultsMock: any = { deletedCount: 1 }
+    reviewModelMock.deleteMany.mockResolvedValue(deleteResultsMock)
+
+    await removeAccessRequestReviews(reviewModelMock.accessRequestId)
 
-    expect(reviewModelMock.delete).toBeCalled()
+    expect(reviewModelMock.deleteMany).toBeCalled()
   })
 
-  test('removeAccessRequestReview > failure', async () => {
-    reviewModelMock.findOne.mockResolvedValue(undefined)
+  test('removeAccessRequestReviews > could not delete failure', async () => {
+    const deleteResultsMock: any = { deletedCount: 0 }
+    reviewModelMock.deleteMany.mockResolvedValue(deleteResultsMock)
 
-    expect(() => removeAccessRequestReview('')).rejects.toThrowError(
-      /^The requested access request review was not found./,
+    expect(() => removeAccessRequestReviews('')).rejects.toThrowError(
+      /^The requested access request reviews could not be deleted./,
     )
   })
 })