Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Canvas] Adds Save and Return Workflow #111411

Merged

Conversation

cqliu1
Copy link
Contributor

@cqliu1 cqliu1 commented Sep 7, 2021

Summary

Related to #108439.
Related to #81812.
Blocked by #104499.

This introduces the save and return flow for creating new by-value embeddables in Canvas and adds support for rendering by-value embeddables in the embeddable renderer.

Note: The Lens option in the Add element menu was only added temporarily for test. It'll be removed in a future PR that introduces a new menu for create new non-native visualizations in Canvas.

Known issues to be addressed in future PRs

  • editing an embeddable and returning will add a new element instead of updating the existing one fixed

Outstanding bugs (will be addressed in separate PRs)

Checklist

Delete any items that are not applicable to this PR.

  • 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

@cqliu1 cqliu1 added enhancement New value added to drive a business result Feature:Canvas Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Sep 7, 2021
@cqliu1 cqliu1 mentioned this pull request Sep 7, 2021
15 tasks
@cqliu1 cqliu1 force-pushed the canvas/save-and-return branch 3 times, most recently from 531e499 to 57720fc Compare September 13, 2021 18:22
@cqliu1 cqliu1 force-pushed the canvas/save-and-return branch 6 times, most recently from 35728ac to 5a8361d Compare September 27, 2021 17:52
@cqliu1 cqliu1 mentioned this pull request Sep 27, 2021
13 tasks
@cqliu1 cqliu1 force-pushed the canvas/save-and-return branch 2 times, most recently from 2657a68 to 40dd372 Compare October 3, 2021 22:48
@cqliu1 cqliu1 changed the base branch from master to canvas/by-value-embeddables October 3, 2021 22:49
@cqliu1 cqliu1 force-pushed the canvas/save-and-return branch from 40dd372 to 02ac36e Compare October 3, 2021 22:55
@cqliu1 cqliu1 added the review label Oct 5, 2021
@cqliu1 cqliu1 marked this pull request as ready for review October 5, 2021 18:44
@cqliu1 cqliu1 requested a review from a team as a code owner October 5, 2021 18:44
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@cqliu1 cqliu1 force-pushed the canvas/by-value-embeddables branch from e669879 to ddb8632 Compare October 5, 2021 18:54
@cqliu1 cqliu1 requested review from a team as code owners October 5, 2021 18:54
@cqliu1 cqliu1 force-pushed the canvas/save-and-return branch from 92d2b5e to 49e479e Compare October 5, 2021 18:58
@cqliu1 cqliu1 force-pushed the canvas/by-value-embeddables branch from ddb8632 to b706dfd Compare October 5, 2021 20:57
@cqliu1 cqliu1 force-pushed the canvas/save-and-return branch from 49e479e to f744dd6 Compare October 5, 2021 20:58
@lukeelmers
Copy link
Member

Removing core & telemetry as reviewers as (I think) this was unintentional

@crob611
Copy link
Contributor

crob611 commented Oct 12, 2021

When editing a Lens, and doing a "Save and Return" it's kicking me to the Canvas home listing instead of back to the workpad. (Or it's hitting a url that's invald and kicking it back there, not sure)

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Oct 14, 2021
@cqliu1 cqliu1 force-pushed the canvas/save-and-return branch 2 times, most recently from 55cefdc to ca60143 Compare October 15, 2021 00:11
@cqliu1 cqliu1 removed the request for review from a team October 15, 2021 00:12
@cqliu1 cqliu1 removed the Feature:Embedding Embedding content via iFrame label Oct 15, 2021
@cqliu1 cqliu1 force-pushed the canvas/by-value-embeddables branch from 2030a15 to afea023 Compare October 15, 2021 17:33
@cqliu1 cqliu1 force-pushed the canvas/save-and-return branch from ca60143 to ac62a31 Compare October 15, 2021 17:34
@cqliu1 cqliu1 force-pushed the canvas/by-value-embeddables branch from afea023 to 6b2bc48 Compare October 15, 2021 19:47
@cqliu1 cqliu1 force-pushed the canvas/save-and-return branch 2 times, most recently from 652f1f8 to cb6749a Compare October 15, 2021 21:29
@cqliu1 cqliu1 force-pushed the canvas/by-value-embeddables branch from 6b2bc48 to 196b94d Compare October 18, 2021 17:08
Updates existing embeddable with updates on edit

