From d1f276515ac28cefb49a40734c0bd40ae8c92ab1 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 4 Jun 2020 10:00:46 +0200 Subject: [PATCH] add description property and check duplicate title on save --- .../helpers/check_for_duplicate_title.ts | 7 ++- .../display_duplicate_title_confirm_modal.ts | 2 +- .../public/saved_object/index.ts | 2 +- .../vis_types/vis_type_alias_registry.ts | 1 + .../lens/public/app_plugin/app.test.tsx | 56 ++++++++++++++++++- x-pack/plugins/lens/public/app_plugin/app.tsx | 26 ++++++++- .../editor_frame_service/editor_frame/save.ts | 1 + .../editor_frame/state_management.test.ts | 2 + .../editor_frame/state_management.ts | 2 + .../persistence/saved_object_store.test.ts | 3 + .../public/persistence/saved_object_store.ts | 1 + x-pack/plugins/lens/public/vis_type_alias.ts | 3 +- x-pack/plugins/lens/server/saved_objects.ts | 3 + 13 files changed, 102 insertions(+), 7 deletions(-) diff --git a/src/plugins/saved_objects/public/saved_object/helpers/check_for_duplicate_title.ts b/src/plugins/saved_objects/public/saved_object/helpers/check_for_duplicate_title.ts index 8895336aa9ffa..0313b7978c5ab 100644 --- a/src/plugins/saved_objects/public/saved_object/helpers/check_for_duplicate_title.ts +++ b/src/plugins/saved_objects/public/saved_object/helpers/check_for_duplicate_title.ts @@ -31,10 +31,13 @@ import { displayDuplicateTitleConfirmModal } from './display_duplicate_title_con * @param services */ export async function checkForDuplicateTitle( - savedObject: SavedObject, + savedObject: Pick< + SavedObject, + 'id' | 'title' | 'getDisplayName' | 'lastSavedTitle' | 'copyOnSave' | 'getEsType' + >, isTitleDuplicateConfirmed: boolean, onTitleDuplicate: (() => void) | undefined, - services: SavedObjectKibanaServices + services: Pick ): Promise { const { savedObjectsClient, overlays } = services; // Don't check for duplicates if user has already confirmed save with duplicate title diff --git a/src/plugins/saved_objects/public/saved_object/helpers/display_duplicate_title_confirm_modal.ts b/src/plugins/saved_objects/public/saved_object/helpers/display_duplicate_title_confirm_modal.ts index 0b02977830fda..1b9e6fb6e996f 100644 --- a/src/plugins/saved_objects/public/saved_object/helpers/display_duplicate_title_confirm_modal.ts +++ b/src/plugins/saved_objects/public/saved_object/helpers/display_duplicate_title_confirm_modal.ts @@ -23,7 +23,7 @@ import { confirmModalPromise } from './confirm_modal_promise'; import { SavedObject } from '../../types'; export function displayDuplicateTitleConfirmModal( - savedObject: SavedObject, + savedObject: Pick, overlays: OverlayStart ): Promise { const confirmMessage = i18n.translate( diff --git a/src/plugins/saved_objects/public/saved_object/index.ts b/src/plugins/saved_objects/public/saved_object/index.ts index 178ffaf88f4be..f189fe724abab 100644 --- a/src/plugins/saved_objects/public/saved_object/index.ts +++ b/src/plugins/saved_objects/public/saved_object/index.ts @@ -19,6 +19,6 @@ export { createSavedObjectClass } from './saved_object'; export { SavedObjectLoader } from './saved_object_loader'; -export { checkForDuplicateTitle } from './helpers/check_for_duplicate_title'; +export { checkForDuplicateTitle, DuplicateTitleError } from './helpers/check_for_duplicate_title'; export { saveWithConfirmation } from './helpers/save_with_confirmation'; export { isErrorNonFatal } from './helpers/save_saved_object'; diff --git a/src/plugins/visualizations/public/vis_types/vis_type_alias_registry.ts b/src/plugins/visualizations/public/vis_types/vis_type_alias_registry.ts index 6f18cbc5026ec..73e3360004e5a 100644 --- a/src/plugins/visualizations/public/vis_types/vis_type_alias_registry.ts +++ b/src/plugins/visualizations/public/vis_types/vis_type_alias_registry.ts @@ -25,6 +25,7 @@ export interface VisualizationListItem { stage: 'experimental' | 'beta' | 'production'; savedObjectType: string; title: string; + description?: string; typeTitle: string; } diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index f1a2edd2d554f..26866949a8895 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -13,7 +13,10 @@ import { AppMountParameters } from 'kibana/public'; import { Storage } from '../../../../../src/plugins/kibana_utils/public'; import { Document, SavedObjectStore } from '../persistence'; import { mount } from 'enzyme'; -import { SavedObjectSaveModal } from '../../../../../src/plugins/saved_objects/public'; +import { + SavedObjectSaveModal, + checkForDuplicateTitle, +} from '../../../../../src/plugins/saved_objects/public'; import { esFilters, FilterManager, @@ -29,6 +32,17 @@ import { coreMock } from 'src/core/public/mocks'; jest.mock('../persistence'); jest.mock('src/core/public'); +jest.mock('../../../../../src/plugins/saved_objects/public', () => { + // eslint-disable-next-line no-shadow + const { SavedObjectSaveModal, SavedObjectSaveModalOrigin } = jest.requireActual( + '../../../../../src/plugins/saved_objects/public' + ); + return { + SavedObjectSaveModal, + SavedObjectSaveModalOrigin, + checkForDuplicateTitle: jest.fn(), + }; +}); const navigationStartMock = navigationPluginMock.createStartContract(); @@ -633,6 +647,46 @@ describe('Lens App', () => { }); }); + it('checks for duplicate title before saving', async () => { + const args = defaultArgs; + args.editorFrame = frame; + (args.docStorage.save as jest.Mock).mockReturnValue(Promise.resolve({ id: '123' })); + + instance = mount(); + + const onChange = frame.mount.mock.calls[0][1].onChange; + await act(async () => + onChange({ + filterableIndexPatterns: [], + doc: ({ id: '123', expression: 'valid expression' } as unknown) as Document, + }) + ); + instance.update(); + await act(async () => { + getButton(instance).run(instance.getDOMNode()); + }); + instance.update(); + + const onTitleDuplicate = jest.fn(); + + await act(async () => { + instance.find(SavedObjectSaveModal).prop('onSave')({ + onTitleDuplicate, + isTitleDuplicateConfirmed: false, + newCopyOnSave: false, + newDescription: '', + newTitle: 'test', + }); + }); + + expect(checkForDuplicateTitle).toHaveBeenCalledWith( + expect.objectContaining({ id: '123' }), + false, + onTitleDuplicate, + expect.anything() + ); + }); + it('does not show the copy button on first save', async () => { const args = defaultArgs; args.editorFrame = frame; diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index ffa59a6fb6bc9..f00d688ca6202 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -16,6 +16,7 @@ import { KibanaContextProvider } from '../../../../../src/plugins/kibana_react/p import { SavedObjectSaveModalOrigin, OnSaveProps, + checkForDuplicateTitle, } from '../../../../../src/plugins/saved_objects/public'; import { Document, SavedObjectStore } from '../persistence'; import { EditorFrameInstance } from '../types'; @@ -230,9 +231,11 @@ export function App({ // state.persistedDoc, ]); - const runSave = ( + const runSave = async ( saveProps: Omit & { returnToOrigin: boolean; + onTitleDuplicate?: OnSaveProps['onTitleDuplicate']; + newDescription?: string; } ) => { if (!lastKnownDoc) { @@ -254,10 +257,30 @@ export function App({ const doc = { ...lastDocWithoutPinned, + description: saveProps.newDescription, id: saveProps.newCopyOnSave ? undefined : lastKnownDoc.id, title: saveProps.newTitle, }; + await checkForDuplicateTitle( + { + ...doc, + copyOnSave: saveProps.newCopyOnSave, + lastSavedTitle: lastKnownDoc?.title, + getEsType: () => 'lens', + getDisplayName: () => + i18n.translate('xpack.lens.app.saveModalType', { + defaultMessage: 'Lens visualization', + }), + }, + saveProps.isTitleDuplicateConfirmed, + saveProps.onTitleDuplicate, + { + savedObjectsClient: core.savedObjects.client, + overlays: core.overlays, + } + ); + const newlyCreated: boolean = saveProps.newCopyOnSave || !lastKnownDoc?.id; docStorage .save(doc) @@ -470,6 +493,7 @@ export function App({ documentInfo={{ id: lastKnownDoc.id, title: lastKnownDoc.title || '', + description: lastKnownDoc.description || '', }} objectType={i18n.translate('xpack.lens.app.saveModalType', { defaultMessage: 'Lens visualization', diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/save.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/save.ts index b292299c569af..d62f3dbcf029a 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/save.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/save.ts @@ -48,6 +48,7 @@ export function getSavedObjectFormat({ return { id: state.persistedId, title: state.title, + description: state.description, type: 'lens', visualizationType: state.visualization.activeId, expression: expression ? toExpression(expression) : '', diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.test.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.test.ts index 71aabaae3c65c..e1151b92aac51 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.test.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.test.ts @@ -388,6 +388,7 @@ describe('editor_frame state management', () => { filters: [], }, title: 'heyo!', + description: 'My lens', type: 'lens', visualizationType: 'line', }, @@ -406,6 +407,7 @@ describe('editor_frame state management', () => { }, persistedId: 'b', title: 'heyo!', + description: 'My lens', visualization: { activeId: 'line', state: { diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts index bb6daf5641a64..09674ebf2ade2 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts @@ -18,6 +18,7 @@ export interface PreviewState { export interface EditorFrameState extends PreviewState { persistedId?: string; title: string; + description?: string; stagedPreview?: PreviewState; activeDatasourceId: string | null; } @@ -157,6 +158,7 @@ export const reducer = (state: EditorFrameState, action: Action): EditorFrameSta ...state, persistedId: action.doc.id, title: action.doc.title, + description: action.doc.description, datasourceStates: Object.entries(action.doc.state.datasourceStates).reduce( (stateMap, [datasourceId, datasourceState]) => ({ ...stateMap, diff --git a/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts b/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts index 515d008d82586..f7caac6549389 100644 --- a/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts +++ b/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts @@ -25,6 +25,7 @@ describe('LensStore', () => { const { client, store } = testStore('FOO'); const doc = await store.save({ title: 'Hello', + description: 'My doc', visualizationType: 'bar', expression: '', state: { @@ -43,6 +44,7 @@ describe('LensStore', () => { expect(doc).toEqual({ id: 'FOO', title: 'Hello', + description: 'My doc', visualizationType: 'bar', expression: '', state: { @@ -61,6 +63,7 @@ describe('LensStore', () => { expect(client.create).toHaveBeenCalledTimes(1); expect(client.create).toHaveBeenCalledWith('lens', { title: 'Hello', + description: 'My doc', visualizationType: 'bar', expression: '', state: { diff --git a/x-pack/plugins/lens/public/persistence/saved_object_store.ts b/x-pack/plugins/lens/public/persistence/saved_object_store.ts index 015f4b9b825f4..7632be3d82046 100644 --- a/x-pack/plugins/lens/public/persistence/saved_object_store.ts +++ b/x-pack/plugins/lens/public/persistence/saved_object_store.ts @@ -13,6 +13,7 @@ export interface Document { type?: string; visualizationType: string | null; title: string; + description?: string; expression: string | null; state: { datasourceMetaData: { diff --git a/x-pack/plugins/lens/public/vis_type_alias.ts b/x-pack/plugins/lens/public/vis_type_alias.ts index a58288191325c..3bb2dbbae1f9c 100644 --- a/x-pack/plugins/lens/public/vis_type_alias.ts +++ b/x-pack/plugins/lens/public/vis_type_alias.ts @@ -34,10 +34,11 @@ export const getLensAliasConfig = (): VisTypeAlias => ({ searchFields: ['title^3'], toListItem(savedObject) { const { id, type, attributes } = savedObject; - const { title } = attributes as { title: string }; + const { title, description } = attributes as { title: string; description?: string }; return { id, title, + description, editUrl: getEditPath(id), editApp: 'lens', icon: 'lensApp', diff --git a/x-pack/plugins/lens/server/saved_objects.ts b/x-pack/plugins/lens/server/saved_objects.ts index 1f7d22e2b5642..a16cc3dab7967 100644 --- a/x-pack/plugins/lens/server/saved_objects.ts +++ b/x-pack/plugins/lens/server/saved_objects.ts @@ -29,6 +29,9 @@ export function setupSavedObjects(core: CoreSetup) { title: { type: 'text', }, + description: { + type: 'text', + }, visualizationType: { type: 'keyword', },