Skip to content

Commit

Permalink
users: On update-user with is_active, usersrealm.nonActiveUsers
Browse files Browse the repository at this point in the history
The `is_active` property of realm_user/update events is new in Zulip
Server 8.0 (FL 222):
  https://zulip.com/api/get-events#realm_user-update

Start handling it by moving the user between `state.users` (where we
keep active users) and `state.realm.nonActiveUsers`.

For servers 10.0+ (starting at FL 303), the client has more to do to
handle user deactivation; that's zulip#5899 "Remove deactivated users
from groups". Best to do this more basic thing first.
  • Loading branch information
chrisbobbe committed Nov 12, 2024
1 parent 715d60a commit 5107a89
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 4 deletions.
15 changes: 13 additions & 2 deletions src/api/eventTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type {
UserSettings,
ClientPresence,
UserTopic,
User,
} from './modelTypes';
import type { RealmDataForUpdate } from './realmDataTypes';

Expand Down Expand Up @@ -422,7 +423,11 @@ export type RealmUserUpdateEventRaw = {|
// the collections of users and bots in the initial data. Ignore.
// avatar_source: string,
// avatar_url_medium: string,
|},
|}
// TODO(flow) should be just one branch with `boolean` instead of
// separate `true` and `false`; investigate errors
| {| +user_id: UserOrBot['user_id'], +is_active: true |}
| {| +user_id: UserOrBot['user_id'], +is_active: false |},
|};

/** A realm_user update event, after we've processed it at the edge. */
Expand All @@ -449,7 +454,13 @@ export type RealmUserUpdateEvent = {|
// the collections of users and bots in the initial data. Ignore.
// avatar_source: string,
// avatar_url_medium: string,
|},
|}
| {|
// Digested from `is_active: true`. We add the full user details
// from our data structures; the event only carries the user ID.
+userToActivate: User,
|}
| {| +userToDeactivate: User |}, // from `is_active: false`
|};

export type UserTopicEvent = {|
Expand Down
26 changes: 26 additions & 0 deletions src/events/eventToAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,32 @@ export default (state: PerAccountState, event: $FlowFixMe): EventAction | null =
},
},
};
} else if (person.is_active === true) {
const userToActivate = state.realm.nonActiveUsers.find(
u => u.user_id === person.user_id,
);
if (userToActivate == null) {
logging.warn(
'Got realm_user/update event to reactivate a user not found in state.realm.nonActiveUsers',
);
return null;
}
return {
type: EVENT,
event: { ...rawEvent, person: { userToActivate } },
};
} else if (person.is_active === false) {
const userToDeactivate = state.users.find(u => u.user_id === person.user_id);
if (userToDeactivate == null) {
logging.warn(
'Got realm_user/update event to deactivate a user not found in state.users',
);
return null;
}
return {
type: EVENT,
event: { ...rawEvent, person: { userToDeactivate } },
};
} else {
return {
type: EVENT,
Expand Down
26 changes: 26 additions & 0 deletions src/realm/__tests__/realmReducer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -576,5 +576,31 @@ describe('realmReducer', () => {
check(false, false);
});
});

describe('type `realm_user`, op `update`', () => {
test('User is deactivated', () => {
const user = eg.makeUser();
const initialState = { ...eg.plusReduxState.realm, nonActiveUsers: [] };
const person = { userToDeactivate: user };
const action = deepFreeze({
type: EVENT,
event: { id: 1, type: EventTypes.realm_user, op: 'update', person },
});
const actualState = realmReducer(initialState, action);
expect(actualState.nonActiveUsers).toEqual([user]);
});

test('User is reactivated', () => {
const user = eg.makeUser();
const initialState = { ...eg.plusReduxState.realm, nonActiveUsers: [user] };
const person = { userToActivate: user };
const action = deepFreeze({
type: EVENT,
event: { id: 1, type: EventTypes.realm_user, op: 'update', person },
});
const actualState = realmReducer(initialState, action);
expect(actualState.nonActiveUsers).toEqual([]);
});
});
});
});
14 changes: 14 additions & 0 deletions src/realm/realmReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,20 @@ export default (
// `op: 'update_dict'` events near the edge.)
return state;

case EventTypes.realm_user: {
const { person } = event;

if (person.userToDeactivate != null) {
return { ...state, nonActiveUsers: [...state.nonActiveUsers, person.userToDeactivate] };
} else if (person.userToActivate != null) {
const userId = person.userToActivate.user_id;
const nonActiveUsers = state.nonActiveUsers.filter(u => u.user_id !== userId);
return { ...state, nonActiveUsers };
}

return state;
}

case EventTypes.user_settings:
if (event.op === 'update') {
const { property, value } = event;
Expand Down
18 changes: 18 additions & 0 deletions src/users/__tests__/usersReducer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,24 @@ describe('usersReducer', () => {
const new_email = randString();
check({ new_email }, { ...theUser, email: new_email });
});

test('When a user is deactivated', () => {
const person = { userToDeactivate: theUser };
const action = deepFreeze({
type: EVENT,
event: { id: 1, type: EventTypes.realm_user, op: 'update', person },
});
expect(usersReducer([theUser], action)).toEqual([]);
});

test('When a user is activated', () => {
const person = { userToActivate: theUser };
const action = deepFreeze({
type: EVENT,
event: { id: 1, type: EventTypes.realm_user, op: 'update', person },
});
expect(usersReducer([], action)).toEqual([theUser]);
});
});

describe('RESET_ACCOUNT_DATA', () => {
Expand Down
16 changes: 14 additions & 2 deletions src/users/usersReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,14 @@ export default (
case EventTypes.realm_user: {
switch (event.op) {
case 'update': {
const { person } = event;
if (person.userToActivate != null) {
return [...state, person.userToActivate];
} else if (person.userToDeactivate != null) {
const userId = person.userToDeactivate.user_id;
return state.filter(u => u.user_id !== userId);
}
return state.map(user => {
const { person } = event;
if (user.user_id !== person.user_id) {
return user;
}
Expand Down Expand Up @@ -70,7 +76,13 @@ export default (
} else if (person.new_email !== undefined) {
return { ...user, email: person.new_email };
} else {
return { ...user, ...person };
// eslint-disable-next-line no-unused-vars
const { userToActivate, userToDeactivate, ...rest } = person;

// TODO(flow) Use `...person`, not `...rest`; Flow should already know
// that userToActivate and userToDeactivate are absent;
// see early-returns before the loop-through-users.
return { ...user, ...rest };
}
});
}
Expand Down

0 comments on commit 5107a89

Please sign in to comment.