Skip to content

Commit

Permalink
usersReducers: Handle 'EVENT_USER_UPDATE' server events
Browse files Browse the repository at this point in the history
Fixes zulip#3397 Part of zulip#3408

Handles the three types of user data updates possible:
* full name
* avatar
* custom profile field

Extra care is taken not to mutate the state if not necessary.

This is done for two reasons:
 * currently editing values throught he web app causes two events,
   one of which is not necessary (maybe should be fixed)
 * this is likely the largest state, doing an extra `isEqual` can
   save copying all this data
  • Loading branch information
borisyankov committed Mar 16, 2019
1 parent a277dab commit f049142
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 4 deletions.
75 changes: 74 additions & 1 deletion src/users/__tests__/usersReducers-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import deepFreeze from 'deep-freeze';

import { REALM_INIT, EVENT_USER_ADD, ACCOUNT_SWITCH } from '../../actionConstants';
import {
REALM_INIT,
EVENT_USER_ADD,
ACCOUNT_SWITCH,
EVENT_USER_UPDATE,
} from '../../actionConstants';
import usersReducers from '../usersReducers';

describe('usersReducers', () => {
Expand Down Expand Up @@ -96,6 +101,74 @@ describe('usersReducers', () => {
});
});

describe('EVENT_USER_UPDATE', () => {
test('updating non existing user does not mutate state', () => {
const initialState = deepFreeze([
{
user_id: 1,
full_name: 'Some Guy',
},
]);
const action = deepFreeze({
type: EVENT_USER_UPDATE,
person: {
user_id: 2,
full_name: 'New Name',
},
});

const actualState = usersReducers(initialState, action);

expect(actualState).toBe(initialState);
});

test('updating an existing user mutates state', () => {
const initialState = deepFreeze([
{
user_id: 1,
full_name: 'Some Guy',
},
]);
const action = deepFreeze({
type: EVENT_USER_UPDATE,
person: {
user_id: 1,
full_name: 'New Name',
},
});
const expectedState = [
{
user_id: 1,
full_name: 'New Name',
},
];

const actualState = usersReducers(initialState, action);

expect(actualState).toEqual(expectedState);
});

test('updating existing user with the same values does not mutate state', () => {
const initialState = deepFreeze([
{
user_id: 1,
full_name: 'Some Guy',
},
]);
const action = deepFreeze({
type: EVENT_USER_UPDATE,
person: {
user_id: 1,
full_name: 'Some Guy',
},
});

const actualState = usersReducers(initialState, action);

expect(actualState).toBe(initialState);
});
});

describe('ACCOUNT_SWITCH', () => {
test('resets state to initial state', () => {
const initialState = deepFreeze([
Expand Down
23 changes: 20 additions & 3 deletions src/users/usersReducers.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* @flow strict-local */
import type { UsersState, Action } from '../types';
import isEqual from 'lodash.isequal';

import type { User, UsersState, Action } from '../types';
import {
LOGOUT,
LOGIN_SUCCESS,
Expand All @@ -10,6 +12,7 @@ import {
EVENT_USER_UPDATE,
} from '../actionConstants';
import { NULL_ARRAY } from '../nullObjects';
import { updateUser } from './userHelpers';

const initialState: UsersState = NULL_ARRAY;

Expand All @@ -29,8 +32,22 @@ export default (state: UsersState = initialState, action: Action): UsersState =>
case EVENT_USER_REMOVE:
return state; // TODO

case EVENT_USER_UPDATE:
return state; // TODO
case EVENT_USER_UPDATE: {
const userIndex = state.findIndex(x => x.user_id === action.person.user_id);
const oldUser: User = state[userIndex];

if (userIndex === -1) {
return state;
}

const updatedUser = updateUser(oldUser, action.person);

if (isEqual(updatedUser, oldUser)) {
return state;
}

return [...state.slice(0, userIndex), updatedUser, ...state.slice(userIndex + 1)];
}

default:
return state;
Expand Down

0 comments on commit f049142

Please sign in to comment.