Select incoming embeddable element on load

Fixed ts errors

Added use incoming embeddable hook

Fixed eslint errors

Added comment

Fixed typo

Fixed story

Fix ts errors

Fixed failing tests

Fixed ts errors

Add addUpdater to pluginServiceRegistry.start

Fixed plugin appUpdater
@cqliu1 cqliu1 force-pushed the canvas/save-and-return branch from cb6749a to cfa5593 Compare October 18, 2021 17:08
@kibanamachine
Copy link
Contributor

💛 Build succeeded, but was flaky


Test Failures

Kibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/ml/jobs/categorization_field_examples·ts.apis Machine Learning jobs Categorization example endpoint - invalid, too many tokens.

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

[00:00:00]     │
[00:00:00]       └-: apis
[00:00:00]         └-> "before all" hook in "apis"
[00:11:04]         └-: Machine Learning
[00:11:04]           └-> "before all" hook in "Machine Learning"
[00:11:04]           └-> "before all" hook in "Machine Learning"
[00:11:04]             │ debg creating role ft_ml_source
[00:11:04]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_ml_source]
[00:11:04]             │ debg creating role ft_ml_source_readonly
[00:11:04]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_ml_source_readonly]
[00:11:04]             │ debg creating role ft_ml_dest
[00:11:04]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_ml_dest]
[00:11:04]             │ debg creating role ft_ml_dest_readonly
[00:11:04]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_ml_dest_readonly]
[00:11:04]             │ debg creating role ft_ml_ui_extras
[00:11:05]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_ml_ui_extras]
[00:11:05]             │ debg creating role ft_default_space_ml_all
[00:11:05]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_default_space_ml_all]
[00:11:05]             │ debg creating role ft_default_space1_ml_all
[00:11:05]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_default_space1_ml_all]
[00:11:05]             │ debg creating role ft_all_spaces_ml_all
[00:11:05]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_all_spaces_ml_all]
[00:11:05]             │ debg creating role ft_default_space_ml_read
[00:11:05]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_default_space_ml_read]
[00:11:05]             │ debg creating role ft_default_space1_ml_read
[00:11:05]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_default_space1_ml_read]
[00:11:05]             │ debg creating role ft_all_spaces_ml_read
[00:11:05]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_all_spaces_ml_read]
[00:11:05]             │ debg creating role ft_default_space_ml_none
[00:11:05]             │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [ft_default_space_ml_none]
[00:11:05]             │ debg creating user ft_ml_poweruser
[00:11:05]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_poweruser]
[00:11:05]             │ debg created user ft_ml_poweruser
[00:11:05]             │ debg creating user ft_ml_poweruser_spaces
[00:11:05]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_poweruser_spaces]
[00:11:05]             │ debg created user ft_ml_poweruser_spaces
[00:11:05]             │ debg creating user ft_ml_poweruser_space1
[00:11:05]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_poweruser_space1]
[00:11:05]             │ debg created user ft_ml_poweruser_space1
[00:11:05]             │ debg creating user ft_ml_poweruser_all_spaces
[00:11:05]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_poweruser_all_spaces]
[00:11:05]             │ debg created user ft_ml_poweruser_all_spaces
[00:11:05]             │ debg creating user ft_ml_viewer
[00:11:05]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_viewer]
[00:11:05]             │ debg created user ft_ml_viewer
[00:11:05]             │ debg creating user ft_ml_viewer_spaces
[00:11:05]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_viewer_spaces]
[00:11:05]             │ debg created user ft_ml_viewer_spaces
[00:11:05]             │ debg creating user ft_ml_viewer_space1
[00:11:06]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_viewer_space1]
[00:11:06]             │ debg created user ft_ml_viewer_space1
[00:11:06]             │ debg creating user ft_ml_viewer_all_spaces
[00:11:06]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_viewer_all_spaces]
[00:11:06]             │ debg created user ft_ml_viewer_all_spaces
[00:11:06]             │ debg creating user ft_ml_unauthorized
[00:11:06]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_unauthorized]
[00:11:06]             │ debg created user ft_ml_unauthorized
[00:11:06]             │ debg creating user ft_ml_unauthorized_spaces
[00:11:06]             │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [ft_ml_unauthorized_spaces]
[00:11:06]             │ debg created user ft_ml_unauthorized_spaces
[00:15:14]           └-: jobs
[00:15:14]             └-> "before all" hook in "jobs"
[00:15:14]             └-: Categorization example endpoint - 
[00:15:14]               └-> "before all" hook for "valid with good number of tokens"
[00:15:14]               └-> "before all" hook for "valid with good number of tokens"
[00:15:14]                 │ info [x-pack/test/functional/es_archives/ml/categorization] Loading "mappings.json"
[00:15:14]                 │ info [x-pack/test/functional/es_archives/ml/categorization] Loading "data.json.gz"
[00:15:14]                 │ info [o.e.c.m.MetadataCreateIndexService] [node-01] [ft_categorization] creating index, cause [api], templates [], shards [1]/[0]
[00:15:15]                 │ info [x-pack/test/functional/es_archives/ml/categorization] Created index "ft_categorization"
[00:15:15]                 │ debg [x-pack/test/functional/es_archives/ml/categorization] "ft_categorization" settings {"index":{"number_of_replicas":"0","number_of_shards":"1"}}
[00:15:16]                 │ info [x-pack/test/functional/es_archives/ml/categorization] Indexed 1501 docs into "ft_categorization"
[00:15:16]                 │ debg applying update to kibana config: {"dateFormat:tz":"UTC"}
[00:15:16]               └-> valid with good number of tokens
[00:15:16]                 └-> "before each" hook: global before each for "valid with good number of tokens"
[00:15:16]                 └- ✓ pass  (147ms)
[00:15:16]               └-> invalid, too many tokens.
[00:15:16]                 └-> "before each" hook: global before each for "invalid, too many tokens."
[00:15:16]                 │ info [r.suppressed] [node-01] path: /_analyze, params: {}
[00:15:16]                 │      org.elasticsearch.transport.RemoteTransportException: [node-01][127.0.0.1:63171][indices:admin/analyze[s]]
[00:15:16]                 │      Caused by: java.lang.IllegalStateException: The number of tokens produced by calling _analyze has exceeded the allowed maximum of [10000]. This limit can be set by changing the [index.analyze.max_token_count] index level setting.
[00:15:16]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction$TokenCounter.increment(TransportAnalyzeAction.java:397) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:15:16]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.simpleAnalyze(TransportAnalyzeAction.java:229) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:15:16]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.analyze(TransportAnalyzeAction.java:204) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:15:16]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.analyze(TransportAnalyzeAction.java:122) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:15:16]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.shardOperation(TransportAnalyzeAction.java:110) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:15:16]                 │      	at org.elasticsearch.action.admin.indices.analyze.TransportAnalyzeAction.shardOperation(TransportAnalyzeAction.java:62) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:15:16]                 │      	at org.elasticsearch.action.support.single.shard.TransportSingleShardAction.lambda$asyncShardOperation$0(TransportSingleShardAction.java:99) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:15:16]                 │      	at org.elasticsearch.action.ActionRunnable.lambda$supply$0(ActionRunnable.java:47) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:15:16]                 │      	at org.elasticsearch.action.ActionRunnable$2.doRun(ActionRunnable.java:62) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:15:16]                 │      	at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:737) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:15:16]                 │      	at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:26) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
[00:15:16]                 │      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
[00:15:16]                 │      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
[00:15:16]                 │      	at java.lang.Thread.run(Thread.java:833) [?:?]
[00:15:16]                 └- ✖ fail: apis Machine Learning jobs Categorization example endpoint -  invalid, too many tokens.
[00:15:16]                 │       Error: expected 'partially_valid' to sort of equal 'invalid'
[00:15:16]                 │       + expected - actual
[00:15:16]                 │ 
[00:15:16]                 │       -partially_valid
[00:15:16]                 │       +invalid
[00:15:16]                 │       
[00:15:16]                 │       at Assertion.assert (/dev/shm/workspace/parallel/17/kibana/node_modules/@kbn/expect/expect.js:100:11)
[00:15:16]                 │       at Assertion.eql (/dev/shm/workspace/parallel/17/kibana/node_modules/@kbn/expect/expect.js:244:8)
[00:15:16]                 │       at Context.<anonymous> (test/api_integration/apis/ml/jobs/categorization_field_examples.ts:302:44)
[00:15:16]                 │       at runMicrotasks (<anonymous>)
[00:15:16]                 │       at processTicksAndRejections (node:internal/process/task_queues:96:5)
[00:15:16]                 │       at Object.apply (/dev/shm/workspace/parallel/17/kibana/node_modules/@kbn/test/target_node/functional_test_runner/lib/mocha/wrap_function.js:87:16)
[00:15:16]                 │ 
[00:15:16]                 │ 

