Skip to content

Commit

Permalink
Update incomplete data messaging (#169578)
Browse files Browse the repository at this point in the history
Closes #167906

PR breaks monolith component `<SearchResponseWarnings/>` into 3 separate
components: `<SearchResponseWarningsBadge/>`,
`<SearchResponseWarningsCallout/>`, and
`<SearchResponseWarningsEmptyPrompt/>`. These components are designed to
display a single messages when provided warnings from multiple requests
and display better messaging around partial results. PR also removes
`message` from `SearchResponseWarning` type.

Collaborated with @gchaps on copy.

### Test setup
1. install sample web logs data set
2. install sample flights data set
3. Create data view.
    1. Set **Index pattern** to `kibana_sample_data*`
    2. Set **Time field** to `timestamp`
4. Open discover
5. Select **kibana_sample_data*** data view
6. set time range to last 24 hours
7. Add filter
    ```
    {
      "error_query": {
        "indices": [
          {
            "error_type": "exception",
            "message": "shard failure message 123",
            "name": "kibana_sample_data_logs",
            "shard_ids": [
              0
            ]
          }
        ]
      }
    }
    ```
8) save search as **kibana_sample_data***

#### Search response warnings callout
1. Open saved search created in test setup
<img width="500" alt="Screenshot 2023-10-24 at 8 49 19 AM"
src="https://github.com/elastic/kibana/assets/373691/867cff58-c201-4a6b-b049-7136b43d053f">
2. Click "expand" icon on left of first row in documents table
3. Click "Surrounding documents"
4. Re-enable "kibana_sample_data_logs failure" filter
<img width="500" alt="Screenshot 2023-10-24 at 8 51 22 AM"
src="https://github.com/elastic/kibana/assets/373691/a50cf033-64de-4909-a47d-6ee07bb915ea">

#### Search response warnings empty prompt
1. Open saved search created in test setup
2. Add filter `DistanceKilometers is -1`
<img width="500" alt="Screenshot 2023-10-24 at 8 44 13 AM"
src="https://github.com/elastic/kibana/assets/373691/e3ae0fac-8bda-4cad-b079-8ace4e01b786">

#### Search response warnings badge
1. create new dashboard
2. add saved search created during test setup
<img width="500" alt="Screenshot 2023-10-26 at 9 15 21 AM"
src="https://github.com/elastic/kibana/assets/373691/0066e3e2-953b-4631-a7aa-f389f7e6dbfc">


#### Search response warnings toast
1. create new table aggregation visualization
2. Use saved search created during test setup as source
<img width="500" alt="Screenshot 2023-10-24 at 2 59 41 PM"
src="https://github.com/elastic/kibana/assets/373691/58aab97e-71d9-49d9-bd67-73484ec54751">

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
nreese and kibanamachine authored Oct 27, 2023
1 parent b601f9a commit 4b27288
Show file tree
Hide file tree
Showing 37 changed files with 900 additions and 743 deletions.
7 changes: 4 additions & 3 deletions packages/kbn-search-response-warnings/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
export type { SearchResponseWarning, WarningHandlerCallback } from './src/types';

export {
SearchResponseWarnings,
type SearchResponseWarningsProps,
SearchResponseWarningsBadge,
SearchResponseWarningsBadgePopoverContent,
SearchResponseWarningsCallout,
SearchResponseWarningsEmptyPrompt,
} from './src/components/search_response_warnings';
export { ViewWarningButton } from './src/components/view_warning_button';

export { handleWarnings } from './src/handle_warnings';
export { hasUnsupportedDownsampledAggregationFailure } from './src/has_unsupported_downsampled_aggregation_failure';
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { SearchResponseWarning } from '../types';

