fix(EMI-1791): Fix unsaving artwork in multiple lists bug (#13862)

* fix(EMI-1791): Fix unsaving artwork in multiple lists bug * remove non-null assertions * re-enable tests
artsy · May 13, 2024 · 5b32ece · 5b32ece
1 parent c575cca
commit 5b32ece
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 29 deletions.
diff --git a/src/Apps/Artwork/Components/ArtworkImageBrowser/ArtworkActionsSaveButton.tsx b/src/Apps/Artwork/Components/ArtworkImageBrowser/ArtworkActionsSaveButton.tsx
@@ -27,7 +27,7 @@ export const ArtworkActionsSaveButton: FC<ArtworkActionsSaveButtonProps> = ({
       internalID: artwork.internalID,
       id: artwork.id,
       slug: artwork.slug,
-      title: artwork.title!,
+      title: artwork.title ?? "",
       year: artwork.date,
       artistNames: artwork.artistNames,
       imageURL: artwork.preview?.url ?? null,

diff --git a/...outes/Saves/Components/SelectArtworkListsModal/__tests__/SelectArtworkListsModal.jest.tsx b/...outes/Saves/Components/SelectArtworkListsModal/__tests__/SelectArtworkListsModal.jest.tsx
@@ -15,6 +15,7 @@ import { useMutation } from "Utils/Hooks/useMutation"
 import { AnalyticsCombinedContextProvider } from "System/Analytics/AnalyticsContext"
 import { FC } from "react"
 import { flushPromiseQueue } from "DevTools/flushPromiseQueue"
+import { MockBoot } from "DevTools/MockBoot"
 
 jest.unmock("react-relay")
 jest.mock("Utils/Hooks/useMutation")
@@ -34,19 +35,17 @@ const { renderWithRelay } = setupTestWrapperTL<
   SelectArtworkListsModal_Test_Query
 >({
   Component: props => {
-    if (!props.me) {
-      return null
-    }
-
     return (
-      <AnalyticsCombinedContextProvider
-        contextPageOwnerId="page-owner-id"
-        path="/artist/page-owner-slug"
-      >
-        <ManageArtworkForSavesProvider artwork={artwork}>
-          <TestComponent {...props} />
-        </ManageArtworkForSavesProvider>
-      </AnalyticsCombinedContextProvider>
+      <MockBoot>
+        <AnalyticsCombinedContextProvider
+          contextPageOwnerId="page-owner-id"
+          path="/artist/page-owner-slug"
+        >
+          <ManageArtworkForSavesProvider artwork={artwork}>
+            <TestComponent {...props} />
+          </ManageArtworkForSavesProvider>
+        </AnalyticsCombinedContextProvider>
+      </MockBoot>
     )
   },
   query: graphql`
@@ -58,21 +57,15 @@ const { renderWithRelay } = setupTestWrapperTL<
   `,
 })
 
-// FIXME:
-describe.skip("SelectArtworkListsModal", () => {
+describe("SelectArtworkListsModal", () => {
   const mockUseMutation = useMutation as jest.Mock
   const mockUseTracking = useTracking as jest.Mock
   const trackEvent = jest.fn()
   const submitMutation = jest.fn()
 
   beforeEach(() => {
-    mockUseMutation.mockImplementation(() => {
-      return { submitMutation }
-    })
-
-    mockUseTracking.mockImplementation(() => ({
-      trackEvent,
-    }))
+    mockUseMutation.mockImplementation(() => ({ submitMutation }))
+    mockUseTracking.mockImplementation(() => ({ trackEvent }))
   })
 
   it("should render artwork data", async () => {

diff --git a/...CollectorProfile/Routes/Saves/Components/SelectArtworkListsModal/useSelectArtworkLists.ts b/...CollectorProfile/Routes/Saves/Components/SelectArtworkListsModal/useSelectArtworkLists.ts
@@ -71,8 +71,11 @@ export const useSelectArtworkLists = (artworkID: string) => {
         artwork.setValue(false, "isSaved")
       }
 
-      // Update `isSavedToList` field to reflect if artwork was saved to any list
-      const hasBeenSavedToList = addedCounts.custom - removedCounts.custom > 0
+      // Update `isSavedToList` field to reflect if artwork was saved to any list or it was already saved
+      const hasBeenSavedToList =
+        addedCounts.custom - removedCounts.custom > 0 ||
+        (removedCounts.custom === 0 && artwork.getValue("isSavedToList"))
+
       artwork.setValue(hasBeenSavedToList, "isSavedToList")
     },
   })

diff --git a/src/Components/Artwork/SaveButton/SaveArtworkToListsButton.tsx b/src/Components/Artwork/SaveButton/SaveArtworkToListsButton.tsx
@@ -31,7 +31,7 @@ const SaveArtworkToListsButton: FC<SaveArtworkToListsButtonProps> = ({
       internalID: artwork.internalID,
       id: artwork.id,
       slug: artwork.slug,
-      title: artwork.title!,
+      title: artwork.title ?? "",
       year: artwork.date,
       artistNames: artwork.artistNames,
       imageURL: artwork.preview?.url ?? null,

diff --git a/src/Components/Artwork/SaveButton/useSaveArtwork.ts b/src/Components/Artwork/SaveButton/useSaveArtwork.ts
@@ -35,8 +35,8 @@ export const useSaveArtwork = ({
           {
             saveArtwork: {
               artwork: {
-                id: artwork.id!,
-                slug: artwork.slug!,
+                id: artwork.id ?? "",
+                slug: artwork.slug ?? "",
                 isSaved: !isSaved,
               },
               /**

diff --git a/src/Components/Artwork/SaveButton/useSaveArtworkToLists.ts b/src/Components/Artwork/SaveButton/useSaveArtworkToLists.ts
@@ -83,8 +83,8 @@ export const useSaveArtworkToLists = (options: SaveArtworkToListsOptions) => {
       {
         saveArtwork: {
           artwork: {
-            id: artwork.id!,
-            slug: artwork.slug!,
+            id: artwork.id,
+            slug: artwork.slug,
             isSaved: !artwork.isSavedToDefaultList,
           },
           /**