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

[visualize embeddable] fix panel disappears from dashboard when canceling edit after dashboard save #193914

Merged
merged 3 commits into from
Sep 26, 2024

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Sep 24, 2024

Part of #193905. Broke work into separate PRs to backport map embeddable changes to 8.15.

Resolves issue for Visualize embeddable and dashboard empty screen. The problem is that Visualize embeddable is using a stale result from parentApi.getAppContext. Dashboard's getAppContext changes the getCurrentPath when the dashboard has a saved object id. By using the stale results, getCurrentPath returned #/create instead of #/view/. The fix is to get a current version of getAppContext when required.

Test steps

  1. create new dashboard
  2. Click "Add panel" (problem also exists when using "Add from library")
  3. Select "Aggregation based"
  4. In editor, click "Save and return"
  5. Save dashboard
  6. Click "Edit" in panel context menu
  7. In editor, click "Cancel"
  8. Ensure visualize panel is still displayed in dashboard

@nreese nreese added v9.0.0 v8.16.0 backport:version Backport to applied version labels labels Sep 24, 2024
@nreese
Copy link
Contributor Author

nreese commented Sep 24, 2024

/ci

? parentApi.timeRange$.getValue()
: {};
const customTimeRange = customTimeRange$.getValue();
const parentApiContext = parentApi.getAppContext();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the fix for the visualize embeddable. Calling getAppContext in the onEdit callback

@nreese nreese marked this pull request as ready for review September 24, 2024 19:41
@nreese nreese requested review from a team as code owners September 24, 2024 19:41
@nreese nreese added release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Sep 24, 2024
@elasticmachine
Copy link
Contributor

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

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
visualizations 475 476 +1

Async chunks

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

id before after diff
dashboard 490.6KB 490.5KB -117.0B
visualizations 315.6KB 315.7KB +52.0B
total -65.0B

Page load bundle

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

id before after diff
visualizations 71.0KB 70.9KB -34.0B

History

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

@nickpeihl nickpeihl self-requested a review September 25, 2024 13:13
Copy link
Member

@nickpeihl nickpeihl left a comment

Choose a reason for hiding this comment

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

lgtm! code review and verified visualization does not disappear when navigating back to dashboard.

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

Tested, works good, the bug is fixed!

