[Visualize] Allows editing broken visualizations caused by runtime fields changes #94798

VladLasitsa · 2021-03-17T13:05:38Z

Summary

Adds possibility to open visualization when a user deleted the field or changed its type so that user can fix it.

Now user can open visualization and see error. Also editor will be in invalid state. Moreover I added error message in validation by focus leaving from combobox if user doesn't select another field.

In case when we delete field

In case when we change type of field

… anymore

src/plugins/data/common/search/aggs/param_types/field.ts

timroes · 2021-03-18T09:51:52Z

A note to the current way this is designed: I don't think we should link the user to the saved object page. There are several reasons this is a bad idea: (a) it needs different permissions than saving (and opening) visualizations, (b) we're anyway considering making that page a read only page potentially, so you could not necessarily edit the saved obejct in the future there.

If we load a visualization in a state where a field doesn't exist or has a wrong type, we should completely stay in the editor and just have the editor be in an invalid state, the same as you'd see if you selected an aggregation but not a field yet. Thus the user needs to change the field to bring the editor in the valid state again and then can save the visualization.

VladLasitsa · 2021-03-18T12:46:11Z

A note to the current way this is designed: I don't think we should link the user to the saved object page. There are several reasons this is a bad idea: (a) it needs different permissions than saving (and opening) visualizations, (b) we're anyway considering making that page a read only page potentially, so you could not necessarily edit the saved obejct in the future there.

If we load a visualization in a state where a field doesn't exist or has a wrong type, we should completely stay in the editor and just have the editor be in an invalid state, the same as you'd see if you selected an aggregation but not a field yet. Thus the user needs to change the field to bring the editor in the valid state again and then can save the visualization.

Okay, got it. I removed link from error message and update editor so that it will be in invalid state in these cases.

VladLasitsa · 2021-03-22T06:56:00Z

@elasticmachine merge upstream

alexwizp · 2021-03-25T08:09:45Z

@elasticmachine merge upstream

stratoula · 2021-03-29T07:54:23Z

@elasticmachine merge upstream

stratoula

Some initial comments:

When I delete the field and refresh my viz, I can see the toast twice. Moreove I would expect to be an error message when I open the split series accordion.
The toasts do not disappear, as a result they "follow" me and if I open the visualization again I can see 4 toasts etc.
For a broken visualization on a dashboard I see an error label and only when I hover it it visible what the problem is. I suggest to be minimal for users with no edit permissions and more explanatory for the users who can actually edit and fix it. @ThomThomson wdyt?

The above applies to fields that their type changes too.

src/plugins/vis_default_editor/public/components/controls/field.tsx

ThomThomson · 2021-03-29T15:04:06Z

@stratoula I like the idea to have the tooltip less descriptive for users without edit permissions, but having a small error label like that doesn't match particularly well with the error reporting for lens. It would be ideal if it was possible to match this errorEmbeddable like design from #90668

stratoula · 2021-03-29T15:08:21Z

Thank you @ThomThomson, I agree. This is what I meant with more explanatory 😄 We should def match with Lens

VladLasitsa · 2021-03-30T18:33:54Z

@elasticmachine merge upstream

elasticmachine · 2021-04-08T08:40:29Z

Pinging @elastic/kibana-app (Team:KibanaApp)

stratoula

Just one comment of a change that we no need anymore. Other than that it seems fine to me.
@ppisljar do you want to also check it?

src/plugins/kibana_utils/common/errors/errors.ts

src/plugins/visualizations/public/embeddable/visualize_embeddable.ts

stratoula

Kibana App code LGTM. I tested it locally and it works fine. I can now edit my broken visualizations and I see a warning icon on the aggregation that causes the problem.

I would like to display the error as in Lens for consistency but I think it is fine for now

ppisljar · 2021-04-08T12:05:22Z

is the error still visible on dashboard ? i think it should be. my reasoning would be that:

