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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Aug 12, 2022
Merged

Sync UserProfile interface in kbn/user-profile-components package with the one in Security plugin. #138704

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
28d0949
Sync `UserProfile` interface in `kbn/user-profile-components` package…
azasypkin Aug 12, 2022
19ae485
Update example plugin.
azasypkin Aug 12, 2022
773ab22
Merge branch 'main' into issue-xxx-user-profile-enabled
kibanamachine Aug 12, 2022
69f4096
Review#1: type mock user profiles in `avatar_demo`.
azasypkin Aug 12, 2022
22973a1
Merge branch 'main' into issue-xxx-user-profile-enabled
kibanamachine Aug 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions examples/user_profile_examples/public/avatar_demo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { PanelWithCodeBlock } from './panel_with_code_block';
export const AvatarDemo: FunctionComponent = () => {
const userProfile = {
uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0',
enabled: true,
Copy link
Contributor

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 🤷‍♂️

Copy link
Member Author

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!

user: {
username: 'delighted_nightingale',
email: 'delighted_nightingale@elastic.co',
Expand Down Expand Up @@ -49,6 +50,7 @@ const code = `import { UserAvatar } from '@kbn/user-profile-components';

const userProfile = {
uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0',
enabled: true,
user: {
username: 'delighted_nightingale',
email: 'delighted_nightingale@elastic.co',
Expand Down
7 changes: 6 additions & 1 deletion examples/user_profile_examples/public/popover_demo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const PopoverDemo: FunctionComponent = () => {
const [selectedOptions, setSelectedOptions] = useState<UserProfileWithAvatar[]>([
{
uid: 'u_BOulL4QMPSyV9jg5lQI2JmCkUnokHTazBnet3xVHNv0_0',
enabled: true,
data: {},
user: {
username: 'delighted_nightingale',
Expand All @@ -26,6 +27,7 @@ export const PopoverDemo: FunctionComponent = () => {
const defaultOptions: UserProfileWithAvatar[] = [
{
uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0',
enabled: true,
data: {},
user: {
username: 'damaged_raccoon',
Expand All @@ -35,6 +37,7 @@ export const PopoverDemo: FunctionComponent = () => {
},
{
uid: 'u_A_tM4n0wPkdiQ9smmd8o0Hr_h61XQfu8aRPh9GMoRoc_0',
enabled: true,
data: {},
user: {
username: 'physical_dinosaur',
Expand All @@ -44,6 +47,7 @@ export const PopoverDemo: FunctionComponent = () => {
},
{
uid: 'u_9xDEQqUqoYCnFnPPLq5mIRHKL8gBTo_NiKgOnd5gGk0_0',
enabled: true,
data: {},
user: {
username: 'wet_dingo',
Expand Down Expand Up @@ -83,6 +87,7 @@ const code = `import { UserProfilesPopover } from '@kbn/user-profile-components'
const [selectedOptions, setSelectedOptions] = useState([
{
uid: 'u_BOulL4QMPSyV9jg5lQI2JmCkUnokHTazBnet3xVHNv0_0',
enabled: true,
data: {},
user: {
username: 'delighted_nightingale',
Expand All @@ -92,7 +97,7 @@ const [selectedOptions, setSelectedOptions] = useState([
},
]);

<UserProfilesPopover
<UserProfilesPopover
title="Edit assignees"
button={
<EuiButton>
Expand Down
5 changes: 5 additions & 0 deletions examples/user_profile_examples/public/selectable_demo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const SelectableDemo: FunctionComponent = () => {
const [selectedOptions, setSelectedOptions] = useState<UserProfileWithAvatar[]>([
{
uid: 'u_BOulL4QMPSyV9jg5lQI2JmCkUnokHTazBnet3xVHNv0_0',
enabled: true,
data: {},
user: {
username: 'delighted_nightingale',
Expand All @@ -25,6 +26,7 @@ export const SelectableDemo: FunctionComponent = () => {
const defaultOptions: UserProfileWithAvatar[] = [
{
uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0',
enabled: true,
data: {},
user: {
username: 'damaged_raccoon',
Expand All @@ -34,6 +36,7 @@ export const SelectableDemo: FunctionComponent = () => {
},
{
uid: 'u_A_tM4n0wPkdiQ9smmd8o0Hr_h61XQfu8aRPh9GMoRoc_0',
enabled: true,
data: {},
user: {
username: 'physical_dinosaur',
Expand All @@ -43,6 +46,7 @@ export const SelectableDemo: FunctionComponent = () => {
},
{
uid: 'u_9xDEQqUqoYCnFnPPLq5mIRHKL8gBTo_NiKgOnd5gGk0_0',
enabled: true,
data: {},
user: {
username: 'wet_dingo',
Expand All @@ -68,6 +72,7 @@ const code = `import { UserProfilesSelectable } from '@kbn/user-profile-componen
const [selectedOptions, setSelectedOptions] = useState([
{
uid: 'u_BOulL4QMPSyV9jg5lQI2JmCkUnokHTazBnet3xVHNv0_0',
enabled: true,
data: {},
user: {
username: 'delighted_nightingale',
Expand Down
5 changes: 5 additions & 0 deletions packages/kbn-user-profile-components/src/user_profile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ export interface UserProfile<D extends UserProfileData = UserProfileData> {
*/
uid: string;

/**
* Indicates whether user profile is enabled or not.
*/
enabled: boolean;
Copy link
Member Author

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,

Copy link
Contributor

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.

Copy link
Member Author

@azasypkin azasypkin Aug 12, 2022 •
edited
Loading

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).


/**
* Information about the user that owns profile.
*/
Expand Down
6 changes: 6 additions & 0 deletions packages/kbn-user-profile-components/src/user_profiles_popover.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { UserProfilesPopover } from './user_profiles_popover';
const userProfiles = [
{
uid: 'u_BOulL4QMPSyV9jg5lQI2JmCkUnokHTazBnet3xVHNv0_0',
enabled: true,
data: {},
user: {
username: 'delighted_nightingale',
Expand All @@ -23,6 +24,7 @@ const userProfiles = [
},
{
uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0',
enabled: true,
data: {},
user: {
username: 'damaged_raccoon',
Expand All @@ -32,6 +34,7 @@ const userProfiles = [
},
{
uid: 'u_A_tM4n0wPkdiQ9smmd8o0Hr_h61XQfu8aRPh9GMoRoc_0',
enabled: true,
data: {},
user: {
username: 'physical_dinosaur',
Expand All @@ -41,6 +44,7 @@ const userProfiles = [
},
{
uid: 'u_9xDEQqUqoYCnFnPPLq5mIRHKL8gBTo_NiKgOnd5gGk0_0',
enabled: true,
data: {},
user: {
username: 'wet_dingo',
Expand Down Expand Up @@ -88,6 +92,7 @@ describe('UserProfilesPopover', () => {
Array [
Object {
"data": Object {},
"enabled": true,
"uid": "u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0",
"user": Object {
"email": "damaged_raccoon@profiles.elastic.co",
Expand All @@ -101,6 +106,7 @@ describe('UserProfilesPopover', () => {
Array [
Object {
"data": Object {},
"enabled": true,
"uid": "u_BOulL4QMPSyV9jg5lQI2JmCkUnokHTazBnet3xVHNv0_0",
"user": Object {
"email": "delighted_nightingale@profiles.elastic.co",
Expand Down
4 changes: 4 additions & 0 deletions packages/kbn-user-profile-components/src/user_profiles_selectable.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { UserProfilesSelectable } from './user_profiles_selectable';
const userProfiles = [
{
uid: 'u_BOulL4QMPSyV9jg5lQI2JmCkUnokHTazBnet3xVHNv0_0',
enabled: true,
data: {},
user: {
username: 'delighted_nightingale',
Expand All @@ -23,6 +24,7 @@ const userProfiles = [
},
{
uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0',
enabled: true,
data: {},
user: {
username: 'damaged_raccoon',
Expand All @@ -32,6 +34,7 @@ const userProfiles = [
},
{
uid: 'u_A_tM4n0wPkdiQ9smmd8o0Hr_h61XQfu8aRPh9GMoRoc_0',
enabled: true,
data: {},
user: {
username: 'physical_dinosaur',
Expand All @@ -41,6 +44,7 @@ const userProfiles = [
},
{
uid: 'u_9xDEQqUqoYCnFnPPLq5mIRHKL8gBTo_NiKgOnd5gGk0_0',
enabled: true,
data: {},
user: {
username: 'wet_dingo',
Expand Down