diff --git a/src/users/__tests__/usersReducer-test.js b/src/users/__tests__/usersReducer-test.js index 3c04721b531..6c6e53ab580 100644 --- a/src/users/__tests__/usersReducer-test.js +++ b/src/users/__tests__/usersReducer-test.js @@ -5,6 +5,7 @@ import * as eg from '../../__tests__/lib/exampleData'; import { UploadedAvatarURL } from '../../utils/avatar'; import { EVENT_USER_ADD, ACCOUNT_SWITCH, EVENT } from '../../actionConstants'; import { EventTypes, type RealmUserUpdateEvent } from '../../api/eventTypes'; +import type { User } from '../../types'; import { RoleValues } from '../../api/permissionsTypes'; import usersReducer from '../usersReducer'; import { randString } from '../../utils/misc'; @@ -54,12 +55,22 @@ describe('usersReducer', () => { * Check that an update event with supplied `person` works. * * May omit `user_id` to avoid repetition. + * + * Normally, the expected User value after the change is + * + * { ...theUser, ...person } + * + * But in at least one case (when updating custom profile fields), that + * isn't even a valid User object because `person`'s properties aren't + * a subset of User's. To handle that, callers can pass an + * `expectedUser`. */ // Tell ESLint to recognize `check` as a helper function that runs // assertions. /* eslint jest/expect-expect: ["error", { "assertFunctionNames": ["expect", "check"] }] */ const check = >( personMaybeWithoutId: $Rest, + expectedUser?: User, ) => { const person: P = { user_id: theUser.user_id, ...personMaybeWithoutId }; @@ -68,7 +79,7 @@ describe('usersReducer', () => { event: { id: 1, type: EventTypes.realm_user, op: 'update', person }, }); - expect(usersReducer(prevState, action)).toEqual([{ ...theUser, ...person }]); + expect(usersReducer(prevState, action)).toEqual([expectedUser ?? { ...theUser, ...person }]); }; /* @@ -79,7 +90,7 @@ describe('usersReducer', () => { * A few properties that we don't handle are commented out. */ - test.skip('When a user changes their full name.', () => { + test('When a user changes their full name.', () => { check({ full_name: randString() }); }); @@ -95,34 +106,38 @@ describe('usersReducer', () => { }); }); - test.skip('When a user changes their timezone setting.', () => { + test('When a user changes their timezone setting.', () => { check({ timezone: randString() }); }); - test.skip('When the owner of a bot changes.', () => { - check({ user_id: theUser.user_id, bot_owner_id: (theUser.bot_owner_id ?? 1) + 1 }); + test('When the owner of a bot changes.', () => { + check({ bot_owner_id: (theUser.bot_owner_id ?? 1) + 1 }); }); test('When the role of a user changes.', () => { check({ role: RoleValues[(RoleValues.indexOf(theUser.role) + 1) % RoleValues.length] }); }); - test.skip("When a user's billing-admin status changes", () => { - check({ user_id: theUser.user_id, is_billing_admin: !theUser.is_billing_admin }); + test("When a user's billing-admin status changes", () => { + check({ is_billing_admin: !theUser.is_billing_admin }); }); - test.skip('When the delivery email of a user changes.', () => { + test('When the delivery email of a user changes.', () => { check({ delivery_email: randString() }); }); - test.skip('When the user updates one of their custom profile fields.', () => { - check({ - custom_profile_field: { id: 4, value: randString(), rendered_value: randString() }, - }); + test('When the user updates one of their custom profile fields.', () => { + const value = randString(); + const rendered_value = randString(); + check( + { custom_profile_field: { id: 4, value, rendered_value } }, + { ...theUser, profile_data: { '4': { value, rendered_value } } }, + ); }); - test.skip('When the Zulip display email address of a user changes', () => { - check({ user_id: theUser.user_id, new_email: randString() }); + test('When the Zulip display email address of a user changes', () => { + const new_email = randString(); + check({ user_id: theUser.user_id, new_email }, { ...theUser, email: new_email }); }); }); diff --git a/src/users/usersReducer.js b/src/users/usersReducer.js index 4312c57c15d..9da8a9dd55c 100644 --- a/src/users/usersReducer.js +++ b/src/users/usersReducer.js @@ -1,5 +1,5 @@ /* @flow strict-local */ -import type { UsersState, PerAccountApplicableAction } from '../types'; +import type { User, UsersState, PerAccountApplicableAction } from '../types'; import { LOGOUT, LOGIN_SUCCESS, @@ -40,13 +40,25 @@ export default ( switch (event.op) { case 'update': { return state.map(user => { - if (user.user_id !== event.person.user_id) { + const { person } = event; + if (user.user_id !== person.user_id) { return user; } - if (event.person.avatar_url || event.person.role) { - return { ...user, ...event.person }; + if (person.custom_profile_field) { + return { + ...user, + profile_data: { + ...(user.profile_data: $PropertyType), + [person.custom_profile_field.id]: { + value: person.custom_profile_field.value, + rendered_value: person.custom_profile_field.rendered_value, + }, + }, + }; + } else if (person.new_email !== undefined) { + return { ...user, email: person.new_email }; } else { - return user; + return { ...user, ...person }; } }); }