Skip to content

Commit e866a79

Browse files
chrisbobbechrisbobbe
committed
user groups tests: Start type-checking; use example data
1 parent 22dab8f commit e866a79

File tree

3 files changed

+71
-123
lines changed

3 files changed

+71
-123
lines changed

‎src/api/initialDataTypes.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,9 @@ export type InitialDataRealmUser = $ReadOnly<{|
415415

416416
export type InitialDataRealmUserGroups = $ReadOnly<{|
417417
// New in Zulip 1.8.
418+
// TODO(#5102): In userGroupsReducer, we still have a fallback for pre-1.8
419+
// servers; remove that, and remove the above comment, which will be
420+
// irrelevant.
418421
realm_user_groups: $ReadOnlyArray<UserGroup>,
419422
|}>;
420423

‎src/user-groups/__tests__/userGroupsReducer-test.js

Lines changed: 67 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
/* @flow strict-local */
2+
13
import deepFreeze from 'deep-freeze';
24

5+
import * as eg from '../../__tests__/lib/exampleData';
36
import {
4-
REGISTER_COMPLETE,
5-
ACCOUNT_SWITCH,
67
EVENT_USER_GROUP_ADD,
78
EVENT_USER_GROUP_REMOVE,
89
EVENT_USER_GROUP_UPDATE,
@@ -14,36 +15,29 @@ import userGroupsReducer from '../userGroupsReducer';
1415
describe('userGroupsReducer', () => {
1516
describe('REGISTER_COMPLETE', () => {
1617
test('when data is provided init state with it', () => {
17-
const prevState = deepFreeze([]);
18-
const action = deepFreeze({
19-
type: REGISTER_COMPLETE,
20-
data: {
21-
realm_user_groups: [
22-
{
23-
id: 1,
24-
name: 'Some user group',
25-
members: [],
26-
},
27-
],
28-
},
29-
});
18+
const group = eg.makeUserGroup();
19+
20+
const prevState = deepFreeze(eg.plusReduxState.userGroups);
21+
22+
const action = eg.mkActionRegisterComplete({ realm_user_groups: [group] });
3023

3124
const actualState = userGroupsReducer(prevState, action);
3225

33-
expect(actualState).toEqual([
34-
{
35-
id: 1,
36-
name: 'Some user group',
37-
members: [],
38-
},
39-
]);
26+
expect(actualState).toEqual([group]);
4027
});
4128

29+
// TODO(#5102): Remove this test case, which is for pre-1.8 servers.
4230
test('when no data is given reset state', () => {
43-
const prevState = deepFreeze([['stream'], ['topic']]);
44-
const action = deepFreeze({
45-
type: REGISTER_COMPLETE,
46-
data: {},
31+
const prevState = deepFreeze([eg.makeUserGroup()]);
32+
const action = eg.mkActionRegisterComplete({
33+
// Hmm, we should need a Flow suppression here. This property is
34+
// marked required in InitialData, and this explicit undefined is
35+
// meant to defy that; see TODO(#5102) above.
36+
// mkActionRegisterComplete is designed to accept input with this or
37+
// any property *omitted*… and I think, as a side effect of handling
38+
// that, Flow mistakenly accepts an explicit undefined here, so it
39+
// doesn't catch the resulting malformed InitialData.
40+
realm_user_groups: undefined,
4741
});
4842
const expectedState = [];
4943

@@ -55,18 +49,9 @@ describe('userGroupsReducer', () => {
5549

5650
describe('ACCOUNT_SWITCH', () => {
5751
test('resets state to initial state', () => {
58-
const prevState = deepFreeze([
59-
{
60-
id: 1,
61-
name: 'Some Group',
62-
description: 'This is some group',
63-
members: [],
64-
},
65-
]);
52+
const prevState = deepFreeze([eg.makeUserGroup()]);
6653

67-
const action = deepFreeze({
68-
type: ACCOUNT_SWITCH,
69-
});
54+
const action = eg.action.account_switch;
7055

7156
const expectedState = [];
7257

@@ -79,13 +64,9 @@ describe('userGroupsReducer', () => {
7964
describe('EVENT_USER_GROUP_ADD', () => {
8065
test('adds a user group to the state', () => {
8166
const prevState = deepFreeze([]);
82-
const group = {
83-
id: 1,
84-
name: 'Some Group',
85-
description: 'This is some group',
86-
members: [123],
87-
};
67+
const group = eg.makeUserGroup();
8868
const action = deepFreeze({
69+
id: 1,
8970
type: EVENT_USER_GROUP_ADD,
9071
op: 'add',
9172
group,
@@ -103,6 +84,7 @@ describe('userGroupsReducer', () => {
10384
test('if user group does not exist state does not change', () => {
10485
const prevState = deepFreeze([]);
10586
const action = deepFreeze({
87+
id: 1,
10688
type: EVENT_USER_GROUP_REMOVE,
10789
op: 'remove',
10890
group_id: 1,
@@ -115,28 +97,18 @@ describe('userGroupsReducer', () => {
11597
});
11698

11799
test('adds a user group to the state', () => {
118-
const prevState = deepFreeze([
119-
{
120-
id: 1,
121-
name: 'Some group',
122-
},
123-
{
124-
id: 2,
125-
name: 'Another group',
126-
},
127-
]);
100+
const group1 = eg.makeUserGroup();
101+
const group2 = eg.makeUserGroup();
102+
103+
const prevState = deepFreeze([group1, group2]);
128104
const action = deepFreeze({
105+
id: 1,
129106
type: EVENT_USER_GROUP_REMOVE,
130107
op: 'remove',
131-
group_id: 1,
108+
group_id: group1.id,
132109
});
133110

134-
const expectedState = [
135-
{
136-
id: 2,
137-
name: 'Another group',
138-
},
139-
];
111+
const expectedState = [group2];
140112

141113
const actualState = userGroupsReducer(prevState, action);
142114

@@ -148,6 +120,7 @@ describe('userGroupsReducer', () => {
148120
test('if user group does not exist state does not change', () => {
149121
const prevState = deepFreeze([]);
150122
const action = deepFreeze({
123+
id: 1,
151124
type: EVENT_USER_GROUP_UPDATE,
152125
op: 'update',
153126
group_id: 1,
@@ -161,32 +134,18 @@ describe('userGroupsReducer', () => {
161134
});
162135

163136
test('updates an existing user group with supplied new values', () => {
164-
const prevState = deepFreeze([
165-
{
166-
id: 1,
167-
name: 'Some group',
168-
},
169-
{
170-
id: 2,
171-
name: 'Another group',
172-
},
173-
]);
137+
const group1 = eg.makeUserGroup();
138+
const group2 = eg.makeUserGroup();
139+
140+
const prevState = deepFreeze([group1, group2]);
174141
const action = deepFreeze({
142+
id: 1,
175143
type: EVENT_USER_GROUP_UPDATE,
176144
op: 'update',
177-
group_id: 2,
145+
group_id: group2.id,
178146
data: { name: 'New name' },
179147
});
180-
const expectedState = [
181-
{
182-
id: 1,
183-
name: 'Some group',
184-
},
185-
{
186-
id: 2,
187-
name: 'New name',
188-
},
189-
];
148+
const expectedState = [group1, { ...group2, name: 'New name' }];
190149

191150
const actualState = userGroupsReducer(prevState, action);
192151

@@ -198,10 +157,11 @@ describe('userGroupsReducer', () => {
198157
test('if user group does not exist state does not change', () => {
199158
const prevState = deepFreeze([]);
200159
const action = deepFreeze({
160+
id: 1,
201161
type: EVENT_USER_GROUP_ADD_MEMBERS,
202162
op: 'add_members',
203163
group_id: 1,
204-
user_ids: [1, 2, 3],
164+
user_ids: [eg.makeUser().user_id, eg.makeUser().user_id, eg.makeUser().user_id],
205165
});
206166
const expectedState = [];
207167

@@ -211,25 +171,17 @@ describe('userGroupsReducer', () => {
211171
});
212172

213173
test('updates an existing user group with supplied new members', () => {
214-
const prevState = deepFreeze([
215-
{
216-
id: 1,
217-
name: 'Some group',
218-
members: [1],
219-
},
220-
]);
174+
const group = eg.makeUserGroup({ members: [eg.selfUser.user_id] });
175+
const prevState = deepFreeze([group]);
221176
const action = deepFreeze({
177+
id: 1,
222178
type: EVENT_USER_GROUP_ADD_MEMBERS,
223179
op: 'add_members',
224-
group_id: 1,
225-
user_ids: [2, 3],
180+
group_id: group.id,
181+
user_ids: [eg.otherUser.user_id, eg.thirdUser.user_id],
226182
});
227183
const expectedState = [
228-
{
229-
id: 1,
230-
name: 'Some group',
231-
members: [1, 2, 3],
232-
},
184+
{ ...group, members: [eg.selfUser.user_id, eg.otherUser.user_id, eg.thirdUser.user_id] },
233185
];
234186

235187
const actualState = userGroupsReducer(prevState, action);
@@ -242,10 +194,11 @@ describe('userGroupsReducer', () => {
242194
test('if user group does not exist state does not change', () => {
243195
const prevState = deepFreeze([]);
244196
const action = deepFreeze({
197+
id: 1,
245198
type: EVENT_USER_GROUP_REMOVE_MEMBERS,
246199
op: 'remove_members',
247200
group_id: 1,
248-
user_ids: [1],
201+
user_ids: [eg.makeUser().user_id],
249202
});
250203
const expectedState = [];
251204

@@ -255,36 +208,27 @@ describe('userGroupsReducer', () => {
255208
});
256209

257210
test('removes members from an existing user group', () => {
258-
const prevState = deepFreeze([
259-
{
260-
id: 1,
261-
name: 'Some group',
262-
members: [1, 2, 3, 4],
263-
},
264-
{
265-
id: 2,
266-
name: 'Another group',
267-
members: [1, 2, 3, 4],
268-
},
269-
]);
211+
const user1 = eg.makeUser();
212+
const user2 = eg.makeUser();
213+
const user3 = eg.makeUser();
214+
const user4 = eg.makeUser();
215+
216+
const group1 = eg.makeUserGroup({
217+
members: [user1.user_id, user2.user_id, user3.user_id, user4.user_id],
218+
});
219+
const group2 = eg.makeUserGroup({
220+
members: [user1.user_id, user2.user_id, user3.user_id, user4.user_id],
221+
});
222+
223+
const prevState = deepFreeze([group1, group2]);
270224
const action = deepFreeze({
225+
id: 1,
271226
type: EVENT_USER_GROUP_REMOVE_MEMBERS,
272227
op: 'remove_members',
273-
group_id: 1,
274-
user_ids: [2, 3],
228+
group_id: group1.id,
229+
user_ids: [user2.user_id, user3.user_id],
275230
});
276-
const expectedState = [
277-
{
278-
id: 1,
279-
name: 'Some group',
280-
members: [1, 4],
281-
},
282-
{
283-
id: 2,
284-
name: 'Another group',
285-
members: [1, 2, 3, 4],
286-
},
287-
];
231+
const expectedState = [{ ...group1, members: [user1.user_id, user4.user_id] }, group2];
288232

289233
const actualState = userGroupsReducer(prevState, action);
290234

‎src/user-groups/userGroupsReducer.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ export default (
5656
return initialState;
5757

5858
case REGISTER_COMPLETE:
59+
// TODO(#5102): Remove fallback for pre-1.8 servers
5960
return action.data.realm_user_groups || initialState;
6061

6162
case EVENT_USER_GROUP_ADD:

0 commit comments

Comments
 (0)