Send only changed data when editing product (#5004) (#5012)

* Send only changed data * Add tests * Create beige-moose-greet.md --------- Co-authored-by: Patryk Andrzejewski <[email protected]> Co-authored-by: Paweł Chyła <[email protected]>
saleor · Jul 3, 2024 · 6b2892c · 6b2892c
1 parent 1b7126b
commit 6b2892c
Show file tree

Hide file tree

Showing 12 changed files with 211 additions and 43 deletions.
diff --git a/.changeset/beige-moose-greet.md b/.changeset/beige-moose-greet.md
@@ -0,0 +1,5 @@
+---
+"saleor-dashboard": patch
+---
+
+Product edition no longer change the others work when changing different fields simultaneously. This means UI sends only form fields that were changed.
diff --git a/src/hooks/useForm.ts → src/hooks/useForm/index.ts b/src/hooks/useForm.ts → src/hooks/useForm/index.ts
@@ -1,4 +1,3 @@
-// @ts-strict-ignore
 import {
   CheckIfSaveIsDisabledFnType,
   FormId,
@@ -11,7 +10,9 @@ import isEqual from "lodash/isEqual";
 import omit from "lodash/omit";
 import React, { useEffect, useState } from "react";
 
-import useStateFromProps from "./useStateFromProps";
+import useStateFromProps from "./../useStateFromProps";
+import { FormData } from "./types";
+import { useChangedData } from "./useChangedData";
 
 export interface ChangeEvent<TData = any> {
   target: {
@@ -51,6 +52,8 @@ export interface UseFormResult<TData>
   setError: (name: keyof TData, error: string | React.ReactNode) => void;
   clearErrors: (name?: keyof TData | Array<keyof TData>) => void;
   setIsSubmitDisabled: (value: boolean) => void;
+  cleanChanged: () => void;
+  changedData: TData;
 }
 
 export interface CommonUseFormResult<TData> {
@@ -65,8 +68,6 @@ export interface CommonUseFormResultWithHandlers<TData, THandlers>
   handlers: THandlers;
 }
 
-type FormData = Record<string, any | any[]>;
-
 function merge<T extends FormData>(prevData: T, prevState: T, data: T): T {
   return Object.keys(prevState).reduce(
     (acc, key) => {
@@ -97,6 +98,8 @@ function useForm<T extends FormData, TErrors>(
     mergeFunc: mergeData ? merge : undefined,
   });
 
+  const { add: addChanged, clean: cleanChanged, data: changed } = useChangedData<T>(data);
+
   const isSaveDisabled = () => {
     if (checkIfSaveIsDisabled) {
       return checkIfSaveIsDisabled(data);
@@ -145,7 +148,7 @@ function useForm<T extends FormData, TErrors>(
 
     if (Array.isArray(field)) {
       handleSetChanged(true);
-
+      addChanged(name);
       setData({
         ...data,
         [name]: toggle(value, field, isEqual),
@@ -163,6 +166,7 @@ function useForm<T extends FormData, TErrors>(
 
     if (Array.isArray(field)) {
       handleSetChanged(true);
+      addChanged(name);
 
       setData({
         ...data,
@@ -186,6 +190,8 @@ function useForm<T extends FormData, TErrors>(
     if (!(name in data)) {
       console.error(`Unknown form field: ${name}`);
     } else {
+      addChanged(name);
+
       if (data[name] !== value) {
         handleSetChanged(true);
       }
@@ -213,6 +219,8 @@ function useForm<T extends FormData, TErrors>(
 
       return result;
     }
+
+    return [];
   }
 
   const setError = (field: keyof T, error: string | React.ReactNode) =>
@@ -229,6 +237,8 @@ function useForm<T extends FormData, TErrors>(
   };
 
   return {
+    changedData: changed,
+    cleanChanged,
     formId,
     setError,
     errors,

diff --git a/src/hooks/useForm/types.ts b/src/hooks/useForm/types.ts
@@ -0,0 +1 @@
+export type FormData = Record<string, any | any[]>;
diff --git a/src/hooks/useForm/useChangedData.test.tsx b/src/hooks/useForm/useChangedData.test.tsx
@@ -0,0 +1,46 @@
+import { renderHook } from "@testing-library/react-hooks";
+
+import { useChangedData } from "./useChangedData";
+
+describe("useForm / useChangedData", () => {
+  it("returns all changed fields", () => {
+    // Arrange
+    const { result } = renderHook(() =>
+      useChangedData({
+        "field-1": "value-1",
+        "field-2": "value-2",
+        "field-3": "value-3",
+        "field-4": "value-4",
+      }),
+    );
+
+    // Act
+    result.current.add("field-1");
+    result.current.add("field-2");
+
+    // Assert
+    expect(result.current.data).toEqual({
+      "field-1": "value-1",
+      "field-2": "value-2",
+    });
+  });
+
+  it("clears changed fields", () => {
+    // Arrange
+    const { result } = renderHook(() =>
+      useChangedData({
+        "field-1": "value-1",
+        "field-2": "value-2",
+        "field-3": "value-3",
+        "field-4": "value-4",
+      }),
+    );
+
+    // Act
+    result.current.add("field-1");
+    result.current.clean();
+
+    // Assert
+    expect(result.current.data).toEqual({});
+  });
+});
diff --git a/src/hooks/useForm/useChangedData.ts b/src/hooks/useForm/useChangedData.ts
@@ -0,0 +1,27 @@
+import { useState } from "react";
+
+import { FormData } from "./types";
+
+export const useChangedData = <T extends FormData>(formData: T) => {
+  const [dirtyFields, setDirtyFields] = useState<string[]>([]);
+
+  const add = (name: string) => {
+    setDirtyFields(fields => {
+      return Array.from(new Set(fields.concat(name)));
+    });
+  };
+
+  const clean = () => {
+    setDirtyFields([]);
+  };
+
+  const data = Object.entries(formData)
+    .filter(([key]) => dirtyFields.includes(key))
+    .reduce((p, [key, value]) => ({ ...p, [key]: value }), {} as T);
+
+  return {
+    add,
+    clean,
+    data,
+  };
+};
diff --git a/src/hooks/useHandleFormSubmit.ts b/src/hooks/useHandleFormSubmit.ts
@@ -1,12 +1,11 @@
-// @ts-strict-ignore
 import { FormId, useExitFormDialog } from "@dashboard/components/Form";
 import { MessageContext } from "@dashboard/components/messages";
 import { SubmitPromise } from "@dashboard/hooks/useForm";
 import { useContext } from "react";
 
 interface UseHandleFormSubmitProps<TData, TError> {
   formId?: FormId;
-  onSubmit: (data: TData) => SubmitPromise<TError[]> | void;
+  onSubmit?: (data: TData) => SubmitPromise<TError[]> | void;
 }
 
 function useHandleFormSubmit<TData, TErrors>({
@@ -25,7 +24,7 @@ function useHandleFormSubmit<TData, TErrors>({
       messageContext.clearErrorNotifications();
     }
 
-    const result = onSubmit(data);
+    const result = onSubmit ? onSubmit(data) : null;
 
     if (!result) {
       return [];

diff --git a/src/products/components/ProductUpdatePage/form.test.ts b/src/products/components/ProductUpdatePage/form.test.ts
@@ -19,30 +19,13 @@ jest.mock("@dashboard/utils/richText/useRichText", () => {
 const baseData = {
   attributes: [],
   attributesWithNewFileValue: [],
-  category: "",
   channels: {
     removeChannels: [],
     updateChannels: [],
   },
-  collections: [],
   description: undefined,
-  globalSoldUnits: 0,
-  globalThreshold: "",
-  hasPreorderEndDate: false,
-  isAvailable: false,
-  isPreorder: false,
   metadata: undefined,
-  name: "",
-  preorderEndDateTime: undefined,
   privateMetadata: undefined,
-  rating: null,
-  seoDescription: "",
-  seoTitle: "",
-  sku: "",
-  slug: "",
-  taxClassId: undefined,
-  trackInventory: false,
-  weight: "",
 };
 
 describe("useProductUpdateForm", () => {
@@ -98,4 +81,55 @@ describe("useProductUpdateForm", () => {
       },
     });
   });
+
+  it("submits form with the only data that was modified", async () => {
+    // Arrange
+    const mockOnSubmit = jest.fn();
+    const { result } = renderHook(() =>
+      useProductUpdateForm(
+        { variants: [], channelListings: [] } as unknown as ProductFragment,
+        mockOnSubmit,
+        false,
+        jest.fn(),
+        {} as UseProductUpdateFormOpts,
+      ),
+    );
+
+    // Act
+    await act(() => {
+      result.current.change({ target: { name: "slug", value: "test-slug-1" } });
+      result.current.change({ target: { name: "category", value: "test-category" } });
+      result.current.change({ target: { name: "collections", value: ["collection-1"] } });
+      result.current.change({ target: { name: "rating", value: 4 } });
+      result.current.change({ target: { name: "seoTitle", value: "seo-title-1" } });
+      result.current.change({ target: { name: "seoDescription", value: "seo-desc-1" } });
+    });
+
+    await act(async () => {
+      await result.current.submit();
+    });
+
+    expect(mockOnSubmit).toHaveBeenCalledWith({
+      attributes: [],
+      attributesWithNewFileValue: [],
+      channels: {
+        removeChannels: [],
+        updateChannels: [],
+      },
+      description: undefined,
+      metadata: undefined,
+      privateMetadata: undefined,
+      slug: "test-slug-1",
+      category: "test-category",
+      collections: ["collection-1"],
+      variants: {
+        added: [],
+        removed: [],
+        updates: [],
+      },
+      rating: 4,
+      seoTitle: "seo-title-1",
+      seoDescription: "seo-desc-1",
+    });
+  });
 });
diff --git a/src/products/components/ProductUpdatePage/form.tsx b/src/products/components/ProductUpdatePage/form.tsx
@@ -67,13 +67,13 @@ export function useProductUpdateForm(
     confirmLeave: true,
     formId: PRODUCT_UPDATE_FORM_ID,
   });
-
   const {
     handleChange,
     triggerChange,
     toggleValues,
     data: formData,
     setIsSubmitDisabled,
+    cleanChanged,
   } = form;
   const { locale } = useLocale();
 
@@ -194,7 +194,7 @@ export function useProductUpdateForm(
   };
 
   const getSubmitData = async (): Promise<ProductUpdateSubmitData> => ({
-    ...data,
+    ...form.changedData,
     ...getMetadata(data, isMetadataModified, isPrivateMetadataModified),
     attributes: mergeAttributes(
       attributes.data,
@@ -210,7 +210,7 @@ export function useProductUpdateForm(
         touchedChannels.current.includes(listing.channelId),
       ),
     },
-    description: await richText.getValue(),
+    description: richText.isDirty ? await richText.getValue() : undefined,
     variants: variants.current,
   });
 
@@ -231,6 +231,8 @@ export function useProductUpdateForm(
 
   const submit = useCallback(async () => {
     const result = await handleFormSubmit(await getSubmitData());
+
+    cleanChanged();
     await refetch();
 
     datagrid.setAdded(prevAdded =>