Skip to content

Commit

Permalink
[Cases] Fix bug where the Cases users API will thrown an error when `…
Browse files Browse the repository at this point in the history
…imageUrl` is set to `null` (#158815)

## Summary

As part of the Serverless work in
#153726, we validate all of our
responses returned by the Cases APIs. The GET case users internal API
validates the schema of the `avatar` persisted in the `data` of the user
profile. It may be possible to set the `imageUrl` to `null` from within
the user profile page. This PR fixes this bug. It also updates the type
of the `imageUrl` in `UserProfileAvatarData`.

## Testing

1. Go to your user profile and press save
2. Go to a case
3. You should see an error toaster. The fix in this RP should eliminate
the error toaster and the Case view page should be shown properly
4. Go to your user profile and change the initials the color and set a
profile image. Go again to a case and verify that the avatar is shown as
expected


### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
cnasikas authored Jun 2, 2023
1 parent cec0a15 commit 7205072
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-user-profile-components/src/user_profile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export interface UserProfileAvatarData {
/**
* Base64 data URL for the user avatar image.
*/
imageUrl?: string;
imageUrl?: string | null;
}

export const USER_AVATAR_FALLBACK_CODE_POINT = 97; // code point for lowercase "a"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export interface UserProfileAvatarData {
/**
* Base64 data URL for the user avatar image.
*/
imageUrl?: string;
imageUrl?: string | null;
}

export const USER_AVATAR_FALLBACK_CODE_POINT = 97; // code point for lowercase "a"
Expand Down
11 changes: 11 additions & 0 deletions x-pack/plugins/cases/common/api/user.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { set } from 'lodash';
import { UserRt, UserWithProfileInfoRt, UsersRt, GetCaseUsersResponseRt } from './user';

describe('User', () => {
Expand Down Expand Up @@ -61,6 +62,16 @@ describe('User', () => {
});
});

it.each(['initials', 'color', 'imageUrl'])('does not returns an error if %s is null', (key) => {
const reqWithNullImage = set(defaultRequest, `avatar.${key}`, null);
const query = UserWithProfileInfoRt.decode(reqWithNullImage);

expect(query).toStrictEqual({
_tag: 'Right',
right: reqWithNullImage,
});
});

it('removes foo:bar attributes from request', () => {
const query = UserWithProfileInfoRt.decode({
...defaultRequest,
Expand Down
8 changes: 7 additions & 1 deletion x-pack/plugins/cases/common/api/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ export const UserWithProfileInfoRt = rt.intersection([
rt.exact(rt.partial({ uid: rt.string })),
rt.exact(
rt.partial({
avatar: rt.exact(rt.partial({ initials: rt.string, color: rt.string, imageUrl: rt.string })),
avatar: rt.exact(
rt.partial({
initials: rt.union([rt.string, rt.null]),
color: rt.union([rt.string, rt.null]),
imageUrl: rt.union([rt.string, rt.null]),
})
),
})
),
]);
Expand Down
21 changes: 19 additions & 2 deletions x-pack/plugins/cases/server/client/user_actions/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
* 2.0.
*/

import type { UserProfileWithAvatar } from '@kbn/user-profile-components';
import { isString } from 'lodash';
import type { UserProfileAvatarData, UserProfileWithAvatar } from '@kbn/user-profile-components';
import type { GetCaseUsersResponse, User, UserWithProfileInfo } from '../../../common/api';
import { decodeOrThrow, GetCaseUsersResponseRt } from '../../../common/api';
import type { OwnerEntity } from '../../authorization';
Expand Down Expand Up @@ -137,11 +138,27 @@ const getUserInformation = (
full_name: userProfile?.user.full_name ?? userInfo?.full_name ?? null,
username: userProfile?.user.username ?? userInfo?.username ?? null,
},
avatar: userProfile?.data.avatar,
avatar: getUserProfileAvatar(userProfile?.data.avatar),
uid: userProfile?.uid ?? uid ?? userInfo?.profile_uid,
};
};

const getUserProfileAvatar = (
avatar?: UserProfileAvatarData | undefined
): UserWithProfileInfo['avatar'] | undefined => {
if (!avatar) {
return avatar;
}

const res = {
...(isString(avatar.initials) ? { initials: avatar.initials } : {}),
...(isString(avatar.color) ? { color: avatar.color } : {}),
...(isString(avatar.imageUrl) ? { imageUrl: avatar.imageUrl } : {}),
};

return res;
};

const removeAllFromSet = (originalSet: Set<string>, values: string[]) => {
const newSet = new Set(originalSet);
values.forEach((value) => newSet.delete(value));
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security/common/model/user_profile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export interface UserProfileAvatarData {
/**
* Base64 data URL for the user avatar image.
*/
imageUrl?: string;
imageUrl?: string | null;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,67 @@ export default ({ getService }: FtrProviderContext): void => {
expect(assignees).to.eql([]);
expect(unassignedUsers).to.eql([]);
});

it('does not throw with imageUrl set to null', async () => {
await updateUserProfileAvatar({
supertest,
req: {
// @ts-expect-error: types are not correct
initials: null,
// @ts-expect-error: types are not correct
color: null,
imageUrl: null,
},
headers: superUserHeaders,
});

const postedCase = await createCase(
supertestWithoutAuth,
getPostCaseRequest(),
200,
null,
superUserHeaders
);

const res = await getCaseUsers({
caseId: postedCase.id,
supertest,
});

expect(res.participants[0].avatar).to.eql({});
expect(res.reporter.avatar).to.eql({});
});

it('does not return any avatar data if they are not a string', async () => {
await updateUserProfileAvatar({
supertest,
req: {
// @ts-expect-error: types are not correct
initials: 4,
// @ts-expect-error: types are not correct
color: true,
// @ts-expect-error: types are not correct
imageUrl: [],
},
headers: superUserHeaders,
});

const postedCase = await createCase(
supertestWithoutAuth,
getPostCaseRequest(),
200,
null,
superUserHeaders
);

const res = await getCaseUsers({
caseId: postedCase.id,
supertest,
});

expect(res.participants[0].avatar).to.eql({});
expect(res.reporter.avatar).to.eql({});
});
});
});

Expand Down

0 comments on commit 7205072

Please sign in to comment.