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

[Embeddable rebuild] ML anomaly swim lane embeddable: ensures dashboard reset works correctly #181346

Merged

Conversation

alvarezmelissa87
Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 commented Apr 22, 2024

Summary

Fixes #181210
The reset functionality was not reverting the embeddable values. This PR ensures the embeddable values are reset on dashboard reset.

Comparators require the next functionality to be wrapped in a function in order to maintain this being defined.
The problem is that BehaviourSubjects are class instances, and if you pass in the next method like [someSubject$, someSubject$.next] you lose the this context.

NOTE: This needs the 8.14.1 label when it's available

Checklist

Delete any items that are not applicable to this PR.

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@kibanamachine kibanamachine added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Apr 22, 2024
@elasticmachine
Copy link
Contributor

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

@alvarezmelissa87 alvarezmelissa87 added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Apr 22, 2024
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

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
ml 4.1MB 4.1MB +24.0B

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

cc @alvarezmelissa87

@nreese
Copy link
Contributor

nreese commented Apr 23, 2024

NOTE: This needs the 8.14.1 label when it's available

question about this. There are still 3 build candidates left for 8.14.0 so this change should easily make it into 8.14.0. Why is 8.14.1 label needed?

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested and LGTM

@peteharverson peteharverson changed the title [Embeddable rebuild] ML swimlane embeddable: ensure dashboard reset works correctly [Embeddable rebuild] ML anomaly swim lane embeddable: ensure dashboard reset works correctly Apr 23, 2024
Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

LGTM

@alvarezmelissa87 alvarezmelissa87 merged commit d4047fe into elastic:main Apr 23, 2024
39 checks passed
@alvarezmelissa87 alvarezmelissa87 deleted the ml-swimlane-embeddable-fix branch April 23, 2024 19:28
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 23, 2024
…d reset works correctly (elastic#181346)

## Summary

Fixes elastic#181210
The reset functionality was not reverting the embeddable values. This PR
ensures the embeddable values are reset on dashboard reset.

Comparators require the `next` functionality to be wrapped in a function
in order to maintain `this` being defined.
The problem is that BehaviourSubjects are class instances, and if you
pass in the next method like `[someSubject$, someSubject$.next]` you
lose the this context.

NOTE: This needs the `8.14.1` label when it's available

### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [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
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

(cherry picked from commit d4047fe)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.14

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Apr 24, 2024
…ashboard reset works correctly (#181346) (#181490)

# Backport

This will backport the following commits from `main` to `8.14`:
- [[Embeddable rebuild] ML anomaly swim lane embeddable: ensure
dashboard reset works correctly
(#181346)](#181346)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Melissa
Alvarez","email":"melissa.alvarez@elastic.co"},"sourceCommit":{"committedDate":"2024-04-23T19:28:03Z","message":"[Embeddable
rebuild] ML anomaly swim lane embeddable: ensure dashboard reset works
correctly (#181346)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/181210\r\nThe reset
functionality was not reverting the embeddable values. This
PR\r\nensures the embeddable values are reset on dashboard
reset.\r\n\r\nComparators require the `next` functionality to be wrapped
in a function\r\nin order to maintain `this` being defined.\r\nThe
problem is that BehaviourSubjects are class instances, and if
you\r\npass in the next method like `[someSubject$, someSubject$.next]`
you\r\nlose the this context.\r\n\r\nNOTE: This needs the `8.14.1` label
when it's available\r\n\r\n### Checklist\r\n\r\nDelete any items that
are not applicable to this PR.\r\n\r\n- [ ] Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"d4047fe716ad946310d3327a95cac58dbb06c152","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Dashboard","release_note:fix",":ml","Team:Presentation","loe:small","impact:low","Feature:Embeddables","project:embeddableRebuild","v8.14.0","v8.15.0"],"title":"[Embeddable
rebuild] ML anomaly swim lane embeddable: ensure dashboard reset works
correctly","number":181346,"url":"https://github.com/elastic/kibana/pull/181346","mergeCommit":{"message":"[Embeddable
rebuild] ML anomaly swim lane embeddable: ensure dashboard reset works
correctly (#181346)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/181210\r\nThe reset
functionality was not reverting the embeddable values. This
PR\r\nensures the embeddable values are reset on dashboard
reset.\r\n\r\nComparators require the `next` functionality to be wrapped
in a function\r\nin order to maintain `this` being defined.\r\nThe
problem is that BehaviourSubjects are class instances, and if
you\r\npass in the next method like `[someSubject$, someSubject$.next]`
you\r\nlose the this context.\r\n\r\nNOTE: This needs the `8.14.1` label
when it's available\r\n\r\n### Checklist\r\n\r\nDelete any items that
are not applicable to this PR.\r\n\r\n- [ ] Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"d4047fe716ad946310d3327a95cac58dbb06c152"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/181346","number":181346,"mergeCommit":{"message":"[Embeddable
rebuild] ML anomaly swim lane embeddable: ensure dashboard reset works
correctly (#181346)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/181210\r\nThe reset
functionality was not reverting the embeddable values. This
PR\r\nensures the embeddable values are reset on dashboard
reset.\r\n\r\nComparators require the `next` functionality to be wrapped
in a function\r\nin order to maintain `this` being defined.\r\nThe
problem is that BehaviourSubjects are class instances, and if
you\r\npass in the next method like `[someSubject$, someSubject$.next]`
you\r\nlose the this context.\r\n\r\nNOTE: This needs the `8.14.1` label
when it's available\r\n\r\n### Checklist\r\n\r\nDelete any items that
are not applicable to this PR.\r\n\r\n- [ ] Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"d4047fe716ad946310d3327a95cac58dbb06c152"}}]}]
BACKPORT-->

Co-authored-by: Melissa Alvarez <melissa.alvarez@elastic.co>
@szabosteve szabosteve changed the title [Embeddable rebuild] ML anomaly swim lane embeddable: ensure dashboard reset works correctly [Embeddable rebuild] ML anomaly swim lane embeddable: ensures dashboard reset works correctly Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features Feature:Embeddables Relating to the Embeddable system impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort :ml project:embeddableRebuild release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.14.0 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Embeddable Rebuild] ML embeddables in dashboard: Reset not working for swimlane embeddable
7 participants