[Lens] move from slice to reducers/actions and simplify loading #113324

mbondyra · 2021-09-28T20:09:11Z

Summary

Fixes #113218 - it was caused by not creating the new searchSession on start.

Instead of using redux-toolkit slice, I converted it to actions + reducers. The reason for this change was to be able to create reducer maker that would get the access to passed datasourceMap & visualizationMap.
Initialization invokes only one single action that is an effect of combining few
I've merged into one reducer switchVisualization and selectSuggestion, as they are almost the same.
I updated some tests and mocks. I tried to make them a bit better by:
- Not testing just middleware but the whole path from mounting the store to checking the state. It’s still not ideal, but better
- Unifying the mocked components we use in all places

mbondyra · 2021-09-30T16:13:33Z

x-pack/plugins/lens/public/state_management/init_middleware/load_initial.ts

+          initEmpty({
+            newState: {
+              ...emptyState,
+              searchSessionId: currentSessionId || data.search.session.start(),


this is what fixes the mentioned bug - somehow it was lost in the previous PRs and undefined searchSessionId caused infinite loop of requesting for data.

mbondyra · 2021-09-30T16:15:17Z

x-pack/plugins/lens/public/state_management/load_initial.test.tsx

+      expect(deps.lensServices.attributeService.unwrapAttributes).not.toHaveBeenCalled();
+    });
+
+    it('starts new searchSessionId', async () => {


this is the new test that tests the bug this PR fixes. The rest is refactored to test the whole path from invoking loadInitial (we do it in mounter) to run through middleware and reducer (before it was only checking middleware in separation)

mbondyra · 2021-09-30T16:15:55Z

x-pack/plugins/lens/public/state_management/time_range_middleware.test.ts

 import moment from 'moment';
 import { initialState } from './lens_slice';
 import { LensAppState } from './types';
 import { PayloadAction } from '@reduxjs/toolkit';
-
-const sessionIdSubject = new Subject<string>();


all these lines are now being shared from mocks file.

elasticmachine · 2021-09-30T16:16:16Z

Pinging @elastic/kibana-vis-editors (Team:VisEditors)

…y similar

dej611

Code review, left a couple fo comments.

dej611 · 2021-10-01T13:15:53Z

x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_helpers.ts

  const suggestions = getSuggestions({
    datasourceMap,
    datasourceStates,
    visualizationMap,
-    activeVisualization,
-    visualizationState,
+    activeVisualization: visualizationMap?.[Object.keys(visualizationMap)[0]] || null,


dej611 · 2021-10-01T13:21:57Z

x-pack/plugins/lens/public/app_plugin/app.test.tsx

@@ -94,7 +100,7 @@ describe('Lens App', () => {

  async function mountWith({
    props = makeDefaultProps(),
-    services = makeDefaultServices(sessionIdSubject),
+    services = makeDefaultServices(sessionIdSubject, 'sessionId-1'),


It can be a constant on top?

mbondyra · 2021-10-04T05:58:10Z

@elasticmachine merge upstream

kibanamachine · 2021-10-04T08:06:50Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`lens`	1.0MB	1.0MB	+1.1KB

Unknown metric groups

References to deprecated APIs

id	before	after	diff
`lens`	248	246	-2

History

💚 Build #157010 succeeded 9cb6f34
💔 Build #156894 failed 448cc018ca57f5f3666ec48545c25e3d8e751ccc
💚 Build #156828 succeeded 27508881f4d5b53e3dc3407009215fe0dc379a48

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

dej611

Tested locally in Chrome 👍

…tic#113324) * structure changes * tests & fix for sessionId * share mocks in time_range_middleware * make switchVisualization and selectSuggestion one reducer as it's very similar * CR Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-10-04T11:35:46Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

) (#113740) * structure changes * tests & fix for sessionId * share mocks in time_range_middleware * make switchVisualization and selectSuggestion one reducer as it's very similar * CR Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Marta Bondyra <[email protected]>

mbondyra added auto-backport Deprecated - use backport:version if exact versions are needed Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.16.0 v8.0.0 labels Sep 28, 2021

mbondyra force-pushed the lens/change_structure_simplify_loading branch 2 times, most recently from 1df0b6d to 2750888 Compare September 30, 2021 16:10

mbondyra commented Sep 30, 2021

View reviewed changes

elastic deleted a comment from kibanamachine Sep 30, 2021

mbondyra marked this pull request as ready for review September 30, 2021 16:16

mbondyra requested a review from a team as a code owner September 30, 2021 16:16

mbondyra force-pushed the lens/change_structure_simplify_loading branch from 2750888 to 448cc01 Compare September 30, 2021 19:57

mbondyra added 4 commits October 1, 2021 08:42

structure changes

91639c7

tests & fix for sessionId

a898d74

share mocks in time_range_middleware

7e24804

make switchVisualization and selectSuggestion one reducer as it's ver…

9cb6f34

…y similar

mbondyra force-pushed the lens/change_structure_simplify_loading branch from 448cc01 to 9cb6f34 Compare October 1, 2021 08:08

dej611 reviewed Oct 1, 2021

View reviewed changes

kibanamachine and others added 2 commits October 4, 2021 01:58

Merge branch 'master' into lens/change_structure_simplify_loading

7f332b8

CR

2b569a2

dej611 approved these changes Oct 4, 2021

View reviewed changes

mbondyra merged commit 6bfa2a4 into elastic:master Oct 4, 2021

mbondyra deleted the lens/change_structure_simplify_loading branch October 4, 2021 11:32

kibanamachine mentioned this pull request Oct 4, 2021

[7.x] [Lens] move from slice to reducers/actions and simplify loading (#113324) #113740

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] move from slice to reducers/actions and simplify loading #113324

[Lens] move from slice to reducers/actions and simplify loading #113324

mbondyra commented Sep 28, 2021 •

edited

Loading

mbondyra Sep 30, 2021

mbondyra Sep 30, 2021

mbondyra Sep 30, 2021

elasticmachine commented Sep 30, 2021

dej611 left a comment

dej611 Oct 1, 2021

dej611 Oct 1, 2021

mbondyra commented Oct 4, 2021

kibanamachine commented Oct 4, 2021

References to deprecated APIs

dej611 left a comment

kibanamachine commented Oct 4, 2021

[Lens] move from slice to reducers/actions and simplify loading #113324

[Lens] move from slice to reducers/actions and simplify loading #113324

Conversation

mbondyra commented Sep 28, 2021 • edited Loading

Summary

mbondyra Sep 30, 2021

Choose a reason for hiding this comment

mbondyra Sep 30, 2021

Choose a reason for hiding this comment

mbondyra Sep 30, 2021

Choose a reason for hiding this comment

elasticmachine commented Sep 30, 2021

dej611 left a comment

Choose a reason for hiding this comment

dej611 Oct 1, 2021

Choose a reason for hiding this comment

dej611 Oct 1, 2021

Choose a reason for hiding this comment

mbondyra commented Oct 4, 2021

kibanamachine commented Oct 4, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

References to deprecated APIs

History

dej611 left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 4, 2021

💚 Backport successful

mbondyra commented Sep 28, 2021 •

edited

Loading