-
Notifications
You must be signed in to change notification settings - Fork 8.6k
synchronously register embeddables and actions to fix race condition causing flaky add panel test #268218
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
synchronously register embeddables and actions to fix race condition causing flaky add panel test #268218
Changes from all commits
69ccde4
b8afc16
336c9f5
54ba095
b007923
570d2bb
fd9fae8
fec79a1
66f97b3
f39a253
4e85d9b
8a479dc
d0ea6b6
4382745
529cd33
6479db8
2e644a4
ab04951
32990f8
df58b19
a047691
b7d40da
d1ea476
646514a
2931dd1
55e7b81
00fa8ea
818c480
4b7ab2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,11 +15,11 @@ import type { | |
| import type { CoreSetup, CoreStart, Plugin } from '@kbn/core/public'; | ||
| import type { DashboardStart } from '@kbn/dashboard-plugin/public'; | ||
| import type { EmbeddableSetup, EmbeddableStart } from '@kbn/embeddable-plugin/public'; | ||
| import type { PresentationUtilPluginStart } from '@kbn/presentation-util-plugin/public'; | ||
| import type { PresentationUtilPluginSetup } from '@kbn/presentation-util-plugin/public'; | ||
| import type { UsageCollectionStart } from '@kbn/usage-collection-plugin/public'; | ||
| import type { VisualizationsSetup } from '@kbn/visualizations-plugin/public'; | ||
|
|
||
| import type { UiActionsPublicStart } from '@kbn/ui-actions-plugin/public/plugin'; | ||
| import type { UiActionsPublicSetup } from '@kbn/ui-actions-plugin/public/plugin'; | ||
| import { ADD_PANEL_TRIGGER } from '@kbn/ui-actions-plugin/common/trigger_ids'; | ||
| import type { LinksEmbeddableState } from '../common'; | ||
| import { | ||
|
|
@@ -39,14 +39,14 @@ export interface LinksSetupDependencies { | |
| embeddable: EmbeddableSetup; | ||
| visualizations: VisualizationsSetup; | ||
| contentManagement: ContentManagementPublicSetup; | ||
| presentationUtil: PresentationUtilPluginSetup; | ||
| uiActions: UiActionsPublicSetup; | ||
| } | ||
|
|
||
| export interface LinksStartDependencies { | ||
| embeddable: EmbeddableStart; | ||
| dashboard: DashboardStart; | ||
| presentationUtil: PresentationUtilPluginStart; | ||
| contentManagement: ContentManagementPublicStart; | ||
| uiActions: UiActionsPublicStart; | ||
| usageCollection?: UsageCollectionStart; | ||
| } | ||
|
|
||
|
|
@@ -56,93 +56,85 @@ export class LinksPlugin | |
| constructor() {} | ||
|
|
||
| public setup(core: CoreSetup<LinksStartDependencies>, plugins: LinksSetupDependencies) { | ||
| core.getStartServices().then(([_, deps]) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great to see this removed. No reason to get start contracts when they aren't used. |
||
| plugins.contentManagement.registry.register({ | ||
| id: CONTENT_ID, | ||
| version: { | ||
| latest: LATEST_VERSION, | ||
| }, | ||
| name: APP_NAME, | ||
| }); | ||
|
|
||
| plugins.embeddable.registerAddFromLibraryType({ | ||
| onAdd: async (container, savedObject) => { | ||
| container.addNewPanel<LinksEmbeddableState>( | ||
| { | ||
| panelType: LINKS_EMBEDDABLE_TYPE, | ||
| serializedState: { | ||
| ref_id: savedObject.id, | ||
| }, | ||
| plugins.contentManagement.registry.register({ | ||
| id: CONTENT_ID, | ||
| version: { | ||
| latest: LATEST_VERSION, | ||
| }, | ||
| name: APP_NAME, | ||
| }); | ||
|
|
||
| plugins.embeddable.registerAddFromLibraryType({ | ||
| onAdd: async (container, savedObject) => { | ||
| container.addNewPanel<LinksEmbeddableState>( | ||
| { | ||
| panelType: LINKS_EMBEDDABLE_TYPE, | ||
| serializedState: { | ||
| ref_id: savedObject.id, | ||
| }, | ||
| { | ||
| displaySuccessMessage: true, | ||
| }, | ||
| { | ||
| displaySuccessMessage: true, | ||
| } | ||
| ); | ||
| }, | ||
| savedObjectType: LINKS_SAVED_OBJECT_TYPE, | ||
| savedObjectName: APP_NAME, | ||
| getIconForSavedObject: () => APP_ICON, | ||
| }); | ||
|
|
||
| plugins.embeddable.registerEmbeddablePublicDefinition(LINKS_EMBEDDABLE_TYPE, async () => { | ||
| const { getLinksEmbeddableFactory } = await import('./embeddable/links_embeddable'); | ||
| return getLinksEmbeddableFactory(); | ||
| }); | ||
|
|
||
| plugins.embeddable.registerLegacyURLTransform(LINKS_EMBEDDABLE_TYPE, async () => { | ||
| const { transformOut } = await import('../common/embeddable/transforms/transform_out'); | ||
| return transformOut; | ||
| }); | ||
|
|
||
| plugins.visualizations.registerAlias({ | ||
| disableCreate: true, // do not allow creation through visualization listing page | ||
| name: CONTENT_ID, | ||
| title: APP_NAME, | ||
| icon: APP_ICON, | ||
| description: i18n.translate('links.description', { | ||
| defaultMessage: 'Use links to navigate to commonly used dashboards and websites.', | ||
| }), | ||
| stage: 'production', | ||
| appExtensions: { | ||
| visualizations: { | ||
| docTypes: [CONTENT_ID], | ||
| searchFields: ['title^3'], | ||
| client: getLinksClient, | ||
| toListItem( | ||
| linkItem: Omit<LinksCrudTypes['Item'], 'attributes'> & { | ||
| attributes: { title: string; description?: string }; | ||
| } | ||
| ); | ||
| }, | ||
| savedObjectType: LINKS_SAVED_OBJECT_TYPE, | ||
| savedObjectName: APP_NAME, | ||
| getIconForSavedObject: () => APP_ICON, | ||
| }); | ||
|
|
||
| plugins.embeddable.registerEmbeddablePublicDefinition(LINKS_EMBEDDABLE_TYPE, async () => { | ||
| const { getLinksEmbeddableFactory } = await import('./embeddable/links_embeddable'); | ||
| return getLinksEmbeddableFactory(); | ||
| }); | ||
|
|
||
| plugins.embeddable.registerLegacyURLTransform(LINKS_EMBEDDABLE_TYPE, async () => { | ||
| const { transformOut } = await import('../common/embeddable/transforms/transform_out'); | ||
| return transformOut; | ||
| }); | ||
|
|
||
| plugins.visualizations.registerAlias({ | ||
| disableCreate: true, // do not allow creation through visualization listing page | ||
| name: CONTENT_ID, | ||
| title: APP_NAME, | ||
| icon: APP_ICON, | ||
| description: i18n.translate('links.description', { | ||
| defaultMessage: 'Use links to navigate to commonly used dashboards and websites.', | ||
| }), | ||
| stage: 'production', | ||
| appExtensions: { | ||
| visualizations: { | ||
| docTypes: [CONTENT_ID], | ||
| searchFields: ['title^3'], | ||
| client: getLinksClient, | ||
| toListItem( | ||
| linkItem: Omit<LinksCrudTypes['Item'], 'attributes'> & { | ||
| attributes: { title: string; description?: string }; | ||
| } | ||
| ) { | ||
| const { id, type, updatedAt, attributes } = linkItem; | ||
| const { title, description } = attributes; | ||
|
|
||
| return { | ||
| id, | ||
| title, | ||
| editor: { | ||
| onEdit: async (refId: string) => { | ||
| const { onVisualizationsEdit } = await import( | ||
| './editor/on_visualizations_edit' | ||
| ); | ||
| onVisualizationsEdit(refId); | ||
| }, | ||
| ) { | ||
| const { id, type, updatedAt, attributes } = linkItem; | ||
| const { title, description } = attributes; | ||
|
|
||
| return { | ||
| id, | ||
| title, | ||
| editor: { | ||
| onEdit: async (refId: string) => { | ||
| const { onVisualizationsEdit } = await import('./editor/on_visualizations_edit'); | ||
| onVisualizationsEdit(refId); | ||
| }, | ||
| description, | ||
| updatedAt, | ||
| icon: APP_ICON, | ||
| typeTitle: APP_NAME, | ||
| stage: 'production', | ||
| savedObjectType: type, | ||
| }; | ||
| }, | ||
| }, | ||
| description, | ||
| updatedAt, | ||
| icon: APP_ICON, | ||
| typeTitle: APP_NAME, | ||
| stage: 'production', | ||
| savedObjectType: type, | ||
| }; | ||
| }, | ||
| }, | ||
| }); | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| public start(core: CoreStart, plugins: LinksStartDependencies) { | ||
| setKibanaServices(core, plugins); | ||
|
|
||
| plugins.uiActions.addTriggerActionAsync( | ||
| ADD_PANEL_TRIGGER, | ||
|
|
@@ -161,6 +153,10 @@ export class LinksPlugin | |
| return { placementSettings }; | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| public start(core: CoreStart, plugins: LinksStartDependencies) { | ||
| setKibanaServices(core, plugins); | ||
|
|
||
| return {}; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,14 +13,28 @@ import { expect } from '@kbn/scout/ui'; | |
| import { DASHBOARD_DEFAULT_INDEX_TITLE, DASHBOARD_SAVED_SEARCH_ARCHIVE } from '../constants'; | ||
|
|
||
| const getExpected = (config: ScoutTestConfig) => { | ||
| if (config.projectType === 'es' || config.projectType === 'security') { | ||
| if (config.projectType === 'es') { | ||
| return { | ||
| groups: [ | ||
| 'visualizationsGroup', | ||
| 'controlsGroup', | ||
| 'annotation-and-navigationGroup', | ||
| 'logs-aiopsGroup', | ||
| 'mlGroup', | ||
| 'legacyGroup', | ||
| ], | ||
| count: 17, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #267977 changed capabilities check from |
||
| }; | ||
| } | ||
|
|
||
| if (config.projectType === 'security') { | ||
| return { | ||
| groups: [ | ||
| 'visualizationsGroup', | ||
| 'controlsGroup', | ||
| 'annotation-and-navigationGroup', | ||
| 'logs-aiopsGroup', | ||
| 'mlGroup', | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| 'legacyGroup', | ||
| ], | ||
| count: 20, | ||
|
|
@@ -32,8 +46,8 @@ const getExpected = (config: ScoutTestConfig) => { | |
| 'visualizationsGroup', | ||
| 'controlsGroup', | ||
| 'annotation-and-navigationGroup', | ||
| 'mlGroup', | ||
| 'logs-aiopsGroup', | ||
| 'mlGroup', | ||
| 'observabilityGroup', | ||
| 'legacyGroup', | ||
| ], | ||
|
|
@@ -47,8 +61,7 @@ const getExpected = (config: ScoutTestConfig) => { | |
| * This test exists to ensures additions to menu | ||
| * notify our team and can be reviewed by design. | ||
| */ | ||
| // Failing: See https://github.com/elastic/kibana/issues/268101 | ||
| spaceTest.describe.skip( | ||
| spaceTest.describe( | ||
| 'Dashboard add panel flyout', | ||
| { | ||
| tag: [ | ||
|
|
@@ -80,7 +93,7 @@ spaceTest.describe.skip( | |
| await scoutSpace.savedObjects.cleanStandardList(); | ||
| }); | ||
|
|
||
| spaceTest('renders add panel flyout', async ({ pageObjects }) => { | ||
| spaceTest('renders add panel flyout', async ({ pageObjects, log }) => { | ||
| await spaceTest.step('open new dashboard and add panel flyout', async () => { | ||
| await pageObjects.dashboard.openNewDashboard(); | ||
| await pageObjects.dashboard.openAddPanelFlyout(); | ||
|
|
@@ -92,7 +105,9 @@ spaceTest.describe.skip( | |
| }); | ||
|
|
||
| await spaceTest.step('verify total panel count', async () => { | ||
| expect(await pageObjects.dashboard.getAddPanelFlyoutPanelTypesCount()).toBe(expectedCount); | ||
| const addPanelActions = await pageObjects.dashboard.getAddPanelFlyoutActions(); | ||
| log.info(`Add panel actions: ${addPanelActions.join(',')}`); | ||
| expect(addPanelActions).toHaveLength(expectedCount); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||

Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Increase expected because plugin was async importing
./embeddable/register_embeddablesduringsetupmethod. This PR replaced the async import with a sync import to better reflect the actual page load metrics