Skip to content

Commit bae752e

Browse files
Fix "now" and mixed format date handling in share modal (#245539)
## Summary This PR fixes a bug where mixed dates (e.g one absolute and one relative) wouldn't work properly and "now" was rendering as 0 seconds in share modal date switcher. <img width="494" height="432" alt="Screenshot 2025-12-09 at 13 13 05" src="https://github.com/user-attachments/assets/4d5eb332-7329-40be-b725-97cf5020b73a" /> It also makes `ShareableUrlLocatorParams` strongly typed, requiring `timeRange` to be always present even if it has an undefined value, in response to a regression where dashboard time range parameter name was renamed to be snake_case, which broke the functionality as `TimeTypeSection` would always expect `timeRange`. Closes: #241084 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
1 parent 39ef2de commit bae752e

12 files changed

Lines changed: 173 additions & 155 deletions

File tree

src/platform/plugins/shared/dashboard/public/dashboard_app/top_nav/share/show_share_modal.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ export function ShowShareModal({
215215
locator: shareService.url.locators.get(
216216
DASHBOARD_APP_LOCATOR
217217
) as LocatorPublic<DashboardLocatorParams>,
218-
params: locatorParams,
218+
params: { ...locatorParams, timeRange: locatorParams.time_range },
219219
},
220220
});
221221
}

