[Workspace]Fix error toasts in sample data page (#8842) (#9015)

* Set default index pattern when workspace disabled * Move saved objects first to avoid partial deleted * Skip ui setting update for non workspace admin * Add UT for sample_data_client * Changeset file for PR #8842 created/updated --------- (cherry picked from commit af429b6) Signed-off-by: Lin Wang <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
opensearch-project · Dec 6, 2024 · bc25b8c · bc25b8c
1 parent eaadd99
commit bc25b8c
Show file tree

Hide file tree

Showing 6 changed files with 215 additions and 23 deletions.
diff --git a/changelogs/fragments/8842.yml b/changelogs/fragments/8842.yml
@@ -0,0 +1,2 @@
+fix:
+- [Workspace]Fix error toasts in sample data page ([#8842](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8842))
diff --git a/src/plugins/home/public/application/opensearch_dashboards_services.ts b/src/plugins/home/public/application/opensearch_dashboards_services.ts
@@ -37,6 +37,7 @@ import {
   SavedObjectsClientContract,
   IUiSettingsClient,
   ApplicationStart,
+  WorkspacesSetup,
 } from 'opensearch-dashboards/public';
 import { UiStatsMetricType } from '@osd/analytics';
 import { TelemetryPluginStart } from '../../../telemetry/public';
@@ -77,6 +78,7 @@ export interface HomeOpenSearchDashboardsServices {
   };
   dataSource?: DataSourcePluginStart;
   sectionTypes: SectionTypeService;
+  workspaces?: WorkspacesSetup;
 }
 
 let services: HomeOpenSearchDashboardsServices | null = null;

diff --git a/src/plugins/home/public/application/sample_data_client.js b/src/plugins/home/public/application/sample_data_client.js
@@ -41,11 +41,26 @@ export async function listSampleDataSets(dataSourceId) {
   return await getServices().http.get(sampleDataUrl, { query });
 }
 
+const canUpdateUISetting = () => {
+  const {
+    application: { capabilities },
+    workspaces,
+  } = getServices();
+  if (
+    capabilities.workspaces &&
+    capabilities.workspaces.enabled &&
+    capabilities.workspaces.permissionEnabled
+  ) {
+    return !!workspaces?.currentWorkspace$.getValue()?.owner;
+  }
+  return true;
+};
+
 export async function installSampleDataSet(id, sampleDataDefaultIndex, dataSourceId) {
   const query = buildQuery(dataSourceId);
   await getServices().http.post(`${sampleDataUrl}/${id}`, { query });
 
-  if (getServices().uiSettings.isDefault('defaultIndex')) {
+  if (canUpdateUISetting() && getServices().uiSettings.isDefault('defaultIndex')) {
     getServices().uiSettings.set('defaultIndex', sampleDataDefaultIndex);
   }
 
@@ -59,6 +74,7 @@ export async function uninstallSampleDataSet(id, sampleDataDefaultIndex, dataSou
   const uiSettings = getServices().uiSettings;
 
   if (
+    canUpdateUISetting() &&
     !uiSettings.isDefault('defaultIndex') &&
     uiSettings.get('defaultIndex') === sampleDataDefaultIndex
   ) {

diff --git a/src/plugins/home/public/application/sample_data_client.test.js b/src/plugins/home/public/application/sample_data_client.test.js
@@ -0,0 +1,167 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { BehaviorSubject } from 'rxjs';
+import { setServices } from '../application/opensearch_dashboards_services';
+import { installSampleDataSet, uninstallSampleDataSet } from './sample_data_client';
+
+const mockHttp = {
+  post: jest.fn(),
+  delete: jest.fn(),
+};
+
+const mockUiSettings = {
+  isDefault: jest.fn(),
+  set: jest.fn(),
+  get: jest.fn(),
+};
+
+const mockApplication = {
+  capabilities: {
+    workspaces: {
+      enabled: false,
+      permissionEnabled: false,
+    },
+  },
+};
+
+const mockIndexPatternService = {
+  clearCache: jest.fn(),
+};
+
+const mockWorkspace = {
+  currentWorkspace$: new BehaviorSubject(),
+};
+
+const mockServices = {
+  workspaces: mockWorkspace,
+  http: mockHttp,
+  uiSettings: mockUiSettings,
+  application: mockApplication,
+  indexPatternService: mockIndexPatternService,
+};
+
+setServices(mockServices);
+
+describe('installSampleDataSet', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+    mockUiSettings.isDefault.mockReturnValue(true);
+    setServices(mockServices);
+  });
+
+  it('should install the sample data set and set the default index', async () => {
+    const id = 'sample-data-id';
+    const sampleDataDefaultIndex = 'sample-data-index';
+    const dataSourceId = 'data-source-id';
+
+    await installSampleDataSet(id, sampleDataDefaultIndex, dataSourceId);
+
+    expect(mockHttp.post).toHaveBeenCalledWith(`/api/sample_data/${id}`, {
+      query: expect.anything(),
+    });
+    expect(mockUiSettings.set).toHaveBeenCalledWith('defaultIndex', sampleDataDefaultIndex);
+    expect(mockIndexPatternService.clearCache).toHaveBeenCalled();
+  });
+
+  it('should install the sample data set and not set the default index when workspace is enabled', async () => {
+    const id = 'sample-data-id';
+    const sampleDataDefaultIndex = 'sample-data-index';
+    const dataSourceId = 'data-source-id';
+
+    setServices({
+      ...mockServices,
+      workspaces: {
+        currentWorkspace$: new BehaviorSubject(),
+      },
+      application: {
+        capabilities: {
+          workspaces: {
+            enabled: true,
+            permissionEnabled: true,
+          },
+        },
+      },
+    });
+
+    await installSampleDataSet(id, sampleDataDefaultIndex, dataSourceId);
+
+    expect(mockHttp.post).toHaveBeenCalledWith(`/api/sample_data/${id}`, {
+      query: expect.anything(),
+    });
+    expect(mockUiSettings.set).not.toHaveBeenCalled();
+    expect(mockIndexPatternService.clearCache).toHaveBeenCalled();
+  });
+});
+
+describe('uninstallSampleDataSet', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+    mockUiSettings.isDefault.mockReturnValue(false);
+    setServices(mockServices);
+  });
+
+  it('should uninstall the sample data set and clear the default index', async () => {
+    const id = 'sample-data-id';
+    const sampleDataDefaultIndex = 'sample-data-index';
+    const dataSourceId = 'data-source-id';
+
+    mockUiSettings.get.mockReturnValue(sampleDataDefaultIndex);
+
+    await uninstallSampleDataSet(id, sampleDataDefaultIndex, dataSourceId);
+
+    expect(mockHttp.delete).toHaveBeenCalledWith(`/api/sample_data/${id}`, {
+      query: expect.anything(),
+    });
+    expect(mockUiSettings.set).toHaveBeenCalledWith('defaultIndex', null);
+    expect(mockIndexPatternService.clearCache).toHaveBeenCalled();
+  });
+
+  it('should uninstall the sample data set and not clear the default index when workspace is enabled', async () => {
+    const id = 'sample-data-id';
+    const sampleDataDefaultIndex = 'sample-data-index';
+    const dataSourceId = 'data-source-id';
+
+    setServices({
+      ...mockServices,
+      workspaces: {
+        currentWorkspace$: new BehaviorSubject(),
+      },
+      application: {
+        capabilities: {
+          workspaces: {
+            enabled: true,
+            permissionEnabled: true,
+          },
+        },
+      },
+    });
+
+    await uninstallSampleDataSet(id, sampleDataDefaultIndex, dataSourceId);
+
+    expect(mockHttp.delete).toHaveBeenCalledWith(`/api/sample_data/${id}`, {
+      query: expect.anything(),
+    });
+    expect(mockUiSettings.set).not.toHaveBeenCalled();
+    expect(mockIndexPatternService.clearCache).toHaveBeenCalled();
+  });
+
+  it('should uninstall the sample data set and not clear the default index when it is not the sample data index', async () => {
+    const id = 'sample-data-id';
+    const sampleDataDefaultIndex = 'sample-data-index';
+    const dataSourceId = 'data-source-id';
+
+    mockUiSettings.isDefault.mockReturnValue(false);
+    mockUiSettings.get.mockReturnValue('other-index');
+
+    await uninstallSampleDataSet(id, sampleDataDefaultIndex, dataSourceId);
+
+    expect(mockHttp.delete).toHaveBeenCalledWith(`/api/sample_data/${id}`, {
+      query: expect.anything(),
+    });
+    expect(mockUiSettings.set).not.toHaveBeenCalled();
+    expect(mockIndexPatternService.clearCache).toHaveBeenCalled();
+  });
+});
diff --git a/src/plugins/home/public/plugin.ts b/src/plugins/home/public/plugin.ts
@@ -156,6 +156,7 @@ export class HomePublicPlugin
         injectedMetadata: coreStart.injectedMetadata,
         dataSource,
         sectionTypes: this.sectionTypeService,
