[workspace] Fix config related issues and dedup the category in landi…

…ng page (opensearch-project#8160) * fix: do not automatically append workspaces params when creating config Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * fix: find global configs when upgrade config Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: dedup category in landing page Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * Changeset file for PR opensearch-project#8160 created/updated --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com> Co-authored-by: SuZhou-Joe <suzhou@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
bandinib-amzn · Sep 12, 2024 · 07c7fa1 · 07c7fa1
1 parent ba8c50b
commit 07c7fa1
Show file tree

Hide file tree

Showing 7 changed files with 154 additions and 4 deletions.
diff --git a/changelogs/fragments/8160.yml 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))
diff --git 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/...ns/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/...ns/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<SavedObject, 'id'> = {
@@ -13,6 +14,13 @@ const dashboard: Omit<SavedObject, 'id'> = {
   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/...ace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts b/...ace/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
@@ -12,8 +12,11 @@ import {
   SavedObjectsCheckConflictsObject,
   OpenSearchDashboardsRequest,
   SavedObjectsFindOptions,
+  SavedObject,
 } from '../../../../core/server';
 
+const UI_SETTINGS_SAVED_OBJECTS_TYPE = 'config';
+
 type WorkspaceOptions = Pick<SavedObjectsBaseOptions, 'workspaces'> | undefined;
 
 export class WorkspaceIdConsumerWrapper {
@@ -39,14 +42,20 @@ export class WorkspaceIdConsumerWrapper {
     };
   }
 
+  private isConfigType(type: string): boolean {
+    return type === UI_SETTINGS_SAVED_OBJECTS_TYPE;
+  }
+
   public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => {
     return {
       ...wrapperOptions.client,
       create: <T>(type: string, attributes: T, options: SavedObjectsCreateOptions = {}) =>
         wrapperOptions.client.create(
           type,
           attributes,
-          this.formatWorkspaceIdParams(wrapperOptions.request, options)
+          this.isConfigType(type)
+            ? options
+            : this.formatWorkspaceIdParams(wrapperOptions.request, options)
         ),
       bulkCreate: <T = unknown>(
         objects: Array<SavedObjectsBulkCreateObject<T>>,
@@ -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
@@ -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
@@ -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],