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

[ML] Fix rison encoding of index ID in the URL which prevented browser refresh #56380

Merged
merged 1 commit into from
Jan 30, 2020
Merged

[ML] Fix rison encoding of index ID in the URL which prevented browser refresh #56380

merged 1 commit into from
Jan 30, 2020

Conversation

darnautov
Copy link
Contributor

@darnautov darnautov commented Jan 30, 2020 •
edited by peteharverson
Loading

Summary

Fixes an issue where query params in the URL besides _a and _q, such as index or savedSearchId, were getting rison encoded when the state of the view was changed. The resulting URL would then contain invalid characters, which would prevent the view reloading correctly when restored from the URL, for example on a browser refresh.

This fixes that behavior by ensuring only the AppState, _a and GlobalState _g params are rison encoded.

Checklist

@darnautov darnautov added :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v7.6.0 labels Jan 30, 2020
@darnautov darnautov requested a review from a team as a code owner January 30, 2020 13:32
@darnautov darnautov self-assigned this Jan 30, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@peteharverson peteharverson added the bug Fixes for quality problems that affect the customer experience label Jan 30, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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

@peteharverson peteharverson changed the title [ML] Fix rison encoding in the URL state [ML] Fix rison encoding of index ID in the URL which prevented browser refresh Jan 30, 2020
peteharverson
peteharverson approved these changes Jan 30, 2020
View reviewed changes
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.

LGTM.

As discussed, I could not reproduce the original issue on Windows (Chrome and Firefox)!

jgowdyelastic
jgowdyelastic approved these changes Jan 30, 2020
View reviewed changes
Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

@darnautov darnautov merged commit ef8dba3 into elastic:master Jan 30, 2020
@darnautov darnautov deleted the ML-fix-appstate-encoding branch January 30, 2020 16:07
@darnautov darnautov mentioned this pull request Jan 31, 2020
Merged
darnautov added a commit to darnautov/kibana that referenced this pull request Jan 31, 2020
@darnautov
[ML] conditional rison encoding for query params (elastic#56380)
207e567
darnautov added a commit to darnautov/kibana that referenced this pull request Jan 31, 2020
@darnautov
[ML] conditional rison encoding for query params (elastic#56380)
dbb4c6d
@darnautov darnautov mentioned this pull request Jan 31, 2020
Merged
darnautov added a commit that referenced this pull request Jan 31, 2020
@darnautov
[ML] conditional rison encoding for query params (#56380) (#56469)
3f5149f
darnautov added a commit that referenced this pull request Jan 31, 2020
@darnautov
[ML] conditional rison encoding for query params (#56380) (#56470)
01368c8
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 31, 2020
@gmmorris
Merge branch 'master' into alerting/details-ui
467bbc4 
* master: (53 commits)
  Fix setting filters without $store value (elastic#56304)
  kuery_autocomplete -> convert remaining items to TS/Jest (elastic#56316)
  [Reporting] New Platform: moves most of our libs/constants and utils to np shims (elastic#55935)
  [reporting] remove a legacy cross-plugin import (elastic#56354)
  [DOCS] Updates example in Timelion doc (elastic#56444)
  [APM] Service map center button (elastic#56434)
  [skip-ci] Add example for migrating pre-handlers (elastic#56080)
  [ML] Fix Data Visualizer responsive layout  (elastic#56372)
  Add missing docker settings (elastic#56411)
  [Endpoint] Add Endpoint Details route (elastic#55746)
  chore(NA): delete data/optimize with kbn clean (elastic#55890)
  System index templates can't be edited (elastic#55229)
  Sync badge (elastic#55113)
  Only fire appState changes when there is a change (elastic#56183)
  Import appropriate files to setup plugin system at the correct time (elastic#55956)
  [Monitoring] Change all configs to `monitoring.*` (elastic#56215)
  [ML] conditional rison encoding for query params (elastic#56380)
  Move tsvb server to new platform (elastic#55310)
  exclude tutorial resources from code ownership (elastic#55987)
  [Watcher] Follow up on flaky functional test (elastic#56384)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 31, 2020
@jloleysens
Merge branch '7.x' of github.com:elastic/kibana into backport/7.x/pr-…
f8afb1b 
…56356

* '7.x' of github.com:elastic/kibana: (23 commits)
  Fix setting filters without $store value (elastic#56304) (elastic#56475)
  [ML] Fix Data Visualizer responsive layout  (elastic#56372) (elastic#56472)
  [ML] conditional rison encoding for query params (elastic#56380) (elastic#56469)
  kuery_autocomplete -> convert remaining items to TS/Jest (elastic#56316) (elastic#56471)
  [APM] Fit service map to container (elastic#56336) (elastic#56463)
  Add animation to service map layout (elastic#56042) (elastic#56460)
  chore(NA): delete data/optimize with kbn clean (elastic#55890) (elastic#56422)
  [APM] Storybook support (elastic#54970) (elastic#56445)
  [DOCS] Updates example in Timelion doc (elastic#56444) (elastic#56454)
  [Logs UI] Fix Check for New Data button on empty indices screen (elastic#56239) (elastic#56320)
  [DOCS] Adds breaking changes for 7.6 (elastic#56437)
  [Monitoring] Change all configs to `monitoring.*` (elastic#56215) (elastic#56421)
  [skip-ci] Add example for migrating pre-handlers (elastic#56080) (elastic#56436)
  [7.x] System index templates can't be edited (elastic#55229) (elastic#56417)
  Add missing docker settings (elastic#56411)
  [Uptime] Use dynamic index pattern in Uptime (elastic#55446) (elastic#56386)
  fix edit rule for detections (elastic#56333) (elastic#56405)
  [Filter Bar] Remove flickering when opening filter bar popover (elastic#56222) (elastic#56385)
  [ILM] Index Lifecycle Policies show wrong unit in Kibana UI (elastic#55228) (elastic#55757)
  Move tsvb server to new platform (elastic#55310) (elastic#56394)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgowdyelastic jgowdyelastic jgowdyelastic approved these changes

@peteharverson peteharverson peteharverson approved these changes

@walterra walterra Awaiting requested review from walterra

Assignees

@darnautov darnautov

Labels
bug Fixes for quality problems that affect the customer experience :ml release_note:skip Skip the PR/issue when compiling release notes v7.6.0 v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@darnautov @elasticmachine @kibanamachine @peteharverson @jgowdyelastic