From 5d395a85f9b1533d8d6167f0508ad4c335b85383 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Thu, 12 Sep 2024 22:19:26 +0800 Subject: [PATCH] [workspace] Fix config related issues and dedup the category in landing page (#8160) (#8162) * fix: do not automatically append workspaces params when creating config * fix: find global configs when upgrade config * feat: dedup category in landing page * Changeset file for PR #8160 created/updated --------- (cherry picked from commit 07c7fa19b73d725f35f292a144ce85ca83835cfa) Signed-off-by: SuZhou-Joe Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] Co-authored-by: SuZhou-Joe Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/8160.yml | 3 ++ .../feature_cards/feature_cards.tsx | 2 +- .../workspace_id_consumer_wrapper.test.ts | 35 +++++++++++++ ...space_saved_objects_client_wrapper.test.ts | 52 +++++++++++++++++++ .../workspace_id_consumer_wrapper.ts | 18 ++++++- ...space_saved_objects_client_wrapper.test.ts | 29 +++++++++++ .../workspace_saved_objects_client_wrapper.ts | 19 ++++++- 7 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/8160.yml diff --git a/changelogs/fragments/8160.yml b/changelogs/fragments/8160.yml new file mode 100644 index 000000000000..2935c9abff02 --- /dev/null +++ b/changelogs/fragments/8160.yml @@ -0,0 +1,3 @@ +fix: +- Config related issues ([#8160](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8160)) +- Dedup the category ([#8160](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8160)) \ No newline at end of file diff --git a/src/plugins/management/public/components/feature_cards/feature_cards.tsx b/src/plugins/management/public/components/feature_cards/feature_cards.tsx index 73a0e300dacb..d308f185d602 100644 --- a/src/plugins/management/public/components/feature_cards/feature_cards.tsx +++ b/src/plugins/management/public/components/feature_cards/feature_cards.tsx @@ -37,7 +37,7 @@ export const FeatureCards = ({ // so it is safe to group the links here. navLinks.forEach((link) => { let lastGroup = grouped.length ? grouped[grouped.length - 1] : undefined; - if (!lastGroup || lastGroup.category !== link.category) { + if (!lastGroup || lastGroup.category?.id !== link.category?.id) { lastGroup = { category: link.category, navLinks: [[]] }; grouped.push(lastGroup); } diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts index 84d09af94362..1586a7dfa9b2 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -5,6 +5,7 @@ import { SavedObject } from 'src/core/types'; import { isEqual } from 'lodash'; +import packageInfo from '../../../../../../package.json'; import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; const dashboard: Omit = { @@ -13,6 +14,13 @@ const dashboard: Omit = { references: [], }; +const config: SavedObject = { + type: 'config', + attributes: {}, + references: [], + id: `config:${packageInfo.version}`, +}; + interface WorkspaceAttributes { id: string; name?: string; @@ -113,6 +121,33 @@ describe('workspace_id_consumer integration test', () => { }); }); + it('create should not append requestWorkspaceId automatically when the type is config', async () => { + await osdTestServer.request.delete( + root, + `/api/saved_objects/${config.type}/${packageInfo.version}` + ); + + // Get page to trigger create config and it should return 200 + await osdTestServer.request + .post( + root, + `/w/${createdFooWorkspace.id}/api/saved_objects/${config.type}/${packageInfo.version}` + ) + .send({ + attributes: { + legacyConfig: 'foo', + }, + }) + .expect(200); + const getConfigResult = await osdTestServer.request.get( + root, + `/api/saved_objects/${config.type}/${packageInfo.version}` + ); + + // workspaces arrtibutes should not be append + expect(!getConfigResult.body.workspaces).toEqual(true); + }); + it('bulk create', async () => { await clearFooAndBar(); const createResultFoo = await osdTestServer.request diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts index 607d55d8d5dd..b5ab4210f1ba 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts @@ -14,6 +14,7 @@ import { WORKSPACE_TYPE, ISavedObjectsRepository, SavedObjectsClientContract, + SavedObjectsBulkCreateObject, } from '../../../../../core/server'; import { httpServerMock } from '../../../../../../src/core/server/mocks'; import * as utilsExports from '../../../../../core/server/utils/auth_info'; @@ -299,6 +300,57 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect.arrayContaining([expect.objectContaining({ id: 'acl-controlled-dashboard-2' })]) ); }); + + it('should return global non-user-level configs when search with sortField buildNum', async () => { + const configsForCreation: SavedObjectsBulkCreateObject[] = [ + { + id: 'user_foo', + type: 'config', + attributes: {}, + }, + { + id: 'user_bar', + type: 'config', + attributes: {}, + }, + { + id: 'global_config', + type: 'config', + attributes: { + buildNum: 1, + }, + }, + ]; + await permittedSavedObjectedClient.bulkCreate(configsForCreation); + const result = await permittedSavedObjectedClient.find({ + type: 'config', + sortField: 'buildNum', + perPage: 999, + page: 1, + }); + + const resultForFindConfig = await permittedSavedObjectedClient.find({ + type: 'config', + perPage: 999, + page: 1, + }); + + expect(result.saved_objects).toEqual( + expect.arrayContaining([expect.objectContaining({ id: 'global_config' })]) + ); + expect(result.saved_objects.length).toEqual(1); + expect(result.total).toEqual(1); + + // Should not be able to find global config if do not find with `sortField: 'buildNum'` + expect(resultForFindConfig.saved_objects.length).toEqual(0); + + // clean up the test configs + await Promise.all( + configsForCreation.map((config) => + permittedSavedObjectedClient.delete(config.type, config.id as string) + ) + ); + }); }); describe('create', () => { diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 1f9e196d1f49..1871a3a9b9f1 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -12,8 +12,11 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, + SavedObject, } from '../../../../core/server'; +const UI_SETTINGS_SAVED_OBJECTS_TYPE = 'config'; + type WorkspaceOptions = Pick | undefined; export class WorkspaceIdConsumerWrapper { @@ -39,6 +42,10 @@ export class WorkspaceIdConsumerWrapper { }; } + private isConfigType(type: string): boolean { + return type === UI_SETTINGS_SAVED_OBJECTS_TYPE; + } + public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { return { ...wrapperOptions.client, @@ -46,7 +53,9 @@ export class WorkspaceIdConsumerWrapper { wrapperOptions.client.create( type, attributes, - this.formatWorkspaceIdParams(wrapperOptions.request, options) + this.isConfigType(type) + ? options + : this.formatWorkspaceIdParams(wrapperOptions.request, options) ), bulkCreate: ( objects: Array>, @@ -67,7 +76,12 @@ export class WorkspaceIdConsumerWrapper { delete: wrapperOptions.client.delete, find: (options: SavedObjectsFindOptions) => { return wrapperOptions.client.find( - this.formatWorkspaceIdParams(wrapperOptions.request, options) + // Based on https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/core/server/ui_settings/create_or_upgrade_saved_config/get_upgradeable_config.ts#L49 + // we need to make sure the find call for upgrade config should be able to find all the global configs as it was before. + // It is a workaround for 2.17, should be optimized in the upcoming 2.18 release. + this.isConfigType(options.type as string) && options.sortField === 'buildNum' + ? options + : this.formatWorkspaceIdParams(wrapperOptions.request, options) ); }, bulkGet: wrapperOptions.client.bulkGet, diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts index 6e3b7f24e62f..c5424c364eb9 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts @@ -872,6 +872,35 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { type: [DATA_SOURCE_SAVED_OBJECT_TYPE], }); }); + it('should call client.find without ACLSearchParams and workspaceOperator when find config and the sortField is buildNum', async () => { + const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper( + DATASOURCE_ADMIN + ); + clientMock.find.mockImplementation(() => ({ + saved_objects: [ + { + id: 'global_config', + type: 'config', + attributes: { + buildNum: 1, + }, + }, + { + id: 'user_config', + type: 'config', + }, + ], + })); + const findResult = await wrapper.find({ + type: 'config', + sortField: 'buildNum', + }); + expect(clientMock.find).toHaveBeenCalledWith({ + type: 'config', + sortField: 'buildNum', + }); + expect(findResult.saved_objects.length).toEqual(1); + }); }); describe('deleteByWorkspace', () => { diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index 6cf972a8a9ab..946ce9369e4e 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -524,7 +524,24 @@ export class WorkspaceSavedObjectsClientWrapper { }) ).saved_objects.map((item) => item.id); - if (!options.workspaces && !options.ACLSearchParams) { + // Based on https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/core/server/ui_settings/create_or_upgrade_saved_config/get_upgradeable_config.ts#L49 + // we need to make sure the find call for upgrade config should be able to find all the global configs as it was before. + // It is a workaround for 2.17, should be optimized in the upcoming 2.18 release. + if (options.type === 'config' && options.sortField === 'buildNum') { + const findResult = await wrapperOptions.client.find<{ buildNum?: number }>(options); + + // There maybe user settings inside the find result, + // so that we need to filter out user configs(user configs are the configs without buildNum attribute). + const finalSavedObjects = findResult.saved_objects.filter( + (savedObject) => !!savedObject.attributes?.buildNum + ); + + return { + ...findResult, + total: finalSavedObjects.length, + saved_objects: finalSavedObjects, + }; + } else if (!options.workspaces && !options.ACLSearchParams) { options.workspaces = permittedWorkspaceIds; options.ACLSearchParams = { permissionModes: [WorkspacePermissionMode.Read, WorkspacePermissionMode.Write],