@nreese nreese merged commit d6c8840 into elastic:main Sep 26, 2024
21 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 26, 2024
…ling edit after dashboard save (elastic#193914)

Part of elastic#193905. Broke work into
separate PRs to backport map embeddable changes to 8.15.

Resolves issue for Visualize embeddable and dashboard empty screen. The
problem is that Visualize embeddable is using a stale result from
`parentApi.getAppContext`. Dashboard's `getAppContext` changes the
`getCurrentPath` when the dashboard has a saved object id. By using the
stale results, `getCurrentPath` returned `#/create` instead of
`#/view/`. The fix is to get a current version of `getAppContext` when
required.

### Test steps
1. create new dashboard
2. Click "Add panel" (problem also exists when using "Add from library")
3. Select "Aggregation based"
4.  In editor, click "Save and return"
5. Save dashboard
6. Click "Edit" in panel context menu
7. In editor, click "Cancel"
8. Ensure visualize panel is still displayed in dashboard

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

💚 All backports created successfully

Status Branch Result
8.x

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 Sep 26, 2024
… canceling edit after dashboard save (#193914) (#194152)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[visualize embeddable] fix panel disappears from dashboard when
canceling edit after dashboard save
(#193914)](#193914)

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

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

<!--BACKPORT [{"author":{"name":"Nathan
Reese","email":"reese.nathan@elastic.co"},"sourceCommit":{"committedDate":"2024-09-26T13:47:04Z","message":"[visualize
embeddable] fix panel disappears from dashboard when canceling edit
after dashboard save (#193914)\n\nPart of
#193905. Broke work
into\r\nseparate PRs to backport map embeddable changes to
8.15.\r\n\r\nResolves issue for Visualize embeddable and dashboard empty
screen. The\r\nproblem is that Visualize embeddable is using a stale
result from\r\n`parentApi.getAppContext`. Dashboard's `getAppContext`
changes the\r\n`getCurrentPath` when the dashboard has a saved object
id. By using the\r\nstale results, `getCurrentPath` returned `#/create`
instead of\r\n`#/view/`. The fix is to get a current version of
`getAppContext` when\r\nrequired.\r\n\r\n### Test steps\r\n1. create new
dashboard\r\n2. Click \"Add panel\" (problem also exists when using
\"Add from library\")\r\n3. Select \"Aggregation based\"\r\n4. In
editor, click \"Save and return\"\r\n5. Save dashboard\r\n6. Click
\"Edit\" in panel context menu\r\n7. In editor, click \"Cancel\"\r\n8.
Ensure visualize panel is still displayed in
dashboard","sha":"d6c8840a561484bee74dbcb335290c879e82d370","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","v9.0.0","project:embeddableRebuild","v8.16.0","backport:version"],"title":"[visualize
embeddable] fix panel disappears from dashboard when canceling edit
after dashboard
save","number":193914,"url":"https://github.com/elastic/kibana/pull/193914","mergeCommit":{"message":"[visualize
embeddable] fix panel disappears from dashboard when canceling edit
after dashboard save (#193914)\n\nPart of
#193905. Broke work
into\r\nseparate PRs to backport map embeddable changes to
8.15.\r\n\r\nResolves issue for Visualize embeddable and dashboard empty
screen. The\r\nproblem is that Visualize embeddable is using a stale
result from\r\n`parentApi.getAppContext`. Dashboard's `getAppContext`
changes the\r\n`getCurrentPath` when the dashboard has a saved object
id. By using the\r\nstale results, `getCurrentPath` returned `#/create`
instead of\r\n`#/view/`. The fix is to get a current version of
`getAppContext` when\r\nrequired.\r\n\r\n### Test steps\r\n1. create new
dashboard\r\n2. Click \"Add panel\" (problem also exists when using
\"Add from library\")\r\n3. Select \"Aggregation based\"\r\n4. In
editor, click \"Save and return\"\r\n5. Save dashboard\r\n6. Click
\"Edit\" in panel context menu\r\n7. In editor, click \"Cancel\"\r\n8.
Ensure visualize panel is still displayed in
dashboard","sha":"d6c8840a561484bee74dbcb335290c879e82d370"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193914","number":193914,"mergeCommit":{"message":"[visualize
embeddable] fix panel disappears from dashboard when canceling edit
after dashboard save (#193914)\n\nPart of
#193905. Broke work
into\r\nseparate PRs to backport map embeddable changes to
8.15.\r\n\r\nResolves issue for Visualize embeddable and dashboard empty
screen. The\r\nproblem is that Visualize embeddable is using a stale
result from\r\n`parentApi.getAppContext`. Dashboard's `getAppContext`
changes the\r\n`getCurrentPath` when the dashboard has a saved object
id. By using the\r\nstale results, `getCurrentPath` returned `#/create`
instead of\r\n`#/view/`. The fix is to get a current version of
`getAppContext` when\r\nrequired.\r\n\r\n### Test steps\r\n1. create new
dashboard\r\n2. Click \"Add panel\" (problem also exists when using
\"Add from library\")\r\n3. Select \"Aggregation based\"\r\n4. In
editor, click \"Save and return\"\r\n5. Save dashboard\r\n6. Click
\"Edit\" in panel context menu\r\n7. In editor, click \"Cancel\"\r\n8.
Ensure visualize panel is still displayed in
dashboard","sha":"d6c8840a561484bee74dbcb335290c879e82d370"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
angorayc pushed a commit to angorayc/kibana that referenced this pull request Oct 1, 2024
…ling edit after dashboard save (elastic#193914)

Part of elastic#193905. Broke work into
separate PRs to backport map embeddable changes to 8.15.

Resolves issue for Visualize embeddable and dashboard empty screen. The
problem is that Visualize embeddable is using a stale result from
`parentApi.getAppContext`. Dashboard's `getAppContext` changes the
`getCurrentPath` when the dashboard has a saved object id. By using the
stale results, `getCurrentPath` returned `#/create` instead of
`#/view/`. The fix is to get a current version of `getAppContext` when
required.

### Test steps
1. create new dashboard
2. Click "Add panel" (problem also exists when using "Add from library")
3. Select "Aggregation based"
4.  In editor, click "Save and return"
5. Save dashboard
6. Click "Edit" in panel context menu
7. In editor, click "Cancel"
8. Ensure visualize panel is still displayed in dashboard
neptunian pushed a commit to neptunian/kibana that referenced this pull request Oct 1, 2024
… canceling edit after dashboard save (elastic#193914) (elastic#194152)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[visualize embeddable] fix panel disappears from dashboard when
canceling edit after dashboard save
(elastic#193914)](elastic#193914)

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

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

<!--BACKPORT [{"author":{"name":"Nathan
Reese","email":"reese.nathan@elastic.co"},"sourceCommit":{"committedDate":"2024-09-26T13:47:04Z","message":"[visualize
embeddable] fix panel disappears from dashboard when canceling edit
after dashboard save (elastic#193914)\n\nPart of
elastic#193905. Broke work
into\r\nseparate PRs to backport map embeddable changes to
8.15.\r\n\r\nResolves issue for Visualize embeddable and dashboard empty
screen. The\r\nproblem is that Visualize embeddable is using a stale
result from\r\n`parentApi.getAppContext`. Dashboard's `getAppContext`
changes the\r\n`getCurrentPath` when the dashboard has a saved object
id. By using the\r\nstale results, `getCurrentPath` returned `#/create`
instead of\r\n`#/view/`. The fix is to get a current version of
`getAppContext` when\r\nrequired.\r\n\r\n### Test steps\r\n1. create new
dashboard\r\n2. Click \"Add panel\" (problem also exists when using
\"Add from library\")\r\n3. Select \"Aggregation based\"\r\n4. In
editor, click \"Save and return\"\r\n5. Save dashboard\r\n6. Click
\"Edit\" in panel context menu\r\n7. In editor, click \"Cancel\"\r\n8.
Ensure visualize panel is still displayed in
dashboard","sha":"d6c8840a561484bee74dbcb335290c879e82d370","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","v9.0.0","project:embeddableRebuild","v8.16.0","backport:version"],"title":"[visualize
embeddable] fix panel disappears from dashboard when canceling edit
after dashboard
save","number":193914,"url":"https://github.com/elastic/kibana/pull/193914","mergeCommit":{"message":"[visualize
embeddable] fix panel disappears from dashboard when canceling edit
after dashboard save (elastic#193914)\n\nPart of
elastic#193905. Broke work
into\r\nseparate PRs to backport map embeddable changes to
8.15.\r\n\r\nResolves issue for Visualize embeddable and dashboard empty
screen. The\r\nproblem is that Visualize embeddable is using a stale
result from\r\n`parentApi.getAppContext`. Dashboard's `getAppContext`
changes the\r\n`getCurrentPath` when the dashboard has a saved object
id. By using the\r\nstale results, `getCurrentPath` returned `#/create`
instead of\r\n`#/view/`. The fix is to get a current version of
`getAppContext` when\r\nrequired.\r\n\r\n### Test steps\r\n1. create new
dashboard\r\n2. Click \"Add panel\" (problem also exists when using
\"Add from library\")\r\n3. Select \"Aggregation based\"\r\n4. In
editor, click \"Save and return\"\r\n5. Save dashboard\r\n6. Click
\"Edit\" in panel context menu\r\n7. In editor, click \"Cancel\"\r\n8.
Ensure visualize panel is still displayed in
dashboard","sha":"d6c8840a561484bee74dbcb335290c879e82d370"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193914","number":193914,"mergeCommit":{"message":"[visualize
embeddable] fix panel disappears from dashboard when canceling edit
after dashboard save (elastic#193914)\n\nPart of
elastic#193905. Broke work
into\r\nseparate PRs to backport map embeddable changes to
8.15.\r\n\r\nResolves issue for Visualize embeddable and dashboard empty
screen. The\r\nproblem is that Visualize embeddable is using a stale
result from\r\n`parentApi.getAppContext`. Dashboard's `getAppContext`
changes the\r\n`getCurrentPath` when the dashboard has a saved object
id. By using the\r\nstale results, `getCurrentPath` returned `#/create`
instead of\r\n`#/view/`. The fix is to get a current version of
`getAppContext` when\r\nrequired.\r\n\r\n### Test steps\r\n1. create new
dashboard\r\n2. Click \"Add panel\" (problem also exists when using
\"Add from library\")\r\n3. Select \"Aggregation based\"\r\n4. In
editor, click \"Save and return\"\r\n5. Save dashboard\r\n6. Click
\"Edit\" in panel context menu\r\n7. In editor, click \"Cancel\"\r\n8.
Ensure visualize panel is still displayed in
dashboard","sha":"d6c8840a561484bee74dbcb335290c879e82d370"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels project:embeddableRebuild release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants