Finish outfit auto-saving!

Hope it actually work-works lol Did some refactors in useOutfitState to support the new reset action we do after auto-saving, in case the server tweaked things like the name.
2021-04-28 15:47:59 -07:00 · 2021-04-28 15:47:59 -07:00 · 1d97988383
commit 1d97988383
parent 217aa8dcc1
4 changed files with 89 additions and 37 deletions
--- a/scripts/setup-mysql.sql
+++ b/scripts/setup-mysql.sql
@ -27,9 +27,9 @@ GRANT INSERT ON modeling_logs TO impress2020;
 -- User data tables
 GRANT SELECT, INSERT, DELETE ON closet_hangers TO impress2020;
 GRANT SELECT, UPDATE ON closet_lists TO impress2020;
-GRANT SELECT ON item_outfit_relationships TO impress2020;
+GRANT SELECT, DELETE ON item_outfit_relationships TO impress2020;
 GRANT SELECT ON neopets_connections TO impress2020;
-GRANT SELECT ON outfits TO impress2020;
+GRANT SELECT, INSERT, UPDATE ON outfits TO impress2020;
 GRANT SELECT, UPDATE ON users TO impress2020;
 GRANT SELECT, UPDATE ON openneo_id.users TO impress2020;

--- a/src/app/WardrobePage/ItemsPanel.js
+++ b/src/app/WardrobePage/ItemsPanel.js
@ -258,7 +258,7 @@ function ItemZoneGroupSkeleton({ itemCount }) {
  );
 }

-function useOutfitSaving(outfitState) {
+function useOutfitSaving(outfitState, dispatchToOutfit) {
  const { isLoggedIn, id: currentUserId } = useCurrentUser();
  const history = useHistory();
  const toast = useToast();
@ -272,9 +272,25 @@ function useOutfitSaving(outfitState) {
  const isNewOutfit = outfitState.id == null;

  // Whether this outfit's latest local changes have been saved to the server.
+  // And log it to the console!
  const latestVersionIsSaved =
    outfitState.savedOutfitState &&
-    outfitStatesAreEqual(outfitState, outfitState.savedOutfitState);
+    outfitStatesAreEqual(
+      outfitState.outfitStateWithoutExtras,
+      outfitState.savedOutfitState
+    );
+  React.useEffect(() => {
+    console.debug(
+      "[useOutfitSaving] Latest version is saved? %s\nCurrent: %o\nSaved: %o",
+      latestVersionIsSaved,
+      outfitState.outfitStateWithoutExtras,
+      outfitState.savedOutfitState
+    );
+  }, [
+    latestVersionIsSaved,
+    outfitState.outfitStateWithoutExtras,
+    outfitState.savedOutfitState,
+  ]);

  // Only logged-in users can save outfits - and they can only save new outfits,
  // or outfits they created.
@ -347,6 +363,15 @@ function useOutfitSaving(outfitState) {
            },
          },
        });
+
+        // Also, send a `reset` action, to show whatever the server returned.
+        // This is important for suffix changes to `name`, but can also be
+        // relevant for graceful failure when a bug causes a change not to
+        // persist.
+        dispatchToOutfit({
+          type: "resetToSavedOutfitData",
+          savedOutfitData: outfit,
+        });
      },
    }
  );
