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

Url service locators #101045

Merged
merged 17 commits into from
Jun 7, 2021
Merged

Url service locators #101045

merged 17 commits into from
Jun 7, 2021

Conversation

streamich
Copy link
Contributor

@streamich streamich commented Jun 1, 2021

Closes #98104

Summary

This PR introduces URL locators as part of the URL Service. This PR just introduces the service, the next piece of work will refactor Kibana URL generators to become locators and update documentation.

Checklist

Delete any items that are not applicable to this PR.

  • Documentation was added for features that require explanation or tutorials
    • Partially adjusted existing docs, new documentation will be added in subsequent PR.
  • Unit or functional tests were updated or added to match the most common scenarios

For maintainers


Plugin API Changes

URL generators are now deprecated, to create a generator for deep links in Kibana use locators in URL Service instead.

Deep link providers

Before you would create a URL generator, like so:

plugins.share.urlGenerators.registerUrlGenerator(/* ... */);

Now, instead, you have to create a "locator", like so:

plugins.share.url.locators.create(/* ... */);

Deep link consumers

Before you would use a URL generator to get a relative deep link in kibana:

plugins.share.urlGenerators.getUrlGenerator('MY_GENERATOR').createUrl(/* ... */);

Now you will create a relative link, or navigate immediately using locators:

plugins.share.locators.get('MY_LOCATOR')!.getLocation(/* ... */);
plugins.share.locators.get('MY_LOCATOR')!.navigate(/* ... */);

@streamich streamich added Feature:SharingURLs Short URLs and Share URL features release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:AppServices v7.14.0 v8.0.0 labels Jun 1, 2021
@streamich streamich marked this pull request as ready for review June 4, 2021 13:02
@streamich streamich requested a review from a team June 4, 2021 13:02
@streamich streamich requested a review from a team as a code owner June 4, 2021 13:02
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@streamich
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

return {
...this.shareMenuRegistry.setup(),
urlGenerators: this.urlGeneratorsService.setup(core),
url: this.url,
Copy link
Member

Choose a reason for hiding this comment

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

we return the same on start and setup contract but we should just allow registering in setup and getting in start

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can harden it if you like, I made it this way as it is cleaner from code perspective and we have other services where we do the same.

@streamich
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM, code owners review, didn't test, just the order on imports was changed and 2 lines were added for consistent styling.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
share 47 53 +6

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
share 3 4 +1

Page load bundle

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

id before after diff
share 67.9KB 71.3KB +3.3KB

History

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

@streamich streamich merged commit 2a71047 into elastic:master Jun 7, 2021
streamich added a commit that referenced this pull request Jun 7, 2021
* feat: 🎸 add url service types

* refactor: 💡 move locator types into its own folder

* feat: 🎸 add abstract locator implementation

* feat: 🎸 implement abstract locator client

* feat: 🎸 add browser-side locators service

* feat: 🎸 implement locator .getLocation()

* feat: 🎸 implement navigate function

* feat: 🎸 implement locator service in /common folder

* feat: 🎸 expose locators client on browser and server

* refactor: 💡 make locators async

* chore: 🤖 add deprecation notice to URL generators

* docs: ✏️ add deprecation notice to readme

* test: 💍 make test locator async

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 7, 2021
* master: (90 commits)
  Fix UI breaks on providing long search keyword in 'Search Box' (elastic#101385)
  Adds css class to EuiDescriptionListDescription in order to break word on exception details card (elastic#101481)
  [Lens] Increase timings for drag and drop tests (elastic#101380)
  [Lens] Fix editor react error on configuration panel (elastic#101367)
  [Fleet] Move integrations to a separate app (elastic#99848)
  Fix incorrect message displayed on importing Timeline Templates (elastic#101288)
  [Cases] RBAC (elastic#95058)
  [APM] Visual improvements for new APM layout with left navigation (elastic#101360)
  [master] More precise alerts matching (elastic#99820)
  [Lens] Value in legend (elastic#101353)
  Revert "[Reporting] ILM policy for managing reporting indices (elastic#100130)" (elastic#101358)
  [Discover] Fix header row of data grid in Firefox (elastic#101374)
  Add link to advanced setting in Discover (elastic#101154)
  Url service locators (elastic#101045)
  [Timelion] Update the removal message to mention the exact version (elastic#100994)
  [Security Solution][Detection Engine] Test cases for alias failure test cases where we don't copy aliases correctly (elastic#101437)
  [Event Log] Adding `type_id` to saved object array in event log (elastic#100939)
  [Reporting] Add `location.url` info to console message logs (elastic#101427)
  [Security Solutions][Detection Engine] Fixes timestamp bugs within source indexes when the formats are not ISO8601 format (elastic#101349)
  Improve Task Manager instrumentation (elastic#99160)
  ...
@streamich streamich mentioned this pull request Jun 25, 2021
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:SharingURLs Short URLs and Share URL features release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. review v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce Locators in share plugin
5 participants