Skip to content

[Discover] Refactor getContextUrl to separate file#84503

Merged
kertal merged 5 commits intoelastic:masterfrom
kertal:kertal-pr-2020-11-30-discover-helper-get-context-url
Dec 3, 2020
Merged

[Discover] Refactor getContextUrl to separate file#84503
kertal merged 5 commits intoelastic:masterfrom
kertal:kertal-pr-2020-11-30-discover-helper-get-context-url

Conversation

@kertal
Copy link
Copy Markdown
Member

@kertal kertal commented Nov 30, 2020 •
edited
Loading

Summary

Extract the function to generate a URL to Discover Context view to a separate file. Why? Since it will be used in 2 places in the future (legacy grid + EUI data grid).

Checklist

@kertal kertal added the Feature:Discover Discover Application label Nov 30, 2020
@kertal kertal self-assigned this Nov 30, 2020
@kertal kertal added Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.11.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Dec 3, 2020
@kertal kertal marked this pull request as ready for review December 3, 2020 08:07
@kertal kertal requested a review from a team December 3, 2020 08:07
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 3, 2020

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

@kertal
Copy link
Copy Markdown
Member Author

kertal commented Dec 3, 2020

@elasticmachine merge upstream

@kertal kertal requested a review from majagrubic December 3, 2020 08:08
majagrubic
majagrubic approved these changes Dec 3, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@majagrubic majagrubic left a comment

Choose a reason for hiding this comment

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

I haven't tested this, but the code change looks pretty straightforward and the most common use-cases are covered with a unit test.

src/plugins/discover/public/application/helpers/get_context_url.test.ts
} as unknown) as FilterManager;

describe('Get context url', () => {
test('returning a valid context url', async () => {
Copy link
Copy Markdown
Contributor

@majagrubic majagrubic Dec 3, 2020

Choose a reason for hiding this comment

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

so great that we now have tests for this!

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Dec 3, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
discover 279 280 +1

Async chunks

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

id before after diff
discover 432.2KB 432.3KB +139.0B

Page load bundle

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

id before after diff
discover 82.1KB 82.3KB +238.0B

History

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

@kertal kertal merged commit ae463cf into elastic:master Dec 3, 2020
kertal added a commit to kertal/kibana that referenced this pull request Dec 3, 2020
@kertal
[Discover] Refactor getContextUrl to separate file (elastic#84503)
0e43385
@kertal kertal mentioned this pull request Dec 3, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 3, 2020
@gmmorris
Merge branch 'master' into alerts/force-schedule
ffcc5ca 
* master: (40 commits)
  fix: 🐛 don't add separator befor group on no main items (elastic#83166)
  [Security Solution][Detections] Implements indicator match rule cypress test (elastic#84323)
  [APM] Add APM agent config options (elastic#84678)
  Fixed a11y issue on rollup jobs table selection (elastic#84567)
  [Discover] Refactor getContextUrl to separate file (elastic#84503)
  [Embeddable] Export CSV action for Lens embeddables in dashboard (elastic#83654)
  [TSVB] [Cleanup] Remove extra dateFormat props (elastic#84749)
  [Lens] Migrate legacy es client and remove total hits as int (elastic#84340)
  Improve logging pipeline in @kbn/legacy-logging (elastic#84629)
  Catch @hapi/podium errors (elastic#84575)
  [Discover] Unskip date histogram test (elastic#84727)
  Rename server.xsrf.whitelist to server.xsrf.allowlist (elastic#84791)
  [Enterprise Search] Fix schema errors button (elastic#84842)
  [APM] Removes react-sticky dependency in favor of using CSS (elastic#84589)
  [Maps] Always initialize routes on server-startup (elastic#84806)
  [Fleet] EPM support to handle uploaded file paths (elastic#84708)
  [Snapshot Restore] Fix initial policy form state (elastic#83928)
  Upgrade Node.js to version 14 (elastic#83425)
  [Security Solution] Keep Endpoint policies up to date with license changes (elastic#83992)
  [Security Solution][Exceptions] Implement exceptions for ML rules (elastic#84006)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 3, 2020
@gmmorris
Merge branch 'master' into alerts/flaky-event-log-tests
5cce0f7 
* master: (236 commits)
  fix: 🐛 don't add separator befor group on no main items (elastic#83166)
  [Security Solution][Detections] Implements indicator match rule cypress test (elastic#84323)
  [APM] Add APM agent config options (elastic#84678)
  Fixed a11y issue on rollup jobs table selection (elastic#84567)
  [Discover] Refactor getContextUrl to separate file (elastic#84503)
  [Embeddable] Export CSV action for Lens embeddables in dashboard (elastic#83654)
  [TSVB] [Cleanup] Remove extra dateFormat props (elastic#84749)
  [Lens] Migrate legacy es client and remove total hits as int (elastic#84340)
  Improve logging pipeline in @kbn/legacy-logging (elastic#84629)
  Catch @hapi/podium errors (elastic#84575)
  [Discover] Unskip date histogram test (elastic#84727)
  Rename server.xsrf.whitelist to server.xsrf.allowlist (elastic#84791)
  [Enterprise Search] Fix schema errors button (elastic#84842)
  [APM] Removes react-sticky dependency in favor of using CSS (elastic#84589)
  [Maps] Always initialize routes on server-startup (elastic#84806)
  [Fleet] EPM support to handle uploaded file paths (elastic#84708)
  [Snapshot Restore] Fix initial policy form state (elastic#83928)
  Upgrade Node.js to version 14 (elastic#83425)
  [Security Solution] Keep Endpoint policies up to date with license changes (elastic#83992)
  [Security Solution][Exceptions] Implement exceptions for ML rules (elastic#84006)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 3, 2020
@gmmorris
Merge remote-tracking branch 'upstream/master' into alerts/custom-rec…
7762864 
…overy-action-group

* upstream/master: (48 commits)
  [Lens] accessibility screen reader issues (elastic#84395)
  [Logs UI] Fetch single log entries via a search strategy (elastic#81710)
  fix: 🐛 don't add separator befor group on no main items (elastic#83166)
  [Security Solution][Detections] Implements indicator match rule cypress test (elastic#84323)
  [APM] Add APM agent config options (elastic#84678)
  Fixed a11y issue on rollup jobs table selection (elastic#84567)
  [Discover] Refactor getContextUrl to separate file (elastic#84503)
  [Embeddable] Export CSV action for Lens embeddables in dashboard (elastic#83654)
  [TSVB] [Cleanup] Remove extra dateFormat props (elastic#84749)
  [Lens] Migrate legacy es client and remove total hits as int (elastic#84340)
  Improve logging pipeline in @kbn/legacy-logging (elastic#84629)
  Catch @hapi/podium errors (elastic#84575)
  [Discover] Unskip date histogram test (elastic#84727)
  Rename server.xsrf.whitelist to server.xsrf.allowlist (elastic#84791)
  [Enterprise Search] Fix schema errors button (elastic#84842)
  [APM] Removes react-sticky dependency in favor of using CSS (elastic#84589)
  [Maps] Always initialize routes on server-startup (elastic#84806)
  [Fleet] EPM support to handle uploaded file paths (elastic#84708)
  [Snapshot Restore] Fix initial policy form state (elastic#83928)
  Upgrade Node.js to version 14 (elastic#83425)
  ...
kertal added a commit that referenced this pull request Dec 3, 2020
@kertal
[Discover] Refactor getContextUrl to separate file (#84503) (#84878)
cb9a208
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@majagrubic majagrubic majagrubic approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kertal kertal

Labels

Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.11.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kertal @elasticmachine @kibanamachine @majagrubic