Skip to content

Commit

Permalink
[dashboard] fix Inspect panel action displays untitled instead of pan…
Browse files Browse the repository at this point in the history
…el title (elastic#177516)

Fixes elastic#176870

PR provides a utility method for fetching title that falls back to
default title when title is not provided.
  • Loading branch information
nreese authored and fkanout committed Mar 4, 2024
1 parent c7eab2c commit db385f1
Show file tree
Hide file tree
Showing 14 changed files with 79 additions and 29 deletions.
1 change: 1 addition & 0 deletions packages/presentation/presentation_publishing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export {
type PublishesWritablePanelDescription,
} from './interfaces/publishes_panel_description';
export {
getPanelTitle,
apiPublishesPanelTitle,
apiPublishesWritablePanelTitle,
useDefaultPanelTitle,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* 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 { BehaviorSubject } from 'rxjs';
import { getPanelTitle } from './publishes_panel_title';

describe('getPanelTitle', () => {
test('should return default title when title is undefined', () => {
const api = {
panelTitle: new BehaviorSubject<string | undefined>(undefined),
defaultPanelTitle: new BehaviorSubject<string | undefined>('default title'),
};
expect(getPanelTitle(api)).toBe('default title');
});

test('should return default title when title is empty string', () => {
const api = {
panelTitle: new BehaviorSubject<string | undefined>(''),
defaultPanelTitle: new BehaviorSubject<string | undefined>('default title'),
};
expect(getPanelTitle(api)).toBe('default title');
});

test('should return title when title is provided', () => {
const api = {
panelTitle: new BehaviorSubject<string | undefined>('custom title'),
defaultPanelTitle: new BehaviorSubject<string | undefined>('default title'),
};
expect(getPanelTitle(api)).toBe('custom title');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ export interface PublishesPanelTitle {
defaultPanelTitle?: PublishingSubject<string | undefined>;
}

export function getPanelTitle(api: Partial<PublishesPanelTitle>): string | undefined {
return api.panelTitle?.value || api.defaultPanelTitle?.value;
}

export type PublishesWritablePanelTitle = PublishesPanelTitle & {
setPanelTitle: (newTitle: string | undefined) => void;
setHidePanelTitle: (hide: boolean | undefined) => void;
Expand Down Expand Up @@ -44,8 +48,11 @@ export const apiPublishesWritablePanelTitle = (
/**
* A hook that gets this API's panel title as a reactive variable which will cause re-renders on change.
*/
export const usePanelTitle = (api: Partial<PublishesPanelTitle> | undefined) =>
useStateFromPublishingSubject(api?.panelTitle);
export const usePanelTitle = (api: Partial<PublishesPanelTitle> | undefined) => {
const title = useStateFromPublishingSubject(api?.panelTitle);
const defaultTitle = useStateFromPublishingSubject(api?.defaultPanelTitle);
return title || defaultTitle;
};

/**
* A hook that gets this API's hide panel title setting as a reactive variable which will cause re-renders on change.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { apiCanLinkToLibrary, CanLinkToLibrary } from '@kbn/presentation-library
import {
apiCanAccessViewMode,
EmbeddableApiContext,
getPanelTitle,
PublishesPanelTitle,
CanAccessViewMode,
getInheritedViewMode,
Expand Down Expand Up @@ -57,7 +58,7 @@ export class AddToLibraryAction implements Action<EmbeddableApiContext> {

public async execute({ embeddable }: EmbeddableApiContext) {
if (!isApiCompatible(embeddable)) throw new IncompatibleActionError();
const panelTitle = embeddable.panelTitle?.value ?? embeddable.defaultPanelTitle?.value;
const panelTitle = getPanelTitle(embeddable);
try {
await embeddable.linkToLibrary();
this.toastsService.addSuccess({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ import {
HasInspectorAdapters,
type Adapters,
} from '@kbn/inspector-plugin/public';
import { EmbeddableApiContext, PublishesPanelTitle } from '@kbn/presentation-publishing';
import {
EmbeddableApiContext,
getPanelTitle,
PublishesPanelTitle,
} from '@kbn/presentation-publishing';
import { pluginServices } from '../services/plugin_services';
import { dashboardExportCsvActionStrings } from './_dashboard_actions_strings';

Expand Down Expand Up @@ -98,7 +102,7 @@ export class ExportCSVAction implements Action<ExportContext> {
const postFix = datatables.length > 1 ? `-${i + 1}` : '';
const untitledFilename = dashboardExportCsvActionStrings.getUntitledFilename();

memo[`${embeddable.panelTitle?.value || untitledFilename}${postFix}.csv`] = {
memo[`${getPanelTitle(embeddable) || untitledFilename}${postFix}.csv`] = {
content: exporters.datatableToCSV(datatable, {
csvSeparator: this.uiSettings.get('csv:separator', ','),
quoteValues: this.uiSettings.get('csv:quoteValues', true),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { EuiFlyoutBody, EuiFlyoutHeader, EuiTitle } from '@elastic/eui';
import React from 'react';

import { Toast } from '@kbn/core/public';

import { getPanelTitle } from '@kbn/presentation-publishing';
import { pluginServices } from '../services/plugin_services';
import { ReplacePanelActionApi } from './replace_panel_action';
import { dashboardReplacePanelActionStrings } from './_dashboard_actions_strings';
Expand Down Expand Up @@ -83,9 +83,7 @@ export class ReplacePanelFlyout extends React.Component<Props> {
<EuiTitle size="m">
<h2>
<span>
{dashboardReplacePanelActionStrings.getFlyoutHeader(
this.props.api.panelTitle?.value ?? this.props.api.defaultPanelTitle?.value
)}
{dashboardReplacePanelActionStrings.getFlyoutHeader(getPanelTitle(this.props.api))}
</span>
</h2>
</EuiTitle>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
CanAccessViewMode,
EmbeddableApiContext,
getInheritedViewMode,
getPanelTitle,
PublishesPanelTitle,
} from '@kbn/presentation-publishing';
import { pluginServices } from '../services/plugin_services';
Expand Down Expand Up @@ -60,7 +61,7 @@ export class UnlinkFromLibraryAction implements Action<EmbeddableApiContext> {

public async execute({ embeddable }: EmbeddableApiContext) {
if (!unlinkActionIsCompatible(embeddable)) throw new IncompatibleActionError();
const title = embeddable.panelTitle?.value ?? embeddable.defaultPanelTitle?.value;
const title = getPanelTitle(embeddable);
try {
await embeddable.unlinkFromLibrary();
this.toastsService.addSuccess({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
PanelNotFoundError,
reactEmbeddableRegistryHasKey,
} from '@kbn/embeddable-plugin/public';
import { apiPublishesPanelTitle } from '@kbn/presentation-publishing';
import { apiPublishesPanelTitle, getPanelTitle } from '@kbn/presentation-publishing';
import { filter, map, max } from 'lodash';
import { v4 as uuidv4 } from 'uuid';
import { DashboardPanelState } from '../../../../common';
Expand Down Expand Up @@ -59,9 +59,7 @@ const duplicateReactEmbeddableInput = async (
const child = dashboard.reactEmbeddableChildren.value[idToDuplicate];
if (!child) throw new PanelNotFoundError();

const lastTitle = apiPublishesPanelTitle(child)
? child.panelTitle.value ?? child.defaultPanelTitle?.value ?? ''
: '';
const lastTitle = apiPublishesPanelTitle(child) ? getPanelTitle(child) ?? '' : '';
const newTitle = await incrementPanelTitle(dashboard, lastTitle);
const id = uuidv4();
const serializedState = await child.serializeState();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { METRIC_TYPE } from '@kbn/analytics';
import { Reference } from '@kbn/content-management-utils';
import type { ControlGroupContainer } from '@kbn/controls-plugin/public';
import type { KibanaExecutionContext, OverlayRef } from '@kbn/core/public';
import { getPanelTitle } from '@kbn/presentation-publishing';
import { RefreshInterval } from '@kbn/data-plugin/public';
import type { DataView } from '@kbn/data-views-plugin/public';
import { reportPerformanceMetricEvent } from '@kbn/ebt-tools';
Expand Down Expand Up @@ -664,10 +665,7 @@ export class DashboardContainer
for (const [id, panel] of Object.entries(this.getInput().panels)) {
const title = await (async () => {
if (reactEmbeddableRegistryHasKey(panel.type)) {
return (
this.reactEmbeddableChildren.value[id]?.panelTitle?.value ??
this.reactEmbeddableChildren.value[id]?.defaultPanelTitle?.value
);
return getPanelTitle(this.reactEmbeddableChildren.value[id]);
}
await this.untilEmbeddableLoaded(id);
const child: IEmbeddable<EmbeddableInput, EmbeddableOutput> = this.getChild(id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';

import { UI_SETTINGS } from '@kbn/data-plugin/public';
import { apiPublishesLocalUnifiedSearch, getInheritedViewMode } from '@kbn/presentation-publishing';
import {
apiPublishesLocalUnifiedSearch,
getInheritedViewMode,
getPanelTitle,
} from '@kbn/presentation-publishing';

import { core } from '../../kibana_services';
import { CustomizePanelActionApi } from './customize_panel_action';
Expand Down Expand Up @@ -59,9 +63,7 @@ export const CustomizePanelEditor = ({
const [panelDescription, setPanelDescription] = useState(
api.panelDescription?.value ?? api.defaultPanelDescription?.value
);
const [panelTitle, setPanelTitle] = useState(
api.panelTitle?.value ?? api.defaultPanelTitle?.value
);
const [panelTitle, setPanelTitle] = useState(getPanelTitle(api));
const [localTimeRange, setLocalTimeRange] = useState(
api.localTimeRange?.value ?? api?.getFallbackTimeRange?.()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { apiHasInspectorAdapters, HasInspectorAdapters } from '@kbn/inspector-pl
import { tracksOverlays } from '@kbn/presentation-containers';
import {
EmbeddableApiContext,
getPanelTitle,
PublishesPanelTitle,
HasParentApi,
} from '@kbn/presentation-publishing';
Expand Down Expand Up @@ -56,7 +57,7 @@ export class InspectPanelAction implements Action<EmbeddableApiContext> {
}

const panelTitle =
embeddable.panelTitle?.value ??
getPanelTitle(embeddable) ||
i18n.translate('presentationPanel.action.inspectPanel.untitledEmbeddableFilename', {
defaultMessage: 'untitled',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import {
type PresentationContainer,
} from '@kbn/presentation-containers';
import {
PublishesPanelTitle,
getPanelTitle,
type PublishesPanelTitle,
type HasUniqueId,
type HasParentApi,
} from '@kbn/presentation-publishing';
Expand Down Expand Up @@ -63,7 +64,7 @@ export const createDrilldownTemplatesFromSiblings = (
id: event.eventId,
name: event.action.name,
icon: 'dashboardApp',
description: child.panelTitle?.value ?? child.uuid ?? '',
description: getPanelTitle(child) ?? child.uuid ?? '',
config: event.action.config,
factoryId: event.action.factoryId,
triggers: event.triggers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {
PublishesLocalUnifiedSearch,
PublishesDataViews,
} from '@kbn/presentation-publishing';
import { getPanelTitle } from '@kbn/presentation-publishing';
import type { UrlTemplateEditorVariable } from '@kbn/kibana-react-plugin/public';
import { txtValue } from './i18n';
import { deleteUndefinedKeys } from './util';
Expand Down Expand Up @@ -78,7 +79,7 @@ export const getContextScopeValues = (context: Partial<EmbeddableApiContext>): C
return {
panel: deleteUndefinedKeys({
id: api.uuid,
title: api.panelTitle?.value ?? api.defaultPanelTitle?.value,
title: getPanelTitle(api),
savedObjectId: api.savedObjectId?.value,
query: api.parentApi?.localQuery?.value,
timeRange: api.localTimeRange?.value ?? api.parentApi?.localTimeRange?.value,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { i18n } from '@kbn/i18n';
import { monaco } from '@kbn/monaco';
import type { PublishesPanelTitle } from '@kbn/presentation-publishing';
import { getPanelTitle, type PublishesPanelTitle } from '@kbn/presentation-publishing';
import {
ChartActionContext,
isRangeSelectTriggerContext,
Expand Down Expand Up @@ -103,9 +103,10 @@ const getEventScopeFromRowClickTriggerContext = (
for (const columnId of columns) {
const column = data.table.columns.find(({ id }) => id === columnId);
if (!column) {
// This should never happe, but in case it does we log data necessary for debugging.
// This should never happen, but in case it does we log data necessary for debugging.
const title = getPanelTitle(api);
// eslint-disable-next-line no-console
console.error(data, api?.panelTitle ? `Embeddable [${api.panelTitle.value}]` : null);
console.error(data, title ? `Embeddable [${title}]` : null);
throw new Error('Could not find a datatable column.');
}
values.push(row[columnId]);
Expand Down

0 comments on commit db385f1

Please sign in to comment.