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

[Lens] allow removing ad-hoc data view from event annotation group #163976

Merged
merged 4 commits into from Aug 16, 2023

Conversation

drewdaemon
Copy link
Contributor

@drewdaemon drewdaemon commented Aug 15, 2023

Summary

Fix #163894

undefined was being used instead of null to indicate no ad-hoc data view spec. This was problematic since

  1. undefined properties are evicted from the object during JSON serialization
  2. the saved object API doesn't automatically remove fields that aren't mentioned in the payload (basically PATCH, not POST)

The bug was introduced with #159692.

I have

  • fixed the bug
  • corrected the type
  • corrected the unit test
  • covered this case with API integration tests

Checklist

@drewdaemon drewdaemon force-pushed the 163894/fix-annotation-save-bug branch from 353a10b to 85a4f74 Compare August 15, 2023 20:58
@drewdaemon
Copy link
Contributor Author

@elasticmachine merge upstream

@drewdaemon drewdaemon added release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure backport:prev-minor Backport to the previous minor version (i.e. one version back from main) v8.9.0 v8.10.0 and removed backport:prev-minor Backport to the previous minor version (i.e. one version back from main) labels Aug 15, 2023
@drewdaemon drewdaemon marked this pull request as ready for review August 15, 2023 23:05
@drewdaemon drewdaemon requested a review from a team as a code owner August 15, 2023 23:05
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@drewdaemon
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 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
eventAnnotation 176.7KB 176.7KB -8.0B

History

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

@drewdaemon drewdaemon merged commit 08a48f3 into elastic:main Aug 16, 2023
23 checks passed
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.9 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 163976

Questions ?

Please refer to the Backport tool documentation

@drewdaemon
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.9

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

Questions ?

Please refer to the Backport tool documentation

drewdaemon added a commit to drewdaemon/kibana that referenced this pull request Aug 16, 2023
…lastic#163976)

(cherry picked from commit 08a48f3)

# Conflicts:
#	.github/CODEOWNERS
drewdaemon added a commit that referenced this pull request Aug 16, 2023
…oup (#163976) (#164101)

# Backport

This will backport the following commits from `main` to `8.9`:
- [[Lens] allow removing ad-hoc data view from event annotation group
(#163976)](#163976)

<!--- Backport version: 8.9.8 -->

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

<!--BACKPORT [{"author":{"name":"Drew
Tate","email":"drew.tate@elastic.co"},"sourceCommit":{"committedDate":"2023-08-16T15:24:15Z","message":"[Lens]
allow removing ad-hoc data view from event annotation group
(#163976)","sha":"08a48f3cf1a928d2e01dbe1e867326ae54afbb5a","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Visualizations","v8.9.0","v8.10.0"],"number":163976,"url":"#163976
allow removing ad-hoc data view from event annotation group
(#163976)","sha":"08a48f3cf1a928d2e01dbe1e867326ae54afbb5a"}},"sourceBranch":"main","suggestedTargetBranches":["8.9"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"#163976
allow removing ad-hoc data view from event annotation group
(#163976)","sha":"08a48f3cf1a928d2e01dbe1e867326ae54afbb5a"}}]}]
BACKPORT-->
@mistic
Copy link
Member

mistic commented Aug 17, 2023

This pr didn't make it into the build candidate for v8.9.1. Updating the labels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.9.2 v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Event annotations] cannot remove ad-hoc data view from group
6 participants