Skip to content

Commit

Permalink
[ContentManagement] Fix Visualize List search and CRUD operations via…
Browse files Browse the repository at this point in the history
… CM (elastic#165485)

## Summary

Fix elastic#163246

This PR fixes the CM problems within the Visualize List page leveraging
the services already in place.
The approach here is lighter than elastic#165292 as it passes each client via
the TypesService already used to register extensions in the
Visualization scope. Also the `search` method now transparently uses the
`mSearch` if more content types are detected without leaking any
implementation detail outside the `VisualizationClient` interface.

More fixes/features:
* fixed Maps update operation definition which was missing the
`overwrite` flag
* Allow `mSearch` to accept an options object argument
* Added new helper functions to interact with the metadata flyout in
Listing page


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Stratoula Kalafateli <[email protected]>
  • Loading branch information
dej611 and stratoula authored Sep 13, 2023
1 parent 691311c commit 01ad4c2
Show file tree
Hide file tree
Showing 33 changed files with 492 additions and 418 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ const deleteVisualization = async (id: string) => {
};

const search = async (query: SearchQuery = {}, options?: VisualizationSearchQuery) => {
if (options && options.types && options.types.length > 1) {
const { types } = options;
return getContentManagement().client.mSearch<VisualizationSearchOut['hits'][number]>({
contentTypes: types.map((type) => ({ contentTypeId: type })),
query,
});
}
return getContentManagement().client.search<VisualizationSearchIn, VisualizationSearchOut>({
contentTypeId: 'visualization',
query,
Expand Down
10 changes: 9 additions & 1 deletion src/plugins/visualizations/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ export { getVisSchemas } from './vis_schemas';
/** @public types */
export type { VisualizationsSetup, VisualizationsStart };
export { VisGroups } from './vis_types/vis_groups_enum';
export type { BaseVisType, VisTypeAlias, VisTypeDefinition, Schema, ISchemas } from './vis_types';
export type {
BaseVisType,
VisTypeAlias,
VisTypeDefinition,
Schema,
ISchemas,
VisualizationClient,
SerializableAttributes,
} from './vis_types';
export type { Vis, SerializedVis, SerializedVisData, VisData } from './vis';
export type VisualizeEmbeddableFactoryContract = PublicContract<VisualizeEmbeddableFactory>;
export type VisualizeEmbeddableContract = PublicContract<VisualizeEmbeddable>;
Expand Down
1 change: 1 addition & 0 deletions src/plugins/visualizations/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ export class VisualizationsPlugin
unifiedSearch: pluginsStart.unifiedSearch,
serverless: pluginsStart.serverless,
noDataPage: pluginsStart.noDataPage,
contentManagement: pluginsStart.contentManagement,
};

params.element.classList.add('visAppWrapper');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,42 @@
import type { SavedObjectsTaggingApi } from '@kbn/saved-objects-tagging-oss-plugin/public';
import type { OverlayStart } from '@kbn/core-overlays-browser';

import { ContentManagementPublicStart } from '@kbn/content-management-plugin/public';
import { extractReferences } from '../saved_visualization_references';
import { visualizationsClient } from '../../content_management';
import { TypesStart } from '../../vis_types';

interface UpdateBasicSoAttributesDependencies {
savedObjectsTagging?: SavedObjectsTaggingApi;
overlays: OverlayStart;
typesService: TypesStart;
contentManagement: ContentManagementPublicStart;
}

function getClientForType(
type: string,
typesService: TypesStart,
contentManagement: ContentManagementPublicStart
) {
const visAliases = typesService.getAliases();
return (
visAliases
.find((v) => v.appExtensions?.visualizations.docTypes.includes(type))
?.appExtensions?.visualizations.client(contentManagement) || visualizationsClient
);
}

function getAdditionalOptionsForUpdate(
type: string,
typesService: TypesStart,
method: 'update' | 'create'
) {
const visAliases = typesService.getAliases();
const aliasType = visAliases.find((v) => v.appExtensions?.visualizations.docTypes.includes(type));
if (!aliasType) {
return { overwrite: true };
}
return aliasType?.appExtensions?.visualizations?.clientOptions?.[method];
}

export const updateBasicSoAttributes = async (
Expand All @@ -27,7 +57,9 @@ export const updateBasicSoAttributes = async (
},
dependencies: UpdateBasicSoAttributesDependencies
) => {
const so = await visualizationsClient.get(soId);
const client = getClientForType(type, dependencies.typesService, dependencies.contentManagement);

const so = await client.get(soId);
const extractedReferences = extractReferences({
attributes: so.item.attributes,
references: so.item.references,
Expand All @@ -48,14 +80,14 @@ export const updateBasicSoAttributes = async (
);
}

return await visualizationsClient.update({
return await client.update({
id: soId,
data: {
...attributes,
},
options: {
overwrite: true,
references,
...getAdditionalOptionsForUpdate(type, dependencies.typesService, 'update'),
},
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ import {
injectSearchSourceReferences,
SerializedSearchSourceFields,
} from '@kbn/data-plugin/public';
import { SerializableRecord } from '@kbn/utility-types';
import { SavedVisState, VisSavedObject } from '../../types';

import { extractTimeSeriesReferences, injectTimeSeriesReferences } from './timeseries_references';
import { extractControlsReferences, injectControlsReferences } from './controls_references';
import type { SerializableAttributes } from '../../vis_types/vis_type_alias_registry';

export function extractReferences({
attributes,
references = [],
}: {
attributes: SerializableRecord;
attributes: SerializableAttributes;
references: SavedObjectReference[];
}) {
const updatedAttributes = { ...attributes };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ jest.mock('../services', () => ({
update: mockUpdateContent,
get: mockGetContent,
search: mockFindContent,
mSearch: mockFindContent,
},
})),
}));
Expand Down Expand Up @@ -358,10 +359,11 @@ describe('saved_visualize_utils', () => {
expect(mockFindContent.mock.calls).toMatchObject([
[
{
options: {
types: ['bazdoc', 'etc', 'visualization'],
searchFields: ['baz', 'bing', 'title^3', 'description'],
},
contentTypes: [
{ contentTypeId: 'bazdoc' },
{ contentTypeId: 'etc' },
{ contentTypeId: 'visualization' },
],
},
],
]);
Expand Down Expand Up @@ -395,10 +397,12 @@ describe('saved_visualize_utils', () => {
expect(mockFindContent.mock.calls).toMatchObject([
[
{
options: {
types: ['bazdoc', 'bar', 'visualization', 'foo'],
searchFields: ['baz', 'bing', 'barfield', 'foofield', 'title^3', 'description'],
},
contentTypes: [
{ contentTypeId: 'bazdoc' },
{ contentTypeId: 'bar' },
{ contentTypeId: 'visualization' },
{ contentTypeId: 'foo' },
],
},
],
]);
Expand Down
1 change: 1 addition & 0 deletions src/plugins/visualizations/public/vis_types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ export { Schemas } from './schemas';
export { VisGroups } from './vis_groups_enum';
export { BaseVisType } from './base_vis_type';
export type { VisTypeDefinition, ISchemas, Schema } from './types';
export type { VisualizationClient, SerializableAttributes } from './vis_type_alias_registry';
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@
* Side Public License, v 1.
*/

import { SearchQuery } from '@kbn/content-management-plugin/common';
import { ContentManagementPublicStart } from '@kbn/content-management-plugin/public';
import {
ContentManagementCrudTypes,
SavedObjectCreateOptions,
SavedObjectUpdateOptions,
} from '@kbn/content-management-utils';
import type { SimpleSavedObject } from '@kbn/core/public';
import { BaseVisType } from './base_vis_type';

Expand All @@ -27,9 +34,54 @@ export interface VisualizationListItem {
type?: BaseVisType | string;
}

export interface SerializableAttributes {
[key: string]: unknown;
}

export type GenericVisualizationCrudTypes<
ContentType extends string,
Attr extends SerializableAttributes
> = ContentManagementCrudTypes<
ContentType,
Attr,
Pick<SavedObjectCreateOptions, 'overwrite' | 'references'>,
Pick<SavedObjectUpdateOptions, 'references'>,
object
>;

export interface VisualizationClient<
ContentType extends string = string,
Attr extends SerializableAttributes = SerializableAttributes
> {
get: (id: string) => Promise<GenericVisualizationCrudTypes<ContentType, Attr>['GetOut']>;
create: (
visualization: Omit<
GenericVisualizationCrudTypes<ContentType, Attr>['CreateIn'],
'contentTypeId'
>
) => Promise<GenericVisualizationCrudTypes<ContentType, Attr>['CreateOut']>;
update: (
visualization: Omit<
GenericVisualizationCrudTypes<ContentType, Attr>['UpdateIn'],
'contentTypeId'
>
) => Promise<GenericVisualizationCrudTypes<ContentType, Attr>['UpdateOut']>;
delete: (id: string) => Promise<GenericVisualizationCrudTypes<ContentType, Attr>['DeleteOut']>;
search: (
query: SearchQuery,
options?: object
) => Promise<GenericVisualizationCrudTypes<ContentType, Attr>['SearchOut']>;
}

export interface VisualizationsAppExtension {
docTypes: string[];
searchFields?: string[];
/** let each visualization client pass its own custom options if required */
clientOptions?: {
update?: { overwrite?: boolean; [otherOption: string]: unknown };
create?: { [otherOption: string]: unknown };
};
client: (contentManagement: ContentManagementPublicStart) => VisualizationClient;
toListItem: (savedObject: SimpleSavedObject<any>) => VisualizationListItem;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ const useTableListViewProps = (
overlays,
toastNotifications,
visualizeCapabilities,
contentManagement,
},
} = useKibana<VisualizeServices>();

Expand Down Expand Up @@ -176,11 +177,16 @@ const useTableListViewProps = (
description: args.description ?? '',
tags: args.tags,
},
{ overlays, savedObjectsTagging }
{
overlays,
savedObjectsTagging,
typesService: getTypes(),
contentManagement,
}
);
}
},
[overlays, savedObjectsTagging]
[overlays, savedObjectsTagging, contentManagement]
);

const contentEditorValidators: OpenContentEditorParams['customValidators'] = useMemo(
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/visualizations/public/visualize_app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import type { SavedObjectsTaggingApi } from '@kbn/saved-objects-tagging-oss-plug
import type { SavedSearch, SavedSearchPublicPluginStart } from '@kbn/saved-search-plugin/public';
import type { ServerlessPluginStart } from '@kbn/serverless/public';
import type { NoDataPagePluginStart } from '@kbn/no-data-page-plugin/public';
import { ContentManagementPublicStart } from '@kbn/content-management-plugin/public';
import type {
Vis,
VisualizeEmbeddableContract,
Expand Down Expand Up @@ -119,6 +120,7 @@ export interface VisualizeServices extends CoreStart {
unifiedSearch: UnifiedSearchPublicPluginStart;
serverless?: ServerlessPluginStart;
noDataPage?: NoDataPagePluginStart;
contentManagement: ContentManagementPublicStart;
}

export interface VisInstance {
Expand Down
12 changes: 4 additions & 8 deletions test/functional/apps/dashboard/group4/dashboard_listing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const PageObjects = getPageObjects(['dashboard', 'header', 'common']);
const browser = getService('browser');
const listingTable = getService('listingTable');
const testSubjects = getService('testSubjects');
const retry = getService('retry');
const dashboardAddPanel = getService('dashboardAddPanel');

describe('dashboard listing page', function describeIndexTests() {
Expand Down Expand Up @@ -217,12 +215,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {

await PageObjects.dashboard.gotoDashboardLandingPage();
await listingTable.searchForItemWithName(`${dashboardName}-editMetaData`);
await testSubjects.click('inspect-action');
await testSubjects.setValue('nameInput', 'new title');
await testSubjects.setValue('descriptionInput', 'new description');
await retry.try(async () => {
await testSubjects.click('saveButton');
await testSubjects.missingOrFail('flyoutTitle');
await listingTable.inspectVisualization();
await listingTable.editVisualizationDetails({
title: 'new title',
description: 'new description',
});

await listingTable.searchAndExpectItemsCount('dashboard', 'new title', 1);
Expand Down
17 changes: 17 additions & 0 deletions test/functional/apps/visualize/group3/_visualize_listing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,5 +84,22 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await listingTable.expectItemsCount('visualize', 0);
});
});

describe('Edit', () => {
before(async () => {
await PageObjects.visualize.gotoVisualizationLandingPage();
});

it('should edit the title and description of a visualization', async () => {
await listingTable.searchForItemWithName('Hello');
await listingTable.inspectVisualization();
await listingTable.editVisualizationDetails({
title: 'new title',
description: 'new description',
});
await listingTable.searchForItemWithName('new title');
await listingTable.expectItemsCount('visualize', 1);
});
});
});
}
29 changes: 29 additions & 0 deletions test/functional/services/listing_table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,35 @@ export class ListingTableService extends FtrService {
return visualizationNames;
}

/**
* Open the inspect flyout
*/
public async inspectVisualization(index: number = 0) {
const inspectButtons = await this.testSubjects.findAll('inspect-action');
await inspectButtons[index].click();
}

/**
* Edit Visualization title and description in the flyout
*/
public async editVisualizationDetails(
{ title, description }: { title?: string; description?: string } = {},
shouldSave: boolean = true
) {
if (title) {
await this.testSubjects.setValue('nameInput', title);
}
if (description) {
await this.testSubjects.setValue('descriptionInput', description);
}
if (shouldSave) {
await this.retry.try(async () => {
await this.testSubjects.click('saveButton');
await this.testSubjects.missingOrFail('flyoutTitle');
});
}
}

/**
* Returns items count on landing page
*/
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/lens/common/content_management/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export type {
LensSearchIn,
LensSearchOut,
LensSearchQuery,
LensCrudTypes,
} from './latest';

export * as LensV1 from './v1';
2 changes: 1 addition & 1 deletion x-pack/plugins/lens/common/content_management/v1/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ export type {
LensSearchIn,
LensSearchOut,
LensSearchQuery,
Reference,
LensCrudTypes,
} from './types';
Loading

0 comments on commit 01ad4c2

Please sign in to comment.