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

[Discover] Encode context link filter part #63831

Conversation

kertal
Copy link
Member

@kertal kertal commented Apr 17, 2020

Summary

When you added a filter in Discover that contained a whitespace, navigate to context view (surrounding documents), you couldn't navigate back with the browser's back button. This PR fixes this by encoding the filter part of the context link.

Fixes #63674

Steps to reproduce:

  • Use kibana_sample_data_logs
  • Navigate to Discover
  • Add a filter for an agent field (those usually contain whitespaces)
  • Expand a document and click on "View surrounding documents"
  • Try to navigate back with the browsers back button
  • Doesn't work in Chrome, Firefox

Checklist

Delete any items that are not applicable to this PR.

@kertal kertal added Feature:Discover Discover Application release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 v7.7.0 v7.8.0 labels Apr 17, 2020
@kertal kertal self-assigned this Apr 17, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested in Chrome and back/forward buttons work as expected, LGTM

@kertal kertal force-pushed the kertal-pr-2020-discover-encode-filters-in-context-url branch from 439eac1 to 4164426 Compare May 12, 2020 15:12
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

  • 💚 Build #41501 succeeded 439eac1eaa321e8e63051eafc900a888674a6f90

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

@kertal kertal marked this pull request as ready for review May 12, 2020 17:15
@kertal kertal requested a review from a team May 12, 2020 17:15
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Approving again because according to GH approvals on draft PRs are not serious enough.

@kertal
Copy link
Member Author

kertal commented May 13, 2020

@flash1293 thx, promise, this is the last time you've to approve. last time, there was a test for this missing

@kertal kertal merged commit e6254f1 into elastic:master May 13, 2020
kertal added a commit to kertal/kibana that referenced this pull request May 13, 2020
* Encode filter part of the URI for context

* Add functional test agent filter with whitespace
v1v added a commit to v1v/kibana that referenced this pull request May 13, 2020
* upstream/master: (223 commits)
  [Ingest] Support root level yaml variables in agent stream template (elastic#66120)
  [Snapshot Restore] Fix error when deleting snapshots behind reverse proxy (elastic#66147)
  [Lens] fix empty state for pie (elastic#66206)
  [APM] Improve e2e tests (elastic#66373)
  [ML] Data Frame Analytics: Fix steps to be named phases. (elastic#65855)
  [Discover] Encode context link filter part (elastic#63831)
  [APM] Scope APM alert creation to environment (elastic#65681)
  Move Kibana Usage collectors outside the telemetry plugin (elastic#65663)
  [ML] Data Frame Analytics: Fix confusion matrix data grid width. (elastic#65818)
  Switch to core application service (elastic#63443)
  Removes use of prefer_v2_templates (elastic#66316)
  [Maps] handle case where fit to bounds does not match any documents (elastic#66307)
  log error instead of throw (elastic#66254)
  [plugin-helpers] remove outdated postinstall task (elastic#66324)
  Spaces - migrate default space and enter space view to KP (elastic#66098)
  [APM] Change plugin id for `apm_oss` to `apmOss` (elastic#66164)
  [Maps] convert map_selectors to TS (elastic#65905)
  [uptime/usage-collector] add missing await (elastic#66079)
  [Ingest] Add additional attributes to the Datasources Saved Object (elastic#66127)
  [Endpoint]EMT-339: add new policy response schema (elastic#66264)
  ...
kertal added a commit that referenced this pull request May 13, 2020
* Encode filter part of the URI for context

* Add functional test agent filter with whitespace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discover] Broken back navigation in context when filter contains whitespace
4 participants