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 1/3] 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);

From be9aea086ee3b7cc6df7d6c8c84907c11b88c455 Mon Sep 17 00:00:00 2001
From: goemen <gnthomiwa@gmail.com>
Date: Mon, 23 Sep 2024 15:27:16 -0700
Subject: [PATCH 2/3] improve code cov

---
 .../__tests__/EditAnnouncementPage.spec.ts    | 53 +++++++++++++++++++
 .../announcements/AnnouncementForm.vue        |  5 +-
 backend/src/v1/routes/announcement-routes.ts  |  1 -
 3 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/admin-frontend/src/components/__tests__/EditAnnouncementPage.spec.ts b/admin-frontend/src/components/__tests__/EditAnnouncementPage.spec.ts
index e7b8b9d3c..3f08149b9 100644
--- a/admin-frontend/src/components/__tests__/EditAnnouncementPage.spec.ts
+++ b/admin-frontend/src/components/__tests__/EditAnnouncementPage.spec.ts
@@ -62,6 +62,7 @@ vi.mock('../../services/notificationService', () => ({
 let store: ReturnType<typeof useAnnouncementSelectionStore>;
 describe('EditAnnouncementPage', () => {
   beforeEach(() => {
+    vi.clearAllMocks();
     setActivePinia(pinia);
     store = useAnnouncementSelectionStore();
     store.reset();
@@ -112,6 +113,44 @@ describe('EditAnnouncementPage', () => {
       });
 
       describe('when publishing announcement', () => {
+        describe('when expires on is before active on', () => {
+          it('should show error message', async () => {
+            const publishDate = LocalDate.now().plusDays(4);
+            store.setAnnouncement({
+              title: 'title',
+              description: 'description',
+              active_on: convert(publishDate).toDate(),
+              expires_on: convert(publishDate.minusDays(1)).toDate(),
+              status: 'DRAFT',
+              announcement_resource: [],
+            } as any);
+            const { getByRole, getByText } = await wrappedRender();
+            const saveButton = getByRole('button', { name: 'Save' });
+            await fireEvent.click(saveButton);
+            // await fireEvent.click(publishButton);
+            await waitFor(() => {
+              expect(
+                getByText('Publish On date should be before expiry date.'),
+              ).toBeVisible();
+            });
+          });
+        });
+        describe('when active on date is not set', () => {
+          it('should show error message', async () => {
+            store.setAnnouncement({
+              title: 'title',
+              description: 'description',
+              status: 'PUBLISHED',
+              announcement_resource: [],
+            } as any);
+            const { getByRole, getByText } = await wrappedRender();
+            const saveButton = getByRole('button', { name: 'Save' });
+            await fireEvent.click(saveButton);
+            await waitFor(() => {
+              expect(getByText('Active On date is required.')).toBeVisible();
+            });
+          });
+        });
         it('should require Active On date to be not in the past', async () => {
           store.setAnnouncement({
             title: 'title',
@@ -242,6 +281,7 @@ describe('EditAnnouncementPage', () => {
             title: 'title',
             description: 'description',
             active_on: new Date(),
+            expires_on: convert(LocalDate.now().plusDays(1)).toDate(),
             status: 'PUBLISHED',
             announcement_resource: [
               {
@@ -259,6 +299,19 @@ describe('EditAnnouncementPage', () => {
           await waitFor(() => {
             expect(fileNameInput).toHaveValue('');
           });
+          const saveButton = getByRole('button', { name: 'Save' });
+          await fireEvent.click(saveButton);
+          await waitFor(async () => {
+            const confirmButton = getByRole('button', { name: 'Confirm' });
+            await fireEvent.click(confirmButton);
+          });
+          await waitFor(() => {
+            const args = mockUpdateAnnouncement.mock.calls[0];
+            const data = args[1];
+            expect(data.fileDisplayName).toBeUndefined();
+            expect(data.attachment).toBeUndefined();
+            expect(data.attachmentId).toBeUndefined();
+          });
         });
       });
 
diff --git a/admin-frontend/src/components/announcements/AnnouncementForm.vue b/admin-frontend/src/components/announcements/AnnouncementForm.vue
index 1fbcd4dce..e0c95718e 100644
--- a/admin-frontend/src/components/announcements/AnnouncementForm.vue
+++ b/admin-frontend/src/components/announcements/AnnouncementForm.vue
@@ -842,7 +842,10 @@ 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,
+    attachmentId:
+      !values.attachment && !values.fileDisplayName
+        ? undefined
+        : values.attachmentId,
   });
 });
 </script>
diff --git a/backend/src/v1/routes/announcement-routes.ts b/backend/src/v1/routes/announcement-routes.ts
index 51bc31069..7b692597a 100644
--- a/backend/src/v1/routes/announcement-routes.ts
+++ b/backend/src/v1/routes/announcement-routes.ts
@@ -23,7 +23,6 @@ import {
   PatchAnnouncementsSchema,
   PatchAnnouncementsType,
 } from '../types/announcements';
-import { omit } from 'lodash';
 import { DateTimeFormatter, ZonedDateTime } from '@js-joda/core';
 
 const router = Router();

From 5cc97be5923a0d0179fd5a515f48db826575c66d Mon Sep 17 00:00:00 2001
From: goemen <gnthomiwa@gmail.com>
Date: Mon, 23 Sep 2024 15:33:27 -0700
Subject: [PATCH 3/3] improve code cov

---
 admin-frontend/src/services/__tests__/apiService.spec.ts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/admin-frontend/src/services/__tests__/apiService.spec.ts b/admin-frontend/src/services/__tests__/apiService.spec.ts
index 1afa611b9..4539bd337 100644
--- a/admin-frontend/src/services/__tests__/apiService.spec.ts
+++ b/admin-frontend/src/services/__tests__/apiService.spec.ts
@@ -462,6 +462,8 @@ describe('ApiService', () => {
           linkUrl: 'https://example.com',
           linkDisplayName: 'example',
           attachment: 'test',
+          fileDisplayName: 'test',
+          attachmentId: '1',
         };
         const mockResponse = {
           data: {},