Repository navigation
[ML] Disabling start trial option when license management ui is disabled - #60987
Conversation
|
Pinging @elastic/ml-ui (:ml) |
loixlab
left a comment
There was a problem hiding this comment.
Overall looks good to me, left a few comments about code preferences and one about declaring the plugin dependency as optional and thus we need to check its existence before reading its config.
| stop() {} | ||
| } | ||
|
|
||
| export type LicenseManagementUIPluginSetup = ReturnType<LicenseManagementUIPlugin['setup']>; |
There was a problem hiding this comment.
I would prefer to declare explicitly the setup interface above the class.
It makes it easier to scan IMO
export interface LicenseManagementUIPluginSetup {
enabled: boolean;
}
export class LicenseManagementUIPlugin ...| */ | ||
| import { PluginInitializerContext } from 'src/core/public'; | ||
|
|
||
| export { LicenseManagementUIPluginSetup, LicenseManagementUIPluginStart } from './plugin'; |
There was a problem hiding this comment.
Do you mind putting the exports below the imports ?
| interface StartPlugins { | ||
| data: DataPublicPluginStart; | ||
| security: SecurityPluginSetup; | ||
| licenseManagement: LicenseManagementUIPluginSetup; |
There was a problem hiding this comment.
As it is optional we need to add the ?
licenseManagement?: LicenseManagementUIPluginSetup;| } = useMlKibana(); | ||
|
|
||
| const startTrialVisible = isFullLicense() === false; | ||
| const startTrialVisible = licenseManagement.enabled === true && isFullLicense() === false; |
There was a problem hiding this comment.
As the licenseManagement plugin is optional, you first need to check for the plugin existence.
| licensing: LicensingPluginSetup; | ||
| management: ManagementSetup; | ||
| usageCollection: UsageCollectionSetup; | ||
| licenseManagement: LicenseManagementUIPluginSetup; |
There was a problem hiding this comment.
Here the same
licenseManagement?: LicenseManagementUIPluginSetup;
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
LGTM! Thanks for making the changes.
Just a note, there are other plugins declared as optional in the kibana.json but are not declared with the ? in the interface (like the "security" plugin). Probably worth a look to check if some code might break if those plugins are disabled.
…led (elastic#60987) * [ML] Disabling start trial option when license management ui is disabled * tiny refactor * changes based on review
…led (elastic#60987) * [ML] Disabling start trial option when license management ui is disabled * tiny refactor * changes based on review
…led (elastic#60987) * [ML] Disabling start trial option when license management ui is disabled * tiny refactor * changes based on review
If the license management plugin is disabled, we should not show the option to navigate to the license management page to start a trial.
Setup contract was needed in the license management plugin.
Fixes #58207