@ -360,8 +385,8 @@ function useOutfitSaving(outfitState) {
          speciesId: outfitState.speciesId,
          colorId: outfitState.colorId,
          pose: outfitState.pose,
-          wornItemIds: outfitState.wornItemIds,
-          closetedItemIds: outfitState.closetedItemIds,
+          wornItemIds: [...outfitState.wornItemIds],
+          closetedItemIds: [...outfitState.closetedItemIds],
        },
      })
        .then(({ data: { outfit } }) => {
@ -416,7 +441,7 @@ function useOutfitSaving(outfitState) {
      !outfitStatesAreEqual(debouncedOutfitState, outfitState.savedOutfitState)
    ) {
      console.info(
-        "[useOutfitSaving] Auto-saving outfit from old state to new state",
+        "[useOutfitSaving] Auto-saving outfit\nSaved: %o\nCurrent (debounced): %o",
        outfitState.savedOutfitState,
        debouncedOutfitState
      );
@ -452,7 +477,7 @@ function useOutfitSaving(outfitState) {
 * OutfitSavingIndicator shows a Save button, or the "Saved" or "Saving" state,
 * if the user can save this outfit. If not, this is empty!
 */
-function OutfitSavingIndicator({ outfitState }) {
+function OutfitSavingIndicator({ outfitState, dispatchToOutfit }) {
  const {
    canSaveOutfit,
    isNewOutfit,
@ -460,7 +485,7 @@ function OutfitSavingIndicator({ outfitState }) {
    latestVersionIsSaved,
    saveError,
    saveOutfit,
-  } = useOutfitSaving(outfitState);
+  } = useOutfitSaving(outfitState, dispatchToOutfit);

  const errorTextColor = useColorModeValue("red.600", "red.400");

@ -579,7 +604,10 @@ function OutfitHeading({ outfitState, dispatchToOutfit }) {
          </Box>
          <Box width="4" flex="1 0 auto" />
          <Box flex="0 0 auto">
-            <OutfitSavingIndicator outfitState={outfitState} />
+            <OutfitSavingIndicator
+              outfitState={outfitState}
+              dispatchToOutfit={dispatchToOutfit}
+            />
          </Box>
          <Box width="2" />
          <Menu placement="bottom-end">
--- a/src/app/WardrobePage/useOutfitState.js
+++ b/src/app/WardrobePage/useOutfitState.js
@ -61,8 +61,8 @@ function useOutfitState() {
      returnPartialData: true,
      onCompleted: (outfitData) => {
        dispatchToOutfit({
-          type: "reset",
-          newState: getOutfitStateFromOutfitData(outfitData.outfit),
+          type: "resetToSavedOutfitData",
+          savedOutfitData: outfitData.outfit,
        });
      },
    }
@ -97,17 +97,17 @@ function useOutfitState() {
  if (urlOutfitState.id === localOutfitState.id) {
    // Use the reducer state: they're both for the same saved outfit, or both
    // for an unsaved outfit (null === null).
-    console.debug("Choosing local outfit state");
+    console.debug("[useOutfitState] Choosing local outfit state");
    outfitState = localOutfitState;
  } else if (urlOutfitState.id && urlOutfitState.id === savedOutfitState.id) {
    // Use the saved outfit state: it's for the saved outfit the URL points to.
-    console.debug("Choosing saved outfit state");
+    console.debug("[useOutfitState] Choosing saved outfit state");
    outfitState = savedOutfitState;
  } else {
    // Use the URL state: it's more up-to-date than any of the others. (Worst
    // case, it's empty except for ID, which is fine while the saved outfit
    // data loads!)
-    console.debug("Choosing URL outfit state");
+    console.debug("[useOutfitState] Choosing URL outfit state");
    outfitState = urlOutfitState;
  }

@ -350,8 +350,8 @@ const outfitStateReducer = (apolloClient) => (baseState, action) => {
        // particular about which version of the pose to show if more than one.
        state.appearanceId = action.appearanceId || null;
      });
-    case "reset":
-      return action.newState;
+    case "resetToSavedOutfitData":
+      return getOutfitStateFromOutfitData(action.savedOutfitData);
    default:
      throw new Error(`unexpected action ${JSON.stringify(action)}`);
  }
--- a/src/server/types/Outfit.js
+++ b/src/server/types/Outfit.js
@ -141,10 +141,6 @@ const resolvers = {
        );
      }

-      if (id) {
-        throw new Error("TODO: Add support for updating existing outfits");
-      }
-
      // Get the base name of the provided name: trim it, and strip any "(1)"
      // suffixes.
      const baseName = (rawName || "Untitled outfit").replace(
@ -153,21 +149,23 @@ const resolvers = {
      );
      const namePlaceholder = baseName.trim().replace(/_%/g, "\\$0") + "%";

-      // Then, look for outfits from this user with the same base name.
+      // Then, look for outfits from this user with the same base name. Omit
+      // the current outfit (if it's already saved), because it's fine for this
+      // outfit to have the same name as itself lol!
      const [outfitRows] = await db.query(
        `
-          SELECT name FROM outfits WHERE user_id = ? AND name LIKE ?;
+          SELECT name FROM outfits WHERE user_id = ? AND name LIKE ? AND id != ?;
        `,
-        [currentUserId, namePlaceholder]
+        [currentUserId, namePlaceholder, id || "<no-ID-new-outfit>"]
      );
      const existingOutfitNames = new Set(
        outfitRows.map(({ name }) => name.trim())
      );

-      // Then, get the unique name to use for this outfit: try the base name
-      // first, but, if it's taken, keep incrementing the "(1)" suffix until
-      // it's not.
-      let name = baseName;
+      // Then, get the unique name to use for this outfit: try the provided
+      // name first, but, if it's taken, add a "(1)" suffix and keep
+      // incrementing it until it's not.
+      let name = rawName;
      for (let i = 1; existingOutfitNames.has(name); i++) {
        name = `${baseName} (${i})`;
      }
@ -197,15 +195,41 @@ const resolvers = {
      await db.beginTransaction();
      let newOutfitId;
      try {
-        const [result] = await db.execute(
-          `
-            INSERT INTO outfits
-              (name, pet_state_id, user_id, created_at, updated_at)
-              VALUES (?, ?, ?, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP());
-          `,
-          [name, petState.id, currentUserId]
-        );
-        newOutfitId = String(result.insertId);
+        // If this is a new outfit, INSERT it. Or, if it's an existing outfit,
+        // UPDATE it.
+        const [result] = id
+          ? await db.execute(
+              `
+              UPDATE outfits
+                SET name = ?, pet_state_id = ?,
+                  updated_at = CURRENT_TIMESTAMP()
+                WHERE id = ?;
+            `,
+              [name, petState.id, id]
+            )
+          : await db.execute(
+              `
+                INSERT INTO outfits
+                  (name, pet_state_id, user_id, created_at, updated_at)
+                  VALUES (?, ?, ?, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP());
+              `,
+              [name, petState.id, currentUserId]
+            );
+        newOutfitId = id || String(result.insertId);
+
+        // If this outfit is already saved, clear the item relationships. We'll
+        // re-insert them in the next query. (We're in a transaction, so this
+        // is safe: on error, we'll roll this back!)
+        //
+        // TODO: This delete-then-insert paradigm isn't super-performant,
+        //       performing the actual needed sync could be better. Keep an eye
+        //       on query perf!
+        if (id) {
+          await db.execute(
+            `DELETE FROM item_outfit_relationships WHERE outfit_id = ?;`,
+            [id]
+          );
+        }

        if (wornItemIds.length > 0 || closetedItemIds.length > 0) {
          const itemRowPlaceholders = [