src/platform/plugins/shared/discover/public/application/main/components/top_nav/app_menu_actions/get_share.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { AppMenuActionId, AppMenuActionType } from '@kbn/discover-utils';
1212
import { omit } from 'lodash';
1313
import { setStateToKbnUrl } from '@kbn/kibana-utils-plugin/public';
1414
import { i18n } from '@kbn/i18n';
15+
import type { TimeRange } from '@kbn/es-query';
1516
import type { DiscoverSession } from '@kbn/saved-search-plugin/common';
1617
import type { DiscoverStateContainer } from '../../../state_management/discover_state';
1718
import { getSharingData, showPublicUrlSwitch } from '../../../../../utils/get_sharing_data';
@@ -64,14 +65,14 @@ export const getShareAppMenuItem = ({
6465
const filters = services.filterManager.getFilters();
6566

6667
// Share -> Get links -> Snapshot
67-
const params: DiscoverAppLocatorParams = {
68+
const params: DiscoverAppLocatorParams & { timeRange: TimeRange | undefined } = {
6869
...omit(currentTab.appState, 'dataSource'),
6970
...(persistedDiscoverSession?.id ? { savedSearchId: persistedDiscoverSession.id } : {}),
7071
...(dataView?.isPersisted()
7172
? { dataViewId: dataView?.id }
7273
: { dataViewSpec: dataView?.toMinimalSpec() }),
7374
filters,
74-
timeRange,
75+
timeRange: timeRange ?? undefined,
7576
refreshInterval,
7677
};
7778

src/platform/plugins/shared/share/moon.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ dependsOn:
4444
- '@kbn/core-logging-server-mocks'
4545
- '@kbn/core-http-router-server-mocks'
4646
- '@kbn/licensing-types'
47+
- '@kbn/es-query'
4748
tags:
4849
- plugin
4950
- prod

src/platform/plugins/shared/share/public/components/share_context_menu.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@ import type { Capabilities } from '@kbn/core/public';
1818

1919
import type { LocatorPublic } from '../../common';
2020
import { UrlPanelContent } from './url_panel_content';
21-
import type { ShareMenuItemLegacy, ShareContextMenuPanelItem, UrlParamExtension } from '../types';
21+
import type {
22+
ShareMenuItemLegacy,
23+
ShareContextMenuPanelItem,
24+
UrlParamExtension,
25+
ShareableUrlLocatorParams,
26+
} from '../types';
2227
import type { AnonymousAccessServiceContract } from '../../common/anonymous_access';
2328
import type { BrowserUrlService } from '../types';
2429

@@ -31,7 +36,7 @@ export interface ShareContextMenuProps {
3136
shareableUrlForSavedObject?: string;
3237
shareableUrlLocatorParams?: {
3338
locator: LocatorPublic<any>;
34-
params: any;
39+
params: ShareableUrlLocatorParams;
3540
};
3641
shareMenuItems: ShareMenuItemLegacy[];
3742
sharingData: any;

src/platform/plugins/shared/share/public/components/tabs/link/link_content.tsx

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99

10-
import type { EuiSwitchEvent } from '@elastic/eui';
1110
import {
1211
copyToClipboard,
1312
EuiButton,
@@ -126,20 +125,23 @@ export const LinkContent = ({
126125
setIsLoading(false);
127126
}, [snapshotUrl, delegatedShareUrlHandler, allowShortUrl, createShortUrl]);
128127

129-
const changeTimeType = (e: EuiSwitchEvent) => {
130-
setIsAbsoluteTime(e.target.checked);
131-
if (urlToCopy?.current && e.target.checked !== isAbsoluteTime) {
132-
urlToCopy.current = undefined;
133-
}
134-
};
128+
const handleTimeTypeChange = useCallback(
129+
(isAbsolute: boolean) => {
130+
if (urlToCopy?.current && isAbsolute !== isAbsoluteTime) {
131+
urlToCopy.current = undefined;
132+
}
133+
setIsAbsoluteTime(isAbsolute);
134+
},
135+
[isAbsoluteTime]
136+
);
135137

136138
return (
137139
<>
138140
<EuiForm>
139141
<TimeTypeSection
140142
timeRange={timeRange}
141-
isAbsoluteTime={isAbsoluteTime}
142-
changeTimeType={changeTimeType}
143+
onTimeTypeChange={handleTimeTypeChange}
144+
isAbsoluteTimeByDefault={isAbsoluteTime}
143145
/>
144146
{isDirty && draftModeCallOut && (
145147
<>

src/platform/plugins/shared/share/public/components/tabs/link/time_type_section.test.tsx

Lines changed: 64 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import React from 'react';
1212
import { __IntlProvider as IntlProvider } from '@kbn/i18n-react';
1313
import { render, screen } from '@testing-library/react';
1414
import { TimeTypeSection } from './time_type_section';
15-
import * as timeUtils from '../../../lib/time_utils';
1615

1716
const renderComponent = (props: ComponentProps<typeof TimeTypeSection>) => {
1817
render(
@@ -25,123 +24,132 @@ const renderComponent = (props: ComponentProps<typeof TimeTypeSection>) => {
2524
describe('TimeTypeSection', () => {
2625
beforeEach(() => {
2726
jest.clearAllMocks();
28-
jest
29-
.spyOn(timeUtils, 'convertRelativeTimeStringToAbsoluteTimeDate')
30-
.mockReturnValue(new Date());
31-
jest
32-
.spyOn(timeUtils, 'getRelativeTimeValueAndUnitFromTimeString')
33-
.mockImplementation((time) => {
34-
if (time === 'now') return { value: 0, unit: 'second', roundingUnit: undefined };
35-
if (time === 'now-1m') return { value: -1, unit: 'minute', roundingUnit: undefined };
36-
if (time === 'now-30m') return { value: -30, unit: 'minute', roundingUnit: undefined };
37-
return { value: 0, unit: 'second', roundingUnit: undefined };
38-
});
39-
jest.spyOn(timeUtils, 'isTimeRangeAbsoluteTime').mockReturnValue(false);
4027
});
4128

42-
it('renders null when timeRange is not provided', () => {
43-
const changeTimeType = jest.fn();
29+
it('should render null when timeRange is not provided', () => {
30+
const onTimeTypeChange = jest.fn();
4431

4532
renderComponent({
46-
isAbsoluteTime: false,
47-
changeTimeType,
33+
isAbsoluteTimeByDefault: false,
34+
onTimeTypeChange,
4835
});
4936

5037
const timeRangeSwitch = screen.queryByRole('switch');
5138

5239
expect(timeRangeSwitch).not.toBeInTheDocument();
5340
});
5441

55-
it('renders with absolute time range', () => {
42+
it('should render absolute time range', () => {
5643
const timeRange = { from: '2022-01-01T00:00:00.000Z', to: '2022-01-02T00:00:00.000Z' };
57-
const changeTimeType = jest.fn();
44+
const onTimeTypeChange = jest.fn();
5845

5946
renderComponent({
6047
timeRange,
61-
isAbsoluteTime: true,
62-
changeTimeType,
48+
isAbsoluteTimeByDefault: true,
49+
onTimeTypeChange,
6350
});
6451

65-
const timeRangeSwitch = screen.getByRole('switch');
66-
67-
expect(timeRangeSwitch).toBeChecked();
68-
6952
const absoluteTimeInfoText = screen.getByTestId('absoluteTimeInfoText');
7053

7154
expect(absoluteTimeInfoText).toBeInTheDocument();
55+
expect(screen.getByText(/January 01, 2022/)).toBeInTheDocument();
56+
expect(screen.getByText(/January 02, 2022/)).toBeInTheDocument();
7257
});
7358

74-
it('renders with relative time range (from now to specific time)', () => {
59+
it('should render relative time range', () => {
7560
const timeRange = { from: 'now', to: 'now+15m' };
76-
const changeTimeType = jest.fn();
61+
const onTimeTypeChange = jest.fn();
7762

7863
renderComponent({
7964
timeRange,
80-
isAbsoluteTime: false,
81-
changeTimeType,
65+
isAbsoluteTimeByDefault: false,
66+
onTimeTypeChange,
8267
});
8368

8469
const timeRangeSwitch = screen.getByRole('switch');
8570

8671
expect(timeRangeSwitch).not.toBeChecked();
8772

88-
const relativeTimeFromNowInfoText = screen.getByTestId('relativeTimeInfoTextFromNow');
73+
expect(screen.getByText('now')).toBeInTheDocument();
74+
expect(screen.getByText('in 15 minutes')).toBeInTheDocument();
75+
});
76+
77+
it('should hide switch when timeRange is already absolute', () => {
78+
const timeRange = { from: '2022-01-01T00:00:00.000Z', to: '2022-01-02T00:00:00.000Z' };
79+
const onTimeTypeChange = jest.fn();
80+
81+
renderComponent({
82+
timeRange,
83+
isAbsoluteTimeByDefault: true,
84+
onTimeTypeChange,
85+
});
8986

90-
expect(relativeTimeFromNowInfoText).toBeInTheDocument();
87+
const timeRangeSwitch = screen.queryByRole('switch');
88+
89+
expect(timeRangeSwitch).not.toBeInTheDocument();
9190
});
9291

93-
it('renders with relative time range (from specific time to now)', () => {
94-
const timeRange = { from: 'now-30m', to: 'now' };
95-
const changeTimeType = jest.fn();
92+
it('should render with mixed time range (absolute from, relative to)', () => {
93+
const timeRange = { from: '2022-01-01T00:00:00.000Z', to: 'now' };
94+
const onTimeTypeChange = jest.fn();
9695

9796
renderComponent({
9897
timeRange,
99-
isAbsoluteTime: false,
100-
changeTimeType,
98+
isAbsoluteTimeByDefault: false,
99+
onTimeTypeChange,
101100
});
102101

103102
const timeRangeSwitch = screen.getByRole('switch');
104103

105104
expect(timeRangeSwitch).not.toBeChecked();
106105

107-
const relativeTimeToNowInfoText = screen.getByTestId('relativeTimeInfoTextToNow');
108-
109-
expect(relativeTimeToNowInfoText).toBeInTheDocument();
106+
expect(screen.getByText(/January 01, 2022/)).toBeInTheDocument();
107+
expect(screen.getByText('now')).toBeInTheDocument();
110108
});
111109

112-
it('renders with relative time range (between two relative times)', () => {
113-
const timeRange = { from: 'now-30m', to: 'now-1m' };
114-
const changeTimeType = jest.fn();
110+
it('should render with mixed time range (relative from, absolute to)', () => {
111+
const timeRange = { from: 'now-30m', to: '2022-01-01T00:00:00.000Z' };
112+
const onTimeTypeChange = jest.fn();
115113

116114
renderComponent({
117115
timeRange,
118-
isAbsoluteTime: false,
119-
changeTimeType,
116+
isAbsoluteTimeByDefault: false,
117+
onTimeTypeChange,
120118
});
121119

122120
const timeRangeSwitch = screen.getByRole('switch');
123121

124122
expect(timeRangeSwitch).not.toBeChecked();
125123

126-
const relativeTimeInfoText = screen.getByTestId('relativeTimeInfoTextDefault');
127-
128-
expect(relativeTimeInfoText).toBeInTheDocument();
124+
expect(screen.getByText('30 minutes ago')).toBeInTheDocument();
125+
expect(screen.getByText(/January 01, 2022/)).toBeInTheDocument();
129126
});
130127

131-
it('disables switch when timeRange is already absolute', () => {
132-
const timeRange = { from: '2022-01-01T00:00:00.000Z', to: '2022-01-02T00:00:00.000Z' };
133-
const changeTimeType = jest.fn();
134-
135-
jest.spyOn(timeUtils, 'isTimeRangeAbsoluteTime').mockReturnValue(true);
128+
it('should render "now"', () => {
129+
const timeRange = { from: 'now-30m', to: 'now' };
130+
const onTimeTypeChange = jest.fn();
136131

137132
renderComponent({
138133
timeRange,
139-
isAbsoluteTime: false,
140-
changeTimeType,
134+
isAbsoluteTimeByDefault: false,
135+
onTimeTypeChange,
141136
});
142137

143-
const timeRangeSwitch = screen.queryByRole('switch');
138+
expect(screen.getByText('30 minutes ago')).toBeInTheDocument();
139+
expect(screen.getByText('now')).toBeInTheDocument();
140+
});
144141

145-
expect(timeRangeSwitch).not.toBeInTheDocument();
142+
it('should handle plain "now" value correctly in mixed ranges', () => {
143+
const timeRange = { from: '2025-11-10T14:17:51.794Z', to: 'now' };
144+
const onTimeTypeChange = jest.fn();
145+
146+
renderComponent({
147+
timeRange,
148+
isAbsoluteTimeByDefault: false,
149+
onTimeTypeChange,
150+
});
151+
152+
expect(screen.getByText(/November 10, 2025/)).toBeInTheDocument();
153+
expect(screen.getByText('now')).toBeInTheDocument();
146154
});
147155
});

0 commit comments

Comments
 (0)