TK-32: Preserve Documents and Knowledge Items When Reverting an ISA to Draft

api/src/controllers/information-sharing-agreements/revert-to-draft-controller.ts

@@ -45,7 +45,7 @@ export class RevertToDraftController extends BaseController<InformationSharingAg
 
   private async loadInformationSharingAgreement(): Promise<InformationSharingAgreement | null> {
     return InformationSharingAgreement.findByPk(this.params.informationSharingAgreementId, {
-      include: ["accessGrants", "informationSharingAgreementArchiveItems"],
+      include: ["accessGrants"],
     })
   }
 

api/src/policies/information-sharing-agreements/revert-to-draft-policy.ts

@@ -1,28 +1,14 @@
-import { isUndefined } from "lodash"
-
 import { InformationSharingAgreement } from "@/models"
 import { PolicyFactory } from "@/policies/base-policy"
 
 export class RevertToDraftPolicy extends PolicyFactory(InformationSharingAgreement) {
   create(): boolean {
     if (!this.record.isSigned()) return false
-    if (this.hasArchiveItems()) return false
 
     if (this.user.id === this.record.creatorId) return true
     if (this.user.isSystemAdmin) return true
     if (this.record.hasAccessGrantFor(this.user.id)) return true
 
     return false
   }
-
-  private hasArchiveItems(): boolean {
-    const { informationSharingAgreementArchiveItems } = this.record
-    if (isUndefined(informationSharingAgreementArchiveItems)) {
-      throw new Error(
-        "Expected informationSharingAgreementArchiveItems association to be pre-loaded."
-      )
-    }
-
-    return informationSharingAgreementArchiveItems.length > 0
-  }
 }

api/src/services/information-sharing-agreements/revert-to-draft-service.ts

@@ -1,14 +1,6 @@
-import { isUndefined } from "lodash"
-
-import db, {
-  Attachment,
-  InformationSharingAgreement,
-  User,
-  InformationSharingAgreementArchiveItem,
-} from "@/models"
-import { AttachmentTargetTypes } from "@/models/attachment"
+import db, { InformationSharingAgreement, User } from "@/models"
 import BaseService from "@/services/base-service"
-import { Attachments, InformationSharingAgreements } from "@/services"
+import { InformationSharingAgreements } from "@/services"
 
 export class RevertToDraftService extends BaseService {
   constructor(
@@ -23,30 +15,11 @@ export class RevertToDraftService extends BaseService {
       throw new Error("Only signed agreements can be reverted to draft.")
     }
 
-    const { informationSharingAgreementArchiveItems } = this.informationSharingAgreement
-    if (isUndefined(informationSharingAgreementArchiveItems)) {
-      throw new Error(
-        "Expected informationSharingAgreementArchiveItems association to be pre-loaded."
-      )
-    }
-    this.assertNoArchiveItemsLinked(informationSharingAgreementArchiveItems)
-
     return db.transaction(async () => {
+      // The groups (and their access grants) are removed on revert, but signed documents and
+      // any linked knowledge items are intentionally kept so that amending a signed agreement
+      // does not lose them. See TK-32.
       await this.destroyGroups(this.informationSharingAgreement, this.currentUser)
-      // Destroyed one at a time through the service so each removal notifies the
-      // designated contacts, as a bulk destroy would not. See TK-6.
-      await Attachment.findEach(
-        {
-          where: {
-            targetId: this.informationSharingAgreement.id,
-            targetType: AttachmentTargetTypes.InformationSharingAgreement,
-            associationName: "signedConfidentialityAcknowledgement",
-          },
-        },
-        async (attachment) => {
-          await Attachments.DestroyService.perform(attachment, this.currentUser)
-        }
-      )
       await this.informationSharingAgreement.update({
         status: InformationSharingAgreement.Status.DRAFT,
         signedById: null,
@@ -62,14 +35,6 @@ export class RevertToDraftService extends BaseService {
     })
   }
 
-  private assertNoArchiveItemsLinked(
-    informationSharingAgreementArchiveItems: InformationSharingAgreementArchiveItem[]
-  ): void {
-    if (informationSharingAgreementArchiveItems.length > 0) {
-      throw new Error("Cannot revert to draft because archive items are linked to this agreement.")
-    }
-  }
-
   private async destroyGroups(
     informationSharingAgreement: InformationSharingAgreement,
     currentUser: User

api/tests/policies/information-sharing-agreements/revert-to-draft-policy.test.ts

@@ -0,0 +1,60 @@
+import { InformationSharingAgreement } from "@/models"
+
+import {
+  archiveItemFactory,
+  informationSharingAgreementArchiveItemFactory,
+  informationSharingAgreementFactory,
+  userFactory,
+} from "@/tests/factories"
+
+import { RevertToDraftPolicy } from "@/policies/information-sharing-agreements"
+
+describe("api/src/policies/information-sharing-agreements/revert-to-draft-policy.ts", () => {
+  describe("RevertToDraftPolicy", () => {
+    describe("#create", () => {
+      test("allows the creator to revert a signed agreement even when knowledge items are linked", async () => {
+        const creator = await userFactory.create()
+        const informationSharingAgreement = await informationSharingAgreementFactory.create({
+          status: InformationSharingAgreement.Status.SIGNED,
+          creatorId: creator.id,
+        })
+        const archiveItem = await archiveItemFactory.create({ userId: creator.id })
+        await informationSharingAgreementArchiveItemFactory.create({
+          informationSharingAgreementId: informationSharingAgreement.id,
+          archiveItemId: archiveItem.id,
+          creatorId: creator.id,
+        })
+
+        const policy = new RevertToDraftPolicy(creator, informationSharingAgreement)
+
+        expect(policy.create()).toBe(true)
+      })
+
+      test("does not allow reverting an agreement that is not signed", async () => {
+        const creator = await userFactory.create()
+        const informationSharingAgreement = await informationSharingAgreementFactory.create({
+          status: InformationSharingAgreement.Status.DRAFT,
+          creatorId: creator.id,
+        })
+
+        const policy = new RevertToDraftPolicy(creator, informationSharingAgreement)
+
+        expect(policy.create()).toBe(false)
+      })
+
+      test("does not allow an unrelated user to revert", async () => {
+        const creator = await userFactory.create()
+        const unrelatedUser = await userFactory.create()
+        const informationSharingAgreement = await informationSharingAgreementFactory.create({
+          status: InformationSharingAgreement.Status.SIGNED,
+          creatorId: creator.id,
+        })
+        await informationSharingAgreement.reload({ include: ["accessGrants"] })
+
+        const policy = new RevertToDraftPolicy(unrelatedUser, informationSharingAgreement)
+
+        expect(policy.create()).toBe(false)
+      })
+    })
+  })
+})

api/tests/services/information-sharing-agreements/revert-to-draft-service.test.ts

@@ -0,0 +1,113 @@
+import {
+  Attachment,
+  Group,
+  InformationSharingAgreement,
+  InformationSharingAgreementArchiveItem,
+} from "@/models"
+import { AttachmentTargetTypes } from "@/models/attachment"
+
+import {
+  archiveItemFactory,
+  attachmentFactory,
+  groupFactory,
+  informationSharingAgreementArchiveItemFactory,
+  informationSharingAgreementFactory,
+  userFactory,
+} from "@/tests/factories"
+
+import RevertToDraftService from "@/services/information-sharing-agreements/revert-to-draft-service"
+
+// Group removal fans out notifications that are irrelevant to reverting; silence them.
+vi.mock("@/mailers/groups/notify-user-of-removal-mailer", () => {
+  const NotifyUserOfRemovalMailerMock = { perform: vi.fn() }
+  return { NotifyUserOfRemovalMailer: NotifyUserOfRemovalMailerMock, default: NotifyUserOfRemovalMailerMock }
+})
+vi.mock("@/mailers/groups/notify-admins-of-removed-user-mailer", () => {
+  const NotifyAdminsOfRemovedUserMailerMock = { perform: vi.fn() }
+  return { NotifyAdminsOfRemovedUserMailer: NotifyAdminsOfRemovedUserMailerMock, default: NotifyAdminsOfRemovedUserMailerMock }
+})
+vi.mock("@/services/notifications/groups/notify-user-of-removal-service", () => {
+  const NotifyUserOfRemovalServiceMock = { perform: vi.fn() }
+  return { NotifyUserOfRemovalService: NotifyUserOfRemovalServiceMock, default: NotifyUserOfRemovalServiceMock }
+})
+vi.mock("@/services/notifications/groups/notify-admins-of-removed-user-service", () => {
+  const NotifyAdminsOfRemovedUserServiceMock = { perform: vi.fn() }
+  return { NotifyAdminsOfRemovedUserService: NotifyAdminsOfRemovedUserServiceMock, default: NotifyAdminsOfRemovedUserServiceMock }
+})
+
+describe("api/src/services/information-sharing-agreements/revert-to-draft-service.ts", () => {
+  describe("RevertToDraftService", () => {
+    describe("#perform", () => {
+      async function buildSignedAgreement() {
+        const currentUser = await userFactory.create()
+        const internalGroup = await groupFactory.create({ isExternal: false })
+        const externalGroup = await groupFactory.create({ isExternal: true })
+        const informationSharingAgreement = await informationSharingAgreementFactory.create({
+          status: InformationSharingAgreement.Status.SIGNED,
+          internalGroupId: internalGroup.id,
+          externalGroupId: externalGroup.id,
+          signedById: currentUser.id,
+          signedAt: new Date(),
+        })
+        return { currentUser, informationSharingAgreement }
+      }
+
+      test("reverts to draft and clears the signing metadata", async () => {
+        const { currentUser, informationSharingAgreement } = await buildSignedAgreement()
+
+        const reverted = await RevertToDraftService.perform(informationSharingAgreement, currentUser)
+
+        expect(reverted.status).toBe(InformationSharingAgreement.Status.DRAFT)
+        expect(reverted.signedById).toBeNull()
+        expect(reverted.signedAt).toBeNull()
+      })
+
+      test("removes the agreement's groups", async () => {
+        const { currentUser, informationSharingAgreement } = await buildSignedAgreement()
+
+        await RevertToDraftService.perform(informationSharingAgreement, currentUser)
+
+        expect(await Group.count()).toBe(0)
+      })
+
+      test("keeps the signed documents attached", async () => {
+        const { currentUser, informationSharingAgreement } = await buildSignedAgreement()
+        const attachment = await attachmentFactory.create({
+          targetId: informationSharingAgreement.id,
+          targetType: AttachmentTargetTypes.InformationSharingAgreement,
+          associationName: "signedConfidentialityAcknowledgement",
+        })
+
+        await RevertToDraftService.perform(informationSharingAgreement, currentUser)
+
+        expect(await Attachment.findByPk(attachment.id)).not.toBeNull()
+      })
+
+      test("reverts even when knowledge items are linked, keeping the links", async () => {
+        const { currentUser, informationSharingAgreement } = await buildSignedAgreement()
+        const archiveItem = await archiveItemFactory.create({ userId: currentUser.id })
+        const link = await informationSharingAgreementArchiveItemFactory.create({
+          informationSharingAgreementId: informationSharingAgreement.id,
+          archiveItemId: archiveItem.id,
+          creatorId: currentUser.id,
+        })
+
+        const reverted = await RevertToDraftService.perform(informationSharingAgreement, currentUser)
+
+        expect(reverted.status).toBe(InformationSharingAgreement.Status.DRAFT)
+        expect(await InformationSharingAgreementArchiveItem.findByPk(link.id)).not.toBeNull()
+      })
+
+      test("throws when the agreement is not signed", async () => {
+        const currentUser = await userFactory.create()
+        const informationSharingAgreement = await informationSharingAgreementFactory.create({
+          status: InformationSharingAgreement.Status.DRAFT,
+        })
+
+        await expect(
+          RevertToDraftService.perform(informationSharingAgreement, currentUser)
+        ).rejects.toThrow("Only signed agreements can be reverted to draft.")
+      })
+    })
+  })
+})

web/src/components/archive-items/ArchiveItemAttachmentsCard.vue

@@ -55,6 +55,7 @@ import { ref, toRefs } from "vue"
 
 import archiveItemsApi from "@/api/archive-items-api"
 import useArchiveItem from "@/use/use-archive-item"
+import useSnack from "@/use/use-snack"
 
 import ArchiveItemFileCard from "@/components/archive-item-files/ArchiveItemFileCard.vue"
 
@@ -68,6 +69,7 @@ const emit = defineEmits<{
 
 const { archiveItemId } = toRefs(props)
 const { archiveItem, policy, refresh } = useArchiveItem(archiveItemId)
+const snack = useSnack()
 
 const filesToUpload = ref<File[]>([])
 const isUploading = ref(false)
@@ -81,6 +83,9 @@ async function uploadFiles(files: File | File[]) {
     await archiveItemsApi.createFiles(archiveItemId.value, filesAsArray)
     await refresh()
     filesToUpload.value = []
+  } catch (error) {
+    console.error("Failed to upload attachment:", error)
+    snack.error("Failed to upload attachment")
   } finally {
     isUploading.value = false
   }

web/src/components/common/AttachmentAttributesRow.vue

@@ -27,7 +27,7 @@
 import { computed } from "vue"
 
 import { type AttachmentAsReference } from "@/api/attachments-api"
-import { formatBytes, formatDate } from "@/utils/formatters"
+import { formatBytes, formatDateTime } from "@/utils/formatters"
 
 const props = defineProps<{
   attachment: AttachmentAsReference
@@ -65,6 +65,6 @@ const icon = computed(() => {
   return "mdi-file"
 })
 
-const formattedDate = computed(() => formatDate(props.attachment.createdAt))
+const formattedDate = computed(() => formatDateTime(props.attachment.createdAt))
 const formattedSize = computed(() => formatBytes(props.attachment.size))
 </script>

web/src/components/information-sharing-agreements/InformationSharingAgreementRevertToDraftDialog.vue

@@ -10,8 +10,8 @@
 
       <v-card-text>
         <p>
-          Are you sure you want to revert this agreement to draft? Any signed documents will be
-          removed.
+          Are you sure you want to revert this agreement to draft? Its groups and access will be
+          removed, but any signed documents and linked knowledge items will be kept.
         </p>
       </v-card-text>
 

web/src/components/information-sharing-agreements/signed/InformationSharingAgreementSignedActionsMenu.vue

@@ -49,7 +49,7 @@
       />
     </v-list-item>
     <v-list-item
-      v-if="!isLoading && !hasKnowledgeItems"
+      v-if="!isLoading"
       class="cursor-pointer"
     >
       <v-list-item-title>Revert to Draft</v-list-item-title>