diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/api/run_save_functions.tsx b/src/plugins/dashboard/public/dashboard_container/embeddable/api/run_save_functions.tsx index 049bfaf00e5dd..67cad4bd7bf83 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/api/run_save_functions.tsx +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/api/run_save_functions.tsx @@ -275,21 +275,18 @@ export async function runInteractiveSave(this: DashboardContainer, interactionMo if (lastSavedId) { const [baseTitle, baseCount] = extractTitleAndCount(newTitle); - let copyCount = baseCount + 1; - newTitle = `${baseTitle} (${copyCount})`; - - // increment count until we find a unique title - while ( - !(await checkForDuplicateDashboardTitle({ - title: newTitle, - lastSavedTitle: currentState.title, - copyOnSave: true, - isTitleDuplicateConfirmed: false, - })) - ) { - copyCount++; - newTitle = `${baseTitle} (${copyCount})`; - } + + newTitle = `${baseTitle} (${baseCount + 1})`; + + await checkForDuplicateDashboardTitle({ + title: newTitle, + lastSavedTitle: currentState.title, + copyOnSave: true, + isTitleDuplicateConfirmed: false, + onTitleDuplicate(speculativeSuggestion) { + newTitle = speculativeSuggestion; + }, + }); switch (interactionMode) { case ViewMode.EDIT: { diff --git a/src/plugins/dashboard/public/services/dashboard_content_management/lib/check_for_duplicate_dashboard_title.test.ts b/src/plugins/dashboard/public/services/dashboard_content_management/lib/check_for_duplicate_dashboard_title.test.ts new file mode 100644 index 0000000000000..edaae6a8b0d92 --- /dev/null +++ b/src/plugins/dashboard/public/services/dashboard_content_management/lib/check_for_duplicate_dashboard_title.test.ts @@ -0,0 +1,120 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { ContentClient } from '@kbn/content-management-plugin/public'; +import { checkForDuplicateDashboardTitle } from './check_for_duplicate_dashboard_title'; +import { extractTitleAndCount } from '../../../dashboard_container/embeddable/api/lib/extract_title_and_count'; + +type ContentManagementStart = Parameters[1]; + +describe('checkForDuplicateDashboardTitle', () => { + const mockedContentManagementClient = { + search: jest.fn(), + } as unknown as ContentClient; + + const newTitle = 'Shiny dashboard (1)'; + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('will only search using the dashboard basename', async () => { + const [baseDashboardName] = extractTitleAndCount(newTitle); + + const pageResults = [ + { + attributes: { + title: baseDashboardName, + }, + }, + ]; + + ( + mockedContentManagementClient.search as jest.MockedFunction + ).mockImplementationOnce(() => + Promise.resolve({ + hits: pageResults, + pagination: { + total: pageResults.length, + }, + }) + ); + + await checkForDuplicateDashboardTitle( + { + title: newTitle, + lastSavedTitle: baseDashboardName, + copyOnSave: true, + isTitleDuplicateConfirmed: false, + }, + { client: mockedContentManagementClient } as ContentManagementStart + ); + + expect(mockedContentManagementClient.search).toHaveBeenCalledWith( + expect.objectContaining({ + query: expect.objectContaining({ + text: `${baseDashboardName}*`, + }), + }) + ); + }); + + it('invokes onTitleDuplicate with a speculative collision free value when the new title provided is a duplicate match', async () => { + const [baseDashboardName] = extractTitleAndCount(newTitle); + + const userTitleInput = `${baseDashboardName} (10)`; + + const pageResults = [ + { + attributes: { + title: baseDashboardName, + }, + }, + ].concat( + Array.from(new Array(5)).map((_, idx) => ({ + attributes: { + title: `${baseDashboardName} (${10 + idx})`, + }, + })) + ); + + const onTitleDuplicate = jest.fn(); + + ( + mockedContentManagementClient.search as jest.MockedFunction + ).mockImplementationOnce(() => + Promise.resolve({ + hits: pageResults, + pagination: { + total: pageResults.length, + }, + }) + ); + + await checkForDuplicateDashboardTitle( + { + title: userTitleInput, + lastSavedTitle: baseDashboardName, + copyOnSave: true, + isTitleDuplicateConfirmed: false, + onTitleDuplicate, + }, + { client: mockedContentManagementClient } as ContentManagementStart + ); + + expect(mockedContentManagementClient.search).toHaveBeenCalledWith( + expect.objectContaining({ + query: expect.objectContaining({ + text: 'Shiny dashboard*', + }), + }) + ); + + expect(onTitleDuplicate).toHaveBeenCalledWith(`${baseDashboardName} (15)`); + }); +}); diff --git a/src/plugins/dashboard/public/services/dashboard_content_management/lib/check_for_duplicate_dashboard_title.ts b/src/plugins/dashboard/public/services/dashboard_content_management/lib/check_for_duplicate_dashboard_title.ts index 0ffcf8c9788e9..a4bd93191a487 100644 --- a/src/plugins/dashboard/public/services/dashboard_content_management/lib/check_for_duplicate_dashboard_title.ts +++ b/src/plugins/dashboard/public/services/dashboard_content_management/lib/check_for_duplicate_dashboard_title.ts @@ -9,12 +9,16 @@ import { DashboardStartDependencies } from '../../../plugin'; import { DASHBOARD_CONTENT_ID } from '../../../dashboard_constants'; import { DashboardCrudTypes } from '../../../../common/content_management'; +import { extractTitleAndCount } from '../../../dashboard_container/embeddable/api/lib/extract_title_and_count'; export interface DashboardDuplicateTitleCheckProps { title: string; copyOnSave: boolean; lastSavedTitle: string; - onTitleDuplicate?: () => void; + /** + * invokes the onTitleDuplicate function if provided with a speculative title that should be collision free + */ + onTitleDuplicate?: (speculativeSuggestion: string) => void; isTitleDuplicateConfirmed: boolean; } @@ -33,6 +37,11 @@ export async function checkForDuplicateDashboardTitle( }: DashboardDuplicateTitleCheckProps, contentManagement: DashboardStartDependencies['contentManagement'] ): Promise { + // Don't check if the title is an empty string + if (!title) { + return true; + } + // Don't check for duplicates if user has already confirmed save with duplicate title if (isTitleDuplicateConfirmed) { return true; @@ -44,21 +53,37 @@ export async function checkForDuplicateDashboardTitle( return true; } + const [baseDashboardName] = extractTitleAndCount(title); + const { hits } = await contentManagement.client.search< DashboardCrudTypes['SearchIn'], DashboardCrudTypes['SearchOut'] >({ contentTypeId: DASHBOARD_CONTENT_ID, query: { - text: title ? `${title}*` : undefined, - limit: 10, + text: `${baseDashboardName}*`, + limit: 20, + }, + options: { + onlyTitle: true, }, - options: { onlyTitle: true }, }); - const duplicate = hits.find((hit) => hit.attributes.title.toLowerCase() === title.toLowerCase()); + + const duplicate = Boolean( + hits.find((hit) => hit.attributes.title.toLowerCase() === title.toLowerCase()) + ); + if (!duplicate) { return true; } - onTitleDuplicate?.(); + + const [largestDuplicationId] = hits + .map((hit) => extractTitleAndCount(hit.attributes.title)[1]) + .sort((a, b) => b - a); + + const speculativeCollisionFreeTitle = `${baseDashboardName} (${largestDuplicationId + 1})`; + + onTitleDuplicate?.(speculativeCollisionFreeTitle); + return false; } diff --git a/test/functional/apps/dashboard/group4/dashboard_clone.ts b/test/functional/apps/dashboard/group4/dashboard_clone.ts index 1e15f4d44e740..25ad42ac10fe7 100644 --- a/test/functional/apps/dashboard/group4/dashboard_clone.ts +++ b/test/functional/apps/dashboard/group4/dashboard_clone.ts @@ -49,5 +49,20 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.dashboard.gotoDashboardLandingPage(); await listingTable.searchAndExpectItemsCount('dashboard', `${dashboardName} (2)`, 1); }); + + it('Clone should always increment from the last duplicated dashboard with a unique title', async function () { + await PageObjects.dashboard.loadSavedDashboard(clonedDashboardName); + // force dashboard duplicate id to increment out of logical progression bounds + await PageObjects.dashboard.duplicateDashboard(`${dashboardName} (20)`); + await PageObjects.dashboard.gotoDashboardLandingPage(); + await listingTable.searchAndExpectItemsCount('dashboard', `${dashboardName} (20)`, 1); + // load dashboard with duplication id 1 + await PageObjects.dashboard.loadSavedDashboard(clonedDashboardName); + // run normal clone + await PageObjects.dashboard.duplicateDashboard(); + await PageObjects.dashboard.gotoDashboardLandingPage(); + // clone gets duplication id, that picks off from last duplicated dashboard + await listingTable.searchAndExpectItemsCount('dashboard', `${dashboardName} (21)`, 1); + }); }); }