Merge branch 'bacpop-163-error-handling' of https://github.com/bacpop…

…/beebop into bacpop-115-about-page

app/client-v2/src/__tests__/components/ProjectView/CytoscapeCanvas.spec.ts

@@ -12,7 +12,7 @@ vitest.mock("cytoscape", () => ({
 vitest.mock("cytoscape-graphml");
 describe("CytoscapeCanvas", () => {
   it("should render div & buttons only for canvas with correct classes for display", async () => {
-    const { container, emitted } = render(CytoscapeCanvas, {
+    const { container } = render(CytoscapeCanvas, {
       props: {
         graph: MOCK_NETWORK_GRAPH,
         cluster: "test-cluster"
@@ -25,7 +25,6 @@ describe("CytoscapeCanvas", () => {
     });
 
     await userEvent.click(screen.getByRole("button", { name: /fullscreen/i }));
-    console.log(emitted());
 
     expect(screen.getByRole("button", { name: /reset/i })).toBeVisible();
     expect(screen.getByRole("button", { name: /fullscreen/i })).toBeVisible();

app/client-v2/src/__tests__/composables/useToastService.spec.ts

@@ -0,0 +1,48 @@
+import { useToastService } from "@/composables/useToastService";
+
+const mockToastAdd = vitest.fn();
+vitest.mock("primevue/usetoast", () => ({
+  useToast: vitest.fn(() => ({
+    add: mockToastAdd
+  }))
+}));
+describe("useToastService", () => {
+  it("should call add with correct params when showSuccessToast is called", () => {
+    const { showSuccessToast } = useToastService();
+
+    showSuccessToast("Success message");
+
+    expect(mockToastAdd).toHaveBeenCalledWith({
+      severity: "success",
+      summary: "Success",
+      detail: "Success message",
+      life: 5000
+    });
+  });
+
+  it("should call add with correct params when showErrorToast is called", () => {
+    const { showErrorToast } = useToastService();
+
+    showErrorToast("Error message");
+
+    expect(mockToastAdd).toHaveBeenCalledWith({
+      severity: "error",
+      summary: "Error",
+      detail: "Error message",
+      life: 5000
+    });
+  });
+
+  it("should call add with correct params when showInfoToast is called", () => {
+    const { showInfoToast } = useToastService();
+
+    showInfoToast("Info message");
+
+    expect(mockToastAdd).toHaveBeenCalledWith({
+      severity: "info",
+      summary: "Info",
+      detail: "Info message",
+      life: 5000
+    });
+  });
+});

app/client-v2/src/__tests__/stores/projectStore.spec.ts

@@ -194,11 +194,11 @@ describe("projectStore", () => {
 
       expect(store.project.samples[0].sketch).toEqual({ filename: "sample1.fasta", md5: "sample1-md5" });
     });
-    it("should remove sample from fileSamples when handleWorkerResponse fails & call showErrorToast", async () => {
+    it("should remove sample from fileSamples when handleWorkerResponse fails & call toast.showErrorToast", async () => {
       const store = useProjectStore();
       store.project.id = "1";
       store.project.samples = [...mockFilesWithHashes];
-      store.showErrorToast = vitest.fn();
+      store.toast.showErrorToast = vitest.fn();
       const eventData = {
         hash: mockFilesWithHashes[0].hash,
         result: JSON.stringify({ Penicillin: 0.24, Chloramphenicol: 0.24 }),
@@ -213,7 +213,9 @@ describe("projectStore", () => {
       await store.handleWorkerResponse("sample1.fasta", { data: eventData } as any);
 
       expect(store.project.samples).toEqual(mockFilesWithHashes.slice(1));
-      expect(store.showErrorToast).toHaveBeenCalledWith("Ensure uploaded sample file is correct or try again later.");
+      expect(store.toast.showErrorToast).toHaveBeenCalledWith(
+        "Ensure uploaded sample file is correct or try again later."
+      );
     });
 
     it("should set pollingIntervalId when pollAnalysisStatus is called & not already set", async () => {
@@ -282,15 +284,15 @@ describe("projectStore", () => {
     it("should stop polling & call showErrorToast if status request fails", async () => {
       const store = useProjectStore();
       store.stopPollingStatus = vitest.fn();
-      store.showErrorToast = vitest.fn();
+      store.toast.showErrorToast = vitest.fn();
       server.use(http.post(statusUri, () => HttpResponse.error()));
 
       await store.getAnalysisStatus();
 
       expect(store.pollingIntervalId).toBeNull();
       expect(store.stopPollingStatus).toHaveBeenCalled();
-      expect(store.showErrorToast).toHaveBeenCalledWith(
-        "Error fetching analysis status. Try again later, or create a new project."
+      expect(store.toast.showErrorToast).toHaveBeenCalledWith(
+        "Error fetching analysis status. Try refresh page, or create a new project."
       );
     });
     it("should not stop polling if status request returns incomplete status", async () => {
@@ -407,16 +409,16 @@ describe("projectStore", () => {
       expect(URL.createObjectURL).not.toHaveBeenCalled();
     });
 
-    it("should call toast with msg passed in when showErrorToast called", () => {
+    it("should call toast with msg passed in when toast.showErrorToast called", () => {
       const store = useProjectStore();
 
-      store.showErrorToast("test-error");
+      store.toast.showErrorToast("test-error");
 
       expect(mockToastAdd).toHaveBeenCalledWith({
         severity: "error",
-        summary: "Error Occurred",
+        summary: "Error",
         detail: "test-error",
-        life: 3000
+        life: 5000
       });
     });
 
@@ -428,19 +430,23 @@ describe("projectStore", () => {
 
       expect(store.project.samples).toEqual(["sample1", "sample2"]);
     });
-    it("should call api with correct params and showErrorToast when removeUploadedFile fails", async () => {
+    it("should call api with correct params and toast.showErrorToast when removeUploadedFile fails", async () => {
       const store = useProjectStore();
-      store.showErrorToast = vitest.fn();
+      store.toast.showErrorToast = vitest.fn();
       store.project.samples = ["sample1", "sample2", "sample3"] as any;
       server.use(
-        http.post(`/project/${store.project.id}/sample/${store.project.samples[2].hash}/delete`, () =>
-          HttpResponse.error()
-        )
+        http.patch(`/project/${store.project.id}/sample/${store.project.samples[2].hash}`, async ({ request }) => {
+          const body = await request.json();
+
+          expect(body).toEqual({ filename: store.project.samples[2].filename });
+
+          return HttpResponse.error();
+        })
       );
 
       await store.removeUploadedFile(2);
 
-      expect(store.showErrorToast).toHaveBeenCalledWith("Error removing file. Try again later.");
+      expect(store.toast.showErrorToast).toHaveBeenCalledWith("Error removing file. Try again later.");
       expect(store.project.samples).toEqual(["sample1", "sample2", "sample3"]);
     });
   });

app/client-v2/src/composables/useToastService.ts

@@ -0,0 +1,17 @@
+import { useToast } from "primevue/usetoast";
+
+export const useToastService = () => {
+  const toast = useToast();
+
+  const showSuccessToast = (msg: string) => {
+    toast.add({ severity: "success", summary: "Success", detail: msg, life: 5000 });
+  };
+  const showErrorToast = (msg: string) => {
+    toast.add({ severity: "error", summary: "Error", detail: msg, life: 5000 });
+  };
+  const showInfoToast = (msg: string) => {
+    toast.add({ severity: "info", summary: "Info", detail: msg, life: 5000 });
+  };
+
+  return { showSuccessToast, showErrorToast, showInfoToast };
+};

app/client-v2/src/stores/projectStore.ts

@@ -1,3 +1,4 @@
+import { useToastService } from "@/composables/useToastService";
 import { getApiUrl } from "@/config";
 import {
   COMPLETE_STATUS_TYPES,
@@ -13,7 +14,6 @@ import {
 } from "@/types/projectTypes";
 import { mande } from "mande";
 import { defineStore } from "pinia";
-import { useToast } from "primevue/usetoast";
 import { Md5 } from "ts-md5";
 
 const baseApi = mande(getApiUrl(), { credentials: "include" });
@@ -22,7 +22,7 @@ export const useProjectStore = defineStore("project", {
   state: () => ({
     project: {} as Project,
     pollingIntervalId: null as ReturnType<typeof setInterval> | null,
-    toast: useToast() as ReturnType<typeof useToast>
+    toast: useToastService() as ReturnType<typeof useToastService>
   }),
 
   getters: {
@@ -63,15 +63,6 @@ export const useProjectStore = defineStore("project", {
       }
     },
 
-    showErrorToast(msg: string) {
-      this.toast.add({
-        severity: "error",
-        summary: "Error Occurred",
-        detail: msg,
-        life: 3000
-      });
-    },
-
     onFilesUpload(files: File | File[]) {
       const arrayFiles = Array.isArray(files) ? files : [files];
       const nonDuplicateFiles = arrayFiles.filter(
@@ -95,7 +86,7 @@ export const useProjectStore = defineStore("project", {
         };
         worker.onerror = (error) => {
           console.error(error);
-          this.showErrorToast("Ensure uploaded sample file is correct or try again later.");
+          this.toast.showErrorToast("Ensure uploaded sample file is correct");
         };
       }
     },
@@ -121,7 +112,7 @@ export const useProjectStore = defineStore("project", {
         );
       } catch (error) {
         console.error(error);
-        this.showErrorToast("Ensure uploaded sample file is correct or try again later.");
+        this.toast.showErrorToast("Ensure uploaded sample file is correct or try again later.");
         if (matchedHashIndex !== -1) {
           this.project.samples.splice(matchedHashIndex, 1);
         }
@@ -153,7 +144,7 @@ export const useProjectStore = defineStore("project", {
           stopPolling = true;
         }
       } catch (error) {
-        this.showErrorToast("Error fetching analysis status. Try again later, or create a new project.");
+        this.toast.showErrorToast("Error fetching analysis status. Try refresh page, or create a new project.");
         console.error(error);
         stopPolling = true;
       } finally {
@@ -190,11 +181,13 @@ export const useProjectStore = defineStore("project", {
     },
     async removeUploadedFile(index: number) {
       try {
-        await baseApi.post(`/project/${this.project.id}/sample/${this.project.samples[index].hash}/delete`);
+        await baseApi.patch(`/project/${this.project.id}/sample/${this.project.samples[index].hash}`, {
+          filename: this.project.samples[index].filename
+        });
         this.project.samples.splice(index, 1);
       } catch (error) {
         console.error(error);
-        this.showErrorToast("Error removing file. Try again later.");
+        this.toast.showErrorToast("Error removing file. Try again later.");
       }
     },
 
@@ -208,7 +201,7 @@ export const useProjectStore = defineStore("project", {
         this.pollAnalysisStatus();
       } catch (error) {
         console.error("Error running analysis", error);
-        this.showErrorToast("Error running analysis. Try again later.");
+        this.toast.showErrorToast("Error running analysis. Try again later.");
         return;
       }
     },
@@ -252,7 +245,7 @@ export const useProjectStore = defineStore("project", {
         URL.revokeObjectURL(link.href);
       } catch (error) {
         console.error(error);
-        this.showErrorToast("Error downloading zip file. Try again later.");
+        this.toast.showErrorToast("Error downloading zip file. Try again later.");
       }
     }
   }

app/client-v2/src/views/HomeView.vue

@@ -6,18 +6,13 @@ import InputGroup from "primevue/inputgroup";
 import { FilterMatchMode } from "primevue/api";
 import type { DataTableRowEditSaveEvent } from "primevue/datatable";
 import type { ProjectsResponse } from "@/types/projectTypes";
-import { useToast } from "primevue/usetoast";
 import Toast from "primevue/toast";
 import { useRouter } from "vue-router";
+import { useToastService } from "@/composables/useToastService";
 
 const router = useRouter();
-const toast = useToast();
-const showSuccessToast = (msg: string) => {
-  toast.add({ severity: "success", summary: "Success", detail: msg, life: 3000 });
-};
-const showErrorToast = (msg: string) => {
-  toast.add({ severity: "error", summary: "Error", detail: msg, life: 3000 });
-};
+const { showErrorToast, showSuccessToast } = useToastService();
+
 const apiUrl = getApiUrl();
 const {
   data: projects,

app/server/src/controllers/projectController.ts

@@ -110,8 +110,11 @@ export default (config) => {
     async deleteSample(request, response, next) {
       await asyncHandler(next, async () => {
         const { projectId, sampleHash } = request.params;
+        const { filename } = request.body as {
+          filename: string;
+        };
         const { redis } = request.app.locals;
-        await userStore(redis).deleteSample(projectId, sampleHash);
+        await userStore(redis).deleteSample(projectId, sampleHash, filename);
         sendSuccess(response, null);
       });
     },

app/server/src/db/userStore.ts

@@ -98,9 +98,8 @@ export class UserStore {
         const amrString = await this._redis.hget(this._projectSampleKey(projectId, sampleId), "amr");
         return JSON.parse(amrString);
     }
-    async deleteSample(projectId: string, sampleHash: string) {
-        const sampleIds = await this._redis.smembers(this._projectSamplesKey(projectId));
-        const sampleId = sampleIds?.find((id) => id.startsWith(sampleHash));
+    async deleteSample(projectId: string, sampleHash: string, filename: string) {
+        const sampleId = this._sampleId(sampleHash, filename);
         await this._redis.srem(this._projectSamplesKey(projectId), sampleId);
         await this._redis.del(this._projectSampleKey(projectId, sampleId));
     }

app/server/src/routes/projectRoutes.ts

@@ -24,7 +24,7 @@ export default {
         app.post('/project/:projectId/rename',
             authCheck,
             controller.renameProject);
-        app.post(`/project/:projectId/sample/:sampleHash/delete`,
+        app.patch(`/project/:projectId/sample/:sampleHash`,
             authCheck, 
             controller.deleteSample);
     }

app/server/tests/unit/controllers/projectController.test.ts

@@ -298,12 +298,15 @@ describe("projectController", () => {
             params: {
                 projectId: "testProjectId",
                 sampleHash: "1234"
+            },
+            body: {
+                filename: "test1.fa"
             }
         };
 
         await projectController(config).deleteSample(req, mockResponse(),jest.fn());
 
         expect(mockUserStore.deleteSample).toHaveBeenCalledTimes(1);
-        expect(mockUserStore.deleteSample).toHaveBeenCalledWith("testProjectId", "1234");
+        expect(mockUserStore.deleteSample).toHaveBeenCalledWith("testProjectId", "1234", "test1.fa");
     })
 });

app/server/tests/unit/db/userStore.test.ts

@@ -232,14 +232,13 @@ describe("UserStore", () => {
         const sut = new UserStore(mockRedis);
         const projectId = "testProjectId";
         const sampleHash = "testSampleHash";
+        const filename = "test.fa";
         const expectedProjectSamplesKey = (sut as any)._projectSamplesKey(projectId);
-        const expectedSampleId = `${sampleHash}:filename`;
+        const expectedSampleId = `${sampleHash}:${filename}`;
         const expectedProjectSampleKey = (sut as any)._projectSampleKey(projectId, expectedSampleId);
-        mockRedis.smembers.mockImplementation(() => [expectedSampleId, "randomSampleId"]);
 
-        await sut.deleteSample(projectId, sampleHash);
+        await sut.deleteSample(projectId, sampleHash, filename);
 
-        expect(mockRedis.smembers).toHaveBeenCalledWith(expectedProjectSamplesKey);
         expect(mockRedis.srem).toHaveBeenCalledWith(expectedProjectSamplesKey, expectedSampleId);
         expect(mockRedis.del).toHaveBeenCalledWith(expectedProjectSampleKey);
     })

scripts/common

@@ -1,3 +1,2 @@
-# TODO: change back to main before merge into main
-export API_BRANCH="bacpop-160-network-tab"
+export API_BRANCH=main
 export DB_LOCATION="./storage/GPS_v6_references"