Bugfix/bai 1464 deleted access request still appearing on the "your reviews" page

backend/src/migrations/008_delete_reviews_with_removed_access_requests.ts

@@ -0,0 +1,13 @@
+import AccessRequestModel from '../models/AccessRequest.js'
+import { removeAccessRequestReviews } from '../services/review.js'
+
+export async function up() {
+  const deletedAccessRequests = await (AccessRequestModel as any).findDeleted()
+  for (const accessRequest of deletedAccessRequests) {
+    await removeAccessRequestReviews(accessRequest.get('id'))
+  }
+}
+
+export async function down() {
+  /* NOOP */
+}

backend/src/routes/v2/model/accessRequest/getAccessRequest.ts

@@ -11,6 +11,7 @@ import { parse } from '../../../../utils/validate.js'
 
 export const getAccessRequestSchema = z.object({
   params: z.object({
+    modelId: z.string(),
     accessRequestId: z.string(),
   }),
 })

backend/src/routes/v2/model/accessRequest/patchAccessRequest.ts

@@ -15,6 +15,7 @@ export const patchAccessRequestSchema = z.object({
     metadata: accessRequestMetadata,
   }),
   params: z.object({
+    modelId: z.string(),
     accessRequestId: z.string(),
   }),
 })

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 } from './review.js'
+import { createAccessRequestReviews, removeAccessRequestReviews } from './review.js'
 import { getSchemaById } from './schema.js'
 import { sendWebhooks } from './webhook.js'
 
@@ -91,6 +91,8 @@ export async function removeAccessRequest(user: UserInterface, accessRequestId:
 
   await accessRequest.delete()
 
+  await removeAccessRequestReviews(accessRequestId)
+
   return { accessRequestId }
 }
 

backend/src/services/review.ts

@@ -7,7 +7,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, 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,6 +89,26 @@ export async function createAccessRequestReviews(model: ModelDoc, accessRequest:
   await Promise.all(createReviews)
 }
 
+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
+}
+
 export async function findReviewForResponse(
   user: UserInterface,
   modelId: string,
@@ -135,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))

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

@@ -57,3 +57,23 @@ exports[`services > review > findReviews > all reviews for user 2`] = `
   },
 ]
 `;
+
+exports[`services > review > findReviewsForAccessRequests > success 1`] = `
+[
+  {
+    "accessRequestId": [
+      "Hello",
+    ],
+  },
+]
+`;
+
+exports[`services > review > removeAccessRequestReviews > successful 1`] = `
+[
+  {
+    "accessRequestId": [
+      "Hello",
+    ],
+  },
+]
+`;

backend/test/services/accessRequest.spec.ts

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

backend/test/services/review.spec.ts

@@ -3,27 +3,34 @@ import { describe, expect, test, vi } from 'vitest'
 import AccessRequest from '../../src/models/AccessRequest.js'
 import Model from '../../src/models/Model.js'
 import Release from '../../src/models/Release.js'
