Skip to content

Commit faa2ad6

Browse files
PhilippeObertieokoneyo
authored andcommitted
[Security Solution] removing preventing save a Timeline with an adhoc dataview (elastic#240537)
## Summary This PR makes a small change to the save Timeline flow and removes the code that was preventing users from saving a Timeline that was using an adhoc dataview. We don't really need to prevent that to happen, as Timeline only saves the list of indices anyway, and it has a way to create an adhoc data view on the fly when opening from that list of indices. #### Previous behavior https://github.com/user-attachments/assets/b316b46b-4436-41c3-851b-e4831ca7e331 ### New behavior https://github.com/user-attachments/assets/1583d1b5-5293-4bf1-b4c6-6699d6289ec5 The PR actually makes a code change that does not save any dataViewId if the data view is adhoc. We do not need to save the id of adhoc data views as they are ephemeris. They will most likely not exist when users reopen the Timeline. We have enough information with the list of indices to rebuilt a new adhoc data view on the fly next time the Timeline is reopened. ## TODO - [x] write unit tests - [ ] create doc ticket ### Checklist - [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 - [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) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels.
1 parent cbd0b7e commit faa2ad6

2 files changed

Lines changed: 56 additions & 20 deletions

File tree

x-pack/solutions/security/plugins/security_solution/public/timelines/store/middlewares/timeline_save.test.ts

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,19 +85,58 @@ describe('Timeline save middleware', () => {
8585
})
8686
);
8787
await store.dispatch(saveTimeline({ id: TimelineId.test, saveAsNew: false }));
88+
expect(mockDataViewManagerState).toBeDefined();
89+
expect(startTimelineSavingMock).toHaveBeenCalled();
90+
expect(persistTimeline as unknown as jest.Mock).toHaveBeenCalledWith(
91+
expect.objectContaining({
92+
timeline: expect.objectContaining({
93+
dataViewId: mockDataViewManagerState.dataViewManager.timeline.dataViewId,
94+
indexNames: ['test'],
95+
}),
96+
})
97+
);
98+
expect(refreshTimelines as unknown as jest.Mock).toHaveBeenCalled();
99+
expect(endTimelineSavingMock).toHaveBeenCalled();
100+
expect(selectTimelineById(store.getState(), TimelineId.test)).toEqual(
101+
expect.objectContaining({
102+
version: 'newVersion',
103+
changed: false,
104+
})
105+
);
106+
});
88107

108+
it('should not save any adhoc dataViewId when persisting a timeline', async () => {
109+
dataView = getMockDataViewWithMatchedIndices();
110+
dataView.version = undefined;
111+
(kibanaMock.plugins.onStart as jest.Mock).mockReturnValue({
112+
dataViews: {
113+
found: true,
114+
contract: { get: () => dataView },
115+
},
116+
});
117+
(persistTimeline as jest.Mock).mockResolvedValue({
118+
savedObjectId: 'soid',
119+
version: 'newVersion',
120+
});
121+
122+
await store.dispatch(setChanged({ id: TimelineId.test, changed: true }));
123+
expect(selectTimelineById(store.getState(), TimelineId.test)).toEqual(
124+
expect.objectContaining({
125+
version: null,
126+
changed: true,
127+
})
128+
);
129+
await store.dispatch(saveTimeline({ id: TimelineId.test, saveAsNew: false }));
89130
expect(mockDataViewManagerState).toBeDefined();
90131
expect(startTimelineSavingMock).toHaveBeenCalled();
91-
expect(persistTimeline as unknown as jest.Mock).toHaveBeenCalled();
92-
// TODO: skipped the assertion below until the new picker is enabled https://github.com/elastic/security-team/issues/11959
93-
// expect(persistTimeline as unknown as jest.Mock).toHaveBeenCalledWith(
94-
// expect.objectContaining({
95-
// timeline: expect.objectContaining({
96-
// dataViewId: mockDataViewManagerState.dataViewManager.timeline.dataViewId,
97-
// indexNames: ['test'],
98-
// }),
99-
// })
100-
// );
132+
expect(persistTimeline as unknown as jest.Mock).toHaveBeenCalledWith(
133+
expect.objectContaining({
134+
timeline: expect.objectContaining({
135+
dataViewId: null,
136+
indexNames: ['test'],
137+
}),
138+
})
139+
);
101140
expect(refreshTimelines as unknown as jest.Mock).toHaveBeenCalled();
102141
expect(endTimelineSavingMock).toHaveBeenCalled();
103142
expect(selectTimelineById(store.getState(), TimelineId.test)).toEqual(

x-pack/solutions/security/plugins/security_solution/public/timelines/store/middlewares/timeline_save.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ export const saveTimelineMiddleware: (kibana: CoreStart) => Middleware<{}, State
8585
storeState.app.enableExperimental.newDataViewPickerEnabled;
8686

8787
let experimentalSelectedPatterns: string[] = [];
88+
let dataViewId: string | null = null;
89+
90+
if (!experimentalIsDataViewEnabled) {
91+
dataViewId = selectedDataViewIdSourcerer;
92+
}
8893

8994
// NOTE: remove eslint override above after the experimental picker is stabilized
9095
if (experimentalIsDataViewEnabled && experimentalDataViewId) {
@@ -95,13 +100,8 @@ export const saveTimelineMiddleware: (kibana: CoreStart) => Middleware<{}, State
95100
if (plugins.dataViews.found) {
96101
const experimentalDataView = await plugins.dataViews.contract.get(experimentalDataViewId);
97102

98-
if (!experimentalDataView.isPersisted()) {
99-
return kibana.notifications.toasts.addError(
100-
new Error('Persisting timelines with adhoc data views is not allowed'),
101-
{
102-
title: 'Error persisting timeline',
103-
}
104-
);
103+
if (experimentalDataView.isPersisted()) {
104+
dataViewId = experimentalDataViewId;
105105
}
106106

107107
experimentalSelectedPatterns = experimentalDataView.getIndexPattern().split(',');
@@ -111,9 +111,6 @@ export const saveTimelineMiddleware: (kibana: CoreStart) => Middleware<{}, State
111111
const indexNames = experimentalIsDataViewEnabled
112112
? experimentalSelectedPatterns
113113
: selectedPatternsSourcerer;
114-
const dataViewId = experimentalIsDataViewEnabled
115-
? experimentalDataViewId
116-
: selectedDataViewIdSourcerer;
117114

118115
store.dispatch(startTimelineSaving({ id: localTimelineId }));
119116

0 commit comments

Comments
 (0)