[Workspace] Fix workspace update issue (opensearch-project#8570)

* fix workspace update issue Signed-off-by: yubonluo <[email protected]> * Changeset file for PR opensearch-project#8570 created/updated * optimize the code Signed-off-by: yubonluo <[email protected]> * optimize the code Signed-off-by: yubonluo <[email protected]> * optimize the code Signed-off-by: yubonluo <[email protected]> --------- Signed-off-by: yubonluo <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Qxisylolo · Oct 30, 2024 · c42d070 · c42d070
1 parent e856cc7
commit c42d070
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 137 deletions.
diff --git a/changelogs/fragments/8570.yml b/changelogs/fragments/8570.yml
@@ -0,0 +1,2 @@
+fix:
+- Fix workspace update issue ([#8570](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8570))
diff --git a/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx b/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx
@@ -138,7 +138,7 @@ export const WorkspaceDetail = (props: WorkspaceDetailPropsWithFormSubmitting) =
   // When user has unsaved changes and navigates to other page, will show a confirm modal.
   useEffect(() => {
     onAppLeave((actions) => {
-      if (!props.isFormSubmitting && isEditing && numberOfChanges > 0) {
+      if (isEditing && numberOfChanges > 0) {
         return actions.confirm(
           i18n.translate('workspace.detail.navigate.message', {
             defaultMessage: 'Any unsaved changes will be lost.',
@@ -151,7 +151,7 @@ export const WorkspaceDetail = (props: WorkspaceDetailPropsWithFormSubmitting) =
       }
       return actions.default();
     });
-  }, [handleResetForm, isEditing, numberOfChanges, onAppLeave, props.isFormSubmitting]);
+  }, [handleResetForm, isEditing, numberOfChanges, onAppLeave]);
 
   const handleSetDefaultWorkspace = useCallback(
     async (workspace: WorkspaceAttribute) => {

diff --git a/src/plugins/workspace/public/components/workspace_detail_app.tsx b/src/plugins/workspace/public/components/workspace_detail_app.tsx
@@ -92,7 +92,7 @@ export const WorkspaceDetailApp = (props: WorkspaceDetailPropsWithOnAppLeave) =>
   }, [currentWorkspace, savedObjects, http, notifications]);
 
   const handleWorkspaceFormSubmit = useCallback(
-    async (data: WorkspaceFormSubmitData, refresh?: boolean) => {
+    async (data: WorkspaceFormSubmitData) => {
       let result;
       if (isFormSubmitting) {
         return;
@@ -121,28 +121,15 @@ export const WorkspaceDetailApp = (props: WorkspaceDetailPropsWithOnAppLeave) =>
           dataSources: selectedDataSourceIds,
           permissions: convertPermissionSettingsToPermissions(permissionSettings),
         });
+        setIsFormSubmitting(false);
         if (result?.success) {
           notifications?.toasts.addSuccess({
             title: i18n.translate('workspace.update.success', {
               defaultMessage: 'Update workspace successfully',
             }),
           });
-          setIsFormSubmitting(false);
-          if (application && http && refresh) {
-            // Redirect page after one second, leave one second time to show update successful toast.
-            window.setTimeout(() => {
-              window.location.href = formatUrlWithWorkspaceId(
-                application.getUrlForApp(WORKSPACE_DETAIL_APP_ID, {
-                  absolute: true,
-                }),
-                currentWorkspace.id,
-                http.basePath
-              );
-            }, 1000);
-          }
           return result;
         } else {
-          setIsFormSubmitting(false);
           throw new Error(result?.error ? result?.error : 'update workspace failed');
         }
       } catch (error) {
@@ -156,7 +143,7 @@ export const WorkspaceDetailApp = (props: WorkspaceDetailPropsWithOnAppLeave) =>
         return;
       }
     },
-    [isFormSubmitting, currentWorkspace, notifications?.toasts, workspaceClient, application, http]
+    [isFormSubmitting, currentWorkspace, notifications?.toasts, workspaceClient]
   );
 
   if (!workspaces || !application || !http || !savedObjects || !currentWorkspaceFormData) {

diff --git a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.test.ts b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.test.ts
@@ -138,8 +138,7 @@ describe('useWorkspaceForm', () => {
       expect.objectContaining({
         name: 'test-workspace-name',
         features: ['use-case-observability'],
-      }),
-      true
+      })
     );
   });
   it('should update selected use case', () => {

diff --git a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts
@@ -89,8 +89,19 @@ export const useWorkspaceForm = ({
     [setFeatureConfigs]
   );
 
+  const getSubmitFormData = (submitFormData: WorkspaceFormDataState) => {
+    return {
+      name: submitFormData.name!,
+      description: submitFormData.description,
+      color: submitFormData.color || '#FFFFFF',
+      features: submitFormData.features,
+      permissionSettings: submitFormData.permissionSettings as WorkspacePermissionSetting[],
+      selectedDataSourceConnections: submitFormData.selectedDataSourceConnections,
+    };
+  };
+
   const handleFormSubmit = useCallback<FormEventHandler>(
-    (e) => {
+    async (e) => {
       e.preventDefault();
       const currentFormData = getFormDataRef.current();
       currentFormData.permissionSettings = currentFormData.permissionSettings.filter(
@@ -107,34 +118,22 @@ export const useWorkspaceForm = ({
         return;
       }
 
-      onSubmit?.(
-        {
-          name: currentFormData.name!,
-          description: currentFormData.description,
-          color: currentFormData.color || '#FFFFFF',
-          features: currentFormData.features,
-          permissionSettings: currentFormData.permissionSettings as WorkspacePermissionSetting[],
-          selectedDataSourceConnections: currentFormData.selectedDataSourceConnections,
-        },
-        true
-      );
+      const submitFormData = getSubmitFormData(currentFormData);
+      const result = await onSubmit?.(submitFormData);
+      if (result?.success) {
+        defaultValuesRef.current = submitFormData;
+        setIsEditing(false);
+      }
     },
     [onSubmit, permissionEnabled]
   );
 
   const handleSubmitPermissionSettings = async (settings: WorkspacePermissionSetting[]) => {
     const currentFormData = getFormDataRef.current();
-    const result = await onSubmit?.(
-      {
-        name: currentFormData.name!,
-        description: currentFormData.description,
-        color: currentFormData.color || '#FFFFFF',
-        features: currentFormData.features,
-        permissionSettings: settings,
-        selectedDataSourceConnections: currentFormData.selectedDataSourceConnections,
-      },
-      false
-    );
+    const result = await onSubmit?.({
+      ...getSubmitFormData(currentFormData),
+      permissionSettings: settings,
+    });
     if (result) {
       setPermissionSettings(settings);
     }
@@ -150,7 +149,6 @@ export const useWorkspaceForm = ({
     setDescription(resetValues?.description ?? '');
     setColor(resetValues?.color);
     setFeatureConfigs(resetValues?.features ?? []);
-    setPermissionSettings(defaultValuesRef.current?.permissionSettings ?? []);
     setFormErrors({});
     setIsEditing(false);
   }, []);

diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts
@@ -553,79 +553,6 @@ describe('getNumberOfChanges', () => {
       )
     ).toEqual(1);
   });
-  it('should return consistent permission settings changes count', () => {
-    expect(
-      getNumberOfChanges(
-        {
-          name: 'foo',
-          permissionSettings: [
-            {
-              id: 0,
-              type: WorkspacePermissionItemType.User,
-              userId: 'user-1',
-              modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite],
-            },
-          ],
-        },
-        {
-          name: 'foo',
-          permissionSettings: [
-            {
-              id: 0,
-              type: WorkspacePermissionItemType.User,
-              userId: 'user-1',
-              modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite],
-            },
-          ],
-        }
-      )
-    ).toEqual(0);
-    // for remove permission setting
-    expect(
-      getNumberOfChanges(
-        {
-          name: 'foo',
-          permissionSettings: [
-            {
-              id: 0,
-              type: WorkspacePermissionItemType.User,
-              userId: 'user-1',
-              modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite],
-            },
-            {
-              id: 1,
-              type: WorkspacePermissionItemType.Group,
-              group: 'group-1',
-              modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite],
-            },
-          ],
-        },
-        {
-          name: 'foo',
-          /**
-           * These include three changes:
-           * 1.Remove permission setting#0
-           * 2.Modify permission setting#1
-           * 3.Add permission setting#2
-           */
-          permissionSettings: [
-            {
-              id: 1,
-              type: WorkspacePermissionItemType.Group,
-              group: 'group-1',
-              modes: [WorkspacePermissionMode.Read, WorkspacePermissionMode.LibraryWrite],
-            },
-            {
-              id: 2,
-              type: WorkspacePermissionItemType.User,
-              userId: 'user-1',
-              modes: [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite],
-            },
-          ],
-        }
-      )
-    ).toEqual(3);
-  });
 });
 
 describe('isWorkspacePermissionSetting', () => {

diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts
@@ -446,25 +446,5 @@ export const getNumberOfChanges = (
   ) {
     count++;
   }
-  // Count all new added permission settings
-  count +=
-    newFormData.permissionSettings?.reduce((prevNewAddedCount, setting) => {
-      if (!initialFormData.permissionSettings?.find((item) => item.id === setting.id)) {
-        prevNewAddedCount += 1;
-      }
-      return prevNewAddedCount;
-    }, 0) ?? 0;
-  count +=
-    initialFormData.permissionSettings?.reduce((prevDeletedAndModifiedCount, setting) => {
-      const newSetting = newFormData.permissionSettings?.find((item) => item.id === setting.id);
-      if (!newSetting) {
-        // Count all delete permission settings
-        prevDeletedAndModifiedCount += 1;
-      } else if (!isSamePermissionSetting(newSetting, setting)) {
-        // Count all modified permission settings
-        prevDeletedAndModifiedCount += 1;
-      }
-      return prevDeletedAndModifiedCount;
-    }, 0) ?? 0;
   return count;
 };
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		fix:
		- Fix workspace update issue ([#8570](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8570))