-import { createAccessRequestReviews, createReleaseReviews, findReviews } from '../../src/services/review.js'
+import {
+  createAccessRequestReviews,
+  createReleaseReviews,
+  findReviews,
+  findReviewsForAccessRequests,
+  removeAccessRequestReviews,
+} from '../../src/services/review.js'
 
 vi.mock('../../src/connectors/authorisation/index.js')
 vi.mock('../../src/connectors/authentication/index.js', async () => ({
   default: { getEntities: vi.fn(() => ['user:test']) },
 }))
 
 const reviewModelMock = vi.hoisted(() => {
-  const obj: any = { kind: 'access', accessRequestId: 'Hello' }
+  const obj: any = { kind: 'access', accessRequestId: 'Hello', role: 'msro' }
 
   obj.aggregate = vi.fn(() => obj)
   obj.match = vi.fn(() => obj)
   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.limit = vi.fn(() => obj)
   obj.unwind = vi.fn(() => obj)
   obj.at = vi.fn(() => obj)
@@ -84,6 +91,12 @@ describe('services > review', () => {
     expect(reviewModelMock.match.mock.calls.at(1)).toMatchSnapshot()
   })
 
+  test('findReviewsForAccessRequests > success', async () => {
+    await findReviewsForAccessRequests([reviewModelMock.accessRequestId])
+
+    expect(reviewModelMock.find.mock.calls.at(0)).toMatchSnapshot()
+  })
+
   test('createReleaseReviews > No entities found for required roles', async () => {
     const result: Promise<void> = createReleaseReviews(new Model(), new Release())
 
@@ -111,4 +124,21 @@ describe('services > review', () => {
     expect(reviewModelMock.save).toBeCalled()
     expect(smtpMock.requestReviewForAccessRequest).toBeCalled()
   })
+
+  test('removeAccessRequestReviews > successful', async () => {
+    await removeAccessRequestReviews(reviewModelMock.accessRequestId)
+
+    expect(reviewModelMock.find.mock.calls.at(0)).toMatchSnapshot()
+    expect(reviewModelMock.delete).toBeCalled()
+  })
+
+  test('removeAccessRequestReviews > could not delete failure', async () => {
+    reviewModelMock.delete.mockImplementationOnce(() => {
+      throw Error('Error deleting object')
+    })
+
+    expect(() => removeAccessRequestReviews('')).rejects.toThrowError(
+      /^The requested access request review could not be deleted./,
+    )
+  })
 })

frontend/src/entry/model/accessRequests/EditableAccessRequestForm.tsx

@@ -7,6 +7,7 @@ import {
   useGetAccessRequestsForModelId,
 } from 'actions/accessRequest'
 import { useGetModel } from 'actions/model'
+import { useGetReviewRequestsForUser } from 'actions/review'
 import { useGetSchema } from 'actions/schema'
 import { useGetCurrentUser } from 'actions/user'
 import { useRouter } from 'next/router'
@@ -44,6 +45,7 @@ export default function EditableAccessRequestForm({
   const { schema, isSchemaLoading, isSchemaError } = useGetSchema(accessRequest.schemaId)
   const { isAccessRequestError, mutateAccessRequest } = useGetAccessRequest(accessRequest.modelId, accessRequest.id)
   const { mutateAccessRequests } = useGetAccessRequestsForModelId(accessRequest.modelId)
+  const { mutateReviews } = useGetReviewRequestsForUser()
   const { model, isModelLoading, isModelError } = useGetModel(accessRequest.modelId, EntryKind.MODEL)
   const { currentUser, isCurrentUserLoading, isCurrentUserError } = useGetCurrentUser()
 
@@ -66,16 +68,17 @@ export default function EditableAccessRequestForm({
   }, [accessRequest.metadata.overview.entities, currentUser, currentUserRoles])
 
   const handleDeleteConfirm = useCallback(async () => {
-    setErrorMessage('')
+    setDeleteErrorMessage('')
     const res = await deleteAccessRequest(accessRequest.modelId, accessRequest.id)
     if (!res.ok) {
       setDeleteErrorMessage(await getErrorMessage(res))
     } else {
       mutateAccessRequests()
+      mutateReviews()
       setOpen(false)
       router.push(`/model/${accessRequest.modelId}?tab=access`)
     }
-  }, [mutateAccessRequests, accessRequest, router])
+  }, [accessRequest.modelId, accessRequest.id, mutateAccessRequests, mutateReviews, router])
 
   async function handleSubmit() {
     if (schema) {
@@ -122,6 +125,10 @@ export default function EditableAccessRequestForm({
     resetForm()
   }
 
+  function handleDelete() {
+    setOpen(true)
+  }
+
   useEffect(() => {
     resetForm()
   }, [resetForm])
@@ -173,7 +180,7 @@ export default function EditableAccessRequestForm({
           onEdit={handleEdit}
           onCancel={handleCancel}
           onSubmit={handleSubmit}
-          onDelete={() => setOpen(true)}
+          onDelete={handleDelete}
           errorMessage={errorMessage}
           showDeleteButton
         />