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.

Sign up for GitHub

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

Jump to bottom

[Discover] Encode context anchor ID #160239

Merged
merged 2 commits into from
Jun 23, 2023
Merged

[Discover] Encode context anchor ID #160239

merged 2 commits into from
Jun 23, 2023

Conversation

jughosta
Copy link
Contributor

@jughosta jughosta commented Jun 22, 2023 •
edited by kibanamachine
Loading

Summary

Added the missing encoding and updated tests.

For testing

An id with special characters like / can be specified via _bulk API.
Example:

POST _bulk
{ "index" : { "_index" : "context-special", "_id" : "test&?#+/=" } }
{ "timestamp":"2023-06-20T20:00:00.123Z", "name": "test" }

@jughosta jughosta added release_note:fix Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Jun 22, 2023
@jughosta jughosta self-assigned this Jun 22, 2023
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

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

id before after diff
discover 28.9KB 29.0KB +20.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 13 15 +2
securitySolution 416 420 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 14 16 +2
securitySolution 497 501 +4
total +6

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

cc @jughosta

@jughosta jughosta marked this pull request as ready for review June 22, 2023 11:24
@jughosta jughosta requested a review from a team as a code owner June 22, 2023 11:24
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

davismcphee
davismcphee approved these changes Jun 22, 2023
View reviewed changes
Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Tested locally and working as expected 👍 Thanks for fixing!

@jughosta jughosta merged commit 78ee25d into elastic:main Jun 23, 2023
@jughosta jughosta deleted the 160212-context-id-encoding branch June 23, 2023 06:04
@kibanamachine kibanamachine mentioned this pull request Jun 23, 2023
Closed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 23, 2023
@jughosta
[Discover] Encode context anchor ID (elastic#160239)
793c130 
- Closes elastic#160212

## Summary

Added the missing encoding and updated tests.

## For testing

An id with special characters like `/` can be specified via `_bulk` API.
Example:

```
POST _bulk
{ "index" : { "_index" : "context-special", "_id" : "test&?#+/=" } }
{ "timestamp":"2023-06-20T20:00:00.123Z", "name": "test" }
```

(cherry picked from commit 78ee25d)
@kibanamachine
Copy link
Contributor

💚 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

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 4, 2024
@jughosta jughosta removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 5, 2024
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 5, 2024
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@jughosta jughosta removed backport missing Added to PRs automatically when the are determined to be missing a backport. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Aug 6, 2024
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 8, 2024
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 160239 locally

@jughosta jughosta added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davismcphee davismcphee davismcphee approved these changes

Assignees

@jughosta jughosta

Labels
backport:skip This commit does not require backporting release_note:fix Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Discover] Surrounding documents fails to load when ID contains a slash
5 participants
@jughosta @kibana-ci @elasticmachine @kibanamachine @davismcphee