Refactor banner and export/download (#2115)

Backend/Controllers/LiftController.cs

@@ -247,12 +247,20 @@ private async Task<IActionResult> ExportLiftFile(string projectId, string userId
         internal async Task<bool> CreateLiftExportThenSignal(string projectId, string userId)
         {
             // Export the data to a zip, read into memory, and delete zip.
-            var exportedFilepath = await CreateLiftExport(projectId);
-
-            // Store the temporary path to the exported file for user to download later.
-            _liftService.StoreExport(userId, exportedFilepath);
-            await _notifyService.Clients.All.SendAsync(CombineHub.DownloadReady, userId);
-            return true;
+            try
+            {
+                var exportedFilepath = await CreateLiftExport(projectId);
+                // Store the temporary path to the exported file for user to download later.
+                _liftService.StoreExport(userId, exportedFilepath);
+                await _notifyService.Clients.All.SendAsync(CombineHub.DownloadReady, userId);
+                return true;
+            }
+            catch
+            {
+                await _notifyService.Clients.All.SendAsync(CombineHub.ExportFailed, userId);
+                _logger.LogError("Error exporting project {ProjectId}", projectId);
+                throw;
+            }
         }
 
         internal async Task<string> CreateLiftExport(string projectId)

Backend/Helper/CombineHub.cs

@@ -5,5 +5,6 @@ namespace BackendFramework.Helper
     public class CombineHub : Hub
     {
         public const string DownloadReady = "DownloadReady";
+        public const string ExportFailed = "ExportFailed";
     }
 }

src/components/AnnouncementBanner/AnnouncementBanner.tsx

@@ -23,11 +23,9 @@ export default function AnnouncementBanner() {
   // Check for announcement banner on (re)load or navigation to a new page.
   useEffect(() => {
     getBannerText(BannerType.Announcement).then((text) => {
-      if (text !== banner && (!text || text !== getClosedBanner())) {
-        setBanner(text);
-      }
-    }); // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [loc]);
+      setBanner(text !== getClosedBanner() ? text : "");
+    });
+  }, [loc, setBanner]);
 
   function closeBanner() {
     setClosedBanner(banner);

src/components/App/SignalRHub.tsx

@@ -3,7 +3,10 @@ import React, { useEffect, useState } from "react";
 
 import { baseURL } from "backend";
 import { getUserId } from "backend/localStorage";
-import { downloadIsReady } from "components/ProjectExport/Redux/ExportProjectActions";
+import {
+  downloadIsReady,
+  failure,
+} from "components/ProjectExport/Redux/ExportProjectActions";
 import { ExportStatus } from "components/ProjectExport/Redux/ExportProjectReduxTypes";
 import { StoreState } from "types";
 import { useAppDispatch, useAppSelector } from "types/hooks";
@@ -34,22 +37,34 @@ export default function SignalRHub() {
 
   useEffect(() => {
     if (connection) {
-      // The methodName must match what is in Backend/Helper/CombineHub.cs.
-      const methodName = "DownloadReady";
+      // Name must match what is in Backend/Helper/CombineHub.cs.
+      const failName = "ExportFailed";
+      const successName = "DownloadReady";
+
       // The method is what the frontend does upon message receipt.
-      const method = (userId: string) => {
+      const failMethod = (userId: string) => {
         if (userId === getUserId()) {
-          downloadIsReady(exportState.projectId)(dispatch);
+          dispatch(failure(exportState.projectId));
           // After dispatch, stop the connection completely.
           // We don't need it active unless a new export is started,
           // and that might be with a different projectId.
           connection.stop();
         }
       };
-      connection
-        .start()
-        .then(() => connection.on(methodName, method))
-        .catch(console.error);
+      const successMethod = (userId: string) => {
+        if (userId === getUserId()) {
+          dispatch(downloadIsReady(exportState.projectId));
+          // After dispatch, stop the connection completely.
+          // We don't need it active unless a new export is started,
+          // and that might be with a different projectId.
+          connection.stop();
+        }
+      };
+
+      connection.start().then(() => {
+        connection.on(failName, failMethod);
+        connection.on(successName, successMethod);
+      });
     }
     // We reference dispatch and exportState, but they're not dependencies.
     // eslint-disable-next-line react-hooks/exhaustive-deps

src/components/ProjectExport/DownloadButton.tsx

@@ -1,6 +1,12 @@
 import { Cached, Error as ErrorIcon } from "@mui/icons-material";
 import { IconButton, Tooltip } from "@mui/material";
-import React, { createRef, ReactElement, useEffect, useState } from "react";
+import React, {
+  createRef,
+  ReactElement,
+  useCallback,
+  useEffect,
+  useState,
+} from "react";
 import { useTranslation } from "react-i18next";
 
 import { getProjectName } from "backend";
@@ -14,6 +20,10 @@ import { useAppDispatch, useAppSelector } from "types/hooks";
 import { themeColors } from "types/theme";
 import { getNowDateTimeString } from "utilities";
 
+function makeExportName(projectName: string) {
+  return `${projectName}_${getNowDateTimeString()}.zip`;
+}
+
 interface DownloadButtonProps {
   colorSecondary?: boolean;
 }
@@ -32,41 +42,37 @@ export default function DownloadButton(props: DownloadButtonProps) {
   const { t } = useTranslation();
   const downloadLink = createRef<HTMLAnchorElement>();
 
+  const reset = useCallback(() => {
+    dispatch(resetExport(exportState.projectId));
+  }, [dispatch, exportState.projectId]);
+
   useEffect(() => {
     if (downloadLink.current && fileUrl) {
       downloadLink.current.click();
       URL.revokeObjectURL(fileUrl);
+      setFileName(undefined);
       setFileUrl(undefined);
     }
   }, [downloadLink, fileUrl]);
 
   useEffect(() => {
-    if (exportState.status === ExportStatus.Success) {
-      download();
-    } // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [exportState.status]);
-
-  function makeExportName(projectName: string) {
-    return `${projectName}_${getNowDateTimeString()}.zip`;
-  }
-
-  function download() {
-    getProjectName(exportState.projectId).then((projectName) => {
-      setFileName(makeExportName(projectName));
-      asyncDownloadExport(exportState.projectId)(dispatch)
-        .then((url) => {
-          if (url) {
-            setFileUrl(url);
-            reset();
-          }
-        })
-        .catch(console.error);
-    });
-  }
+    if (fileName) {
+      dispatch(asyncDownloadExport(exportState.projectId)).then((url) => {
+        if (url) {
+          setFileUrl(url);
+          reset();
+        }
+      });
+    }
+  }, [dispatch, exportState.projectId, fileName, reset, setFileUrl]);
 
-  function reset() {
-    resetExport(exportState.projectId)(dispatch);
-  }
+  useEffect(() => {
+    if (exportState.status === ExportStatus.Success) {
+      getProjectName(exportState.projectId).then((projectName) => {
+        setFileName(makeExportName(projectName));
+      });
+    }
+  }, [exportState, setFileName]);
 
   function textId(): string {
     switch (exportState.status) {

src/components/ProjectExport/Redux/ExportProjectActions.ts

@@ -50,7 +50,7 @@ function success(projectId: string): ExportProjectAction {
     projectId,
   };
 }
-function failure(projectId: string): ExportProjectAction {
+export function failure(projectId: string): ExportProjectAction {
   return {
     type: ExportStatus.Failure,
     projectId,