190: Fix loginToSave, and ensure cache is preserved after login

src/components/SaveButton/SaveButton.jsx

@@ -31,8 +31,18 @@ const SaveButton = ({ className, type, fill = false }) => {
       window.plausible("Save button");
     }
     document.dispatchEvent(logInEvent);
+
+    // The project is not persisted to localStorage correctly after logging out, so this
+    // workaround ensures it will be whenever the user clicks login to save
+    if (project && Object.keys(project).length > 0) {
+      localStorage.setItem(
+        project.identifier || "project",
+        JSON.stringify(project),
+      );
+    }
+
     dispatch(triggerSave());
-  }, [dispatch]);
+  }, [dispatch, project]);
 
   const projectOwner = isOwner(user, project);
 

src/components/SaveButton/SaveButton.test.js

@@ -110,14 +110,17 @@ describe("When project is loaded", () => {
 
   describe("Without a logged in user", () => {
     let store;
+    let project;
 
     beforeEach(() => {
       const middlewares = [];
       const mockStore = configureStore(middlewares);
+      project = { identifier: "testProject", data: "testData" };
       const initialState = {
         editor: {
           loading: "success",
           webComponent: false,
+          project,
         },
         auth: {},
       };
@@ -148,7 +151,16 @@ describe("When project is loaded", () => {
       fireEvent.click(saveButton);
       expect(logInHandler).toHaveBeenCalled();
     });
+
+    test("Clicking save triggers the project to be saved to localStorage", () => {
+      const saveButton = screen.queryByText("header.loginToSave").parentElement;
+      fireEvent.click(saveButton);
+      expect(localStorage.getItem("testProject")).toEqual(
+        JSON.stringify(project),
+      );
+    });
   });
+
   describe("with webComponent=false", () => {
     let store;
 

src/hooks/useProject.js

@@ -47,7 +47,13 @@ export const useProject = ({
         cachedProject.identifier === projectIdentifier;
       const is_cached_unsaved_project = !projectIdentifier && cachedProject;
 
-      if (loadCache && (is_cached_saved_project || is_cached_unsaved_project)) {
+      const isAwaitingSave =
+        cachedProject && localStorage.getItem("awaitingSave");
+
+      if (
+        (loadCache && (is_cached_saved_project || is_cached_unsaved_project)) ||
+        isAwaitingSave
+      ) {
         loadCachedProject();
         return;
       }

src/hooks/useProject.test.js

@@ -166,6 +166,22 @@ describe("When not embedded", () => {
     );
   });
 
+  test("If awaitingSave is true, loads cached project", () => {
+    localStorage.setItem("awaitingSave", "true");
+    localStorage.setItem(
+      cachedProject.identifier,
+      JSON.stringify(cachedProject),
+    );
+
+    renderHook(
+      () => useProject({ projectIdentifier: cachedProject.identifier }),
+      { wrapper },
+    );
+
+    expect(setProject).toHaveBeenCalledWith(cachedProject);
+    localStorage.removeItem("awaitingSave");
+  });
+
   test("If requested locale does not match the set language, does not set project", async () => {
     syncProject.mockImplementationOnce(jest.fn((_) => jest.fn()));
     renderHook(

src/hooks/useProjectPersistence.js

@@ -38,36 +38,27 @@ export const useProjectPersistence = ({
   useEffect(() => {
     const saveProject = async () => {
       if (Object.keys(project).length !== 0) {
-        if (saveTriggered || (user && localStorage.getItem("awaitingSave"))) {
+        const identifier = project?.identifier;
+        const accessToken = user?.access_token;
+        const params = { reactAppApiEndpoint, accessToken };
+
+        // Note the awaitingSave flag must be removed conditionally, or it happens too soon breaking the next load
+        if (saveTriggered) {
           if (isOwner(user, project)) {
-            dispatch(
-              syncProject("save")({
-                reactAppApiEndpoint,
-                project,
-                accessToken: user.access_token,
-                autosave: false,
-              }),
-            );
-          } else if (user && project.identifier) {
             await dispatch(
-              syncProject("remix")({
-                reactAppApiEndpoint,
-                project,
-                accessToken: user.access_token,
-              }),
+              syncProject("save")({ ...params, project, autosave: false }),
             );
-            // Ensure the remixed project is loaded, otherwise we'll get in a mess
+            localStorage.removeItem("awaitingSave");
+          } else if (user && identifier) {
+            await dispatch(syncProject("remix")({ ...params, project }));
             if (loadRemix) {
-              dispatch(
-                syncProject("loadRemix")({
-                  reactAppApiEndpoint,
-                  identifier: project.identifier,
-                  accessToken: user.access_token,
-                }),
+              // Ensure the remixed project is loaded, otherwise we'll get in a mess
+              await dispatch(
+                syncProject("loadRemix")({ ...params, identifier }),
               );
             }
+            localStorage.removeItem("awaitingSave");
           }
-          localStorage.removeItem("awaitingSave");
         }
       }
     };

src/hooks/useProjectPersistence.test.js

@@ -195,32 +195,11 @@ describe("When logged in", () => {
       });
     });
 
-    describe("When project has identifier and awaiting save", () => {
-      beforeEach(() => {
-        localStorage.setItem("awaitingSave", "true");
-        syncProject.mockImplementationOnce(jest.fn((_) => remixProject));
-        syncProject.mockImplementationOnce(jest.fn((_) => loadProject));
-        renderHook(() =>
-          useProjectPersistence({
-            user: user2,
-            project: project,
-          }),
-        );
-        jest.runAllTimers();
-      });
-
-      test("Project remixed and saved to database", () => {
-        expect(remixProject).toHaveBeenCalledWith({
-          project,
-          accessToken: user2.access_token,
-        });
-      });
-    });
-
     describe("When project has identifier and save triggered", () => {
       beforeEach(() => {
         syncProject.mockImplementationOnce(jest.fn((_) => remixProject));
         syncProject.mockImplementationOnce(jest.fn((_) => loadProject));
+        localStorage.setItem("awaitingSave", "true");
 
         renderHook(() =>
           useProjectPersistence({
@@ -246,12 +225,17 @@ describe("When logged in", () => {
           accessToken: user2.access_token,
         });
       });
+
+      test("The awaitingSave flag is removed", async () => {
+        expect(localStorage.getItem("awaitingSave")).toBeNull();
+      });
     });
 
     describe("When project has identifier and save triggered without loadRemix", () => {
       beforeEach(() => {
         syncProject.mockImplementationOnce(jest.fn((_) => remixProject));
         syncProject.mockImplementationOnce(jest.fn((_) => loadProject));
+        localStorage.setItem("awaitingSave", "true");
 
         renderHook(() =>
           useProjectPersistence({
@@ -275,27 +259,9 @@ describe("When logged in", () => {
         await remixProject();
         await expect(loadProject).not.toHaveBeenCalled();
       });
-    });
 
-    describe("When project has no identifier and awaiting save", () => {
-      beforeEach(() => {
-        localStorage.setItem("awaitingSave", "true");
-        syncProject.mockImplementationOnce(jest.fn((_) => saveProject));
-        renderHook(() =>
-          useProjectPersistence({
-            user: user2,
-            project: { ...project, identifier: null },
-          }),
-        );
-        jest.runAllTimers();
-      });
-
-      test("Project saved to database", () => {
-        expect(saveProject).toHaveBeenCalledWith({
-          project: { ...project, identifier: null },
-          accessToken: user2.access_token,
-          autosave: false,
-        });
+      test("The awaitingSave flag is removed", async () => {
+        expect(localStorage.getItem("awaitingSave")).toBeNull();
       });
     });
 
@@ -309,6 +275,7 @@ describe("When logged in", () => {
             saveTriggered: true,
           }),
         );
+        localStorage.setItem("awaitingSave", "true");
         jest.runAllTimers();
       });
 
@@ -319,6 +286,10 @@ describe("When logged in", () => {
           autosave: false,
         });
       });
+
+      test("The awaitingSave flag is removed", async () => {
+        expect(localStorage.getItem("awaitingSave")).toBeNull();
+      });
     });
   });