Skip to content

Commit

Permalink
[Workspace]Fix error toasts in sample data page (#8842)
Browse files Browse the repository at this point in the history
* Set default index pattern when workspace disabled

Signed-off-by: Lin Wang <[email protected]>

* Move saved objects first to avoid partial deleted

Signed-off-by: Lin Wang <[email protected]>

* Skip ui setting update for non workspace admin

Signed-off-by: Lin Wang <[email protected]>

* Add UT for sample_data_client

Signed-off-by: Lin Wang <[email protected]>

* Changeset file for PR #8842 created/updated

---------

Signed-off-by: Lin Wang <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit af429b6)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent eaadd99 commit 0c7bdd4
Show file tree
Hide file tree
Showing 6 changed files with 215 additions and 23 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/8842.yml
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))
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
SavedObjectsClientContract,
IUiSettingsClient,
ApplicationStart,
WorkspacesSetup,
} from 'opensearch-dashboards/public';
import { UiStatsMetricType } from '@osd/analytics';
import { TelemetryPluginStart } from '../../../telemetry/public';
Expand Down Expand Up @@ -77,6 +78,7 @@ export interface HomeOpenSearchDashboardsServices {
};
dataSource?: DataSourcePluginStart;
sectionTypes: SectionTypeService;
workspaces?: WorkspacesSetup;
}

let services: HomeOpenSearchDashboardsServices | null = null;
Expand Down
18 changes: 17 additions & 1 deletion src/plugins/home/public/application/sample_data_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -59,6 +74,7 @@ export async function uninstallSampleDataSet(id, sampleDataDefaultIndex, dataSou
const uiSettings = getServices().uiSettings;

if (
canUpdateUISetting() &&
!uiSettings.isDefault('defaultIndex') &&
uiSettings.get('defaultIndex') === sampleDataDefaultIndex
) {
Expand Down
167 changes: 167 additions & 0 deletions src/plugins/home/public/application/sample_data_client.test.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
1 change: 1 addition & 0 deletions src/plugins/home/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ export class HomePublicPlugin
injectedMetadata: coreStart.injectedMetadata,
dataSource,
sectionTypes: this.sectionTypeService,
workspaces: core.workspaces,
...homeOpenSearchDashboardsServices,
});
};
Expand Down
48 changes: 26 additions & 22 deletions src/plugins/home/server/services/sample_data/routes/uninstall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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({

Check warning on line 105 in src/plugins/home/server/services/sample_data/routes/uninstall.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/home/server/services/sample_data/routes/uninstall.ts#L105

Added line #L105 was not covered by tests
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);

Expand Down

0 comments on commit 0c7bdd4

Please sign in to comment.