+        workspaces: core.workspaces,
         ...homeOpenSearchDashboardsServices,
       });
     };

diff --git a/src/plugins/home/server/services/sample_data/routes/uninstall.ts b/src/plugins/home/server/services/sample_data/routes/uninstall.ts
@@ -62,27 +62,10 @@ export function createUninstallRoute(
         return response.notFound();
       }
 
-      const caller = dataSourceId
-        ? context.dataSource.opensearch.legacy.getClient(dataSourceId).callAPI
-        : context.core.opensearch.legacy.client.callAsCurrentUser;
-
-      for (let i = 0; i < sampleDataset.dataIndices.length; i++) {
-        const dataIndexConfig = sampleDataset.dataIndices[i];
-        const index =
-          dataIndexConfig.indexName ?? createIndexName(sampleDataset.id, dataIndexConfig.id);
-
-        try {
-          await caller('indices.delete', { index });
-        } catch (err) {
-          return response.customError({
-            statusCode: err.status,
-            body: {
-              message: `Unable to delete sample data index "${index}", error: ${err.message}`,
-            },
-          });
-        }
-      }
-
+      /**
+       * Delete saved objects before removing the data index to avoid partial deletion
+       * of sample data when a read-only workspace user attempts to remove sample data.
+       */
       const savedObjectsList = getFinalSavedObjects({
         dataset: sampleDataset,
         workspaceId,
@@ -99,14 +82,35 @@ export function createUninstallRoute(
         // ignore 404s since users could have deleted some of the saved objects via the UI
         if (_.get(err, 'output.statusCode') !== 404) {
           return response.customError({
-            statusCode: err.status,
+            statusCode: err.status || _.get(err, 'output.statusCode'),
             body: {
               message: `Unable to delete sample dataset saved objects, error: ${err.message}`,
             },
           });
         }
       }
 
+      const caller = dataSourceId
+        ? context.dataSource.opensearch.legacy.getClient(dataSourceId).callAPI
+        : context.core.opensearch.legacy.client.callAsCurrentUser;
+
+      for (let i = 0; i < sampleDataset.dataIndices.length; i++) {
+        const dataIndexConfig = sampleDataset.dataIndices[i];
+        const index =
+          dataIndexConfig.indexName ?? createIndexName(sampleDataset.id, dataIndexConfig.id);
+
+        try {
+          await caller('indices.delete', { index });
+        } catch (err) {
+          return response.customError({
+            statusCode: err.status,
+            body: {
+              message: `Unable to delete sample data index "${index}", error: ${err.message}`,
+            },
+          });
+        }
+      }
+
       // track the usage operation in a non-blocking way
       usageTracker.addUninstall(request.params.id);
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		fix:
		- [Workspace]Fix error toasts in sample data page ([#8842](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8842))