Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] [workspace] Fix config related issues and dedup the category in landing page #8162

Merged
merged 1 commit into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading