-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Migrate ui/registry/feature_catalogue to New Platform plugin #48818
Conversation
Pinging @elastic/kibana-platform (Team:Platform) |
src/plugins/feature_catalogue/public/services/feature_catalogue_registry.ts
Outdated
Show resolved
Hide resolved
💚 Build Succeeded
|
return { | ||
get: (): readonly FeatureCatalogueEntry[] => | ||
[...this.features.values()] | ||
.filter(entry => capabilities.catalogue[entry.id] !== false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this is desired behaviour https://github.com/elastic/kibana/pull/48818/files#diff-a757f884a32fbf9bab5a0c977e69bb9eR54
just curious why? to allow OSS ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's for plugins that haven't registered features, eg. 3rd party plugins, functional test plugins, etc.
src/plugins/feature_catalogue/public/services/feature_catalogue_registry.test.ts
Outdated
Show resolved
Hide resolved
src/plugins/feature_catalogue/public/services/feature_catalogue_registry.test.ts
Outdated
Show resolved
Hide resolved
}); | ||
|
||
describe('setup', () => { | ||
test('wires up and returns registry', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional: not sure those are really useful tests if TS enforces the contract
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can go either way here, but I prefer some coverage here over nothing.
# Feature catalogue plugin | ||
|
||
Replaces the legacy `ui/registry/feature_catalogue` module for registering "features" that should be showed in the home | ||
page's feature catalogue. This should not be confused with the "feature" plugin for registering features used to derive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then probably we should rename either?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will be more clear once this registry moves into a "home" plugin with the actual UI. I opted not to name this plugin "home" right now since it may not be split out until 8.x and I thought that would be confusing in the meantime.
b56a005
to
f612fb9
Compare
💔 Build Failed |
@elasticmachine merge upstream |
💚 Build Succeeded
|
@elasticmachine merge upstream |
merge conflict between base and head |
7b58502
to
e6a27c2
Compare
(Private(FeatureCatalogueRegistryProvider as any) as any).inTitleOrder.map( | ||
npSetup.plugins.feature_catalogue.register | ||
); | ||
return npStart.plugins.feature_catalogue.get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@flash1293 Any issues with doing this here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all, that's fine 👍
💚 Build Succeeded |
Summary
Fixes #46738
This PR adds a new
feature_catalogue
plugin which provides an interface for registering and reading a catalogue items.For the time being, the Home app will register all items out of the legacy registry with the new plugin, and then read these items out of the new plugin.
Dev Docs
The
ui/registries/feature_catalogue
module has been deprecated for removal in 8.0Plugins wishing to migrate may remove their usage of
ui/registries/feature_catalogue
and rely on either:Note that the old module supported providing a Angular DI function to receive Angular dependencies. This is no longer supported as we migrate away from Angular.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers