From bf61ef435591c5497af223930b47a20cb99aefcb Mon Sep 17 00:00:00 2001
From: goemen <gnthomiwa@gmail.com>
Date: Mon, 23 Sep 2024 14:41:35 -0700
Subject: [PATCH] Display error message when download fails Fix: disappearing
 file on edit save

---
 .../announcements/AnnouncementForm.vue        |  2 +
 .../announcements/AnnouncementItem.vue        |  7 +--
 .../announcements/AttachmentResource.vue      | 13 ++++-
 .../__tests__/AttachmentResource.spec.ts      | 50 +++++++++++++++++++
 admin-frontend/src/constants/index.ts         |  3 ++
 admin-frontend/src/services/apiService.ts     | 10 +++-
 backend/src/v1/routes/announcement-routes.ts  |  3 +-
 7 files changed, 78 insertions(+), 10 deletions(-)
 create mode 100644 admin-frontend/src/components/announcements/__tests__/AttachmentResource.spec.ts

diff --git a/admin-frontend/src/components/announcements/AnnouncementForm.vue b/admin-frontend/src/components/announcements/AnnouncementForm.vue
index 979cbddb1..1fbcd4dce 100644
--- a/admin-frontend/src/components/announcements/AnnouncementForm.vue
+++ b/admin-frontend/src/components/announcements/AnnouncementForm.vue
@@ -573,6 +573,7 @@ const handleDeleteLink = () => {
 };
 
 const handleDeleteFile = () => {
+  attachment.value = undefined;
   fileDisplayName.value = undefined;
   fileDisplayOnly.value = false;
 };
@@ -841,6 +842,7 @@ const handleSave = handleSubmit(async (values) => {
     linkUrl: isEmpty(values.linkUrl) ? undefined : values.linkUrl,
     status: status.value,
     attachment: attachment.value,
+    attachmentId: !values.attachment && !values.fileDisplayName ? undefined : values.attachmentId,
   });
 });
 </script>
diff --git a/admin-frontend/src/components/announcements/AnnouncementItem.vue b/admin-frontend/src/components/announcements/AnnouncementItem.vue
index e2c51817a..10e62f327 100644
--- a/admin-frontend/src/components/announcements/AnnouncementItem.vue
+++ b/admin-frontend/src/components/announcements/AnnouncementItem.vue
@@ -52,6 +52,7 @@ import { sanitizeUrl } from '@braintree/sanitize-url';
 import ApiService from '../../services/apiService';
 import { saveAs } from 'file-saver';
 import { NotificationService } from '../../services/notificationService';
