Skip to content

Commit

Permalink
[8.x] [Lens] Correctly use UserMessage longMessage as function (#192492
Browse files Browse the repository at this point in the history
…) (#196194)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Lens] Correctly use UserMessage longMessage as function
(#192492)](#192492)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marco
Vettorello","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-14T17:34:36Z","message":"[Lens]
Correctly use UserMessage longMessage as function (#192492)\n\n##
Summary\r\n\r\nAfter #167205 was
merged, the\r\n`UserMessage.longMessage` was typed as `longMessage:
string |\r\nReact.ReactNode | ((closePopover: () => void) =>
React.ReactNode);`\r\n\r\nWith the upcoming React 18 upgrade, an error
will become visible because\r\n`((closePopover: () => void) =>
React.ReactNode);` can't be used as a\r\nReactNode but it correctly
needs to be called.\r\n\r\nIn this PR I've made the `closePopover`
function being optional (to\r\nsimplify the refactoring) and I've added
the typecheck where
needed.","sha":"e35507a27d9c8df3fe5947c7227d6072d007dfa5","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Feature:Lens","v9.0.0","backport:prev-minor","v8.16.0"],"title":"[Lens]
Correctly use UserMessage longMessage as function
","number":192492,"url":"https://github.com/elastic/kibana/pull/192492","mergeCommit":{"message":"[Lens]
Correctly use UserMessage longMessage as function (#192492)\n\n##
Summary\r\n\r\nAfter #167205 was
merged, the\r\n`UserMessage.longMessage` was typed as `longMessage:
string |\r\nReact.ReactNode | ((closePopover: () => void) =>
React.ReactNode);`\r\n\r\nWith the upcoming React 18 upgrade, an error
will become visible because\r\n`((closePopover: () => void) =>
React.ReactNode);` can't be used as a\r\nReactNode but it correctly
needs to be called.\r\n\r\nIn this PR I've made the `closePopover`
function being optional (to\r\nsimplify the refactoring) and I've added
the typecheck where
needed.","sha":"e35507a27d9c8df3fe5947c7227d6072d007dfa5"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192492","number":192492,"mergeCommit":{"message":"[Lens]
Correctly use UserMessage longMessage as function (#192492)\n\n##
Summary\r\n\r\nAfter #167205 was
merged, the\r\n`UserMessage.longMessage` was typed as `longMessage:
string |\r\nReact.ReactNode | ((closePopover: () => void) =>
React.ReactNode);`\r\n\r\nWith the upcoming React 18 upgrade, an error
will become visible because\r\n`((closePopover: () => void) =>
React.ReactNode);` can't be used as a\r\nReactNode but it correctly
needs to be called.\r\n\r\nIn this PR I've made the `closePopover`
function being optional (to\r\nsimplify the refactoring) and I've added
the typecheck where
needed.","sha":"e35507a27d9c8df3fe5947c7227d6072d007dfa5"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marco Vettorello <[email protected]>
  • Loading branch information
kibanamachine and markov00 authored Oct 14, 2024
1 parent d48190e commit 7e8d54c
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 7e8d54c

Please sign in to comment.