on dashboard we are rendering an embeddable ... something wen't wrong, let the user know
in visualize we are editing a visualization. if configuration is broken we should not even attempt to render as we know that it will fail. The current solution will hide specific errors, but it will actually try to render, so this probably has edge cases it doesn't cover (theoretically it could be something else in the configuration that's broken, not related to saved fields, not sure if it ever really happens).

kibanamachine · 2021-04-08T12:09:14Z

💚 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
`visDefaultEditor`	225.7KB	226.7KB	+1004.0B
`visualize`	91.3KB	91.7KB	+416.0B
total			+1.4KB

Page load bundle

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

id	before	after	diff
`data`	783.8KB	784.5KB	+756.0B
`embeddable`	179.0KB	179.0KB	-65.0B
`kibanaUtils`	143.1KB	143.8KB	+687.0B
`visualizations`	91.8KB	91.8KB	+3.0B
`visualize`	26.7KB	27.0KB	+238.0B
total			+1.6KB

History

💚 Build #118142 succeeded e2fce06
💚 Build #117761 succeeded 9f91d27
💔 Build #117549 failed f6e60c6
💔 Build #117486 failed e7e199a
💔 Build #117501 failed 3fdff74

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

cc @VladLasitsa

stratoula · 2021-04-08T12:13:01Z

Yes Peter the error is displayed as:

ppisljar

appservices code LGTM

stratoula · 2021-04-08T14:21:07Z

@elasticmachine run elasticsearch-ci/docs