+import { FILE_DOWNLOAD_ERROR } from '../../constants';
 
 defineProps<{
   announcement: Announcement;
@@ -67,11 +68,7 @@ async function downloadAnnouncementResource(
       );
     } catch (error) {
       console.error(error);
-      NotificationService.pushNotificationError(
-        'There is a problem with this link/file, please try again later or contact the helpdesk.',
-        '',
-        30000,
-      );
+      NotificationService.pushNotificationError(FILE_DOWNLOAD_ERROR, '', 30000);
     }
   } else if (announcementResource.announcement_resource_file) {
     //When a resource with type ATTACHMENT hasn't yet been uploaded to the
diff --git a/admin-frontend/src/components/announcements/AttachmentResource.vue b/admin-frontend/src/components/announcements/AttachmentResource.vue
index ae7cbf682..f56821fa7 100644
--- a/admin-frontend/src/components/announcements/AttachmentResource.vue
+++ b/admin-frontend/src/components/announcements/AttachmentResource.vue
@@ -1,6 +1,6 @@
 <template>
   <div class="root">
-    <a @click="ApiService.downloadFile(id)">
+    <a :aria-label="name" @click="downloadFile(id)">
       {{ name }}
     </a>
     <v-btn
@@ -21,6 +21,8 @@
 <script setup lang="ts">
 import { defineProps } from 'vue';
 import ApiService from '../../services/apiService';
+import { NotificationService } from '../../services/notificationService';
+import { FILE_DOWNLOAD_ERROR } from '../../constants';
 
 interface AttachmentResourceProps {
   id: string;
@@ -29,6 +31,15 @@ interface AttachmentResourceProps {
 const { id, name } = defineProps<AttachmentResourceProps>();
 
 const emits = defineEmits(['onEdit', 'onDelete']);
+
+const downloadFile = async (id: string) => {
+  try {
+    await ApiService.downloadFile(id);
+  } catch (error) {
+    console.error(error);
+    NotificationService.pushNotificationError(FILE_DOWNLOAD_ERROR, '', 30000);
+  }
+};
 </script>
 <style scoped lang="scss">
 .root {
diff --git a/admin-frontend/src/components/announcements/__tests__/AttachmentResource.spec.ts b/admin-frontend/src/components/announcements/__tests__/AttachmentResource.spec.ts
new file mode 100644
index 000000000..ee201a28c
--- /dev/null
+++ b/admin-frontend/src/components/announcements/__tests__/AttachmentResource.spec.ts
@@ -0,0 +1,50 @@
+import { fireEvent, render } from '@testing-library/vue';
+import AttachmentResource from '../AttachmentResource.vue';
+import { describe, it, expect, vi } from 'vitest';
+import { FILE_DOWNLOAD_ERROR } from '../../../constants';
+
+const mockDownloadFile = vi.fn();
+
+vi.mock('../../../services/apiService', () => ({
+  default: {
+    downloadFile: (...args) => mockDownloadFile(...args),
+  },
+}));
+
+const pushNotificationErrorMock = vi.fn();
+vi.mock('../../../services/notificationService', () => ({
+  NotificationService: {
+    pushNotificationError: (...args) => pushNotificationErrorMock(...args),
+  },
+}));
+
+describe('AttachmentResource', () => {
+  it('should download file', async () => {
+    const attachment = {
+      id: 1,
+      name: 'Test file',
+    };
+    const { getByLabelText } = await render(AttachmentResource, {
+      props: { ...attachment },
+    });
+    const link = await getByLabelText('Test file');
+    expect(link).toBeInTheDocument();
+    await fireEvent.click(link);
+    await expect(mockDownloadFile).toHaveBeenCalledWith(1);
+  });
+
+  it('should display error message when download fails', async () => {
+    mockDownloadFile.mockRejectedValue(new Error('Download failed'));
+    const attachment = {
+      id: 1,
+      name: 'Test file',
+    };
+    const { getByLabelText } = await render(AttachmentResource, {
+      props: { ...attachment },
+    });
+    const link = await getByLabelText('Test file');
+    expect(link).toBeInTheDocument();
+    await fireEvent.click(link);
+    await expect(pushNotificationErrorMock).toHaveBeenCalledWith(FILE_DOWNLOAD_ERROR, '', 30000);
+  });
+});
diff --git a/admin-frontend/src/constants/index.ts b/admin-frontend/src/constants/index.ts
index 238356011..0078e5390 100644
--- a/admin-frontend/src/constants/index.ts
+++ b/admin-frontend/src/constants/index.ts
@@ -17,3 +17,6 @@ export const RoleOptions = [
   { label: RoleLabels['PTRT-ADMIN'], value: 'PTRT-ADMIN' },
   { label: RoleLabels['PTRT-USER'], value: 'PTRT-USER' },
 ];
+
+export const FILE_DOWNLOAD_ERROR =
+  'There is a problem with this link/file, please try again later or contact the helpdesk.';
diff --git a/admin-frontend/src/services/apiService.ts b/admin-frontend/src/services/apiService.ts
index 5e0e7632e..59cb1567d 100644
--- a/admin-frontend/src/services/apiService.ts
+++ b/admin-frontend/src/services/apiService.ts
@@ -534,9 +534,15 @@ const buildFormData = (data: AnnouncementFormValue) => {
   }
 
   if (data.attachment) {
-    formData.append('fileDisplayName', data.fileDisplayName!);
-    formData.append('attachmentId', data.attachmentId!);
     formData.append('file', data.attachment);
   }
+
+  if (data.fileDisplayName) {
+    formData.append('fileDisplayName', data.fileDisplayName);
+  }
+
+  if (data.attachmentId) {
+    formData.append('attachmentId', data.attachmentId);
+  }
   return formData;
 };
diff --git a/backend/src/v1/routes/announcement-routes.ts b/backend/src/v1/routes/announcement-routes.ts
index 0011b2073..51bc31069 100644
--- a/backend/src/v1/routes/announcement-routes.ts
+++ b/backend/src/v1/routes/announcement-routes.ts
@@ -159,8 +159,7 @@ router.put(
       // Create announcement
       const announcement = await updateAnnouncement(
         req.params.id,
-        /* istanbul ignore next */
-        file ? data : omit(data, 'attachmentId'),
+        data,
         user.admin_user_id,
       );
       return res.json(announcement);