Skip to content

Commit

Permalink
[workspace] Fix config related issues and dedup the category in landi…
Browse files Browse the repository at this point in the history
…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>
  • Loading branch information
3 people authored Sep 12, 2024
1 parent ba8c50b commit 07c7fa1
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 4 deletions.
3 changes: 3 additions & 0 deletions changelogs/fragments/8160.yml
Original file line number Diff line number Diff line change
@@ -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))
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'> = {
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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>>,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down

0 comments on commit 07c7fa1

Please sign in to comment.