Skip to content

Commit

Permalink
[Serverless][DataUsage] UX updates from demo feedback (elastic#201638)
Browse files Browse the repository at this point in the history
## Summary

Follow up of elastic#200911

- [x] Treat date picker values as UTC date/time
- [ ] Tooltip for date picker
- [x] Align filters (right aligned for all screens and responsive)
- [x] Disable `refresh` button if no `metricTypes` or `dataStreams` or
invalid date range
- [x] Validate selected date/time range before sending requests

![Screenshot 2024-11-25 at 17 25
43](https://github.com/user-attachments/assets/bc61ad73-2c85-4b40-9156-c5ccc87e0d7e)
![Screenshot 2024-11-25 at 17 27
53](https://github.com/user-attachments/assets/3aef26d1-6df2-4315-a939-0807c9da18c7)
![Screenshot 2024-11-26 at 15 45
56](https://github.com/user-attachments/assets/f7e1da72-4559-42e0-bee2-2e53709fd812)

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...
  • Loading branch information
ashokaditya authored Nov 28, 2024
1 parent f50b93c commit d0d33aa
Show file tree
Hide file tree
Showing 17 changed files with 241 additions and 67 deletions.
12 changes: 12 additions & 0 deletions x-pack/plugins/data_usage/common/rest_types/usage_metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ export const METRIC_TYPE_API_VALUES_TO_UI_OPTIONS_MAP = Object.freeze<Record<Met
search_rate: 'Search Rate',
});

export const METRIC_TYPE_UI_OPTIONS_VALUES_TO_API_MAP = Object.freeze<Record<string, MetricTypes>>({
'Data Retained in Storage': 'storage_retained',
'Data Ingested': 'ingest_rate',
'Search VCU': 'search_vcu',
'Ingest VCU': 'ingest_vcu',
'ML VCU': 'ml_vcu',
'Index Latency': 'index_latency',
'Index Rate': 'index_rate',
'Search Latency': 'search_latency',
'Search Rate': 'search_rate',
});

// type guard for MetricTypes
export const isMetricType = (type: string): type is MetricTypes =>
METRIC_TYPE_VALUES.includes(type as MetricTypes);
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/data_usage/common/test_utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,3 @@

export { TestProvider } from './test_provider';
export { dataUsageTestQueryClientOptions } from './test_query_client_options';
export { timeXMinutesAgo } from './time_ago';
9 changes: 0 additions & 9 deletions x-pack/plugins/data_usage/common/test_utils/time_ago.ts

This file was deleted.

33 changes: 33 additions & 0 deletions x-pack/plugins/data_usage/common/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* 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 { isDateRangeValid } from './utils';

describe('isDateRangeValid', () => {
describe('Valid ranges', () => {
it.each([
['both start and end date is `now`', { start: 'now', end: 'now' }],
['start date is `now-10s` and end date is `now`', { start: 'now-10s', end: 'now' }],
['bounded within the min and max date range', { start: 'now-8d', end: 'now-4s' }],
])('should return true if %s', (_, { start, end }) => {
expect(isDateRangeValid({ start, end })).toBe(true);
});
});

describe('Invalid ranges', () => {
it.each([
['starts before the min date', { start: 'now-11d', end: 'now-5s' }],
['ends after the max date', { start: 'now-9d', end: 'now+2s' }],
[
'end date is before the start date even when both are within min and max date range',
{ start: 'now-3s', end: 'now-10s' },
],
])('should return false if the date range %s', (_, { start, end }) => {
expect(isDateRangeValid({ start, end })).toBe(false);
});
});
});
47 changes: 46 additions & 1 deletion x-pack/plugins/data_usage/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,50 @@
*/

import dateMath from '@kbn/datemath';
export const dateParser = (date: string) => dateMath.parse(date)?.toISOString();

export const DEFAULT_DATE_RANGE_OPTIONS = Object.freeze({
autoRefreshOptions: {
enabled: false,
duration: 10000,
},
startDate: 'now-24h/h',
endDate: 'now',
maxDate: 'now+1s',
minDate: 'now-9d',
recentlyUsedDateRanges: [],
});

export const momentDateParser = (date: string) => dateMath.parse(date);
export const transformToUTCtime = ({
start,
end,
isISOString = false,
}: {
start: string;
end: string;
isISOString?: boolean;
}) => {
const utcOffset = momentDateParser(start)?.utcOffset() ?? 0;
const utcStart = momentDateParser(start)?.utc().add(utcOffset, 'm');
const utcEnd = momentDateParser(end)?.utc().add(utcOffset, 'm');
return {
start: isISOString ? utcStart?.toISOString() : momentDateParser(start),
end: isISOString ? utcEnd?.toISOString() : momentDateParser(end),
};
};

export const isDateRangeValid = ({ start, end }: { start: string; end: string }): boolean => {
const startDate = momentDateParser(start);
const endDate = momentDateParser(end);

if (!startDate || !endDate) {
return false;
}
const minDate = momentDateParser(DEFAULT_DATE_RANGE_OPTIONS.minDate);
const maxDate = momentDateParser(DEFAULT_DATE_RANGE_OPTIONS.maxDate);
return (
startDate.isSameOrAfter(minDate, 's') &&
endDate.isSameOrBefore(maxDate, 's') &&
startDate <= endDate
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -344,18 +344,27 @@ describe('DataUsageMetrics', () => {
});

it('should refetch usage metrics with `Refresh` button click', async () => {
const refetch = jest.fn();
mockUseGetDataUsageMetrics.mockReturnValue({
...getBaseMockedDataUsageMetrics,
data: ['.ds-1', '.ds-2'],
isFetched: true,
mockUseGetDataUsageDataStreams.mockReturnValue({
error: undefined,
data: generateDataStreams(3),
isFetching: false,
});
const refetch = jest.fn();
mockUseGetDataUsageMetrics.mockReturnValue({
...getBaseMockedDataUsageMetrics,
isFetched: true,
data: {},
isFetched: false,
refetch,
});
const { getByTestId } = renderComponent();
const { getByTestId, getAllByTestId } = renderComponent();

const toggleFilterButton = getByTestId(`${testIdFilter}-dataStreams-popoverButton`);

expect(toggleFilterButton).toHaveTextContent('Data streams3');
await user.click(toggleFilterButton);
const allFilterOptions = getAllByTestId('dataStreams-filter-option');
await user.click(allFilterOptions[2]);

const refreshButton = getByTestId(`${testIdFilter}-super-refresh-button`);
// click refresh 5 times
for (let i = 0; i < 5; i++) {
Expand All @@ -364,7 +373,7 @@ describe('DataUsageMetrics', () => {

expect(mockUseGetDataUsageMetrics).toHaveBeenLastCalledWith(
expect.any(Object),
expect.objectContaining({ enabled: false })
expect.objectContaining({ enabled: true })
);
expect(refetch).toHaveBeenCalledTimes(5);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ import { PLUGIN_NAME } from '../../translations';
import { useGetDataUsageMetrics } from '../../hooks/use_get_usage_metrics';
import { useGetDataUsageDataStreams } from '../../hooks/use_get_data_streams';
import { useDataUsageMetricsUrlParams } from '../hooks/use_charts_url_params';
import { DEFAULT_DATE_RANGE_OPTIONS, useDateRangePicker } from '../hooks/use_date_picker';
import {
DEFAULT_DATE_RANGE_OPTIONS,
transformToUTCtime,
isDateRangeValid,
} from '../../../common/utils';
import { useDateRangePicker } from '../hooks/use_date_picker';
import { ChartFilters, ChartFiltersProps } from './filters/charts_filters';
import { ChartsLoading } from './charts_loading';
import { NoDataCallout } from './no_data_callout';
Expand Down Expand Up @@ -104,11 +109,39 @@ export const DataUsageMetrics = memo(
...prevState,
metricTypes: metricTypesFromUrl?.length ? metricTypesFromUrl : prevState.metricTypes,
dataStreams: dataStreamsFromUrl?.length ? dataStreamsFromUrl : prevState.dataStreams,
from: startDateFromUrl ?? prevState.from,
to: endDateFromUrl ?? prevState.to,
}));
}, [metricTypesFromUrl, dataStreamsFromUrl]);
}, [metricTypesFromUrl, dataStreamsFromUrl, startDateFromUrl, endDateFromUrl]);

const { dateRangePickerState, onRefreshChange, onTimeChange } = useDateRangePicker();

const isValidDateRange = useMemo(
() =>
isDateRangeValid({
start: dateRangePickerState.startDate,
end: dateRangePickerState.endDate,
}),
[dateRangePickerState.endDate, dateRangePickerState.startDate]
);

const enableFetchUsageMetricsData = useMemo(
() =>
isValidDateRange &&
metricsFilters.dataStreams.length > 0 &&
metricsFilters.metricTypes.length > 0,
[isValidDateRange, metricsFilters.dataStreams, metricsFilters.metricTypes]
);

const utcTimeRange = useMemo(
() =>
transformToUTCtime({
start: dateRangePickerState.startDate,
end: dateRangePickerState.endDate,
isISOString: true,
}),
[dateRangePickerState]
);
const {
error: errorFetchingDataUsageMetrics,
data: usageMetricsData,
Expand All @@ -118,12 +151,12 @@ export const DataUsageMetrics = memo(
} = useGetDataUsageMetrics(
{
...metricsFilters,
from: dateRangePickerState.startDate,
to: dateRangePickerState.endDate,
from: utcTimeRange.start as string,
to: utcTimeRange.end as string,
},
{
retry: false,
enabled: !!(metricsFilters.dataStreams.length && metricsFilters.metricTypes.length),
enabled: enableFetchUsageMetricsData,
}
);

Expand All @@ -134,8 +167,11 @@ export const DataUsageMetrics = memo(
}, [isFetching, hasFetchedDataUsageMetricsData]);

const onRefresh = useCallback(() => {
if (!enableFetchUsageMetricsData) {
return;
}
refetchDataUsageMetrics();
}, [refetchDataUsageMetrics]);
}, [enableFetchUsageMetricsData, refetchDataUsageMetrics]);

const onChangeDataStreamsFilter = useCallback(
(selectedDataStreams: string[]) => {
Expand Down Expand Up @@ -206,6 +242,7 @@ export const DataUsageMetrics = memo(
<ChartFilters
dateRangePickerState={dateRangePickerState}
isDataLoading={isFetchingDataStreams}
isUpdateDisabled={!enableFetchUsageMetricsData}
onClick={refetchDataUsageMetrics}
onRefresh={onRefresh}
onRefreshChange={onRefreshChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
* 2.0.
*/

import { orderBy, findKey } from 'lodash/fp';
import { orderBy } from 'lodash/fp';
import React, { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { EuiPopoverTitle, EuiSelectable, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';

import { useTestIdGenerator } from '../../../hooks/use_test_id_generator';
import { METRIC_TYPE_API_VALUES_TO_UI_OPTIONS_MAP } from '../../../../common/rest_types';
import { METRIC_TYPE_UI_OPTIONS_VALUES_TO_API_MAP } from '../../../../common/rest_types';
import { UX_LABELS } from '../../../translations';
import { ChartsFilterPopover } from './charts_filter_popover';
import { ToggleAllButton } from './toggle_all_button';
Expand Down Expand Up @@ -171,7 +171,7 @@ export const ChartsFilter = memo<ChartsFilterProps>(
if (isMetricsFilter) {
setUrlMetricTypesFilter(
optionsToSelect
.map((option) => findKey(METRIC_TYPE_API_VALUES_TO_UI_OPTIONS_MAP, option))
.map((option) => METRIC_TYPE_UI_OPTIONS_VALUES_TO_API_MAP[option])
.join(',')
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { FilterName } from '../../hooks';
export interface ChartFiltersProps {
dateRangePickerState: DateRangePickerValues;
isDataLoading: boolean;
isUpdateDisabled: boolean;
filterOptions: Record<FilterName, ChartsFilterProps['filterOptions']>;
onRefresh: () => void;
onRefreshChange: (evt: OnRefreshChangeProps) => void;
Expand All @@ -33,6 +34,7 @@ export const ChartFilters = memo<ChartFiltersProps>(
({
dateRangePickerState,
isDataLoading,
isUpdateDisabled,
filterOptions,
onClick,
onRefresh,
Expand All @@ -59,7 +61,8 @@ export const ChartFilters = memo<ChartFiltersProps>(
const onClickRefreshButton = useCallback(() => onClick(), [onClick]);

return (
<EuiFlexGroup responsive gutterSize="m">
<EuiFlexGroup responsive gutterSize="m" alignItems="center" justifyContent="flexEnd">
<EuiFlexItem grow={2} />
<EuiFlexItem grow={1}>
<EuiFilterGroup>{filters}</EuiFilterGroup>
</EuiFlexItem>
Expand All @@ -78,6 +81,7 @@ export const ChartFilters = memo<ChartFiltersProps>(
data-test-subj={getTestId('super-refresh-button')}
fill={false}
isLoading={isDataLoading}
isDisabled={isUpdateDisabled}
onClick={onClickRefreshButton}
/>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ import type {
OnRefreshChangeProps,
} from '@elastic/eui/src/components/date_picker/types';
import { UI_SETTINGS } from '@kbn/data-plugin/common';
import { momentDateParser } from '../../../../common/utils';
import { momentDateParser, DEFAULT_DATE_RANGE_OPTIONS } from '../../../../common/utils';
import { useTestIdGenerator } from '../../../hooks/use_test_id_generator';
import { DEFAULT_DATE_RANGE_OPTIONS } from '../../hooks/use_date_picker';

export interface DateRangePickerValues {
autoRefreshOptions: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { useCallback, useEffect, useMemo, useState } from 'react';
import { useHistory, useLocation } from 'react-router-dom';
import { type MetricTypes, isMetricType } from '../../../common/rest_types';
import { useUrlParams } from '../../hooks/use_url_params';
import { DEFAULT_DATE_RANGE_OPTIONS } from './use_date_picker';
import { DEFAULT_DATE_RANGE_OPTIONS } from '../../../common/utils';

interface UrlParamsDataUsageMetricsFilters {
metricTypes: string;
Expand Down
13 changes: 1 addition & 12 deletions x-pack/plugins/data_usage/public/app/hooks/use_date_picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,7 @@ import type {
} from '@elastic/eui/src/components/date_picker/types';
import { useDataUsageMetricsUrlParams } from './use_charts_url_params';
import { DateRangePickerValues } from '../components/filters/date_picker';

export const DEFAULT_DATE_RANGE_OPTIONS = Object.freeze({
autoRefreshOptions: {
enabled: false,
duration: 10000,
},
startDate: 'now-24h/h',
endDate: 'now',
maxDate: 'now+1s',
minDate: 'now-9d',
recentlyUsedDateRanges: [],
});
import { DEFAULT_DATE_RANGE_OPTIONS } from '../../../common/utils';

export const useDateRangePicker = () => {
const {
Expand Down
Loading

0 comments on commit d0d33aa

Please sign in to comment.