From eda0f885a46ea78b8982b205890f8b1a500b76a6 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 30 Mar 2023 15:48:33 -0700 Subject: [PATCH 1/4] presence tests: Remove one that expects fallback we don't need We removed the explicit fallback code this was testing in b94544258. As it turns out, the way we use Immutable.Map (new in that commit's same PR) effectively handles the ancient-server behavior as well as the explicit fallback was, and so this test still passes. But that's incidental and not something we need to guarantee. --- src/presence/__tests__/presenceModel-test.js | 22 -------------------- 1 file changed, 22 deletions(-) diff --git a/src/presence/__tests__/presenceModel-test.js b/src/presence/__tests__/presenceModel-test.js index 35d5b4c7a65..ed6fb9461ec 100644 --- a/src/presence/__tests__/presenceModel-test.js +++ b/src/presence/__tests__/presenceModel-test.js @@ -224,28 +224,6 @@ describe('presenceReducer', () => { const expectedState = makePresenceState([[user, userPresence]]); expect(presenceReducer(prevState, action)).toEqual(expectedState); }); - - // TODO(#5102): Delete; see comment on implementation. - test('when no `presence` data is given reset state', () => { - const user = eg.otherUser; - const userPresence = { - aggregated: { client: 'website', status: 'active', timestamp: 123 }, - website: { client: 'website', status: 'active', timestamp: 123 }, - }; - const prevState = makePresenceState([[user, userPresence]]); - const action = eg.mkActionRegisterComplete({ - // Hmm, we should need a Flow suppression here. This property is - // marked required in InitialData, and this explicit undefined is - // meant to defy that; see TODO(#5102) above. - // mkActionRegisterComplete is designed to accept input with this or - // any property *omitted*… and I think, as a side effect of handling - // that, Flow mistakenly accepts an explicit undefined here, so it - // doesn't catch the resulting malformed InitialData. - presences: undefined, - }); - const expectedState = makePresenceState([]); - expect(presenceReducer(prevState, action)).toEqual(expectedState); - }); }); describe('PRESENCE_RESPONSE', () => { From 100c6a0d435657ef7dc44de68041aad9cd014fce Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 30 Mar 2023 15:49:26 -0700 Subject: [PATCH 2/4] fetchActions [nfc]: Remove already-done TODO about ancient servers :tada: We took care of this in 3b811d6af. --- src/message/fetchActions.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/message/fetchActions.js b/src/message/fetchActions.js index 2acff38769c..119992ad3f6 100644 --- a/src/message/fetchActions.js +++ b/src/message/fetchActions.js @@ -374,7 +374,6 @@ export async function fetchServerSettings(realm: URL): Promise< > { try { return { type: 'success', value: await api.getServerSettings(realm) }; - // TODO(#5102): Disallow connecting to ancient servers } catch (errorIllTyped) { const error: mixed = errorIllTyped; From b303fdaeae1141bfae907a440eef509fb9f6d3e8 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 30 Mar 2023 16:04:55 -0700 Subject: [PATCH 3/4] mute: Remove ancient-server support from the reasons for a fallback --- src/mute/__tests__/muteModel-test.js | 10 ---------- src/mute/muteModel.js | 10 ++++------ 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/mute/__tests__/muteModel-test.js b/src/mute/__tests__/muteModel-test.js index 4b58ac91b17..461d5441db6 100644 --- a/src/mute/__tests__/muteModel-test.js +++ b/src/mute/__tests__/muteModel-test.js @@ -95,16 +95,6 @@ describe('reducer', () => { expect(newState).toBeTruthy(); expect(newState && getMute(newState)).toEqual(makeMuteState([[eg.stream, 'topic']])); }); - - // TODO(#5102): Delete; see comment on implementation. - test('in ancient no-muted-topics format', () => { - const state = makeMuteState([[eg.stream, 'topic']]); - const action = eg.mkActionRegisterComplete({ - muted_topics: undefined, - user_topics: undefined, - }); - expect(reducer(state, action, eg.plusReduxState)).toEqual(initialState); - }); }); describe('RESET_ACCOUNT_DATA', () => { diff --git a/src/mute/muteModel.js b/src/mute/muteModel.js index 647af8cb898..e7790946ddb 100644 --- a/src/mute/muteModel.js +++ b/src/mute/muteModel.js @@ -111,12 +111,10 @@ export const reducer = ( // TODO(server-6.0): Stop caring about that, when we cut muted_topics. return convertLegacy( action.data.muted_topics - // TODO(#5102): Delete fallback once we enforce any threshold for - // ancient servers we refuse to connect to. It was added in - // #2878 (2018-11-16), but it wasn't clear even then, it seems, - // whether any servers actually omit the data. The API doc - // doesn't mention any servers that omit it, and our Flow types - // mark it required. + // Unnecessary fallback just to satisfy Flow: the old + // `muted_topics` is absent only when the new `user_topics` is + // present (ignoring ancient unsupported servers), but Flow + // doesn't track that. ?? [], getStreamsByName(globalState), ); From cbf8b3aa5038ba8d60bd2a6d1bd34c0cf39db44f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 30 Mar 2023 15:48:21 -0700 Subject: [PATCH 4/4] model: Remove various fallbacks for ancient, sometimes prehistoric servers This completes all our `TODO(#5102)`s. --- .../__tests__/alertWordsReducer-test.js | 20 ------------------- src/alertWords/alertWordsReducer.js | 10 +--------- src/api/initialDataTypes.js | 4 ---- src/unread/unreadHuddlesReducer.js | 8 +------- src/unread/unreadMentionsReducer.js | 8 +------- src/unread/unreadModel.js | 6 +----- src/unread/unreadPmsReducer.js | 8 +------- .../__tests__/userGroupsReducer-test.js | 20 ------------------- src/user-groups/userGroupsReducer.js | 3 +-- 9 files changed, 6 insertions(+), 81 deletions(-) diff --git a/src/alertWords/__tests__/alertWordsReducer-test.js b/src/alertWords/__tests__/alertWordsReducer-test.js index c85803dd07a..ccb50fc3c9c 100644 --- a/src/alertWords/__tests__/alertWordsReducer-test.js +++ b/src/alertWords/__tests__/alertWordsReducer-test.js @@ -26,26 +26,6 @@ describe('alertWordsReducer', () => { ), ).toEqual(['word', '@mobile-core', 'alert']); }); - - // TODO(#5102): Delete; see comment on implementation. - test('when no `alert_words` data is given reset state', () => { - const prevState = deepFreeze(['word']); - const actualState = alertWordsReducer( - prevState, - eg.mkActionRegisterComplete({ - // Hmm, we should need a Flow suppression here. This property is - // marked required in InitialData, and this explicit undefined is - // meant to defy that; see TODO(#5102) above. - // mkActionRegisterComplete is designed to accept input with this or - // any property *omitted*… and I think, as a side effect of handling - // that, Flow mistakenly accepts an explicit undefined here, so it - // doesn't catch the resulting malformed InitialData. - alert_words: undefined, - }), - ); - - expect(actualState).toEqual([]); - }); }); describe('EVENT_ALERT_WORDS', () => { diff --git a/src/alertWords/alertWordsReducer.js b/src/alertWords/alertWordsReducer.js index 4fbd6430f80..5f143a342ce 100644 --- a/src/alertWords/alertWordsReducer.js +++ b/src/alertWords/alertWordsReducer.js @@ -14,15 +14,7 @@ export default ( return initialState; case REGISTER_COMPLETE: - return ( - action.data.alert_words - // TODO(#5102): Delete fallback once we enforce any threshold for - // ancient servers we refuse to connect to. It was added in #2878 - // (2018-11-16), but it wasn't clear even then, it seems, whether - // any servers actually omit the data. The API doc doesn't mention - // any servers that omit it, and our Flow types mark it required. - || initialState - ); + return action.data.alert_words; case EVENT_ALERT_WORDS: return action.alert_words || initialState; diff --git a/src/api/initialDataTypes.js b/src/api/initialDataTypes.js index 9f11c5c00e0..e9fb85b018e 100644 --- a/src/api/initialDataTypes.js +++ b/src/api/initialDataTypes.js @@ -419,10 +419,6 @@ export type InitialDataRealmUser = $ReadOnly<{| |}>; export type InitialDataRealmUserGroups = $ReadOnly<{| - // New in Zulip 1.8. - // TODO(#5102): In userGroupsReducer, we still have a fallback for pre-1.8 - // servers; remove that, and remove the above comment, which will be - // irrelevant. realm_user_groups: $ReadOnlyArray, |}>; diff --git a/src/unread/unreadHuddlesReducer.js b/src/unread/unreadHuddlesReducer.js index b78eb9f40ea..8f03c1482ce 100644 --- a/src/unread/unreadHuddlesReducer.js +++ b/src/unread/unreadHuddlesReducer.js @@ -94,13 +94,7 @@ export default ( return initialState; case REGISTER_COMPLETE: - return ( - (action.data.unread_msgs && action.data.unread_msgs.huddles) - // TODO(#5102): Delete fallback once we refuse to connect to Zulip - // servers before 1.7.0, when it seems this feature was added; see - // comment on InitialDataUpdateMessageFlags. - || initialState - ); + return action.data.unread_msgs.huddles; case EVENT_NEW_MESSAGE: return eventNewMessage(state, action); diff --git a/src/unread/unreadMentionsReducer.js b/src/unread/unreadMentionsReducer.js index 4220d33d49d..4639b25fdc8 100644 --- a/src/unread/unreadMentionsReducer.js +++ b/src/unread/unreadMentionsReducer.js @@ -52,13 +52,7 @@ export default ( return initialState; case REGISTER_COMPLETE: - return ( - (action.data.unread_msgs && action.data.unread_msgs.mentions) - // TODO(#5102): Delete fallback once we refuse to connect to Zulip - // servers before 1.7.0, when it seems this feature was added; see - // comment on InitialDataUpdateMessageFlags. - || initialState - ); + return action.data.unread_msgs.mentions; case EVENT_NEW_MESSAGE: { const { flags } = action.message; diff --git a/src/unread/unreadModel.js b/src/unread/unreadModel.js index 4b0916e9548..f365c6245f3 100644 --- a/src/unread/unreadModel.js +++ b/src/unread/unreadModel.js @@ -129,11 +129,7 @@ function streamsReducer( return initialStreamsState; case REGISTER_COMPLETE: { - // TODO(#5102): Delete fallback once we refuse to connect to Zulip - // servers before 1.7.0, when it seems this feature was added; see - // comment on InitialDataUpdateMessageFlags. - // flowlint-next-line unnecessary-optional-chain:off - const data = action.data.unread_msgs?.streams ?? []; + const data = action.data.unread_msgs.streams; // First, collect together all the data for a given stream, just in a // plain old Array. diff --git a/src/unread/unreadPmsReducer.js b/src/unread/unreadPmsReducer.js index 1c1ac6e8865..4bf166ca206 100644 --- a/src/unread/unreadPmsReducer.js +++ b/src/unread/unreadPmsReducer.js @@ -94,13 +94,7 @@ export default ( return initialState; case REGISTER_COMPLETE: - return ( - (action.data.unread_msgs && action.data.unread_msgs.pms) - // TODO(#5102): Delete fallback once we refuse to connect to Zulip - // servers before 1.7.0, when it seems this feature was added; see - // comment on InitialDataUpdateMessageFlags. - || initialState - ); + return action.data.unread_msgs.pms; case EVENT_NEW_MESSAGE: return eventNewMessage(state, action); diff --git a/src/user-groups/__tests__/userGroupsReducer-test.js b/src/user-groups/__tests__/userGroupsReducer-test.js index 99453063aa0..37aebe9bf85 100644 --- a/src/user-groups/__tests__/userGroupsReducer-test.js +++ b/src/user-groups/__tests__/userGroupsReducer-test.js @@ -22,26 +22,6 @@ describe('userGroupsReducer', () => { userGroupsReducer(prevState, eg.mkActionRegisterComplete({ realm_user_groups: [group] })), ).toEqual([group]); }); - - // TODO(#5102): Remove this test case, which is for pre-1.8 servers. - test('when no data is given reset state', () => { - const prevState = deepFreeze([eg.makeUserGroup()]); - expect( - userGroupsReducer( - prevState, - eg.mkActionRegisterComplete({ - // Hmm, we should need a Flow suppression here. This property is - // marked required in InitialData, and this explicit undefined is - // meant to defy that; see TODO(#5102) above. - // mkActionRegisterComplete is designed to accept input with this or - // any property *omitted*… and I think, as a side effect of handling - // that, Flow mistakenly accepts an explicit undefined here, so it - // doesn't catch the resulting malformed InitialData. - realm_user_groups: undefined, - }), - ), - ).toEqual([]); - }); }); describe('RESET_ACCOUNT_DATA', () => { diff --git a/src/user-groups/userGroupsReducer.js b/src/user-groups/userGroupsReducer.js index 3f7d1dc2d8b..7ee86da4c7f 100644 --- a/src/user-groups/userGroupsReducer.js +++ b/src/user-groups/userGroupsReducer.js @@ -52,8 +52,7 @@ export default ( return initialState; case REGISTER_COMPLETE: - // TODO(#5102): Remove fallback for pre-1.8 servers - return action.data.realm_user_groups || initialState; + return action.data.realm_user_groups; case EVENT_USER_GROUP_ADD: return [...state, action.group];