export const searchResponseIncompleteWarningLocalCluster: SearchResponseWarning = {
type: 'incomplete',
message: 'The data might be incomplete or wrong.',
requestName: 'My request',
clusters: {
'(local)': {
status: 'partial',
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* 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 React, { useState } from 'react';
import { i18n } from '@kbn/i18n';
import { css } from '@emotion/react';
import { EuiButton, EuiIcon, EuiPopover, useEuiTheme, useEuiFontSize } from '@elastic/eui';
import { SearchResponseWarningsBadgePopoverContent } from './badge_popover_content';
import type { SearchResponseWarning } from '../../types';

interface Props {
warnings: SearchResponseWarning[];
}

export const SearchResponseWarningsBadge = (props: Props) => {
const [isPopoverOpen, setIsPopoverOpen] = useState<boolean>(false);
const { euiTheme } = useEuiTheme();
const xsFontSize = useEuiFontSize('xs').fontSize;

if (!props.warnings.length) {
return null;
}

return (
<EuiPopover
panelPaddingSize="none"
button={
<EuiButton
minWidth={0}
size="s"
color="warning"
onClick={() => setIsPopoverOpen(!isPopoverOpen)}
data-test-subj="searchResponseWarningsBadgeToogleButton"
title={i18n.translate('searchResponseWarnings.badgeButtonLabel', {
defaultMessage: '{warningCount} {warningCount, plural, one {warning} other {warnings}}',
values: {
warningCount: props.warnings.length,
},
})}
css={css`
block-size: ${euiTheme.size.l};
font-size: ${xsFontSize};
padding: 0 ${euiTheme.size.xs};
& > * {
gap: ${euiTheme.size.xs};
}
`}
>
<EuiIcon
type="warning"
css={css`
margin-left: ${euiTheme.size.xxs};
`}
/>
{props.warnings.length}
</EuiButton>
}
isOpen={isPopoverOpen}
closePopover={() => setIsPopoverOpen(false)}
>
<SearchResponseWarningsBadgePopoverContent
onViewDetailsClick={() => {
setIsPopoverOpen(false);
}}
warnings={props.warnings}
/>
</EuiPopover>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
* 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 React from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import { SearchResponseWarningsBadgePopoverContent } from './badge_popover_content';
import type { SearchResponseWarning } from '../../types';

describe('SearchResponseWarningsBadgePopoverContent', () => {
describe('single warning', () => {
test('Clicking "view details" should open warning details', () => {
const mockOpenInInspector = jest.fn();
const mockOnViewDetailsClick = jest.fn();
const warnings = [
{
type: 'incomplete',
requestName: 'My request',
clusters: {
remote1: {
status: 'partial',
indices: '',
timed_out: false,
},
},
openInInspector: mockOpenInInspector,
} as SearchResponseWarning,
];
render(
<SearchResponseWarningsBadgePopoverContent
onViewDetailsClick={mockOnViewDetailsClick}
warnings={warnings}
/>
);
const viewDetailsButton = screen.getByRole('button');
fireEvent.click(viewDetailsButton);
expect(mockOpenInInspector).toHaveBeenCalled();
expect(mockOnViewDetailsClick).toHaveBeenCalled();
});
});

describe('multiple warnings', () => {
const request1MockOpenInInspector = jest.fn();
const request2MockOpenInInspector = jest.fn();
const warnings = [
{
type: 'incomplete',
requestName: 'My first request',
clusters: {
remote1: {
status: 'partial',
indices: '',
timed_out: false,
},
},
openInInspector: request1MockOpenInInspector,
} as SearchResponseWarning,
{
type: 'incomplete',
requestName: 'My second request',
clusters: {
remote1: {
status: 'partial',
indices: '',
timed_out: false,
},
},
openInInspector: request2MockOpenInInspector,
} as SearchResponseWarning,
];

beforeEach(() => {
request1MockOpenInInspector.mockReset();
request2MockOpenInInspector.mockReset();
});

test('Clicking "view details" should open content panel with button to view details for each warning', () => {
const mockOnViewDetailsClick = jest.fn();
render(
<SearchResponseWarningsBadgePopoverContent
onViewDetailsClick={mockOnViewDetailsClick}
warnings={warnings}
/>
);
const viewDetailsButton = screen.getByRole('button');
fireEvent.click(viewDetailsButton);
expect(request1MockOpenInInspector).not.toHaveBeenCalled();
expect(request2MockOpenInInspector).not.toHaveBeenCalled();
expect(mockOnViewDetailsClick).not.toHaveBeenCalled();

const openRequest1Button = screen.getByRole('button', { name: 'My first request' });
fireEvent.click(openRequest1Button);
expect(request1MockOpenInInspector).toHaveBeenCalled();
expect(mockOnViewDetailsClick).toHaveBeenCalled();
expect(request2MockOpenInInspector).not.toHaveBeenCalled();
});

test('Should ensure unique request names by numbering duplicate request names', () => {
const warningsWithDuplicateRequestNames = warnings.map((warning) => {
return {
...warning,
requestName: 'Request',
};
});
render(
<SearchResponseWarningsBadgePopoverContent warnings={warningsWithDuplicateRequestNames} />
);
const viewDetailsButton = screen.getByRole('button');
fireEvent.click(viewDetailsButton);

screen.getByRole('button', { name: 'Request' });
screen.getByRole('button', { name: 'Request (2)' });
});
});
});
Loading

0 comments on commit 4b27288

Please sign in to comment.