…elds changes (elastic#94798) * Add possibility to open visualization when saved field doesn't exists anymore * Fix tests * Fix some remarks * Remove unneeded code * Fix tests * Fix tests * Fix some remarks * Fixed problem with double error popover in visualizations * Fix CI * Fix type * Fix API docs * Don't show error popup for error related to runtime fields * Fix some remarks Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…elds changes (#94798) (#96580) * Add possibility to open visualization when saved field doesn't exists anymore * Fix tests * Fix some remarks * Remove unneeded code * Fix tests * Fix tests * Fix some remarks * Fixed problem with double error popover in visualizations * Fix CI * Fix type * Fix API docs * Don't show error popup for error related to runtime fields * Fix some remarks Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Uladzislau Lasitsa <Uladzislau_Lasitsa@epam.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Add possibility to open visualization when saved field doesn't exists…

dcedad7

… anymore

VladLasitsa requested review from stratoula and alexwizp March 17, 2021 13:05

Fix tests

1aaff69

VladLasitsa requested a review from ppisljar March 18, 2021 07:59

ppisljar reviewed Mar 18, 2021

View reviewed changes

src/plugins/data/common/search/aggs/param_types/field.ts Outdated Show resolved Hide resolved

Fix some remarks

daa53b3

VladLasitsa self-assigned this Mar 18, 2021

VladLasitsa changed the title ~~[Visualize] User should be able to edit a broken visualization from a runtime field that doesn't exist anymore~~ [Visualize] Support runtime fields editor Mar 18, 2021

VladLasitsa added 3 commits March 18, 2021 16:10

Remove unneeded code

2348add

Fix tests

2135c11

Fix tests

0ef5b75

Merge branch 'master' into 93924

5cdc9b3

Merge branch 'master' into 93924

7371dda

Merge branch 'master' into 93924

62eae83

stratoula reviewed Mar 29, 2021

View reviewed changes

src/plugins/vis_default_editor/public/components/controls/field.tsx Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/master' into 93924

783b0b4

VladLasitsa added 2 commits March 30, 2021 12:46

Fix some remarks

6dde4be

Fixed problem with double error popover in visualizations

c225ba3

kibanamachine and others added 3 commits March 30, 2021 14:34

Merge branch 'master' into 93924

6106aba

Fix CI

1acfa03

Fix type

bb80f46

stratoula requested a review from a team April 8, 2021 08:39

stratoula requested a review from a team as a code owner April 8, 2021 08:39

stratoula added release_note:feature Makes this part of the condensed release notes v7.13.0 v8.0.0 Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Apr 8, 2021

stratoula added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:enhancement and removed release_note:feature Makes this part of the condensed release notes labels Apr 8, 2021

stratoula changed the title ~~[Visualize] Support runtime fields editor~~ [Visualize] Allows editing broken visualizations caused by runtime fields changes Apr 8, 2021

stratoula reviewed Apr 8, 2021

View reviewed changes

src/plugins/kibana_utils/common/errors/errors.ts Outdated Show resolved Hide resolved

src/plugins/visualizations/public/embeddable/visualize_embeddable.ts Outdated Show resolved Hide resolved

botelastic bot added the Feature:Embedding Embedding content via iFrame label Apr 8, 2021

Fix some remarks

e983cfd

stratoula approved these changes Apr 8, 2021

View reviewed changes

alexwizp approved these changes Apr 8, 2021

View reviewed changes

ppisljar approved these changes Apr 8, 2021

View reviewed changes

stratoula merged commit e3f31ea into elastic:master Apr 8, 2021

VladLasitsa mentioned this pull request Apr 8, 2021

[7.x] [Visualize] Allows editing broken visualizations caused by runtime fields changes (#94798) #96579

Closed

stratoula mentioned this pull request Apr 8, 2021

[7.x] [Visualize] Allows editing broken visualizations caused by runtime fields changes (#94798) #96580

Merged

stratoula mentioned this pull request Apr 14, 2021

[Visualize] User should be able to edit a broken visualization from a runtime field that doesn't exist anymore #93924

Closed

ppisljar mentioned this pull request May 3, 2021

Changing sibling pipeline aggregation throws browser console error #91689

Closed

drewdaemon mentioned this pull request Dec 12, 2022

[Lens] missing field validation as warnings #147053

Closed

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Visualize] Allows editing broken visualizations caused by runtime fields changes #94798

[Visualize] Allows editing broken visualizations caused by runtime fields changes #94798

VladLasitsa commented Mar 17, 2021 •

edited by stratoula

Loading

timroes commented Mar 18, 2021

VladLasitsa commented Mar 18, 2021

VladLasitsa commented Mar 22, 2021

alexwizp commented Mar 25, 2021

stratoula commented Mar 29, 2021

stratoula left a comment •

edited

Loading

ThomThomson commented Mar 29, 2021

stratoula commented Mar 29, 2021

VladLasitsa commented Mar 30, 2021

elasticmachine commented Apr 8, 2021

stratoula left a comment

stratoula left a comment

ppisljar commented Apr 8, 2021

kibanamachine commented Apr 8, 2021

stratoula commented Apr 8, 2021

ppisljar left a comment

stratoula commented Apr 8, 2021

[Visualize] Allows editing broken visualizations caused by runtime fields changes #94798

[Visualize] Allows editing broken visualizations caused by runtime fields changes #94798

Conversation

VladLasitsa commented Mar 17, 2021 • edited by stratoula Loading

Summary

In case when we delete field

In case when we change type of field

timroes commented Mar 18, 2021

VladLasitsa commented Mar 18, 2021

VladLasitsa commented Mar 22, 2021

alexwizp commented Mar 25, 2021

stratoula commented Mar 29, 2021

stratoula left a comment • edited Loading

Choose a reason for hiding this comment

ThomThomson commented Mar 29, 2021

stratoula commented Mar 29, 2021

VladLasitsa commented Mar 30, 2021

elasticmachine commented Apr 8, 2021

stratoula left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

ppisljar commented Apr 8, 2021

kibanamachine commented Apr 8, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

stratoula commented Apr 8, 2021

ppisljar left a comment

Choose a reason for hiding this comment

stratoula commented Apr 8, 2021

VladLasitsa commented Mar 17, 2021 •

edited by stratoula

Loading

stratoula left a comment •

edited

Loading