Skip to content

Commit a595be8

Browse files
chrisbobbegnprice
authored andcommitted
model [nfc]: Comment on rest of REGISTER_COMPLETE handlers that reset state
This addresses the following item in the list at #5605: > - [ ] Some reducers don't have a comment saying why they reset > state on `REGISTER_COMPLETE` instead of taking data from its > payload: > - For example, `flagsReducer` should point out that > `messagesReducer` doesn't store messages on > `REGISTER_COMPLETE`, and `messagesReducer` should say that > it doesn't do that because we fetch messages with > `GET /messages`. > - (Probably plenty of other instances.) Fixes-partly: #5605
1 parent 1408d4d commit a595be8

File tree

6 files changed

+32
-8
lines changed

6 files changed

+32
-8
lines changed

src/caughtup/caughtUpReducer.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,14 @@ export default (
3232
action: PerAccountApplicableAction,
3333
): CaughtUpState => {
3434
switch (action.type) {
35-
case REGISTER_COMPLETE:
3635
case RESET_ACCOUNT_DATA:
3736
return initialState;
3837

38+
// Reset because `caughtUp` is server-data metadata, and we're resetting
39+
// the server data it's supposed to apply to: `state.narrows`.
40+
case REGISTER_COMPLETE:
41+
return initialState;
42+
3943
case MESSAGE_FETCH_START: {
4044
// We don't want to accumulate old searches that we'll never
4145
// need again.

src/chat/fetchingReducer.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,11 @@ export default (
7171
return initialState;
7272

7373
// 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.
74+
// resetting the server data it's supposed to apply to
75+
// (`state.narrows`)… But really, we should have canceled any
76+
// in-progress message fetches by now; that's #5623. Still, even if
77+
// there is an in-progress fetch, we probably don't want to show loading
78+
// indicators for it in the UI.
7879
// TODO(#5623): Remove reference to #5623.
7980
case REGISTER_COMPLETE:
8081
return initialState;

src/chat/flagsReducer.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,16 @@ export default (
131131
action: PerAccountApplicableAction,
132132
): FlagsState => {
133133
switch (action.type) {
134-
case REGISTER_COMPLETE:
135134
case RESET_ACCOUNT_DATA:
136135
return initialState;
137136

137+
// Reset to clear stale data. We don't initialize the
138+
// messages/narrows/flags model using initial data; instead, we fetch
139+
// chunks of data as needed with api.getMessages. See
140+
// https://zulip.readthedocs.io/en/latest/subsystems/events-system.html#messages
141+
case REGISTER_COMPLETE:
142+
return initialState;
143+
138144
case MESSAGE_FETCH_COMPLETE:
139145
return processFlagsForMessages(state, action.messages);
140146

src/chat/narrowsReducer.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,10 +194,16 @@ export default (
194194
action: PerAccountApplicableAction,
195195
): NarrowsState => {
196196
switch (action.type) {
197-
case REGISTER_COMPLETE:
198197
case RESET_ACCOUNT_DATA:
199198
return initialState;
200199

200+
// Reset to clear stale data. We don't initialize the
201+
// messages/narrows/flags model using initial data; instead, we fetch
202+
// chunks of data as needed with api.getMessages. See
203+
// https://zulip.readthedocs.io/en/latest/subsystems/events-system.html#messages
204+
case REGISTER_COMPLETE:
205+
return initialState;
206+
201207
case MESSAGE_FETCH_START: {
202208
// We don't want to accumulate old searches that we'll never need again.
203209
if (isSearchNarrow(action.narrow)) {

src/message/messagesReducer.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,16 @@ export default (
139139
globalState: PerAccountState,
140140
): MessagesState => {
141141
switch (action.type) {
142-
case REGISTER_COMPLETE:
143142
case RESET_ACCOUNT_DATA:
144143
return initialState;
145144

145+
// Reset to clear stale data. We don't initialize the
146+
// messages/narrows/flags model using initial data; instead, we fetch
147+
// chunks of data as needed with api.getMessages. See
148+
// https://zulip.readthedocs.io/en/latest/subsystems/events-system.html#messages
149+
case REGISTER_COMPLETE:
150+
return initialState;
151+
146152
case MESSAGE_FETCH_COMPLETE:
147153
return state.merge(
148154
Immutable.Map(

src/outbox/outboxReducer.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export default (
2626
action: PerAccountApplicableAction,
2727
): OutboxState => {
2828
switch (action.type) {
29+
// TODO(#3881): Figure out if we want this.
2930
case REGISTER_COMPLETE:
3031
return filterArray(state, (outbox: Outbox) => !outbox.isSent);
3132

0 commit comments

Comments
 (0)