From 9cad0a31cc2ca783e9cc17ea45300aaa79fffc55 Mon Sep 17 00:00:00 2001 From: yuboluo Date: Mon, 14 Oct 2024 10:57:13 +0800 Subject: [PATCH] [Workspace] Fix workspace update issue (#8570) * fix workspace update issue Signed-off-by: yubonluo * Changeset file for PR #8570 created/updated * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo --------- Signed-off-by: yubonluo Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/8570.yml | 2 + .../workspace_detail/workspace_detail.tsx | 4 +- .../components/workspace_detail_app.tsx | 19 +---- .../workspace_form/use_workspace_form.test.ts | 3 +- .../workspace_form/use_workspace_form.ts | 46 ++++++------ .../components/workspace_form/utils.test.ts | 73 ------------------- .../public/components/workspace_form/utils.ts | 20 ----- 7 files changed, 30 insertions(+), 137 deletions(-) create mode 100644 changelogs/fragments/8570.yml diff --git a/changelogs/fragments/8570.yml b/changelogs/fragments/8570.yml new file mode 100644 index 000000000000..e967ac10c560 --- /dev/null +++ b/changelogs/fragments/8570.yml @@ -0,0 +1,2 @@ +fix: +- Fix workspace update issue ([#8570](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8570)) \ No newline at end of file diff --git a/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx b/src/plugins/workspace/public/components/workspace_detail/workspace_detail.tsx index e027c7d5f586..627ecb4d0271 100644 --- 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 index b67d0eef6544..2ece6e4e73bb 100644 --- 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 index ecb36acaa125..19b9299c52dc 100644 --- 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 index 70fe9fdb2c81..d9e4a24f3399 100644 --- 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( - (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 index e387393c4a31..6c480a70f714 100644 --- 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 index 9d0e2a47cbc3..0bdb861a55bc 100644 --- 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; };