-
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
Sync UserProfile
interface in kbn/user-profile-components
package with the one in Security plugin.
#138704
Sync UserProfile
interface in kbn/user-profile-components
package with the one in Security plugin.
#138704
Conversation
…s with the one in Security plugin.
/** | ||
* Indicates whether user profile is enabled or not. | ||
*/ | ||
enabled: boolean; |
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.
note: Covered the rationale here
kibana/x-pack/plugins/security/server/user_profile/user_profile_service.ts
Lines 182 to 185 in ea7bc36
// Get User Profile API returns `enabled` property, but Suggest User Profile API doesn't since it's assumed that the | |
// API returns only enabled profiles. To simplify the API in Kibana we use the same interfaces for user profiles | |
// irrespective to the source they are coming from, so we need to "normalize" `enabled` property here. | |
enabled: rawUserProfile.enabled ?? true, |
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.
Ah nice, thanks for this Oleg. I wrongly removed this prop from the interface thinking it wasn't used anymore.
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.
No worries, I reviewed that PR and thought that was fine to remove it too 🙈 Only when migrating to the new Bulk Get API I realized that our consumers might need this field to recognize not active user profiles (e.g. in cases assignments).
UserProfile
interface in kbn/user-profile-components
packages with the one in Security plugin.UserProfile
interface in kbn/user-profile-components
package with the one in Security plugin.
@elasticmachine merge upstream |
Pinging @elastic/kibana-security (Team:Security) |
@@ -13,6 +13,7 @@ import { PanelWithCodeBlock } from './panel_with_code_block'; | |||
export const AvatarDemo: FunctionComponent = () => { | |||
const userProfile = { | |||
uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0', | |||
enabled: true, |
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.
nit: might be worth typing these so that we get type errors before test failures 🤷♂️
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.
Yep, it's a good idea, will do!
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.
LGTM!
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]History
To update your PR or re-run it, just comment with: |
… with the one in Security plugin. (elastic#138704)
Summary
Sync
UserProfile
interface inkbn/user-profile-components
packages with the one in Security plugin.