[Lens] fix dimension panel datasource updates #161881

mbondyra · 2023-07-13T15:04:08Z

Summary

Fixes #161759
Fixes #161854
Fixes #161988
Fixes #161666

This bug surfaced by my change with NativeRenderer removal PR. It happens because sometimes there are two updates happening at the same time, in this case
*setState coming from optimizing middleware to update the relative timerange in a call to es

updating datasource.

Two updates is not a problem and works correctly for the same case for visualizations etc, but for the updateDatasourceState we break one of the fundamental rules of redux by passing non-serializable prop to action (an updater function that is then called internally with the previous state). The problem is that in the meantime the state gets updated from the previous setState call. To avoid this problem I rewrote the updateDatasourceState action to not accept updater function. To avoid these problems in the future I've also included a serializableCheck that ignores dataViews and some other properties (they contain some non-serializable data, I hope we'll take care of them in the future)

 serializableCheck: {
        ignoredActionPaths: ['lens.dataViews.indexPatterns'],
        ignoredPaths: ['lens.dataViews.indexPatterns'],
      },

I also fixed a few small issues, will leave comments in the code.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

mbondyra · 2023-07-13T16:05:23Z

...lugins/chart_expressions/expression_heatmap/public/expression_renderers/heatmap_renderer.tsx

-            syncCursor={config.syncCursor}
-          />
-        </div>
+        <I18nProvider>


non related to the main issue, but just saw it in the console and decided to fix

mbondyra · 2023-07-13T16:06:14Z

x-pack/plugins/lens/public/datasources/form_based/datapanel.tsx

@@ -40,7 +40,7 @@ import { FieldItem } from '../common/field_item';

 export type Props = Omit<
  DatasourceDataPanelProps<FormBasedPrivateState>,
-  'core' | 'onChangeIndexPattern' | 'dragDropContext'
+  'core' | 'onChangeIndexPattern'


non related to the main issue, but it's a leftover I forgot about

mbondyra · 2023-07-13T16:06:25Z

x-pack/plugins/lens/public/datasources/form_based/dimension_panel/dimension_editor.tsx

@@ -276,7 +276,7 @@ export function DimensionEditor(props: DimensionEditorProps) {
  // changes from the static value operation (which has to be a function)
  // Note: it forced a rerender at this point to avoid UI glitches in async updates (another hack upstream)
  // TODO: revisit this once we get rid of updateDatasourceAsync upstream
-  const moveDefinetelyToStaticValueAndUpdate = (
+  const moveDefinitelyToStaticValueAndUpdate = (


just a typo fix :)

elasticmachine · 2023-07-13T16:17:48Z

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

mbondyra · 2023-07-14T08:36:41Z

x-pack/plugins/lens/public/datasources/form_based/dimension_panel/reference_editor.tsx

@@ -82,7 +82,6 @@ export interface ReferenceEditorProps {
  labelAppend?: EuiFormRowProps['labelAppend'];
  isFullscreen: boolean;
  toggleFullscreen: () => void;
-  setIsCloseable: (isCloseable: boolean) => void;


I removed all code associated with it because seems like this is not needed anymore... I am not sure what changed it but it was introduced 2 years ago with formula editor PR (#99297). The comment says it solves ' Popover steals focus from text input, and doesn't close when clicking the monaco editor. Needs an alternative design', but I couldn't reproduce it and tested quite extensively.
I even checked 7.14 version where formula was introduced and the behavior is the same.

stratoula

@mbondyra this looks fantastic and I think it solves all the bugs that were created.
@dej611 can you also take a look?

kibana-ci · 2023-07-17T08:10:26Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: ea85498

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #59 / alerting api integration security and spaces enabled - Group 2 Alerts legacy alerts alerts "after all" hook in "alerts"
[job] [logs] FTR Configs #59 / alerting api integration security and spaces enabled - Group 2 Alerts legacy alerts alerts "before all" hook in "alerts"
[job] [logs] FTR Configs #10 / EPM Endpoints EPM - get Installed Packages Allows the fetching of installed packages

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.3MB	1.3MB	-646.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`expressionHeatmap`	14.7KB	14.7KB	+43.0B

History

💚 Build #142450 succeeded 3844b40
💚 Build #142429 succeeded 75ef9566cecb61c43d7acae31fb20140f8f1df4f
💔 Build #142392 failed 16f7476
💔 Build #142324 failed 7a0abaa5e8498bb47bdc4a0d9714045daebaa962

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

dej611

Tested locally with Safari 👍

mbondyra force-pushed the lens/fix_top_values_updates branch from a21164a to 7a0abaa Compare July 13, 2023 15:56

mbondyra marked this pull request as ready for review July 13, 2023 16:04

mbondyra requested a review from a team as a code owner July 13, 2023 16:04

mbondyra commented Jul 13, 2023

View reviewed changes

mbondyra added Team:Visualizations Visualization editors, elastic-charts and infrastructure release_note:skip Skip the PR/issue when compiling release notes Feature:Lens v8.10.0 labels Jul 13, 2023

mbondyra marked this pull request as draft July 13, 2023 17:00

[Lens] fix dimension panel datasource updates

16f7476

mbondyra force-pushed the lens/fix_top_values_updates branch from 7a0abaa to 16f7476 Compare July 13, 2023 20:28

remove not needed code

3844b40

mbondyra force-pushed the lens/fix_top_values_updates branch from 75ef956 to 3844b40 Compare July 14, 2023 08:29

mbondyra commented Jul 14, 2023

View reviewed changes

mbondyra marked this pull request as ready for review July 14, 2023 08:42

stratoula mentioned this pull request Jul 17, 2023

[Lens] editor crashes when switching quick function #161988

Closed

Merge branch 'main' into lens/fix_top_values_updates

ea85498

stratoula mentioned this pull request Jul 17, 2023

Failing test: Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/lens/group5/formula·ts - lens app - group 5 lens formula should insert single quotes and escape when needed to create valid field name #161666

Closed

stratoula approved these changes Jul 17, 2023

View reviewed changes

dej611 approved these changes Jul 17, 2023

View reviewed changes

mbondyra merged commit 9f67549 into elastic:main Jul 17, 2023

kibanamachine added the backport:skip This commit does not require backporting label Jul 17, 2023

mbondyra deleted the lens/fix_top_values_updates branch July 17, 2023 11:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] fix dimension panel datasource updates #161881

[Lens] fix dimension panel datasource updates #161881

mbondyra commented Jul 13, 2023 •

edited

Loading

mbondyra Jul 13, 2023

mbondyra Jul 13, 2023

mbondyra Jul 13, 2023

elasticmachine commented Jul 13, 2023

mbondyra Jul 14, 2023 •

edited

Loading

stratoula left a comment

kibana-ci commented Jul 17, 2023

dej611 left a comment

[Lens] fix dimension panel datasource updates #161881

[Lens] fix dimension panel datasource updates #161881

Conversation

mbondyra commented Jul 13, 2023 • edited Loading

Summary

Checklist

Risk Matrix

For maintainers

mbondyra Jul 13, 2023

Choose a reason for hiding this comment

mbondyra Jul 13, 2023

Choose a reason for hiding this comment

mbondyra Jul 13, 2023

Choose a reason for hiding this comment

elasticmachine commented Jul 13, 2023

mbondyra Jul 14, 2023 • edited Loading

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

kibana-ci commented Jul 17, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Async chunks

Page load bundle

History

dej611 left a comment

Choose a reason for hiding this comment

mbondyra commented Jul 13, 2023 •

edited

Loading

mbondyra Jul 14, 2023 •

edited

Loading