diff --git a/docs/howto/debugging.md b/docs/howto/debugging.md index 5887243db54..50a14055700 100644 --- a/docs/howto/debugging.md +++ b/docs/howto/debugging.md @@ -14,6 +14,7 @@ A variety of tools are available to help us do that. * ... [with React DevTools](#react-devtools) * ... [with Reactotron](#reactotron) * ... [with redux-logger](#redux-logger) + * ... [with immutable-devtools](#immutable-devtools) * [Debugging the message list](#webview) in its WebView * ... on iOS, [with Safari developer tools](#webview-safari) * ... on Android, [with the Chrome Developer Tools](#webview-chrome) @@ -158,6 +159,33 @@ call to `createLogger` in `src/boot/middleware.js`. doc](https://github.com/evgenyrodionov/redux-logger#options). +
+ +## immutable-devtools: inspect Immutable.js objects in Chrome + +Some of the data in the Redux state is stored in Immutable.js data +structures. It's normally awkward to inspect these data structures; an +`Immutable.Map` object, for example, is full of properties like +`size`, `__altered`, `__hash`, and `__ownerID`, which you have to dig +around to get at a clear representation of the items contained. + +[`immutable-devtools`](https://github.com/andrewdavey/immutable-devtools) +fixes this problem in Google Chrome v47+ by installing a "custom +formatter". + +(Alternatively, we might have recommended a [Chrome +extension](https://github.com/mattzeunert/immutable-object-formatter-extension), +which `immutable-devtools` mentions in its setup doc. But using the +NPM package provides the formatter for `zulip-mobile` just as +effectively without making widespread changes in behavior when you +browse the Web.) + +To enable it, open Chrome Dev Tools and press F1 to open the settings. +In the "Console" section, tick "Enable custom formatters". If it isn't +working, please consult the project's issue tracker before opening an +issue in `zulip-mobile`. + +
# Tools: Debugging the message list (in WebView) diff --git a/flow-typed/npm/immutable-devtools_vx.x.x.js b/flow-typed/npm/immutable-devtools_vx.x.x.js new file mode 100644 index 00000000000..e26a0ebc482 --- /dev/null +++ b/flow-typed/npm/immutable-devtools_vx.x.x.js @@ -0,0 +1,35 @@ +// flow-typed signature: 14523166b9c09a9ed88bca4845a653b1 +// flow-typed version: <>/immutable-devtools_v0.1.5/flow_v0.113.0 + +/** + * This is an autogenerated libdef stub for: + * + * 'immutable-devtools' + * + * Fill this stub out by replacing all the `any` types. + * + * Once filled out, we encourage you to share your work with the + * community by sending a pull request to: + * https://github.com/flowtype/flow-typed + */ + +declare module 'immutable-devtools' { + declare module.exports: any; +} + +/** + * We include stubs for each file inside this npm package in case you need to + * require those files directly. Feel free to delete any files that aren't + * needed. + */ +declare module 'immutable-devtools/dist' { + declare module.exports: any; +} + +// Filename aliases +declare module 'immutable-devtools/dist/index' { + declare module.exports: $Exports<'immutable-devtools/dist'>; +} +declare module 'immutable-devtools/dist/index.js' { + declare module.exports: $Exports<'immutable-devtools/dist'>; +} diff --git a/package.json b/package.json index da0bf7ccb0e..97a00a5a1a3 100644 --- a/package.json +++ b/package.json @@ -120,6 +120,7 @@ "flow-bin": "^0.113.0", "flow-coverage-report": "^0.6.0", "flow-typed": "^2.4.0", + "immutable-devtools": "^0.1.5", "jest": "^26.4.1", "jest-cli": "^26.4.1", "jest-environment-jsdom": "^26.3.0", diff --git a/src/__tests__/lib/exampleData.js b/src/__tests__/lib/exampleData.js index 3720c191363..8f7a2f3d48d 100644 --- a/src/__tests__/lib/exampleData.js +++ b/src/__tests__/lib/exampleData.js @@ -576,5 +576,5 @@ export const eventNewMessageActionBase /* \: $Diff, type: typeof EVENT_NEW_MESSAGE, caughtUp: CaughtUpState, - ownEmail: string, + ownUser: User, |}; type EventSubmessageAction = {| diff --git a/src/boot/store.js b/src/boot/store.js index 2d26c347899..e978ff1da1c 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -19,6 +19,18 @@ import createMigration from '../redux-persist-migrate/index'; import { provideLoggingContext } from './loggingContext'; import { tryGetActiveAccount } from '../account/accountsSelectors'; +if (process.env.NODE_ENV === 'development') { + // Chrome dev tools for Immutable. + // + // To enable, press F1 from the Chrome dev tools to open the + // settings. In the "Console" section, check "Enable custom + // formatters". + // + // eslint-disable-next-line import/no-extraneous-dependencies, global-require + const installDevTools = require('immutable-devtools'); + installDevTools(Immutable); +} + // AsyncStorage.clear(); // use to reset storage during development /** @@ -199,6 +211,12 @@ const migrations: { [string]: (GlobalState) => GlobalState } = { })), }), + // Convert `narrows` from object-as-map to `Immutable.Map`. + '16': state => ({ + ...state, + narrows: Immutable.Map(state.narrows), + }), + // TIP: When adding a migration, consider just using `dropCache`. }; diff --git a/src/chat/__tests__/narrowsReducer-test.js b/src/chat/__tests__/narrowsReducer-test.js index 9eec8565ff1..337af9263f7 100644 --- a/src/chat/__tests__/narrowsReducer-test.js +++ b/src/chat/__tests__/narrowsReducer-test.js @@ -1,5 +1,6 @@ /* @flow strict-local */ import deepFreeze from 'deep-freeze'; +import Immutable from 'immutable'; import narrowsReducer from '../narrowsReducer'; import { @@ -32,9 +33,7 @@ describe('narrowsReducer', () => { describe('EVENT_NEW_MESSAGE', () => { test('if not caught up in narrow, do not add message in home narrow', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2] }); const message = eg.streamMessage({ id: 3, flags: [] }); @@ -45,17 +44,13 @@ describe('narrowsReducer', () => { const newState = narrowsReducer(initialState, action); - const expectedState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2], - }); + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2] }); expect(newState).toEqual(expectedState); }); test('appends message to state producing a copy of messages', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2] }); const message = eg.streamMessage({ id: 3, flags: [] }); @@ -70,9 +65,9 @@ describe('narrowsReducer', () => { }, }); - const expectedState = { + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2, 3], - }; + }); const newState = narrowsReducer(initialState, action); @@ -81,9 +76,7 @@ describe('narrowsReducer', () => { }); test('if new message key does not exist do not create it', () => { - const initialState = deepFreeze({ - [topicNarrowStr]: [1, 2], - }); + const initialState = Immutable.Map({ [topicNarrowStr]: [1, 2] }); const message = eg.streamMessage({ id: 3, flags: [], stream: eg.makeStream() }); @@ -94,17 +87,15 @@ describe('narrowsReducer', () => { const newState = narrowsReducer(initialState, action); - const expectedState = { + const expectedState = Immutable.Map({ [topicNarrowStr]: [1, 2], - }; + }); expect(newState).toEqual(expectedState); }); }); test('if new message is private or group add it to the "allPrivate" narrow', () => { - const initialState = deepFreeze({ - [ALL_PRIVATE_NARROW_STR]: [], - }); + const initialState = Immutable.Map({ [ALL_PRIVATE_NARROW_STR]: [] }); const message = eg.pmMessage({ id: 1, recipients: [eg.selfUser, eg.otherUser, eg.thirdUser], @@ -117,9 +108,9 @@ describe('narrowsReducer', () => { [ALL_PRIVATE_NARROW_STR]: { older: true, newer: true }, }, }); - const expectedState = { + const expectedState = Immutable.Map({ [ALL_PRIVATE_NARROW_STR]: [1], - }; + }); const actualState = narrowsReducer(initialState, action); @@ -127,10 +118,7 @@ describe('narrowsReducer', () => { }); test('message sent to topic is stored correctly', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2], - [topicNarrowStr]: [2], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2], [topicNarrowStr]: [2] }); const message = eg.streamMessage({ id: 3, flags: [] }); @@ -148,10 +136,10 @@ describe('narrowsReducer', () => { }, }, }); - const expectedState = { + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2], [topicNarrowStr]: [2, message.id], - }; + }); const newState = narrowsReducer(initialState, action); @@ -160,7 +148,7 @@ describe('narrowsReducer', () => { test('message sent to self is stored correctly', () => { const narrowWithSelfStr = JSON.stringify(pmNarrowFromEmail(eg.selfUser.email)); - const initialState = deepFreeze({ + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [], [narrowWithSelfStr]: [], }); @@ -181,10 +169,10 @@ describe('narrowsReducer', () => { }, }); - const expectedState = { + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [message.id], [narrowWithSelfStr]: [message.id], - }; + }); const newState = narrowsReducer(initialState, action); @@ -193,7 +181,7 @@ describe('narrowsReducer', () => { }); test('appends stream message to all cached narrows that match and are caught up', () => { - const initialState = deepFreeze({ + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2], [ALL_PRIVATE_NARROW_STR]: [1, 2], [streamNarrowStr]: [2, 3], @@ -214,14 +202,14 @@ describe('narrowsReducer', () => { }, }); - const expectedState = { + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2, message.id], [ALL_PRIVATE_NARROW_STR]: [1, 2], [streamNarrowStr]: [2, 3, message.id], [topicNarrowStr]: [2, 3, message.id], [privateNarrowStr]: [2, 4], [groupNarrowStr]: [2, 4], - }; + }); const newState = narrowsReducer(initialState, action); @@ -230,9 +218,7 @@ describe('narrowsReducer', () => { }); test('does not append stream message to not cached narrows', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1] }); const message = eg.streamMessage({ id: 3, flags: [] }); @@ -244,9 +230,9 @@ describe('narrowsReducer', () => { }, }); - const expectedState = { + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1, message.id], - }; + }); const newState = narrowsReducer(initialState, action); @@ -255,7 +241,7 @@ describe('narrowsReducer', () => { }); test('appends private message to multiple cached narrows', () => { - const initialState = deepFreeze({ + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2], [ALL_PRIVATE_NARROW_STR]: [1, 2], [streamNarrowStr]: [2, 3], @@ -274,19 +260,17 @@ describe('narrowsReducer', () => { const action = deepFreeze({ ...eg.eventNewMessageActionBase, message, - caughtUp: Object.fromEntries( - Object.keys(initialState).map(key => [key, { older: false, newer: true }]), - ), + caughtUp: initialState.map(_ => ({ older: false, newer: true })).toObject(), }); - const expectedState = { + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2, message.id], [ALL_PRIVATE_NARROW_STR]: [1, 2, message.id], [streamNarrowStr]: [2, 3], [topicNarrowStr]: [2, 3], [privateNarrowStr]: [2, 4, message.id], [groupNarrowStr]: [2, 4], - }; + }); const newState = narrowsReducer(initialState, action); @@ -296,10 +280,7 @@ describe('narrowsReducer', () => { describe('EVENT_MESSAGE_DELETE', () => { test('if a message does not exist no changes are made', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2], - [privateNarrowStr]: [], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2], [privateNarrowStr]: [] }); const action = deepFreeze({ type: EVENT_MESSAGE_DELETE, @@ -312,18 +293,12 @@ describe('narrowsReducer', () => { }); test('if a message exists in one or more narrows delete it from there', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2, 3], - [privateNarrowStr]: [2], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2, 3], [privateNarrowStr]: [2] }); const action = deepFreeze({ type: EVENT_MESSAGE_DELETE, messageIds: [2], }); - const expectedState = deepFreeze({ - [HOME_NARROW_STR]: [1, 3], - [privateNarrowStr]: [], - }); + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1, 3], [privateNarrowStr]: [] }); const newState = narrowsReducer(initialState, action); @@ -331,18 +306,12 @@ describe('narrowsReducer', () => { }); test('if multiple messages indicated, delete the ones that exist', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2, 3], - [privateNarrowStr]: [2], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2, 3], [privateNarrowStr]: [2] }); const action = deepFreeze({ type: EVENT_MESSAGE_DELETE, messageIds: [2, 3, 4], }); - const expectedState = deepFreeze({ - [HOME_NARROW_STR]: [1], - [privateNarrowStr]: [], - }); + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1], [privateNarrowStr]: [] }); const newState = narrowsReducer(initialState, action); @@ -352,9 +321,7 @@ describe('narrowsReducer', () => { describe('MESSAGE_FETCH_START', () => { test('if fetching for a search narrow, ignore', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2] }); const action = deepFreeze({ ...eg.action.message_fetch_start, @@ -373,7 +340,7 @@ describe('narrowsReducer', () => { // MESSAGE_FETCH_START applies the identity function to the // state (i.e., it doesn't do anything to it). Reversing that // effect is also done with the identity function. - const initialState = deepFreeze({ + const initialState = Immutable.Map({ [HOME_NARROW_STR]: { older: true, newer: true, @@ -401,9 +368,7 @@ describe('narrowsReducer', () => { describe('MESSAGE_FETCH_COMPLETE', () => { test('if no messages returned still create the key in state', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2, 3], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2, 3] }); const action = deepFreeze({ type: MESSAGE_FETCH_COMPLETE, @@ -416,10 +381,10 @@ describe('narrowsReducer', () => { foundOldest: false, }); - const expectedState = { + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2, 3], [JSON.stringify(pmNarrowFromEmail(eg.otherUser.email))]: [], - }; + }); const newState = narrowsReducer(initialState, action); @@ -427,9 +392,7 @@ describe('narrowsReducer', () => { }); test('no duplicate messages', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2, 3], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2, 3] }); const action = deepFreeze({ type: MESSAGE_FETCH_COMPLETE, @@ -446,9 +409,9 @@ describe('narrowsReducer', () => { foundOldest: false, }); - const expectedState = { + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2, 3, 4], - }; + }); const newState = narrowsReducer(initialState, action); @@ -457,9 +420,7 @@ describe('narrowsReducer', () => { }); test('added messages are sorted by id', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2] }); const action = deepFreeze({ type: MESSAGE_FETCH_COMPLETE, @@ -475,9 +436,9 @@ describe('narrowsReducer', () => { foundOldest: false, }); - const expectedState = { + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2, 3, 4], - }; + }); const newState = narrowsReducer(initialState, action); @@ -486,9 +447,7 @@ describe('narrowsReducer', () => { }); test('when anchor is FIRST_UNREAD_ANCHOR previous messages are replaced', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2] }); const action = deepFreeze({ anchor: FIRST_UNREAD_ANCHOR, @@ -504,9 +463,9 @@ describe('narrowsReducer', () => { foundOldest: false, }); - const expectedState = { + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [3, 4], - }; + }); const newState = narrowsReducer(initialState, action); @@ -514,9 +473,7 @@ describe('narrowsReducer', () => { }); test('when anchor is LAST_MESSAGE_ANCHOR previous messages are replaced', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2] }); const action = deepFreeze({ anchor: LAST_MESSAGE_ANCHOR, @@ -532,9 +489,9 @@ describe('narrowsReducer', () => { foundOldest: false, }); - const expectedState = { + const expectedState = Immutable.Map({ [HOME_NARROW_STR]: [3, 4], - }; + }); const newState = narrowsReducer(initialState, action); @@ -542,9 +499,7 @@ describe('narrowsReducer', () => { }); test('if fetched messages are from a search narrow, ignore them', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2] }); const action = deepFreeze({ ...eg.action.message_fetch_complete, @@ -567,9 +522,7 @@ describe('narrowsReducer', () => { }; test('Do nothing if the is:starred narrow has not been fetched', () => { - const initialState = deepFreeze({ - [HOME_NARROW_STR]: [1, 2], - }); + const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2] }); const action = deepFreeze({ type: EVENT_UPDATE_MESSAGE_FLAGS, @@ -587,9 +540,7 @@ describe('narrowsReducer', () => { }); test("Do nothing if action.flag is not 'starred'", () => { - const initialState = deepFreeze({ - [STARRED_NARROW_STR]: [1, 2], - }); + const initialState = Immutable.Map({ [STARRED_NARROW_STR]: [1, 2] }); const action = deepFreeze({ type: EVENT_UPDATE_MESSAGE_FLAGS, @@ -610,9 +561,7 @@ describe('narrowsReducer', () => { 'Adds, while maintaining chronological order, ' + 'newly starred messages to the is:starred narrow', () => { - const initialState = deepFreeze({ - [STARRED_NARROW_STR]: [1, 3, 5], - }); + const initialState = Immutable.Map({ [STARRED_NARROW_STR]: [1, 3, 5] }); const action = deepFreeze({ type: EVENT_UPDATE_MESSAGE_FLAGS, @@ -624,9 +573,9 @@ describe('narrowsReducer', () => { messages: [4, 2], }); - const expectedState = { + const expectedState = Immutable.Map({ [STARRED_NARROW_STR]: [1, 2, 3, 4, 5], - }; + }); const newState = narrowsReducer(initialState, action); @@ -638,9 +587,7 @@ describe('narrowsReducer', () => { 'Removes, while maintaining chronological order, ' + 'newly unstarred messages from the is:starred narrow', () => { - const initialState = deepFreeze({ - [STARRED_NARROW_STR]: [1, 2, 3, 4, 5], - }); + const initialState = Immutable.Map({ [STARRED_NARROW_STR]: [1, 2, 3, 4, 5] }); const action = deepFreeze({ type: EVENT_UPDATE_MESSAGE_FLAGS, @@ -652,9 +599,9 @@ describe('narrowsReducer', () => { messages: [4, 2], }); - const expectedState = { + const expectedState = Immutable.Map({ [STARRED_NARROW_STR]: [1, 3, 5], - }; + }); const newState = narrowsReducer(initialState, action); diff --git a/src/chat/__tests__/narrowsSelectors-test.js b/src/chat/__tests__/narrowsSelectors-test.js index 420edcca029..3ffc90b4bea 100644 --- a/src/chat/__tests__/narrowsSelectors-test.js +++ b/src/chat/__tests__/narrowsSelectors-test.js @@ -1,4 +1,6 @@ /* @flow strict-local */ +import Immutable from 'immutable'; + import { getFirstMessageId, getLastMessageId, @@ -29,9 +31,9 @@ describe('getMessagesForNarrow', () => { test('if no outbox messages returns messages with no change', () => { const state = eg.reduxState({ - narrows: { + narrows: Immutable.Map({ '[]': [123], - }, + }), messages, outbox: [], realm: eg.realmState({ email: eg.selfUser.email }), @@ -44,9 +46,9 @@ describe('getMessagesForNarrow', () => { test('combine messages and outbox in same narrow', () => { const state = eg.reduxState({ - narrows: { + narrows: Immutable.Map({ '[]': [123], - }, + }), messages, outbox: [outboxMessage], caughtUp: { @@ -62,9 +64,9 @@ describe('getMessagesForNarrow', () => { test('do not combine messages and outbox if not caught up', () => { const state = eg.reduxState({ - narrows: { + narrows: Immutable.Map({ [HOME_NARROW_STR]: [123], - }, + }), messages, outbox: [outboxMessage], realm: eg.realmState({ email: eg.selfUser.email }), @@ -77,9 +79,9 @@ describe('getMessagesForNarrow', () => { test('do not combine messages and outbox in different narrow', () => { const state = eg.reduxState({ - narrows: { + narrows: Immutable.Map({ [JSON.stringify(pmNarrowFromEmail('john@example.com'))]: [123], - }, + }), messages, outbox: [outboxMessage], realm: eg.realmState({ email: eg.selfUser.email }), @@ -94,9 +96,9 @@ describe('getMessagesForNarrow', () => { describe('getFirstMessageId', () => { test('return undefined when there are no messages', () => { const state = eg.reduxState({ - narrows: { + narrows: Immutable.Map({ '[]': [], - }, + }), outbox: [], }); @@ -107,9 +109,9 @@ describe('getFirstMessageId', () => { test('returns first message id', () => { const state = eg.reduxState({ - narrows: { + narrows: Immutable.Map({ '[]': [1, 2, 3], - }, + }), messages: { [1]: eg.streamMessage({ id: 1 }) /* eslint-disable-line no-useless-computed-key */, [2]: eg.streamMessage({ id: 2 }) /* eslint-disable-line no-useless-computed-key */, @@ -127,9 +129,9 @@ describe('getFirstMessageId', () => { describe('getLastMessageId', () => { test('return undefined when there are no messages', () => { const state = eg.reduxState({ - narrows: { + narrows: Immutable.Map({ '[]': [], - }, + }), messages: {}, outbox: [], }); @@ -141,9 +143,9 @@ describe('getLastMessageId', () => { test('returns last message id', () => { const state = eg.reduxState({ - narrows: { + narrows: Immutable.Map({ '[]': [1, 2, 3], - }, + }), messages: { [1]: eg.streamMessage({ id: 1 }) /* eslint-disable-line no-useless-computed-key */, [2]: eg.streamMessage({ id: 2 }) /* eslint-disable-line no-useless-computed-key */, diff --git a/src/chat/narrowsReducer.js b/src/chat/narrowsReducer.js index c360d1d86a6..7e37a275ed0 100644 --- a/src/chat/narrowsReducer.js +++ b/src/chat/narrowsReducer.js @@ -1,5 +1,6 @@ /* @flow strict-local */ import union from 'lodash.union'; +import Immutable from 'immutable'; import type { NarrowsState, Action } from '../types'; import { ensureUnreachable } from '../types'; @@ -17,14 +18,13 @@ import { } from '../actionConstants'; import { LAST_MESSAGE_ANCHOR, FIRST_UNREAD_ANCHOR } from '../anchor'; import { - isMessageInNarrow, + getNarrowsForMessage, MENTIONED_NARROW_STR, STARRED_NARROW_STR, isSearchNarrow, } from '../utils/narrow'; -import { NULL_OBJECT } from '../nullObjects'; -const initialState: NarrowsState = NULL_OBJECT; +const initialState: NarrowsState = Immutable.Map(); const messageFetchComplete = (state, action) => { // We don't want to accumulate old searches that we'll never need again. @@ -35,62 +35,82 @@ const messageFetchComplete = (state, action) => { const fetchedMessageIds = action.messages.map(message => message.id); const replaceExisting = action.anchor === FIRST_UNREAD_ANCHOR || action.anchor === LAST_MESSAGE_ANCHOR; - return { - ...state, - [key]: replaceExisting + return state.set( + key, + replaceExisting ? fetchedMessageIds - : union(state[key], fetchedMessageIds).sort((a, b) => a - b), - }; + : union(state.get(key), fetchedMessageIds).sort((a, b) => a - b), + ); }; const eventNewMessage = (state, action) => { - let stateChange = false; - const newState: NarrowsState = {}; - Object.keys(state).forEach(key => { - const { flags } = action.message; - if (!flags) { - throw new Error('EVENT_NEW_MESSAGE message missing flags'); - } - const isInNarrow = isMessageInNarrow(action.message, flags, JSON.parse(key), action.ownEmail); - const isCaughtUp = action.caughtUp[key] && action.caughtUp[key].newer; - const messageDoesNotExist = state[key].find(id => action.message.id === id) === undefined; - - if (isInNarrow && isCaughtUp && messageDoesNotExist) { - stateChange = true; - newState[key] = [...state[key], action.message.id]; - } else { - newState[key] = state[key]; - } + const { message } = action; + const { flags } = message; + + if (!flags) { + throw new Error('EVENT_NEW_MESSAGE message missing flags'); + } + + return state.withMutations(stateMutable => { + const narrowsForMessage = getNarrowsForMessage(message, action.ownUser, flags); + + narrowsForMessage.forEach(narrow => { + const key = JSON.stringify(narrow); + const value = stateMutable.get(key); + + if (!value) { + // We haven't loaded this narrow. The time to add a new key + // isn't now; we do that in MESSAGE_FETCH_COMPLETE, when we + // might have a reasonably long, contiguous list of messages + // to show. + return; // i.e., continue + } + + // (No guarantee that `key` is in `action.caughtUp`) + // flowlint-next-line unnecessary-optional-chain:off + if (!action.caughtUp[key]?.newer) { + // Don't add a message to the end of the list unless we know + // it's the most recent message, i.e., unless we know we're + // currently looking at (caught up with) the newest messages in + // the narrow. + return; // i.e., continue + } + + if (value.some(id => action.message.id === id)) { + // Don't add a message that's already been added. It's probably + // very rare for a message to have already been added when we + // get an EVENT_NEW_MESSAGE, and perhaps impossible. (TODO: + // investigate?) + return; // i.e., continue + } + + stateMutable.set(key, [...value, message.id]); + }); }); - return stateChange ? newState : state; }; const eventMessageDelete = (state, action) => { let stateChange = false; - const newState: NarrowsState = {}; - Object.keys(state).forEach(key => { - newState[key] = state[key].filter(id => !action.messageIds.includes(id)); - stateChange = stateChange || newState[key].length < state[key].length; + const newState = state.map((value, key) => { + const result = value.filter(id => !action.messageIds.includes(id)); + stateChange = stateChange || result.length < value.length; + return result; }); return stateChange ? newState : state; }; const updateFlagNarrow = (state, narrowStr, operation, messageIds): NarrowsState => { - if (!state[narrowStr]) { + const value = state.get(narrowStr); + if (!value) { return state; } switch (operation) { - case 'add': - return { - ...state, - [narrowStr]: [...state[narrowStr], ...messageIds].sort((a, b) => a - b), - }; + case 'add': { + return state.set(narrowStr, [...value, ...messageIds].sort((a, b) => a - b)); + } case 'remove': { const messageIdSet = new Set(messageIds); - return { - ...state, - [narrowStr]: state[narrowStr].filter(id => !messageIdSet.has(id)), - }; + return state.set(narrowStr, value.filter(id => !messageIdSet.has(id))); } default: ensureUnreachable(operation); diff --git a/src/chat/narrowsSelectors.js b/src/chat/narrowsSelectors.js index 7ffb100b8f8..192c28c2e4d 100644 --- a/src/chat/narrowsSelectors.js +++ b/src/chat/narrowsSelectors.js @@ -57,7 +57,7 @@ export const outboxMessagesForNarrow: Selector = createSelecto ); export const getFetchedMessageIdsForNarrow = (state: GlobalState, narrow: Narrow) => - getAllNarrows(state)[JSON.stringify(narrow)] || NULL_ARRAY; + getAllNarrows(state).get(JSON.stringify(narrow)) || NULL_ARRAY; const getFetchedMessagesForNarrow: Selector = createSelector( getFetchedMessageIdsForNarrow, diff --git a/src/events/eventToAction.js b/src/events/eventToAction.js index ac5410bbfad..ddc74f49e72 100644 --- a/src/events/eventToAction.js +++ b/src/events/eventToAction.js @@ -31,7 +31,7 @@ import { EVENT_SUBSCRIPTION, EVENT, } from '../actionConstants'; -import { getOwnEmail, getOwnUserId } from '../users/userSelectors'; +import { getOwnUser, getOwnUserId } from '../users/userSelectors'; const opToActionUserGroup = { add: EVENT_USER_GROUP_ADD, @@ -86,7 +86,7 @@ export default (state: GlobalState, event: $FlowFixMe): EventAction => { }, local_message_id: event.local_message_id, caughtUp: state.caughtUp, - ownEmail: getOwnEmail(state), + ownUser: getOwnUser(state), }; // Before server feature level 13, or if we don't specify the diff --git a/src/message/__tests__/fetchActions-test.js b/src/message/__tests__/fetchActions-test.js index 52893aa1486..04251957e77 100644 --- a/src/message/__tests__/fetchActions-test.js +++ b/src/message/__tests__/fetchActions-test.js @@ -3,6 +3,7 @@ import invariant from 'invariant'; import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import omit from 'lodash.omit'; +import Immutable from 'immutable'; import type { GlobalState } from '../../reduxTypes'; import type { Action } from '../../actionTypes'; @@ -41,9 +42,9 @@ describe('fetchActions', () => { older: true, }, }, - narrows: { + narrows: Immutable.Map({ [streamNarrowStr]: [], - }, + }), streams: [eg.makeStream({ name: 'some stream' })], }); @@ -63,9 +64,9 @@ describe('fetchActions', () => { older: false, }, }, - narrows: { + narrows: Immutable.Map({ [streamNarrowStr]: [1], - }, + }), messages: { [message1.id]: message1, [message2.id]: message2, @@ -115,9 +116,9 @@ describe('fetchActions', () => { const baseState = eg.reduxState({ accounts: [eg.makeAccount()], - narrows: { + narrows: Immutable.Map({ [streamNarrowStr]: [message1.id], - }, + }), }); describe('success', () => { @@ -293,9 +294,9 @@ describe('fetchActions', () => { const store = mockStore( eg.reduxState({ accounts: [eg.selfAccount], - narrows: { + narrows: Immutable.Map({ [streamNarrowStr]: [1], - }, + }), }), ); @@ -318,9 +319,9 @@ describe('fetchActions', () => { const store = mockStore( eg.reduxState({ accounts: [eg.selfAccount], - narrows: { + narrows: Immutable.Map({ [streamNarrowStr]: [1], - }, + }), }), ); @@ -342,9 +343,9 @@ describe('fetchActions', () => { const store = mockStore( eg.reduxState({ accounts: [eg.selfAccount], - narrows: { + narrows: Immutable.Map({ [streamNarrowStr]: [1], - }, + }), }), ); @@ -369,11 +370,12 @@ describe('fetchActions', () => { const baseState = eg.reduxState({ accounts: [eg.selfAccount], - narrows: { - ...eg.baseReduxState.narrows, - [streamNarrowStr]: [message2.id], - [HOME_NARROW_STR]: [message1.id, message2.id], - }, + narrows: eg.baseReduxState.narrows.merge( + Immutable.Map({ + [streamNarrowStr]: [message2.id], + [HOME_NARROW_STR]: [message1.id, message2.id], + }), + ), messages: { ...eg.baseReduxState.messages, [message1.id]: message1, @@ -461,11 +463,12 @@ describe('fetchActions', () => { const baseState = eg.reduxState({ accounts: [eg.selfAccount], - narrows: { - ...eg.baseReduxState.narrows, - [streamNarrowStr]: [message2.id], - [HOME_NARROW_STR]: [message1.id, message2.id], - }, + narrows: eg.baseReduxState.narrows.merge( + Immutable.Map({ + [streamNarrowStr]: [message2.id], + [HOME_NARROW_STR]: [message1.id, message2.id], + }), + ), messages: { ...eg.baseReduxState.messages, [message1.id]: message1, diff --git a/src/message/messageSelectors.js b/src/message/messageSelectors.js index af874bc3fc2..1dcefa2ac8c 100644 --- a/src/message/messageSelectors.js +++ b/src/message/messageSelectors.js @@ -39,7 +39,7 @@ export const getPrivateMessages: Selector = createSelector( const privateMessages: Message[] = []; const unknownIds: number[] = []; - const pmIds = narrows[ALL_PRIVATE_NARROW_STR] || NULL_ARRAY; + const pmIds = narrows.get(ALL_PRIVATE_NARROW_STR) || NULL_ARRAY; pmIds.forEach(id => { const msg = messages[id]; if (msg !== undefined) { diff --git a/src/pm-conversations/__tests__/pmConversationsSelectors-test.js b/src/pm-conversations/__tests__/pmConversationsSelectors-test.js index e9ea2d9ef21..7759bfa8e52 100644 --- a/src/pm-conversations/__tests__/pmConversationsSelectors-test.js +++ b/src/pm-conversations/__tests__/pmConversationsSelectors-test.js @@ -1,4 +1,6 @@ /* @flow strict-local */ +import Immutable from 'immutable'; + import { getRecentConversations } from '../pmConversationsSelectors'; import { ALL_PRIVATE_NARROW_STR } from '../../utils/narrow'; import * as eg from '../../__tests__/lib/exampleData'; @@ -11,9 +13,9 @@ describe('getRecentConversations', () => { const state = eg.reduxState({ realm: eg.realmState({ email: eg.selfUser.email }), users: [eg.selfUser], - narrows: { + narrows: Immutable.Map({ [ALL_PRIVATE_NARROW_STR]: [], - }, + }), unread: { ...eg.baseReduxState.unread, pms: [], @@ -36,7 +38,7 @@ describe('getRecentConversations', () => { const state = eg.reduxState({ realm: eg.realmState({ email: eg.selfUser.email }), users: [eg.selfUser, userJohn, userMark], - narrows: { + narrows: Immutable.Map({ [ALL_PRIVATE_NARROW_STR]: [ meJohnAndMarkPm.id, meAndJohnPm1.id, @@ -44,7 +46,7 @@ describe('getRecentConversations', () => { meAndJohnPm2.id, meOnlyPm.id, ], - }, + }), messages: { [meAndJohnPm1.id]: meAndJohnPm1, [meAndMarkPm.id]: meAndMarkPm, @@ -128,7 +130,7 @@ describe('getRecentConversations', () => { const state = eg.reduxState({ realm: eg.realmState({ email: eg.selfUser.email }), users: [eg.selfUser, userJohn, userMark], - narrows: { + narrows: Immutable.Map({ [ALL_PRIVATE_NARROW_STR]: [ meAndMarkPm1.id, meAndJohnPm1.id, @@ -137,7 +139,7 @@ describe('getRecentConversations', () => { meJohnAndMarkPm.id, meOnlyPm.id, ], - }, + }), messages: { [meAndJohnPm1.id]: meAndJohnPm1, [meAndMarkPm1.id]: meAndMarkPm1, diff --git a/src/reduxTypes.js b/src/reduxTypes.js index cf9a4f94b15..08d459d4e26 100644 --- a/src/reduxTypes.js +++ b/src/reduxTypes.js @@ -7,7 +7,7 @@ * * @flow strict-local */ - +import type Immutable from 'immutable'; import type { InputSelector } from 'reselect'; import type { Account, Outbox } from './types'; @@ -185,9 +185,7 @@ export type MuteState = MuteTuple[]; * * `FetchingState` for information about which narrows we're actively * fetching more messages from. */ -export type NarrowsState = { - [narrow: string]: number[], -}; +export type NarrowsState = Immutable.Map; export type NavigationRouteState = { key: string, diff --git a/src/topics/__tests__/topicsSelectors-test.js b/src/topics/__tests__/topicsSelectors-test.js index 65e3de63ad2..50729663700 100644 --- a/src/topics/__tests__/topicsSelectors-test.js +++ b/src/topics/__tests__/topicsSelectors-test.js @@ -1,4 +1,6 @@ /* @flow strict-local */ +import Immutable from 'immutable'; + import { getTopicsForNarrow, getLastMessageTopic, getTopicsForStream } from '../topicSelectors'; import { HOME_NARROW, streamNarrow } from '../../utils/narrow'; import * as eg from '../../__tests__/lib/exampleData'; @@ -30,7 +32,7 @@ describe('getTopicsForNarrow', () => { describe('getLastMessageTopic', () => { test('when no messages in narrow return an empty string', () => { const state = eg.reduxState({ - narrows: {}, + narrows: Immutable.Map({}), realm: eg.realmState({ email: eg.selfUser.email }), }); @@ -44,9 +46,9 @@ describe('getLastMessageTopic', () => { const message1 = eg.streamMessage({ id: 1 }); const message2 = eg.streamMessage({ id: 2, subject: 'some topic' }); const state = eg.reduxState({ - narrows: { + narrows: Immutable.Map({ [JSON.stringify(narrow)]: [1, 2], - }, + }), messages: { [message1.id]: message1, [message2.id]: message2, diff --git a/src/unread/__tests__/unreadHuddlesReducer-test.js b/src/unread/__tests__/unreadHuddlesReducer-test.js index fbd73f180f6..51724dee6b8 100644 --- a/src/unread/__tests__/unreadHuddlesReducer-test.js +++ b/src/unread/__tests__/unreadHuddlesReducer-test.js @@ -1,13 +1,14 @@ +/* @flow strict-local */ import deepFreeze from 'deep-freeze'; import unreadHuddlesReducer from '../unreadHuddlesReducer'; import { - REALM_INIT, ACCOUNT_SWITCH, EVENT_NEW_MESSAGE, EVENT_UPDATE_MESSAGE_FLAGS, } from '../../actionConstants'; import { NULL_ARRAY } from '../../nullObjects'; +import * as eg from '../../__tests__/lib/exampleData'; describe('unreadHuddlesReducer', () => { describe('ACCOUNT_SWITCH', () => { @@ -21,6 +22,7 @@ describe('unreadHuddlesReducer', () => { const action = deepFreeze({ type: ACCOUNT_SWITCH, + index: 1, }); const expectedState = []; @@ -36,17 +38,19 @@ describe('unreadHuddlesReducer', () => { const initialState = deepFreeze([]); const action = deepFreeze({ - type: REALM_INIT, + ...eg.action.realm_init, data: { + ...eg.action.realm_init.data, unread_msgs: { - streams: [{}, {}], + ...eg.action.realm_init.data.unread_msgs, + streams: [], huddles: [ { user_ids_string: '0,1,2', unread_message_ids: [1, 2, 4, 5], }, ], - pms: [{}, {}], + pms: [], mentions: [1, 2, 3], }, }, @@ -67,18 +71,25 @@ describe('unreadHuddlesReducer', () => { describe('EVENT_NEW_MESSAGE', () => { test('if message id already exists, do not mutate state', () => { + const user1 = { ...eg.makeUser(), user_id: 1 }; + const user2 = { ...eg.makeUser(), user_id: 2 }; + const user3 = { ...eg.makeUser(), user_id: 3 }; + + const message1 = eg.pmMessage({ id: 1, recipients: [user1, user2, user3] }); + const message2 = eg.pmMessage({ id: 2, recipients: [user1, user2, user3] }); + const message3 = eg.pmMessage({ id: 3, recipients: [user1, user2, user3] }); + const initialState = deepFreeze([ { - user_ids_string: '1,2,3', - unread_message_ids: [1, 2, 3], + user_ids_string: `${user1.user_id},${user2.user_id},${user3.user_id}`, + unread_message_ids: [message1.id, message2.id, message3.id], }, ]); const action = deepFreeze({ type: EVENT_NEW_MESSAGE, - message: { - id: 2, - }, + ...eg.eventNewMessageActionBase, + message: message2, }); const actualState = unreadHuddlesReducer(initialState, action); @@ -87,6 +98,7 @@ describe('unreadHuddlesReducer', () => { }); test('if message is not group, return original state', () => { + const streamMessage = eg.streamMessage(); const initialState = deepFreeze([ { user_ids_string: '1,2,3', @@ -96,15 +108,8 @@ describe('unreadHuddlesReducer', () => { const action = deepFreeze({ type: EVENT_NEW_MESSAGE, - message: { - id: 4, - type: 'stream', - sender_id: 1, - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 1, email: 'john@example.com' }, - ], - }, + ...eg.eventNewMessageActionBase, + message: streamMessage, }); const actualState = unreadHuddlesReducer(initialState, action); @@ -113,22 +118,22 @@ describe('unreadHuddlesReducer', () => { }); test('if message is sent by self, do not mutate state', () => { + const selfUser = { ...eg.selfUser, user_id: 1 }; + const user2 = { ...eg.otherUser, user_id: 2 }; + const user3 = { ...eg.thirdUser, user_id: 3 }; + const initialState = deepFreeze([]); + const message2 = eg.pmMessage({ + sender: selfUser, + recipients: [selfUser, user2, user3], + }); + const action = deepFreeze({ type: EVENT_NEW_MESSAGE, - message: { - id: 2, - type: 'private', - sender_id: 1, - sender_email: 'me@example.com', - display_recipient: [ - { email: 'john@example.com' }, - { email: 'mark@example.com' }, - { email: 'me@example.com' }, - ], - }, - ownEmail: 'me@example.com', + ...eg.eventNewMessageActionBase, + message: message2, + ownUser: selfUser, }); const actualState = unreadHuddlesReducer(initialState, action); @@ -137,29 +142,28 @@ describe('unreadHuddlesReducer', () => { }); test('if message id does not exist, append to state', () => { + const selfUser = { ...eg.selfUser, user_id: 1 }; + const user2 = { ...eg.otherUser, user_id: 2 }; + const user3 = { ...eg.thirdUser, user_id: 3 }; + + const message4 = eg.pmMessage({ id: 4, recipients: [selfUser, user2, user3] }); + const initialState = deepFreeze([ { - user_ids_string: '0,1,2', + user_ids_string: `${selfUser.user_id},${user2.user_id},${user3.user_id}`, unread_message_ids: [1, 2, 3], }, ]); const action = deepFreeze({ type: EVENT_NEW_MESSAGE, - message: { - id: 4, - type: 'private', - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 1, email: 'john@example.com' }, - { id: 2, email: 'mark@example.com' }, - ], - }, + ...eg.eventNewMessageActionBase, + message: message4, }); const expectedState = [ { - user_ids_string: '0,1,2', + user_ids_string: `${selfUser.user_id},${user2.user_id},${user3.user_id}`, unread_message_ids: [1, 2, 3, 4], }, ]; @@ -169,7 +173,12 @@ describe('unreadHuddlesReducer', () => { expect(actualState).toEqual(expectedState); }); - test('if sender id does not exist, append to state as new sender', () => { + test('if sender-ids string does not exist, append to state as new', () => { + const user1 = { ...eg.selfUser, user_id: 1 }; + const user2 = { ...eg.otherUser, user_id: 2 }; + const user3 = { ...eg.thirdUser, user_id: 3 }; + + const message4 = eg.pmMessage({ id: 4, recipients: [user1, user2, user3] }); const initialState = deepFreeze([ { user_ids_string: '0,3,4', @@ -179,16 +188,8 @@ describe('unreadHuddlesReducer', () => { const action = deepFreeze({ type: EVENT_NEW_MESSAGE, - message: { - id: 4, - type: 'private', - sender_id: 2, - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 1, email: 'john@example.com' }, - { id: 2, email: 'mark@example.com' }, - ], - }, + ...eg.eventNewMessageActionBase, + message: message4, }); const expectedState = [ @@ -197,7 +198,7 @@ describe('unreadHuddlesReducer', () => { unread_message_ids: [1, 2, 3], }, { - user_ids_string: '0,1,2', + user_ids_string: `${user1.user_id},${user2.user_id},${user3.user_id}`, unread_message_ids: [4], }, ]; @@ -213,7 +214,10 @@ describe('unreadHuddlesReducer', () => { const initialState = deepFreeze([]); const action = { + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [1, 2, 3], flag: 'star', operation: 'add', @@ -237,7 +241,10 @@ describe('unreadHuddlesReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [6, 7], flag: 'read', operation: 'add', @@ -261,7 +268,10 @@ describe('unreadHuddlesReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [3, 4, 5, 6], flag: 'read', operation: 'add', @@ -288,7 +298,10 @@ describe('unreadHuddlesReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [1, 2], flag: 'read', operation: 'remove', @@ -308,11 +321,13 @@ describe('unreadHuddlesReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: true, + allMessages: {}, messages: [], flag: 'read', operation: 'add', - all: true, }); const actualState = unreadHuddlesReducer(initialState, action); diff --git a/src/unread/__tests__/unreadPmsReducer-test.js b/src/unread/__tests__/unreadPmsReducer-test.js index 5e3c5e4b48d..d8708a589da 100644 --- a/src/unread/__tests__/unreadPmsReducer-test.js +++ b/src/unread/__tests__/unreadPmsReducer-test.js @@ -1,13 +1,10 @@ +/* @flow strict-local */ import deepFreeze from 'deep-freeze'; import unreadPmsReducer from '../unreadPmsReducer'; -import { - REALM_INIT, - ACCOUNT_SWITCH, - EVENT_NEW_MESSAGE, - EVENT_UPDATE_MESSAGE_FLAGS, -} from '../../actionConstants'; +import { ACCOUNT_SWITCH, EVENT_UPDATE_MESSAGE_FLAGS } from '../../actionConstants'; import { NULL_ARRAY } from '../../nullObjects'; +import * as eg from '../../__tests__/lib/exampleData'; describe('unreadPmsReducer', () => { describe('ACCOUNT_SWITCH', () => { @@ -21,6 +18,7 @@ describe('unreadPmsReducer', () => { const action = deepFreeze({ type: ACCOUNT_SWITCH, + index: 1, }); const expectedState = []; @@ -35,27 +33,35 @@ describe('unreadPmsReducer', () => { test('received data from "unread_msgs.pms" key replaces the current state ', () => { const initialState = deepFreeze([]); + const message1 = eg.pmMessage({ id: 1, sender_id: 1 }); + const message2 = eg.pmMessage({ id: 2, sender_id: 1 }); + const message3 = eg.pmMessage({ id: 3, sender_id: 1 }); + const message4 = eg.pmMessage({ id: 4, sender_id: 1 }); + const message5 = eg.pmMessage({ id: 5, sender_id: 1 }); + const action = deepFreeze({ - type: REALM_INIT, + ...eg.action.realm_init, data: { + ...eg.action.realm_init.data, unread_msgs: { - streams: [{}, {}], - huddles: [{}, {}, {}], + ...eg.action.realm_init.data.unread_msgs, + streams: [], + huddles: [], pms: [ { - sender_id: 1, - unread_message_ids: [1, 2, 4, 5], + sender_id: message1.sender_id, + unread_message_ids: [message1.id, message2.id, message4.id, message5.id], }, ], - mentions: [1, 2, 3], + mentions: [message1.id, message2.id, message3.id], }, }, }); const expectedState = [ { - sender_id: 1, - unread_message_ids: [1, 2, 4, 5], + sender_id: message1.sender_id, + unread_message_ids: [message1.id, message2.id, message4.id, message5.id], }, ]; @@ -67,18 +73,17 @@ describe('unreadPmsReducer', () => { describe('EVENT_NEW_MESSAGE', () => { test('if message id already exists, do not mutate state', () => { + const message1 = eg.pmMessage({ sender_id: 1 }); const initialState = deepFreeze([ { - sender_id: 1, - unread_message_ids: [1, 2, 3], + sender_id: message1.sender_id, + unread_message_ids: [message1.id], }, ]); const action = deepFreeze({ - type: EVENT_NEW_MESSAGE, - message: { - id: 2, - }, + ...eg.eventNewMessageActionBase, + message: message1, }); const actualState = unreadPmsReducer(initialState, action); @@ -86,19 +91,18 @@ describe('unreadPmsReducer', () => { expect(actualState).toBe(initialState); }); - test('if message is sent by self, do not mutate state', () => { - const initialState = deepFreeze([]); - - const action = deepFreeze({ - type: EVENT_NEW_MESSAGE, - message: { - id: 2, - type: 'private', + test('if message is not private, return original state', () => { + const message4 = eg.streamMessage({ id: 4 }); + const initialState = deepFreeze([ + { sender_id: 1, - sender_email: 'me@example.com', - display_recipient: [{ email: 'john@example.com' }, { email: 'me@example.com' }], + unread_message_ids: [1, 2, 3], }, - ownEmail: 'me@example.com', + ]); + + const action = deepFreeze({ + ...eg.eventNewMessageActionBase, + message: message4, }); const actualState = unreadPmsReducer(initialState, action); @@ -106,21 +110,17 @@ describe('unreadPmsReducer', () => { expect(actualState).toBe(initialState); }); - test('if message is not private, return original state', () => { - const initialState = deepFreeze([ - { - sender_id: 1, - unread_message_ids: [1, 2, 3], - }, - ]); + test('if message is sent by self, do not mutate state', () => { + const initialState = deepFreeze([]); + const message1 = eg.pmMessage({ + sender: eg.selfUser, + recipients: [eg.otherUser, eg.selfUser], + }); const action = deepFreeze({ - type: EVENT_NEW_MESSAGE, - message: { - id: 4, - type: 'stream', - sender_id: 1, - }, + ...eg.eventNewMessageActionBase, + message: message1, + ownUser: eg.selfUser, }); const actualState = unreadPmsReducer(initialState, action); @@ -129,27 +129,27 @@ describe('unreadPmsReducer', () => { }); test('if message id does not exist, append to state', () => { + const message1 = eg.pmMessage({ id: 1, sender_id: 1 }); + const message2 = eg.pmMessage({ id: 2, sender_id: 1 }); + const message3 = eg.pmMessage({ id: 3, sender_id: 1 }); + const message4 = eg.pmMessage({ id: 4, sender_id: 1 }); + const initialState = deepFreeze([ { - sender_id: 1, - unread_message_ids: [1, 2, 3], + sender_id: message4.sender_id, + unread_message_ids: [message1.id, message2.id, message3.id], }, ]); const action = deepFreeze({ - type: EVENT_NEW_MESSAGE, - message: { - id: 4, - type: 'private', - sender_id: 1, - display_recipient: [{ email: 'john@example.com' }, { email: 'me@example.com' }], - }, + ...eg.eventNewMessageActionBase, + message: message4, }); const expectedState = [ { - sender_id: 1, - unread_message_ids: [1, 2, 3, 4], + sender_id: message4.sender_id, + unread_message_ids: [message1.id, message2.id, message3.id, message4.id], }, ]; @@ -159,6 +159,7 @@ describe('unreadPmsReducer', () => { }); test('if sender id does not exist, append to state as new sender', () => { + const message4 = eg.pmMessage({ id: 4, sender_id: 2 }); const initialState = deepFreeze([ { sender_id: 1, @@ -167,13 +168,8 @@ describe('unreadPmsReducer', () => { ]); const action = deepFreeze({ - type: EVENT_NEW_MESSAGE, - message: { - id: 4, - type: 'private', - sender_id: 2, - display_recipient: [{ email: 'john@example.com' }, { email: 'me@example.com' }], - }, + ...eg.eventNewMessageActionBase, + message: message4, }); const expectedState = [ @@ -182,8 +178,8 @@ describe('unreadPmsReducer', () => { unread_message_ids: [1, 2, 3], }, { - sender_id: 2, - unread_message_ids: [4], + sender_id: message4.sender_id, + unread_message_ids: [message4.id], }, ]; @@ -198,7 +194,10 @@ describe('unreadPmsReducer', () => { const initialState = deepFreeze([]); const action = { + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [1, 2, 3], flag: 'star', operation: 'add', @@ -222,7 +221,10 @@ describe('unreadPmsReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [6, 7], flag: 'read', operation: 'add', @@ -246,7 +248,10 @@ describe('unreadPmsReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [3, 4, 5, 6], flag: 'read', operation: 'add', @@ -273,7 +278,10 @@ describe('unreadPmsReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [1, 2], flag: 'read', operation: 'remove', @@ -293,11 +301,13 @@ describe('unreadPmsReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: true, + allMessages: {}, messages: [], flag: 'read', operation: 'add', - all: true, }); const actualState = unreadPmsReducer(initialState, action); diff --git a/src/unread/__tests__/unreadStreamsReducer-test.js b/src/unread/__tests__/unreadStreamsReducer-test.js index c755ec07c74..548694bbca5 100644 --- a/src/unread/__tests__/unreadStreamsReducer-test.js +++ b/src/unread/__tests__/unreadStreamsReducer-test.js @@ -1,13 +1,10 @@ +/* @flow strict-local */ import deepFreeze from 'deep-freeze'; import unreadStreamsReducer from '../unreadStreamsReducer'; -import { - REALM_INIT, - ACCOUNT_SWITCH, - EVENT_NEW_MESSAGE, - EVENT_UPDATE_MESSAGE_FLAGS, -} from '../../actionConstants'; +import { ACCOUNT_SWITCH, EVENT_UPDATE_MESSAGE_FLAGS } from '../../actionConstants'; import { NULL_ARRAY } from '../../nullObjects'; +import * as eg from '../../__tests__/lib/exampleData'; describe('unreadStreamsReducer', () => { describe('ACCOUNT_SWITCH', () => { @@ -22,6 +19,7 @@ describe('unreadStreamsReducer', () => { const action = deepFreeze({ type: ACCOUNT_SWITCH, + index: 1, }); const expectedState = []; @@ -36,29 +34,33 @@ describe('unreadStreamsReducer', () => { test('received data from "unread_msgs.streams" key replaces the current state ', () => { const initialState = deepFreeze([]); + const message1 = eg.streamMessage({ id: 1 }); + const action = deepFreeze({ - type: REALM_INIT, + ...eg.action.realm_init, data: { + ...eg.action.realm_init.data, unread_msgs: { + ...eg.action.realm_init.data.unread_msgs, streams: [ { - stream_id: 1, - topic: 'some topic', - unread_message_ids: [1, 2], + stream_id: message1.stream_id, + topic: message1.subject, + unread_message_ids: [message1.id, 2], }, ], - huddles: [{}, {}, {}], - pms: [{}, {}, {}], - mentions: [1, 2, 3], + huddles: [], + pms: [], + mentions: [message1.id, 2, 3], }, }, }); const expectedState = [ { - stream_id: 1, - topic: 'some topic', - unread_message_ids: [1, 2], + stream_id: message1.stream_id, + topic: message1.subject, + unread_message_ids: [message1.id, 2], }, ]; @@ -70,22 +72,18 @@ describe('unreadStreamsReducer', () => { describe('EVENT_NEW_MESSAGE', () => { test('if message id already exists, do not mutate state', () => { + const message1 = eg.streamMessage({ id: 1 }); const initialState = deepFreeze([ { - stream_id: 1, - topic: 'some topic', - unread_message_ids: [1, 2], + stream_id: message1.stream_id, + topic: message1.subject, + unread_message_ids: [message1.id], }, ]); const action = deepFreeze({ - type: EVENT_NEW_MESSAGE, - message: { - id: 1, - type: 'stream', - stream_id: 1, - subject: 'some topic', - }, + ...eg.eventNewMessageActionBase, + message: message1, }); const actualState = unreadStreamsReducer(initialState, action); @@ -94,20 +92,18 @@ describe('unreadStreamsReducer', () => { }); test('if message is not stream, return original state', () => { + const message4 = eg.pmMessage({ id: 4 }); const initialState = deepFreeze([ { - sender_id: 1, + stream_id: message4.stream_id, + topic: message4.subject, unread_message_ids: [1, 2, 3], }, ]); const action = deepFreeze({ - type: EVENT_NEW_MESSAGE, - message: { - id: 4, - type: 'private', - sender_id: 1, - }, + ...eg.eventNewMessageActionBase, + message: message4, }); const actualState = unreadStreamsReducer(initialState, action); @@ -117,17 +113,12 @@ describe('unreadStreamsReducer', () => { test('if message is sent by self, do not mutate state', () => { const initialState = deepFreeze([]); + const message1 = eg.streamMessage({ sender: eg.selfUser }); const action = deepFreeze({ - type: EVENT_NEW_MESSAGE, - message: { - id: 1, - type: 'stream', - stream_id: 2, - subject: 'another topic', - sender_email: 'me@example.com', - }, - ownEmail: 'me@example.com', + ...eg.eventNewMessageActionBase, + message: message1, + ownUser: eg.selfUser, }); const actualState = unreadStreamsReducer(initialState, action); @@ -136,29 +127,26 @@ describe('unreadStreamsReducer', () => { }); test('if message id does not exist, append to state', () => { + const message4 = eg.streamMessage({ id: 4 }); + const initialState = deepFreeze([ { - stream_id: 1, - topic: 'some topic', + stream_id: message4.stream_id, + topic: message4.subject, unread_message_ids: [1, 2, 3], }, ]); const action = deepFreeze({ - type: EVENT_NEW_MESSAGE, - message: { - id: 4, - type: 'stream', - stream_id: 1, - subject: 'some topic', - }, + ...eg.eventNewMessageActionBase, + message: message4, }); const expectedState = [ { - stream_id: 1, - topic: 'some topic', - unread_message_ids: [1, 2, 3, 4], + stream_id: message4.stream_id, + topic: message4.subject, + unread_message_ids: [1, 2, 3, message4.id], }, ]; @@ -168,6 +156,8 @@ describe('unreadStreamsReducer', () => { }); test('if stream with topic does not exist, append to state', () => { + const message4 = eg.streamMessage({ id: 4, stream_id: 2, subject: 'another topic' }); + const initialState = deepFreeze([ { stream_id: 1, @@ -177,13 +167,8 @@ describe('unreadStreamsReducer', () => { ]); const action = deepFreeze({ - type: EVENT_NEW_MESSAGE, - message: { - id: 4, - type: 'stream', - stream_id: 2, - subject: 'another topic', - }, + ...eg.eventNewMessageActionBase, + message: message4, }); const expectedState = [ @@ -193,9 +178,9 @@ describe('unreadStreamsReducer', () => { unread_message_ids: [1, 2, 3], }, { - stream_id: 2, - topic: 'another topic', - unread_message_ids: [4], + stream_id: message4.stream_id, + topic: message4.subject, + unread_message_ids: [message4.id], }, ]; @@ -210,7 +195,10 @@ describe('unreadStreamsReducer', () => { const initialState = deepFreeze([]); const action = { + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [1, 2, 3], flag: 'star', operation: 'add', @@ -236,7 +224,10 @@ describe('unreadStreamsReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [6, 7], flag: 'read', operation: 'add', @@ -262,7 +253,10 @@ describe('unreadStreamsReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [3, 4, 5, 6], flag: 'read', operation: 'add', @@ -291,7 +285,10 @@ describe('unreadStreamsReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: false, + allMessages: {}, messages: [1, 2], flag: 'read', operation: 'remove', @@ -312,11 +309,13 @@ describe('unreadStreamsReducer', () => { ]); const action = deepFreeze({ + id: 1, type: EVENT_UPDATE_MESSAGE_FLAGS, + all: true, + allMessages: {}, messages: [], flag: 'read', operation: 'add', - all: true, }); const actualState = unreadStreamsReducer(initialState, action); diff --git a/src/unread/unreadHuddlesReducer.js b/src/unread/unreadHuddlesReducer.js index d74e62bee92..a8fe4dbae4f 100644 --- a/src/unread/unreadHuddlesReducer.js +++ b/src/unread/unreadHuddlesReducer.js @@ -23,7 +23,7 @@ const eventNewMessage = (state, action) => { return state; } - if (action.ownEmail && action.ownEmail === action.message.sender_email) { + if (action.ownUser.email && action.ownUser.email === action.message.sender_email) { return state; } diff --git a/src/unread/unreadPmsReducer.js b/src/unread/unreadPmsReducer.js index 7b14784a342..ab3426cc1bb 100644 --- a/src/unread/unreadPmsReducer.js +++ b/src/unread/unreadPmsReducer.js @@ -23,7 +23,7 @@ const eventNewMessage = (state, action) => { return state; } - if (action.ownEmail && action.ownEmail === action.message.sender_email) { + if (action.ownUser.email && action.ownUser.email === action.message.sender_email) { return state; } diff --git a/src/unread/unreadStreamsReducer.js b/src/unread/unreadStreamsReducer.js index b338ba274a8..1a34ace84d8 100644 --- a/src/unread/unreadStreamsReducer.js +++ b/src/unread/unreadStreamsReducer.js @@ -18,7 +18,7 @@ const eventNewMessage = (state, action) => { return state; } - if (action.ownEmail && action.ownEmail === action.message.sender_email) { + if (action.ownUser.email && action.ownUser.email === action.message.sender_email) { return state; } diff --git a/yarn.lock b/yarn.lock index 1811a4d59cd..6355be3d68d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6154,6 +6154,11 @@ image-size@^0.6.0: resolved "https://registry.yarnpkg.com/image-size/-/image-size-0.6.3.tgz#e7e5c65bb534bd7cdcedd6cb5166272a85f75fb2" integrity sha512-47xSUiQioGaB96nqtp5/q55m0aBQSQdyIloMOc/x+QVTDZLNmXE892IIDrJ0hM1A5vcNUDD5tDffkSP5lCaIIA== +immutable-devtools@^0.1.5: + version "0.1.5" + resolved "https://registry.yarnpkg.com/immutable-devtools/-/immutable-devtools-0.1.5.tgz#32a653c8ba8258bfed37680a860a3b5fa2675693" + integrity sha512-bgQP4q+RiD1Oolw8c0sfNrCpShQIEdqJIGmPPrcG6efyJrX00hNzzM1noe3FsQdDwj2eQqL8JEtukCrwFQbt/w== + immutable@^4.0.0-rc.12: version "4.0.0-rc.12" resolved "https://registry.yarnpkg.com/immutable/-/immutable-4.0.0-rc.12.tgz#ca59a7e4c19ae8d9bf74a97bdf0f6e2f2a5d0217"