From 290bc8b3590b5dfb29c3b29ed189edcaac936949 Mon Sep 17 00:00:00 2001 From: Devon Thomson Date: Tue, 23 Jan 2024 13:29:07 -0500 Subject: [PATCH] [Embeddable Refactor] Publish phase events (#175245) Makes the Legacy Embeddable API properly implement the `publishesPhaseEvents` interface. --- .../presentation_publishing/index.ts | 6 +-- ...se_events.ts => publishes_phase_events.ts} | 10 ++-- .../compatibility/legacy_embeddable_to_api.ts | 54 ++++++++++++++++++- .../lib/embeddables/embeddable.test.tsx | 26 +++++++++ .../public/lib/embeddables/embeddable.tsx | 2 + .../public/lib/embeddables/i_embeddable.ts | 2 + .../presentation_panel_internal.tsx | 8 +-- 7 files changed, 97 insertions(+), 11 deletions(-) rename packages/presentation/presentation_publishing/interfaces/{fires_phase_events.ts => publishes_phase_events.ts} (74%) diff --git a/packages/presentation/presentation_publishing/index.ts b/packages/presentation/presentation_publishing/index.ts index 722227a7cce35..a50763b7b2098 100644 --- a/packages/presentation/presentation_publishing/index.ts +++ b/packages/presentation/presentation_publishing/index.ts @@ -18,11 +18,11 @@ export { type CanAccessViewMode, } from './interfaces/can_access_view_mode'; export { - apiFiresPhaseEvents, - type FiresPhaseEvents, + apiPublishesPhaseEvents, + type PublishesPhaseEvents, type PhaseEvent, type PhaseEventType, -} from './interfaces/fires_phase_events'; +} from './interfaces/publishes_phase_events'; export { hasEditCapabilities, type HasEditCapabilities } from './interfaces/has_edit_capabilities'; export { apiHasParentApi, type HasParentApi } from './interfaces/has_parent_api'; export { diff --git a/packages/presentation/presentation_publishing/interfaces/fires_phase_events.ts b/packages/presentation/presentation_publishing/interfaces/publishes_phase_events.ts similarity index 74% rename from packages/presentation/presentation_publishing/interfaces/fires_phase_events.ts rename to packages/presentation/presentation_publishing/interfaces/publishes_phase_events.ts index 874e9d86c677d..72dbf70393a90 100644 --- a/packages/presentation/presentation_publishing/interfaces/fires_phase_events.ts +++ b/packages/presentation/presentation_publishing/interfaces/publishes_phase_events.ts @@ -20,10 +20,12 @@ export interface PhaseEvent { timeToEvent: number; } -export interface FiresPhaseEvents { - onPhaseChange: PublishingSubject; +export interface PublishesPhaseEvents { + onPhaseChange: PublishingSubject; } -export const apiFiresPhaseEvents = (unknownApi: null | unknown): unknownApi is FiresPhaseEvents => { - return Boolean(unknownApi && (unknownApi as FiresPhaseEvents)?.onPhaseChange !== undefined); +export const apiPublishesPhaseEvents = ( + unknownApi: null | unknown +): unknownApi is PublishesPhaseEvents => { + return Boolean(unknownApi && (unknownApi as PublishesPhaseEvents)?.onPhaseChange !== undefined); }; diff --git a/src/plugins/embeddable/public/lib/embeddables/compatibility/legacy_embeddable_to_api.ts b/src/plugins/embeddable/public/lib/embeddables/compatibility/legacy_embeddable_to_api.ts index a557878728973..e76253c278d4e 100644 --- a/src/plugins/embeddable/public/lib/embeddables/compatibility/legacy_embeddable_to_api.ts +++ b/src/plugins/embeddable/public/lib/embeddables/compatibility/legacy_embeddable_to_api.ts @@ -10,8 +10,10 @@ import { DataView } from '@kbn/data-views-plugin/common'; import { AggregateQuery, compareFilters, Filter, Query, TimeRange } from '@kbn/es-query'; import type { ErrorLike } from '@kbn/expressions-plugin/common'; import { i18n } from '@kbn/i18n'; +import { PhaseEvent, PhaseEventType } from '@kbn/presentation-publishing'; import deepEqual from 'fast-deep-equal'; -import { BehaviorSubject, Subscription } from 'rxjs'; +import { isNil } from 'lodash'; +import { BehaviorSubject, map, Subscription, distinct } from 'rxjs'; import { embeddableStart } from '../../../kibana_services'; import { isFilterableEmbeddable } from '../../filterable_embeddable'; import { @@ -40,6 +42,18 @@ function isVisualizeEmbeddable( return embeddable.type === 'visualization'; } +const getEventStatus = (output: EmbeddableOutput): PhaseEventType => { + if (!isNil(output.error)) { + return 'error'; + } else if (output.rendered === true) { + return 'rendered'; + } else if (output.loading === false) { + return 'loaded'; + } else { + return 'loading'; + } +}; + export const legacyEmbeddableToApi = ( embeddable: CommonLegacyEmbeddable ): { api: Omit; destroyAPI: () => void } => { @@ -66,6 +80,42 @@ export const legacyEmbeddableToApi = ( }); const isEditingEnabled = () => canEditEmbeddable(embeddable); + /** + * Performance tracking + */ + const onPhaseChange = new BehaviorSubject(undefined); + + let loadingStartTime = 0; + subscriptions.add( + embeddable + .getOutput$() + .pipe( + // Map loaded event properties + map((output) => { + if (output.loading === true) { + loadingStartTime = performance.now(); + } + return { + id: embeddable.id, + status: getEventStatus(output), + error: output.error, + }; + }), + // Dedupe + distinct((output) => loadingStartTime + output.id + output.status + !!output.error), + // Map loaded event properties + map((output): PhaseEvent => { + return { + ...output, + timeToEvent: performance.now() - loadingStartTime, + }; + }) + ) + .subscribe((statusOutput) => { + onPhaseChange.next(statusOutput); + }) + ); + /** * Publish state for Presentation panel */ @@ -155,6 +205,8 @@ export const legacyEmbeddableToApi = ( dataLoading, blockingError, + onPhaseChange, + onEdit, isEditingEnabled, getTypeDisplayName, diff --git a/src/plugins/embeddable/public/lib/embeddables/embeddable.test.tsx b/src/plugins/embeddable/public/lib/embeddables/embeddable.test.tsx index 079a31089d707..b3f4d4d915b74 100644 --- a/src/plugins/embeddable/public/lib/embeddables/embeddable.test.tsx +++ b/src/plugins/embeddable/public/lib/embeddables/embeddable.test.tsx @@ -43,6 +43,17 @@ class OutputTestEmbeddable extends Embeddable { reload() {} } +class PhaseTestEmbeddable extends Embeddable { + public readonly type = 'phaseTest'; + constructor() { + super({ id: 'phaseTest', viewMode: ViewMode.VIEW }, {}); + } + public reportsEmbeddableLoad(): boolean { + return true; + } + reload() {} +} + test('Embeddable calls input subscribers when changed', (done) => { const hello = new ContactCardEmbeddable( { id: '123', firstName: 'Brienne', lastName: 'Tarth' }, @@ -104,6 +115,21 @@ test('updating output state retains instance information', async () => { expect(outputTest.getOutput().testClass).toBeInstanceOf(TestClass); }); +test('fires phase events when output changes', async () => { + const phaseEventTest = new PhaseTestEmbeddable(); + let phaseEventCount = 0; + phaseEventTest.onPhaseChange.subscribe((event) => { + if (event) { + phaseEventCount++; + } + }); + expect(phaseEventCount).toBe(1); // loading is true by default which fires an event. + phaseEventTest.updateOutput({ loading: false }); + expect(phaseEventCount).toBe(2); + phaseEventTest.updateOutput({ rendered: true }); + expect(phaseEventCount).toBe(3); +}); + test('updated$ called after reload and batches input/output changes', async () => { const hello = new ContactCardEmbeddable( { id: '123', firstName: 'Brienne', lastName: 'Tarth' }, diff --git a/src/plugins/embeddable/public/lib/embeddables/embeddable.tsx b/src/plugins/embeddable/public/lib/embeddables/embeddable.tsx index cb0f94b46cc58..4ea27da2f617f 100644 --- a/src/plugins/embeddable/public/lib/embeddables/embeddable.tsx +++ b/src/plugins/embeddable/public/lib/embeddables/embeddable.tsx @@ -126,6 +126,7 @@ export abstract class Embeddable< dataLoading: this.dataLoading, localFilters: this.localFilters, blockingError: this.blockingError, + onPhaseChange: this.onPhaseChange, setPanelTitle: this.setPanelTitle, linkToLibrary: this.linkToLibrary, hidePanelTitle: this.hidePanelTitle, @@ -164,6 +165,7 @@ export abstract class Embeddable< public panelTitle: LegacyEmbeddableAPI['panelTitle']; public dataLoading: LegacyEmbeddableAPI['dataLoading']; public localFilters: LegacyEmbeddableAPI['localFilters']; + public onPhaseChange: LegacyEmbeddableAPI['onPhaseChange']; public linkToLibrary: LegacyEmbeddableAPI['linkToLibrary']; public blockingError: LegacyEmbeddableAPI['blockingError']; public setPanelTitle: LegacyEmbeddableAPI['setPanelTitle']; diff --git a/src/plugins/embeddable/public/lib/embeddables/i_embeddable.ts b/src/plugins/embeddable/public/lib/embeddables/i_embeddable.ts index 5dbfbcc63be32..21f22f9fb7f84 100644 --- a/src/plugins/embeddable/public/lib/embeddables/i_embeddable.ts +++ b/src/plugins/embeddable/public/lib/embeddables/i_embeddable.ts @@ -22,6 +22,7 @@ import { PublishesViewMode, PublishesWritablePanelDescription, PublishesWritablePanelTitle, + PublishesPhaseEvents, } from '@kbn/presentation-publishing'; import { Observable } from 'rxjs'; import { EmbeddableInput } from '../../../common/types'; @@ -38,6 +39,7 @@ export type { EmbeddableInput }; */ export type LegacyEmbeddableAPI = HasType & HasUniqueId & + PublishesPhaseEvents & PublishesViewMode & PublishesDataViews & HasEditCapabilities & diff --git a/src/plugins/presentation_panel/public/panel_component/presentation_panel_internal.tsx b/src/plugins/presentation_panel/public/panel_component/presentation_panel_internal.tsx index b4d57cb46c45e..8af8e22cbeede 100644 --- a/src/plugins/presentation_panel/public/panel_component/presentation_panel_internal.tsx +++ b/src/plugins/presentation_panel/public/panel_component/presentation_panel_internal.tsx @@ -9,7 +9,7 @@ import { EuiFlexGroup, EuiPanel, htmlIdGenerator } from '@elastic/eui'; import { PanelLoader } from '@kbn/panel-loader'; import { - apiFiresPhaseEvents, + apiPublishesPhaseEvents, apiHasParentApi, apiPublishesViewMode, useBatchedPublishingSubjects, @@ -82,8 +82,10 @@ export const PresentationPanelInternal = < useEffect(() => { let subscription: Subscription; - if (api && onPanelStatusChange && apiFiresPhaseEvents(api)) { - subscription = api.onPhaseChange.subscribe((phase) => onPanelStatusChange(phase)); + if (api && onPanelStatusChange && apiPublishesPhaseEvents(api)) { + subscription = api.onPhaseChange.subscribe((phase) => { + if (phase) onPanelStatusChange(phase); + }); } return () => subscription?.unsubscribe(); }, [api, onPanelStatusChange]);