Skip to content

Commit

Permalink
[Lens] Correctly use UserMessage longMessage as function (#192492)
Browse files Browse the repository at this point in the history
## Summary

After #167205 was merged, the
`UserMessage.longMessage` was typed as `longMessage: string |
React.ReactNode | ((closePopover: () => void) => React.ReactNode);`

With the upcoming React 18 upgrade, an error will become visible because
`((closePopover: () => void) => React.ReactNode);` can't be used as a
ReactNode but it correctly needs to be called.

In this PR I've made the `closePopover` function being optional (to
simplify the refactoring) and I've added the typecheck where needed.
  • Loading branch information
markov00 authored Oct 14, 2024
1 parent 7312155 commit e35507a
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from './get_application_user_messages';
import { cleanup, render, screen } from '@testing-library/react';
import { I18nProvider } from '@kbn/i18n-react';
import { getLongMessage } from '../user_messages_utils';

jest.mock('@kbn/shared-ux-link-redirect-app', () => {
const original = jest.requireActual('@kbn/shared-ux-link-redirect-app');
Expand Down Expand Up @@ -164,15 +165,9 @@ describe('application-level user messages', () => {
visualization: {} as Visualization,
visualizationState: { activeId: 'foo', state: {} },
};
const firstMessage = getApplicationUserMessages({ ...props, ...propsOverrides }).at(0);
const rtlRender = render(
<I18nProvider>
{
getApplicationUserMessages({
...props,
...propsOverrides,
})[0].longMessage as React.ReactNode
}
</I18nProvider>
<I18nProvider>{firstMessage && getLongMessage(firstMessage)}</I18nProvider>
);
return rtlRender;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3154,9 +3154,7 @@ describe('IndexPattern Data Source', () => {
values={
Object {
"position": 1,
"wrappedMessage": <React.Fragment>
error 1
</React.Fragment>,
"wrappedMessage": "error 1",
}
}
/>,
Expand All @@ -3177,9 +3175,7 @@ describe('IndexPattern Data Source', () => {
values={
Object {
"position": 1,
"wrappedMessage": <React.Fragment>
error 2
</React.Fragment>,
"wrappedMessage": "error 2",
}
}
/>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ import { LayerSettingsPanel } from './layer_settings';
import { FormBasedLayer, LastValueIndexPatternColumn } from '../..';
import { filterAndSortUserMessages } from '../../app_plugin/get_application_user_messages';
import { EDITOR_INVALID_DIMENSION } from '../../user_messages_ids';
import { getLongMessage } from '../../user_messages_utils';
export type { OperationType, GenericIndexPatternColumn } from './operations';
export { deleteColumn } from './operations';

Expand Down Expand Up @@ -995,7 +996,7 @@ function getLayerErrorMessages(
defaultMessage="Layer {position} error: {wrappedMessage}"
values={{
position: index + 1,
wrappedMessage: <>{error.longMessage}</>,
wrappedMessage: getLongMessage(error),
}}
/>
),
Expand Down
21 changes: 13 additions & 8 deletions x-pack/plugins/lens/public/datasources/form_based/utils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import type { FramePublicAPI, IndexPattern } from '../../types';
import { TermsIndexPatternColumn } from './operations';
import { FormBasedLayer } from './types';
import { createMockedIndexPatternWithAdditionalFields } from './mocks';
import { getLongMessage } from '../../user_messages_utils';

describe('indexpattern_datasource utils', () => {
describe('getPrecisionErrorWarningMessages', () => {
Expand Down Expand Up @@ -121,10 +122,11 @@ describe('indexpattern_datasource utils', () => {
);

expect(warningMessages).toHaveLength(1);
const { longMessage, ...rest } = warningMessages[0];

expect({ ...warningMessages[0], longMessage: '' }).toMatchSnapshot();
expect({ ...rest, longMessage: '' }).toMatchSnapshot();

render(<I18nProvider>{warningMessages[0].longMessage as React.ReactNode}</I18nProvider>);
render(<I18nProvider>{getLongMessage(warningMessages[0])}</I18nProvider>);

expect(screen.getByTestId('lnsPrecisionWarningEnableAccuracy')).toBeInTheDocument();
await userEvent.click(screen.getByTestId('lnsPrecisionWarningEnableAccuracy'));
Expand All @@ -145,11 +147,12 @@ describe('indexpattern_datasource utils', () => {
);

expect(warningMessages).toHaveLength(1);
const { longMessage, ...rest } = warningMessages[0];

expect({ ...warningMessages[0], longMessage: '' }).toMatchSnapshot();
expect({ ...rest, longMessage: '' }).toMatchSnapshot();

const { container } = render(
<I18nProvider>{warningMessages[0].longMessage as React.ReactNode}</I18nProvider>
<I18nProvider>{getLongMessage(warningMessages[0])}</I18nProvider>
);
expect(container).toHaveTextContent(
'might be an approximation. For more precise results, try increasing the number of Top Values or using Filters instead.'
Expand Down Expand Up @@ -178,18 +181,20 @@ describe('indexpattern_datasource utils', () => {
} as unknown as GenericIndexPatternColumn,
};
const setState = jest.fn();
const warnings = getPrecisionErrorWarningMessages(
const warningMessages = getPrecisionErrorWarningMessages(
datatableUtilitites,
state,
framePublicAPI,
docLinks,
setState
);

expect(warnings).toHaveLength(1);
expect({ ...warnings[0], longMessage: '' }).toMatchSnapshot();
expect(warningMessages).toHaveLength(1);
const { longMessage, ...rest } = warningMessages[0];

expect({ ...rest, longMessage: '' }).toMatchSnapshot();

render(<I18nProvider>{warnings[0].longMessage as React.ReactNode}</I18nProvider>);
render(<I18nProvider>{getLongMessage(warningMessages[0])}</I18nProvider>);
await userEvent.click(screen.getByText('Rank by rarity'));
const stateSetter = setState.mock.calls[0][0];
const newState = stateSetter(state);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
import { getSharedActions } from './layer_actions/layer_actions';
import { FlyoutContainer } from '../../../shared_components/flyout_container';
import { FakeDimensionButton } from './buttons/fake_dimension_button';
import { getLongMessage } from '../../../user_messages_utils';

export function LayerPanel(props: LayerPanelProps) {
const [openDimension, setOpenDimension] = useState<{
Expand Down Expand Up @@ -518,6 +519,7 @@ export function LayerPanel(props: LayerPanelProps) {
props?.getUserMessages?.('dimensionButton', {
dimensionId: columnId,
}) ?? [];
const firstMessage = messages.at(0);

return (
<DraggableDimensionButton
Expand Down Expand Up @@ -567,11 +569,15 @@ export function LayerPanel(props: LayerPanelProps) {
props.onRemoveDimension({ columnId: id, layerId });
removeButtonRef(id);
}}
message={{
severity: messages[0]?.severity,
content: (messages[0]?.shortMessage ||
messages[0]?.longMessage) as React.ReactNode,
}}
message={
firstMessage
? {
severity: firstMessage.severity,
content:
firstMessage.shortMessage || getLongMessage(firstMessage),
}
: undefined
}
>
{layerDatasource ? (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
import type { LensInspector } from '../../lens_inspector_service';
import { ErrorBoundary, showMemoizedErrorNotification } from '../../lens_ui_errors';
import { IndexPatternServiceAPI } from '../../data_views_service/service';
import { getLongMessage } from '../../user_messages_utils';

export interface EditorFrameProps {
datasourceMap: DatasourceMap;
Expand Down Expand Up @@ -128,9 +129,7 @@ export function EditorFrame(props: EditorFrameProps) {
bannerMessages={
bannerMessages.length ? (
<ErrorBoundary onError={onError}>
<BannerWrapper
nodes={bannerMessages.map(({ longMessage }) => longMessage as React.ReactNode)}
/>
<BannerWrapper nodes={bannerMessages.map(getLongMessage)} />
</ErrorBoundary>
) : undefined
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
EuiText,
} from '@elastic/eui';
import type { UserMessage } from '../../../types';
import { getLongMessage } from '../../../user_messages_utils';

interface Props {
errors: Array<string | UserMessage>;
Expand Down Expand Up @@ -56,7 +57,7 @@ export function WorkspaceErrors(props: Props) {
{activeError.longMessage ? (
<>
<EuiSpacer />
<EuiText size="s"> {activeError.longMessage as React.ReactNode}</EuiText>
<EuiText size="s"> {getLongMessage(activeError)}</EuiText>
</>
) : null}
</div>
Expand Down
9 changes: 5 additions & 4 deletions x-pack/plugins/lens/public/embeddable/embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ import { EmbeddableFeatureBadge } from './embeddable_info_badges';
import { getDatasourceLayers } from '../state_management/utils';
import type { EditLensConfigurationProps } from '../app_plugin/shared/edit_on_the_fly/get_edit_lens_configuration';
import { TextBasedPersistedState } from '../datasources/text_based/types';
import { getLongMessage } from '../user_messages_utils';

export type LensSavedObjectAttributes = Omit<Document, 'savedObjectId' | 'type'>;

Expand Down Expand Up @@ -251,7 +252,7 @@ export interface ViewUnderlyingDataArgs {
}

function VisualizationErrorPanel({ errors, canEdit }: { errors: UserMessage[]; canEdit: boolean }) {
const showMore = errors.length > 1;
const firstError = errors.at(0);
const canFixInLens = canEdit && errors.some(({ fixableInEditor }) => fixableInEditor);
return (
<div className="lnsEmbeddedError">
Expand All @@ -261,10 +262,10 @@ function VisualizationErrorPanel({ errors, canEdit }: { errors: UserMessage[]; c
data-test-subj="embeddable-lens-failure"
body={
<>
{errors.length ? (
{firstError ? (
<>
<p>{errors[0].longMessage as React.ReactNode}</p>
{showMore && !canFixInLens ? (
<p>{getLongMessage(firstError)}</p>
{errors.length > 1 && !canFixInLens ? (
<p>
<FormattedMessage
id="xpack.lens.embeddable.moreErrors"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import React, { Fragment } from 'react';
import { useState } from 'react';
import type { UserMessage } from '../types';
import './embeddable_info_badges.scss';
import { getLongMessage } from '../user_messages_utils';

export const EmbeddableFeatureBadge = ({ messages }: { messages: UserMessage[] }) => {
const { euiTheme } = useEuiTheme();
Expand Down Expand Up @@ -98,8 +99,8 @@ export const EmbeddableFeatureBadge = ({ messages }: { messages: UserMessage[] }
<h3>{shortMessage}</h3>
</EuiTitle>
<ul className="lnsEmbeddablePanelFeatureList">
{messageGroup.map(({ longMessage }, i) => (
<Fragment key={`${uniqueId}-${i}`}>{longMessage as React.ReactNode}</Fragment>
{messageGroup.map((message, i) => (
<Fragment key={`${uniqueId}-${i}`}>{getLongMessage(message)}</Fragment>
))}
</ul>
</aside>
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/lens/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ export interface UserMessage {
severity: 'error' | 'warning' | 'info';
hidePopoverIcon?: boolean;
shortMessage: string;
longMessage: string | React.ReactNode | ((closePopover: () => void) => React.ReactNode);
longMessage: string | React.ReactNode | ((closePopover?: () => void) => React.ReactNode);
fixableInEditor: boolean;
displayLocations: UserMessageDisplayLocation[];
}
Expand Down
12 changes: 12 additions & 0 deletions x-pack/plugins/lens/public/user_messages_utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { UserMessage } from './types';

export function getLongMessage(msg: UserMessage) {
return typeof msg.longMessage === 'function' ? msg.longMessage() : msg.longMessage;
}

0 comments on commit e35507a

Please sign in to comment.