Stack Trace

Error: expected 'partially_valid' to sort of equal 'invalid'
    at Assertion.assert (/dev/shm/workspace/parallel/17/kibana/node_modules/@kbn/expect/expect.js:100:11)
    at Assertion.eql (/dev/shm/workspace/parallel/17/kibana/node_modules/@kbn/expect/expect.js:244:8)
    at Context.<anonymous> (test/api_integration/apis/ml/jobs/categorization_field_examples.ts:302:44)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at Object.apply (/dev/shm/workspace/parallel/17/kibana/node_modules/@kbn/test/target_node/functional_test_runner/lib/mocha/wrap_function.js:87:16) {
  actual: 'partially_valid',
  expected: 'invalid',
  showDiff: true
}

History

  • 💚 Build #161685 succeeded cb6749a27b127dafd01b884494552e07fdba710a
  • 💚 Build #161643 succeeded 652f1f8433128b928f35139685aab2136e733188
  • 💔 Build #161566 failed ac62a31d829e6545ffa43bcbd1b62f3c0e69761b
  • 💔 Build #161219 failed ca60143b15217de8d208ecac4798df6212146f0f
  • 💔 Build #161078 failed 91b62dab40955d718d14086d8a95c7fb4cc03f94

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

Copy link
Contributor

@crob611 crob611 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@cqliu1 cqliu1 merged this pull request into elastic:canvas/by-value-embeddables Oct 19, 2021
@cqliu1 cqliu1 deleted the canvas/save-and-return branch October 19, 2021 15:50
@cqliu1 cqliu1 mentioned this pull request Oct 19, 2021
11 tasks
cqliu1 added a commit that referenced this pull request Oct 19, 2021
cqliu1 added a commit that referenced this pull request Oct 20, 2021
cqliu1 added a commit that referenced this pull request Oct 25, 2021
cqliu1 added a commit that referenced this pull request Oct 25, 2021
cqliu1 added a commit that referenced this pull request Oct 26, 2021
cqliu1 added a commit that referenced this pull request Oct 27, 2021
* [Canvas] Generic embeddable function (#104499)

* Created generic embeddable function

    Fixed telemetry

    Updates expression on input change

    Fixed ts errors

Store embeddable input to expression

Added lib functions

Added comments

Fixed type errors

Fixed ts errors

Clean up

Removed extraneous import

Added context type to embeddable function def

Fix import

Update encode/decode fns

Moved embeddable data url lib file

Added embeddable test

Updated comment

* Fix reference extract/inject in embeddable fn

* Simplify embeddable toExpression

* Moved labsService to flyout.tsx

* Added comment

* [Canvas] Adds Save and Return Workflow (#111411)

* [Canvas] Adds editor menu to Canvas (#113194)

* Merge existing embeddable input with incoming embeddable input (#116026)

* [Canvas] Extract and inject references for by-value embeddables (#115124)

* Extract/inject references for by-value embeddables in embeddable function

Fixed server interpreter setup

Register external functions in canvas_plugin_src plugin def

* Fixed ref name in embeddable.inject

* Fixed ts errors

* Fix missing type error

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
cqliu1 added a commit that referenced this pull request Oct 28, 2021
cqliu1 added a commit that referenced this pull request Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Canvas review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants