Skip to content

Commit 77b7e9c

Browse files
committed
model: Make REGISTER_COMPLETE reset some state we forgot about
This addresses the second item in the list at #5605: > - [ ] `REGISTER_COMPLETE` should reset or replace all server data > and server-data metadata, but it isn't in some cases: > - server data: `topics`, `typing` (should reset) > - server-data metadata: `fetching` (should reset) Although, I've left a comment on `fetchingReducer`, where doing so felt a bit funny because morally it shouldn't be necessary. Fixes-partly: #5605
1 parent d88ec6a commit 77b7e9c

File tree

6 files changed

+59
-1
lines changed

6 files changed

+59
-1
lines changed

src/chat/__tests__/fetchingReducer-test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,15 @@ describe('fetchingReducer', () => {
2323
expect(fetchingReducer(prevState, eg.action.reset_account_data)).toEqual(initialState);
2424
});
2525

26+
test('REGISTER_COMPLETE', () => {
27+
const initialState = eg.baseReduxState.fetching;
28+
const action1 = { type: MESSAGE_FETCH_START, narrow: HOME_NARROW, numBefore: 10, numAfter: 10 };
29+
const prevState = fetchingReducer(initialState, action1);
30+
expect(prevState).not.toEqual(initialState);
31+
32+
expect(fetchingReducer(prevState, eg.action.register_complete)).toEqual(initialState);
33+
});
34+
2635
describe('MESSAGE_FETCH_START', () => {
2736
test('if messages are fetched before or after the corresponding flag is set', () => {
2837
const prevState = deepFreeze({ [HOME_NARROW_STR]: { older: false, newer: false } });

src/chat/fetchingReducer.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
MESSAGE_FETCH_ERROR,
66
MESSAGE_FETCH_COMPLETE,
77
RESET_ACCOUNT_DATA,
8+
REGISTER_COMPLETE,
89
} from '../actionConstants';
910
import { NULL_OBJECT } from '../nullObjects';
1011
import { DEFAULT_FETCHING } from './fetchingSelectors';
@@ -69,6 +70,15 @@ export default (
6970
case RESET_ACCOUNT_DATA:
7071
return initialState;
7172

73+
// Reset just because `fetching` is server-data metadata, and we're
74+
// resetting the server data it's supposed to apply to… But really, we
75+
// should have canceled any in-progress message fetches by now; that's
76+
// #5623. Still, even if there is an in-progress fetch, we probably
77+
// don't want to show loading indicators for it in the UI.
78+
// TODO(#5623): Remove reference to #5623.
79+
case REGISTER_COMPLETE:
80+
return initialState;
81+
7282
case MESSAGE_FETCH_START:
7383
return messageFetchStart(state, action);
7484

src/topics/__tests__/topicsReducer-test.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@ import { INIT_TOPICS } from '../../actionConstants';
88
import { NULL_OBJECT } from '../../nullObjects';
99

1010
describe('topicsReducer', () => {
11+
describe('REGISTER_COMPLETE', () => {
12+
test('resets state to initial state', () => {
13+
const prevState = deepFreeze({ [eg.stream.stream_id]: [{ max_id: 1, name: 'some topic' }] });
14+
const action = eg.action.register_complete;
15+
expect(topicsReducer(prevState, action)).toEqual(eg.baseReduxState.topics);
16+
});
17+
});
18+
1119
describe('RESET_ACCOUNT_DATA', () => {
1220
test('resets state to initial state', () => {
1321
const prevState = deepFreeze({ [eg.stream.stream_id]: [{ max_id: 1, name: 'some topic' }] });

src/topics/topicsReducer.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
/* @flow strict-local */
22
import type { TopicsState, PerAccountApplicableAction } from '../types';
3-
import { INIT_TOPICS, EVENT_NEW_MESSAGE, RESET_ACCOUNT_DATA } from '../actionConstants';
3+
import {
4+
INIT_TOPICS,
5+
EVENT_NEW_MESSAGE,
6+
REGISTER_COMPLETE,
7+
RESET_ACCOUNT_DATA,
8+
} from '../actionConstants';
49
import { NULL_OBJECT } from '../nullObjects';
510
import { replaceItemInArray } from '../utils/immutability';
611

@@ -44,6 +49,10 @@ export default (
4449
case RESET_ACCOUNT_DATA:
4550
return initialState;
4651

52+
// Reset to clear stale data; payload has no initial data for this model
53+
case REGISTER_COMPLETE:
54+
return initialState;
55+
4756
case INIT_TOPICS:
4857
return {
4958
...state,

src/typing/__tests__/typingReducer-test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,23 @@ describe('typingReducer', () => {
4646
expect(typingReducer(prevState, eg.action.reset_account_data)).toEqual(initialState);
4747
});
4848

49+
describe('REGISTER_COMPLETE', () => {
50+
test('resets state to initial state', () => {
51+
const initialState = eg.baseReduxState.typing;
52+
const action1 = egTypingAction({
53+
op: 'start',
54+
sender: user1,
55+
recipients: [user1, eg.selfUser],
56+
time: 123456889,
57+
});
58+
const prevState = typingReducer(initialState, action1);
59+
expect(prevState).not.toEqual(initialState);
60+
61+
const action2 = eg.action.register_complete;
62+
expect(typingReducer(prevState, action2)).toEqual(eg.baseReduxState.typing);
63+
});
64+
});
65+
4966
describe('EVENT_TYPING_START', () => {
5067
test('adds sender as currently typing user', () => {
5168
const prevState = NULL_OBJECT;

src/typing/typingReducer.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
CLEAR_TYPING,
77
DEAD_QUEUE,
88
RESET_ACCOUNT_DATA,
9+
REGISTER_COMPLETE,
910
} from '../actionConstants';
1011
import { pmTypingKeyFromRecipients } from '../utils/recipient';
1112
import { NULL_OBJECT } from '../nullObjects';
@@ -99,6 +100,10 @@ export default (
99100
case RESET_ACCOUNT_DATA:
100101
return initialState;
101102

103+
// Reset to clear stale data; payload has no initial data for this model
104+
case REGISTER_COMPLETE:
105+
return initialState;
106+
102107
default:
103108
return state;
104109
}

0 commit comments

Comments
 (0)