diff --git a/jest.config.js b/jest.config.js index a8267491857..b8834e9e9ed 100644 --- a/jest.config.js +++ b/jest.config.js @@ -29,7 +29,7 @@ module.exports = { // Finding and transforming source code. - testPathIgnorePatterns: ['/node_modules/', '/src/__tests__/lib/'], + testPathIgnorePatterns: ['/node_modules/', '/src/__tests__/lib/', '-testlib.js$'], // When some source file foo.js says `import 'bar'`, Jest looks in the // directories above foo.js for a directory like `node_modules` to find diff --git a/src/boot/reducers.js b/src/boot/reducers.js index 61f5b3d86e3..ed9d4fd650a 100644 --- a/src/boot/reducers.js +++ b/src/boot/reducers.js @@ -1,12 +1,12 @@ /* @flow strict-local */ import { combineReducers } from 'redux'; -import type { CombinedReducer, Reducer } from 'redux'; +import type { CombinedReducer } from 'redux'; import { enableBatching } from 'redux-batched-actions'; import config from '../config'; import { logSlowReducers } from '../utils/redux'; import { NULL_OBJECT } from '../nullObjects'; -import type { Action, GlobalState, MigrationsState, UnreadState } from '../types'; +import type { Action, GlobalState, MigrationsState } from '../types'; import accounts from '../account/accountsReducer'; import alertWords from '../alertWords/alertWordsReducer'; @@ -27,10 +27,7 @@ import streams from '../streams/streamsReducer'; import subscriptions from '../subscriptions/subscriptionsReducer'; import topics from '../topics/topicsReducer'; import typing from '../typing/typingReducer'; -import unreadHuddles from '../unread/unreadHuddlesReducer'; -import unreadMentions from '../unread/unreadMentionsReducer'; -import unreadPms from '../unread/unreadPmsReducer'; -import unreadStreams from '../unread/unreadStreamsReducer'; +import { reducer as unread } from '../unread/unreadModel'; import userGroups from '../user-groups/userGroupsReducer'; import userStatus from '../user-status/userStatusReducer'; import users from '../users/usersReducer'; @@ -56,12 +53,7 @@ const reducers = { subscriptions, topics, typing, - unread: (combineReducers({ - streams: unreadStreams, - pms: unreadPms, - huddles: unreadHuddles, - mentions: unreadMentions, - }): Reducer), + unread, userGroups, userStatus, users, diff --git a/src/directSelectors.js b/src/directSelectors.js index a9061fcbdd2..16d720f288c 100644 --- a/src/directSelectors.js +++ b/src/directSelectors.js @@ -13,11 +13,7 @@ import type { RealmEmojiById, RealmState, SettingsState, - StreamUnreadItem, TypingState, - UnreadHuddlesState, - UnreadMentionsState, - UnreadPmsState, Account, Debug, Subscription, @@ -88,14 +84,6 @@ export const getPresence = (state: GlobalState): PresenceState => state.presence export const getOutbox = (state: GlobalState): Outbox[] => state.outbox; -export const getUnreadStreams = (state: GlobalState): StreamUnreadItem[] => state.unread.streams; - -export const getUnreadPms = (state: GlobalState): UnreadPmsState => state.unread.pms; - -export const getUnreadHuddles = (state: GlobalState): UnreadHuddlesState => state.unread.huddles; - -export const getUnreadMentions = (state: GlobalState): UnreadMentionsState => state.unread.mentions; - export const getRealm = (state: GlobalState): RealmState => state.realm; export const getCrossRealmBots = (state: GlobalState): CrossRealmBot[] => diff --git a/src/reduxTypes.js b/src/reduxTypes.js index 8519ef11372..1daca61ceac 100644 --- a/src/reduxTypes.js +++ b/src/reduxTypes.js @@ -14,15 +14,12 @@ import type { Account, Outbox } from './types'; import type { Action } from './actionTypes'; import type { Topic, - HuddlesUnreadItem, Message, MuteTuple, - PmsUnreadItem, CrossRealmBot, RealmEmojiById, RealmFilter, Stream, - StreamUnreadItem, Subscription, User, UserGroup, @@ -32,6 +29,7 @@ import type { import type { Narrow } from './utils/narrow'; import type { SessionState } from './session/sessionReducer'; import type { PmConversationsState } from './pm-conversations/pmConversationsModel'; +import type { UnreadState } from './unread/unreadModelTypes'; export type * from './actionTypes'; @@ -301,26 +299,6 @@ export type TypingState = { }, }; -// These four are fragments of UnreadState; see below. -export type UnreadStreamsState = StreamUnreadItem[]; -export type UnreadHuddlesState = HuddlesUnreadItem[]; -export type UnreadPmsState = PmsUnreadItem[]; -export type UnreadMentionsState = number[]; - -/** - * A summary of (almost) all unread messages, even those we don't have. - * - * The initial version the server gives us for this data is `unread_msgs` in - * the `/register` initial state, and we largely follow the structure of - * that. See there (in `src/api/initialDataTypes.js`) for details. - */ -export type UnreadState = {| - streams: UnreadStreamsState, - huddles: UnreadHuddlesState, - pms: UnreadPmsState, - mentions: UnreadMentionsState, -|}; - export type UserGroupsState = UserGroup[]; export type UserStatusState = UserStatusMapObject; diff --git a/src/topics/__tests__/topicsSelectors-test.js b/src/topics/__tests__/topicsSelectors-test.js index b8f6258b759..374bbeca944 100644 --- a/src/topics/__tests__/topicsSelectors-test.js +++ b/src/topics/__tests__/topicsSelectors-test.js @@ -3,7 +3,9 @@ import Immutable from 'immutable'; import { getTopicsForNarrow, getLastMessageTopic, getTopicsForStream } from '../topicSelectors'; import { HOME_NARROW, streamNarrow, keyFromNarrow } from '../../utils/narrow'; +import { reducer as unreadReducer } from '../../unread/unreadModel'; import * as eg from '../../__tests__/lib/exampleData'; +import { mkMessageAction } from '../../unread/__tests__/unread-testlib'; describe('getTopicsForNarrow', () => { test('when no topics return an empty list', () => { @@ -67,10 +69,6 @@ describe('getTopicsForStream', () => { streams: [], topics: {}, mute: [], - unread: { - ...eg.baseReduxState.unread, - streams: [], - }, }); const topics = getTopicsForStream(state, 123); @@ -86,10 +84,6 @@ describe('getTopicsForStream', () => { [stream.stream_id]: [{ name: 'topic', max_id: 456 }], }, mute: [], - unread: { - ...eg.baseReduxState.unread, - streams: [], - }, }); const topics = getTopicsForStream(state, stream.stream_id); @@ -112,21 +106,16 @@ describe('getTopicsForStream', () => { ], }, mute: [['stream 1', 'topic 1'], ['stream 1', 'topic 3'], ['stream 2', 'topic 2']], - unread: { - ...eg.baseReduxState.unread, - streams: [ - { - stream_id: 1, - topic: 'topic 2', - unread_message_ids: [1, 5, 6], - }, - { - stream_id: 1, - topic: 'topic 4', - unread_message_ids: [7, 8], - }, - ], - }, + unread: [ + eg.streamMessage({ stream_id: 1, subject: 'topic 2', id: 1 }), + eg.streamMessage({ stream_id: 1, subject: 'topic 2', id: 5 }), + eg.streamMessage({ stream_id: 1, subject: 'topic 2', id: 6 }), + eg.streamMessage({ stream_id: 1, subject: 'topic 4', id: 7 }), + eg.streamMessage({ stream_id: 1, subject: 'topic 4', id: 8 }), + ].reduce( + (st, message) => unreadReducer(st, mkMessageAction(message)), + eg.baseReduxState.unread, + ), }); const expected = [ { name: 'topic 1', max_id: 5, isMuted: true, unreadCount: 0 }, diff --git a/src/topics/topicSelectors.js b/src/topics/topicSelectors.js index b6354f1369f..f25b482b22c 100644 --- a/src/topics/topicSelectors.js +++ b/src/topics/topicSelectors.js @@ -2,18 +2,15 @@ import { createSelector } from 'reselect'; import type { - MuteState, Narrow, GlobalState, Selector, - Stream, StreamsState, - StreamUnreadItem, - Topic, TopicExtended, TopicsState, } from '../types'; -import { getMute, getStreams, getTopics, getUnreadStreams } from '../directSelectors'; +import { getMute, getStreams, getTopics } from '../directSelectors'; +import { getUnreadStreams } from '../unread/unreadModel'; import { getShownMessagesForNarrow } from '../chat/narrowsSelectors'; import { getStreamsById } from '../subscriptions/subscriptionSelectors'; import { NULL_ARRAY } from '../nullObjects'; @@ -48,12 +45,7 @@ export const getTopicsForStream: Selector = createSe state => getMute(state), (state, streamId) => getStreamsById(state).get(streamId), state => getUnreadStreams(state), - ( - topicList: Topic[], - mute: MuteState, - stream: Stream | void, - unreadStreams: StreamUnreadItem[], - ) => { + (topicList, mute, stream, unreadStreams) => { if (!topicList || !stream) { return undefined; } diff --git a/src/unread/__tests__/unread-testlib.js b/src/unread/__tests__/unread-testlib.js new file mode 100644 index 00000000000..b33c656d12f --- /dev/null +++ b/src/unread/__tests__/unread-testlib.js @@ -0,0 +1,46 @@ +/* @flow strict-local */ + +import type { Message } from '../../types'; +import { reducer } from '../unreadModel'; +import * as eg from '../../__tests__/lib/exampleData'; + +export const initialState = reducer(undefined, ({ type: eg.randString() }: $FlowFixMe)); + +export const mkMessageAction = (message: Message) => ({ + ...eg.eventNewMessageActionBase, + message: { ...message, flags: message.flags ?? [] }, +}); + +export const stream0 = { ...eg.makeStream({ name: 'stream 0' }), stream_id: 0 }; +export const stream2 = { ...eg.makeStream({ name: 'stream 2' }), stream_id: 2 }; + +const [user0, user1, user2, user3, user4, user5] = [0, 1, 2, 3, 4, 5].map(user_id => + eg.makeUser({ user_id }), +); + +export const selectorBaseState = (() => { + let state = initialState; + for (const message of [ + eg.streamMessage({ stream_id: 0, subject: 'a topic', id: 1, flags: ['mentioned'] }), + eg.streamMessage({ stream_id: 0, subject: 'a topic', id: 2, flags: ['mentioned'] }), + eg.streamMessage({ stream_id: 0, subject: 'a topic', id: 3, flags: ['mentioned'] }), + eg.streamMessage({ stream_id: 0, subject: 'another topic', id: 4 }), + eg.streamMessage({ stream_id: 0, subject: 'another topic', id: 5 }), + eg.streamMessage({ stream_id: 2, subject: 'some other topic', id: 6 }), + eg.streamMessage({ stream_id: 2, subject: 'some other topic', id: 7 }), + // We take user1 to be self. + eg.pmMessageFromTo(user0, [user1], { id: 11 }), + eg.pmMessageFromTo(user0, [user1], { id: 12 }), + eg.pmMessageFromTo(user2, [user1], { id: 13 }), + eg.pmMessageFromTo(user2, [user1], { id: 14 }), + eg.pmMessageFromTo(user2, [user1], { id: 15 }), + eg.pmMessageFromTo(user2, [user1, user3], { id: 21 }), + eg.pmMessageFromTo(user2, [user1, user3], { id: 22 }), + eg.pmMessageFromTo(user4, [user1, user5], { id: 23 }), + eg.pmMessageFromTo(user4, [user1, user5], { id: 24 }), + eg.pmMessageFromTo(user4, [user1, user5], { id: 25 }), + ]) { + state = reducer(state, mkMessageAction(message)); + } + return state; +})(); diff --git a/src/unread/__tests__/unreadModel-test.js b/src/unread/__tests__/unreadModel-test.js new file mode 100644 index 00000000000..8bfc56ff1b2 --- /dev/null +++ b/src/unread/__tests__/unreadModel-test.js @@ -0,0 +1,193 @@ +/* @flow strict-local */ +import Immutable from 'immutable'; + +import { ACCOUNT_SWITCH, EVENT_UPDATE_MESSAGE_FLAGS } from '../../actionConstants'; +import { reducer } from '../unreadModel'; +import * as eg from '../../__tests__/lib/exampleData'; +import { initialState, mkMessageAction } from './unread-testlib'; + +// These are the tests corresponding to unreadStreamsReducer-test.js. +// Ultimately we'll want to flip this way of organizing the tests, and +// test the whole model together rather than streams/mentions/etc.; +// but this way simplifies the conversion from the old tests. +describe('stream substate', () => { + // Summarize the state, for convenient comparison to expectations. + // In particular, abstract away irrelevant details of the ordering of + // streams and topics in the data structure -- those should never matter + // to selectors, and in a better data structure they wouldn't exist in the + // first place. + const summary = state => { + // prettier-ignore + const result: Immutable.Map> = + Immutable.Map().asMutable(); + for (const { stream_id, topic, unread_message_ids } of state.streams) { + result.setIn([stream_id, topic], unread_message_ids); + } + return result.asImmutable(); + }; + + describe('ACCOUNT_SWITCH', () => { + test('resets state to initial state', () => { + const state = reducer(initialState, mkMessageAction(eg.streamMessage())); + expect(state).not.toEqual(initialState); + + const action = { type: ACCOUNT_SWITCH, index: 1 }; + expect(reducer(state, action)).toEqual(initialState); + }); + }); + + describe('REALM_INIT', () => { + test('received data from "unread_msgs.streams" key replaces the current state ', () => { + const message1 = eg.streamMessage({ id: 1 }); + + const action = { + ...eg.action.realm_init, + data: { + ...eg.action.realm_init.data, + unread_msgs: { + ...eg.action.realm_init.data.unread_msgs, + streams: [ + { + stream_id: message1.stream_id, + topic: message1.subject, + unread_message_ids: [message1.id, 2], + }, + ], + huddles: [], + pms: [], + mentions: [message1.id, 2, 3], + }, + }, + }; + + // prettier-ignore + expect(summary(reducer(initialState, action))).toEqual(Immutable.Map([ + [message1.stream_id, Immutable.Map([[message1.subject, [1, 2]]])], + ])); + }); + }); + + describe('EVENT_NEW_MESSAGE', () => { + const action = mkMessageAction; + + const baseState = (() => { + let state = initialState; + state = reducer(state, action(eg.streamMessage({ id: 1, subject: 'some topic' }))); + return state; + })(); + + test('(base state, for comparison)', () => { + // prettier-ignore + expect(summary(baseState)).toEqual(Immutable.Map([ + [eg.stream.stream_id, Immutable.Map([['some topic', [1]]])], + ])); + }); + + test('if message id already exists, do not mutate state', () => { + const state = reducer(baseState, action(eg.streamMessage({ id: 1, subject: 'some topic' }))); + expect(state).toBe(baseState); + }); + + test('if message is not stream, return original state', () => { + const state = reducer(baseState, action(eg.pmMessage({ id: 4 }))); + expect(state.streams).toBe(baseState.streams); + }); + + test('if message is sent by self, do not mutate state', () => { + const state = reducer(baseState, action(eg.streamMessage({ sender: eg.selfUser }))); + expect(state).toBe(baseState); + }); + + test('if message id does not exist, append to state', () => { + const state = reducer(baseState, action(eg.streamMessage({ id: 4, subject: 'some topic' }))); + // prettier-ignore + expect(summary(state)).toEqual(Immutable.Map([ + [eg.stream.stream_id, Immutable.Map([['some topic', [1, 4]]])], + ])); + }); + + test('known stream, new topic', () => { + const message = eg.streamMessage({ id: 4, subject: 'another topic' }); + const state = reducer(baseState, action(message)); + // prettier-ignore + expect(summary(state)).toEqual(Immutable.Map([ + [eg.stream.stream_id, Immutable.Map([ + ['some topic', [1]], + ['another topic', [4]], + ])], + ])); + }); + + test('if stream with topic does not exist, append to state', () => { + const message = eg.streamMessage({ id: 4, stream_id: 2, subject: 'another topic' }); + const state = reducer(baseState, action(message)); + // prettier-ignore + expect(summary(state)).toEqual(Immutable.Map([ + [eg.stream.stream_id, Immutable.Map([['some topic', [1]]])], + [message.stream_id, Immutable.Map([['another topic', [4]]])], + ])); + }); + }); + + describe('EVENT_UPDATE_MESSAGE_FLAGS', () => { + const mkAction = args => { + const { all = false, messages, flag = 'read', operation = 'add' } = args; + return { + id: 1, + type: EVENT_UPDATE_MESSAGE_FLAGS, + allMessages: {}, + all, + messages, + flag, + operation, + }; + }; + + const baseState = (() => { + const streamAction = args => mkMessageAction(eg.streamMessage(args)); + let state = initialState; + state = reducer(state, streamAction({ stream_id: 123, subject: 'foo', id: 1 })); + state = reducer(state, streamAction({ stream_id: 123, subject: 'foo', id: 2 })); + state = reducer(state, streamAction({ stream_id: 123, subject: 'foo', id: 3 })); + state = reducer(state, streamAction({ stream_id: 234, subject: 'bar', id: 4 })); + state = reducer(state, streamAction({ stream_id: 234, subject: 'bar', id: 5 })); + return state; + })(); + + test('(base state, for comparison)', () => { + // prettier-ignore + expect(summary(baseState)).toEqual(Immutable.Map([ + [123, Immutable.Map([['foo', [1, 2, 3]]])], + [234, Immutable.Map([['bar', [4, 5]]])], + ])); + }); + + test('when operation is "add" but flag is not "read" do not mutate state', () => { + const action = mkAction({ messages: [1, 2, 3], flag: 'star' }); + expect(reducer(initialState, action)).toBe(initialState); + }); + + test('if id does not exist do not mutate state', () => { + const action = mkAction({ messages: [6, 7] }); + expect(reducer(baseState, action)).toBe(baseState); + }); + + test('if ids are in state remove them', () => { + const action = mkAction({ messages: [3, 4, 5, 6] }); + // prettier-ignore + expect(summary(reducer(baseState, action))).toEqual(Immutable.Map([ + [123, Immutable.Map([['foo', [1, 2]]])], + ])); + }); + + test('when operation is "remove" do nothing', () => { + const action = mkAction({ messages: [1, 2], operation: 'remove' }); + expect(reducer(baseState, action)).toBe(baseState); + }); + + test('when "all" is true reset state', () => { + const action = mkAction({ messages: [], all: true }); + expect(reducer(baseState, action).streams).toBe(initialState.streams); + }); + }); +}); diff --git a/src/unread/__tests__/unreadSelectors-test.js b/src/unread/__tests__/unreadSelectors-test.js index d183260519b..35590e29e26 100644 --- a/src/unread/__tests__/unreadSelectors-test.js +++ b/src/unread/__tests__/unreadSelectors-test.js @@ -1,5 +1,6 @@ import deepFreeze from 'deep-freeze'; +import { reducer } from '../unreadModel'; import { getUnreadByStream, getUnreadStreamTotal, @@ -13,55 +14,14 @@ import { getUnreadStreamsAndTopicsSansMuted, } from '../unreadSelectors'; -const unreadStreamData = [ - { - stream_id: 0, - topic: 'a topic', - unread_message_ids: [1, 2, 3], - }, - { - stream_id: 0, - topic: 'another topic', - unread_message_ids: [4, 5], - }, - { - stream_id: 2, - topic: 'some other topic', - unread_message_ids: [6, 7], - }, -]; - -const unreadPmsData = [ - { - sender_id: 0, - unread_message_ids: [1, 2], - }, - { - sender_id: 2, - unread_message_ids: [3, 4, 5], - }, -]; - -const unreadHuddlesData = [ - { - user_ids_string: '1,2,3', - unread_message_ids: [1, 2], - }, - { - user_ids_string: '4,5', - unread_message_ids: [3, 4, 5], - }, -]; - -const unreadMentionsData = [1, 2, 3]; +import * as eg from '../../__tests__/lib/exampleData'; +import { initialState, mkMessageAction, selectorBaseState as unreadState } from './unread-testlib'; describe('getUnreadByStream', () => { test('when no items in streams key, the result is an empty object', () => { const state = deepFreeze({ subscriptions: [], - unread: { - streams: [], - }, + unread: initialState, }); const unreadByStream = getUnreadByStream(state); @@ -69,7 +29,7 @@ describe('getUnreadByStream', () => { expect(unreadByStream).toEqual({}); }); - test('when there are unread stream messages, returns a list with counts per stream_id ', () => { + test('when there are unread stream messages, returns their counts', () => { const state = deepFreeze({ subscriptions: [ { @@ -83,9 +43,7 @@ describe('getUnreadByStream', () => { in_home_view: true, }, ], - unread: { - streams: unreadStreamData, - }, + unread: unreadState, mute: [['stream 0', 'a topic']], }); @@ -98,9 +56,7 @@ describe('getUnreadByStream', () => { describe('getUnreadStreamTotal', () => { test('when no items in "streams" key, there are unread message', () => { const state = deepFreeze({ - unread: { - streams: [], - }, + unread: initialState, subscriptions: [], mute: [], }); @@ -112,9 +68,7 @@ describe('getUnreadStreamTotal', () => { test('count all the unread messages listed in "streams" key', () => { const state = deepFreeze({ - unread: { - streams: unreadStreamData, - }, + unread: unreadState, subscriptions: [ { stream_id: 0, @@ -141,9 +95,7 @@ describe('getUnreadStreamTotal', () => { describe('getUnreadByPms', () => { test('when no items in streams key, the result is an empty array', () => { const state = deepFreeze({ - unread: { - pms: [], - }, + unread: initialState, }); const unreadByStream = getUnreadByPms(state); @@ -153,9 +105,7 @@ describe('getUnreadByPms', () => { test('when there are unread private messages, returns counts by sender_id', () => { const state = deepFreeze({ - unread: { - pms: unreadPmsData, - }, + unread: unreadState, }); const unreadByStream = getUnreadByPms(state); @@ -167,9 +117,7 @@ describe('getUnreadByPms', () => { describe('getUnreadPmsTotal', () => { test('when no items in "pms" key, there are unread private messages', () => { const state = deepFreeze({ - unread: { - pms: [], - }, + unread: initialState, }); const unreadCount = getUnreadPmsTotal(state); @@ -179,9 +127,7 @@ describe('getUnreadPmsTotal', () => { test('when there are keys in "pms", sum up all unread private message counts', () => { const state = deepFreeze({ - unread: { - pms: unreadPmsData, - }, + unread: unreadState, }); const unreadCount = getUnreadPmsTotal(state); @@ -193,9 +139,7 @@ describe('getUnreadPmsTotal', () => { describe('getUnreadByHuddles', () => { test('when no items in streams key, the result is an empty array', () => { const state = deepFreeze({ - unread: { - huddles: [], - }, + unread: initialState, }); const unreadByStream = getUnreadByHuddles(state); @@ -205,23 +149,19 @@ describe('getUnreadByHuddles', () => { test('when there are unread stream messages, returns a ', () => { const state = deepFreeze({ - unread: { - huddles: unreadHuddlesData, - }, + unread: unreadState, }); const unreadByStream = getUnreadByHuddles(state); - expect(unreadByStream).toEqual({ '1,2,3': 2, '4,5': 3 }); + expect(unreadByStream).toEqual({ '1,2,3': 2, '1,4,5': 3 }); }); }); describe('getUnreadHuddlesTotal', () => { test('when no items in "huddles" key, there are unread group messages', () => { const state = deepFreeze({ - unread: { - huddles: [], - }, + unread: initialState, }); const unreadCount = getUnreadHuddlesTotal(state); @@ -231,9 +171,7 @@ describe('getUnreadHuddlesTotal', () => { test('when there are keys in "huddles", sum up all unread group message counts', () => { const state = deepFreeze({ - unread: { - huddles: unreadHuddlesData, - }, + unread: unreadState, }); const unreadCount = getUnreadHuddlesTotal(state); @@ -245,9 +183,7 @@ describe('getUnreadHuddlesTotal', () => { describe('getUnreadMentionsTotal', () => { test('unread mentions count is equal to the unread array length', () => { const state = deepFreeze({ - unread: { - mentions: [1, 2, 3], - }, + unread: unreadState, }); const unreadCount = getUnreadMentionsTotal(state); @@ -259,12 +195,7 @@ describe('getUnreadMentionsTotal', () => { describe('getUnreadTotal', () => { test('if no key has any items then no unread messages', () => { const state = deepFreeze({ - unread: { - streams: [], - pms: [], - huddles: [], - mentions: [], - }, + unread: initialState, subscriptions: [], mute: [], }); @@ -276,12 +207,7 @@ describe('getUnreadTotal', () => { test('calculates total unread of streams + pms + huddles', () => { const state = deepFreeze({ - unread: { - streams: unreadStreamData, - pms: unreadPmsData, - huddles: unreadHuddlesData, - mentions: unreadMentionsData, - }, + unread: unreadState, subscriptions: [ { stream_id: 0, @@ -309,9 +235,7 @@ describe('getUnreadStreamsAndTopics', () => { test('if no key has any items then no unread messages', () => { const state = deepFreeze({ subscriptions: [], - unread: { - streams: [], - }, + unread: initialState, }); const unreadCount = getUnreadStreamsAndTopics(state); @@ -335,9 +259,7 @@ describe('getUnreadStreamsAndTopics', () => { in_home_view: false, }, ], - unread: { - streams: unreadStreamData, - }, + unread: unreadState, mute: [], }); @@ -402,9 +324,7 @@ describe('getUnreadStreamsAndTopics', () => { in_home_view: true, }, ], - unread: { - streams: unreadStreamData, - }, + unread: unreadState, mute: [['stream 0', 'a topic']], }); @@ -470,9 +390,7 @@ describe('getUnreadStreamsAndTopics', () => { in_home_view: true, }, ], - unread: { - streams: unreadStreamData, - }, + unread: unreadState, mute: [], }); @@ -543,40 +461,19 @@ describe('getUnreadStreamsAndTopics', () => { pin_to_top: false, }, ], - unread: { - streams: [ - { - stream_id: 0, - topic: 'z topic', - unread_message_ids: [1, 2, 3], - }, - { - stream_id: 0, - topic: 'a topic', - unread_message_ids: [4, 5], - }, - { - stream_id: 2, - topic: 'b topic', - unread_message_ids: [6, 7], - }, - { - stream_id: 2, - topic: 'c topic', - unread_message_ids: [7, 8], - }, - { - stream_id: 1, - topic: 'e topic', - unread_message_ids: [10], - }, - { - stream_id: 1, - topic: 'd topic', - unread_message_ids: [9], - }, - ], - }, + unread: [ + eg.streamMessage({ stream_id: 0, subject: 'z topic', id: 1 }), + eg.streamMessage({ stream_id: 0, subject: 'z topic', id: 2 }), + eg.streamMessage({ stream_id: 0, subject: 'z topic', id: 3 }), + eg.streamMessage({ stream_id: 0, subject: 'a topic', id: 4 }), + eg.streamMessage({ stream_id: 0, subject: 'a topic', id: 5 }), + eg.streamMessage({ stream_id: 2, subject: 'b topic', id: 6 }), + eg.streamMessage({ stream_id: 2, subject: 'b topic', id: 7 }), + eg.streamMessage({ stream_id: 2, subject: 'c topic', id: 7 }), + eg.streamMessage({ stream_id: 2, subject: 'c topic', id: 8 }), + eg.streamMessage({ stream_id: 1, subject: 'e topic', id: 10 }), + eg.streamMessage({ stream_id: 1, subject: 'd topic', id: 9 }), + ].reduce((st, message) => reducer(st, mkMessageAction(message)), initialState), mute: [['def stream', 'c topic']], }); @@ -643,9 +540,7 @@ describe('getUnreadStreamsAndTopicsSansMuted', () => { in_home_view: false, }, ], - unread: { - streams: unreadStreamData, - }, + unread: unreadState, mute: [], }); @@ -664,9 +559,7 @@ describe('getUnreadStreamsAndTopicsSansMuted', () => { in_home_view: true, }, ], - unread: { - streams: unreadStreamData, - }, + unread: unreadState, mute: [['stream 0', 'a topic']], }); diff --git a/src/unread/__tests__/unreadStreamsReducer-test.js b/src/unread/__tests__/unreadStreamsReducer-test.js deleted file mode 100644 index 548694bbca5..00000000000 --- a/src/unread/__tests__/unreadStreamsReducer-test.js +++ /dev/null @@ -1,326 +0,0 @@ -/* @flow strict-local */ -import deepFreeze from 'deep-freeze'; - -import unreadStreamsReducer from '../unreadStreamsReducer'; -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', () => { - test('resets state to initial state', () => { - const initialState = deepFreeze([ - { - stream_id: 1, - topic: 'some topic', - unread_message_ids: [1, 2, 3], - }, - ]); - - const action = deepFreeze({ - type: ACCOUNT_SWITCH, - index: 1, - }); - - const expectedState = []; - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toEqual(expectedState); - }); - }); - - describe('REALM_INIT', () => { - test('received data from "unread_msgs.streams" key replaces the current state ', () => { - const initialState = deepFreeze([]); - - const message1 = eg.streamMessage({ id: 1 }); - - const action = deepFreeze({ - ...eg.action.realm_init, - data: { - ...eg.action.realm_init.data, - unread_msgs: { - ...eg.action.realm_init.data.unread_msgs, - streams: [ - { - stream_id: message1.stream_id, - topic: message1.subject, - unread_message_ids: [message1.id, 2], - }, - ], - huddles: [], - pms: [], - mentions: [message1.id, 2, 3], - }, - }, - }); - - const expectedState = [ - { - stream_id: message1.stream_id, - topic: message1.subject, - unread_message_ids: [message1.id, 2], - }, - ]; - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toEqual(expectedState); - }); - }); - - 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: message1.stream_id, - topic: message1.subject, - unread_message_ids: [message1.id], - }, - ]); - - const action = deepFreeze({ - ...eg.eventNewMessageActionBase, - message: message1, - }); - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toBe(initialState); - }); - - test('if message is not stream, return original state', () => { - const message4 = eg.pmMessage({ id: 4 }); - const initialState = deepFreeze([ - { - stream_id: message4.stream_id, - topic: message4.subject, - unread_message_ids: [1, 2, 3], - }, - ]); - - const action = deepFreeze({ - ...eg.eventNewMessageActionBase, - message: message4, - }); - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toBe(initialState); - }); - - test('if message is sent by self, do not mutate state', () => { - const initialState = deepFreeze([]); - const message1 = eg.streamMessage({ sender: eg.selfUser }); - - const action = deepFreeze({ - ...eg.eventNewMessageActionBase, - message: message1, - ownUser: eg.selfUser, - }); - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toBe(initialState); - }); - - test('if message id does not exist, append to state', () => { - const message4 = eg.streamMessage({ id: 4 }); - - const initialState = deepFreeze([ - { - stream_id: message4.stream_id, - topic: message4.subject, - unread_message_ids: [1, 2, 3], - }, - ]); - - const action = deepFreeze({ - ...eg.eventNewMessageActionBase, - message: message4, - }); - - const expectedState = [ - { - stream_id: message4.stream_id, - topic: message4.subject, - unread_message_ids: [1, 2, 3, message4.id], - }, - ]; - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toEqual(expectedState); - }); - - 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, - topic: 'some topic', - unread_message_ids: [1, 2, 3], - }, - ]); - - const action = deepFreeze({ - ...eg.eventNewMessageActionBase, - message: message4, - }); - - const expectedState = [ - { - stream_id: 1, - topic: 'some topic', - unread_message_ids: [1, 2, 3], - }, - { - stream_id: message4.stream_id, - topic: message4.subject, - unread_message_ids: [message4.id], - }, - ]; - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toEqual(expectedState); - }); - }); - - describe('EVENT_UPDATE_MESSAGE_FLAGS', () => { - test('when operation is "add" but flag is not "read" do not mutate state', () => { - const initialState = deepFreeze([]); - - const action = { - id: 1, - type: EVENT_UPDATE_MESSAGE_FLAGS, - all: false, - allMessages: {}, - messages: [1, 2, 3], - flag: 'star', - operation: 'add', - }; - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toBe(initialState); - }); - - test('if id does not exist do not mutate state', () => { - const initialState = deepFreeze([ - { - stream_id: 1, - topic: 'some topic', - unread_message_ids: [1, 2, 3, 4, 5], - }, - { - stream_id: 2, - topic: 'another topic', - unread_message_ids: [4, 5], - }, - ]); - - const action = deepFreeze({ - id: 1, - type: EVENT_UPDATE_MESSAGE_FLAGS, - all: false, - allMessages: {}, - messages: [6, 7], - flag: 'read', - operation: 'add', - }); - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toBe(initialState); - }); - - test('if ids are in state remove them', () => { - const initialState = deepFreeze([ - { - stream_id: 1, - topic: 'some topic', - unread_message_ids: [1, 2, 3], - }, - { - stream_id: 2, - topic: 'another topic', - unread_message_ids: [4, 5], - }, - ]); - - const action = deepFreeze({ - id: 1, - type: EVENT_UPDATE_MESSAGE_FLAGS, - all: false, - allMessages: {}, - messages: [3, 4, 5, 6], - flag: 'read', - operation: 'add', - }); - - const expectedState = [ - { - stream_id: 1, - topic: 'some topic', - unread_message_ids: [1, 2], - }, - ]; - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toEqual(expectedState); - }); - - test('when operation is "remove" do nothing', () => { - const initialState = deepFreeze([ - { - stream_id: 1, - topic: 'some topic', - unread_message_ids: [1, 2, 3, 4, 5], - }, - ]); - - const action = deepFreeze({ - id: 1, - type: EVENT_UPDATE_MESSAGE_FLAGS, - all: false, - allMessages: {}, - messages: [1, 2], - flag: 'read', - operation: 'remove', - }); - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toBe(initialState); - }); - - test('when "all" is true reset state', () => { - const initialState = deepFreeze([ - { - stream_id: 1, - topic: 'some topic', - unread_message_ids: [1, 2, 3, 4, 5], - }, - ]); - - const action = deepFreeze({ - id: 1, - type: EVENT_UPDATE_MESSAGE_FLAGS, - all: true, - allMessages: {}, - messages: [], - flag: 'read', - operation: 'add', - }); - - const actualState = unreadStreamsReducer(initialState, action); - - expect(actualState).toBe(NULL_ARRAY); - }); - }); -}); diff --git a/src/unread/unreadHuddlesReducer.js b/src/unread/unreadHuddlesReducer.js index a8fe4dbae4f..d2ddcfb7c1c 100644 --- a/src/unread/unreadHuddlesReducer.js +++ b/src/unread/unreadHuddlesReducer.js @@ -1,5 +1,6 @@ /* @flow strict-local */ -import type { UnreadHuddlesState, Action } from '../types'; +import type { Action } from '../types'; +import type { UnreadHuddlesState } from './unreadModelTypes'; import { REALM_INIT, LOGOUT, diff --git a/src/unread/unreadMentionsReducer.js b/src/unread/unreadMentionsReducer.js index 932ad7d13fc..0c1b888e3e0 100644 --- a/src/unread/unreadMentionsReducer.js +++ b/src/unread/unreadMentionsReducer.js @@ -1,5 +1,6 @@ /* @flow strict-local */ -import type { UnreadMentionsState, Action } from '../types'; +import type { Action } from '../types'; +import type { UnreadMentionsState } from './unreadModelTypes'; import { REALM_INIT, LOGOUT, diff --git a/src/unread/unreadModel.js b/src/unread/unreadModel.js new file mode 100644 index 00000000000..c7f5a19b7a5 --- /dev/null +++ b/src/unread/unreadModel.js @@ -0,0 +1,31 @@ +/* @flow strict-local */ +import { combineReducers, type Reducer } from 'redux'; + +import type { Action } from '../actionTypes'; +import type { + UnreadState, + UnreadPmsState, + UnreadHuddlesState, + UnreadMentionsState, + UnreadStreamsState, +} from './unreadModelTypes'; +import type { GlobalState } from '../reduxTypes'; +import unreadStreamsReducer from './unreadStreamsReducer'; +import unreadPmsReducer from './unreadPmsReducer'; +import unreadHuddlesReducer from './unreadHuddlesReducer'; +import unreadMentionsReducer from './unreadMentionsReducer'; + +export const getUnreadStreams = (state: GlobalState): UnreadStreamsState => state.unread.streams; + +export const getUnreadPms = (state: GlobalState): UnreadPmsState => state.unread.pms; + +export const getUnreadHuddles = (state: GlobalState): UnreadHuddlesState => state.unread.huddles; + +export const getUnreadMentions = (state: GlobalState): UnreadMentionsState => state.unread.mentions; + +export const reducer: Reducer = combineReducers({ + streams: unreadStreamsReducer, + pms: unreadPmsReducer, + huddles: unreadHuddlesReducer, + mentions: unreadMentionsReducer, +}); diff --git a/src/unread/unreadModelTypes.js b/src/unread/unreadModelTypes.js new file mode 100644 index 00000000000..e495bce9203 --- /dev/null +++ b/src/unread/unreadModelTypes.js @@ -0,0 +1,23 @@ +/* @flow strict-local */ + +import type { HuddlesUnreadItem, PmsUnreadItem, StreamUnreadItem } from '../api/apiTypes'; + +// These four are fragments of UnreadState; see below. +export type UnreadStreamsState = StreamUnreadItem[]; +export type UnreadHuddlesState = HuddlesUnreadItem[]; +export type UnreadPmsState = PmsUnreadItem[]; +export type UnreadMentionsState = number[]; + +/** + * A summary of (almost) all unread messages, even those we don't have. + * + * The initial version the server gives us for this data is `unread_msgs` in + * the `/register` initial state, and we largely follow the structure of + * that. See there (in `src/api/initialDataTypes.js`) for details. + */ +export type UnreadState = {| + streams: UnreadStreamsState, + huddles: UnreadHuddlesState, + pms: UnreadPmsState, + mentions: UnreadMentionsState, +|}; diff --git a/src/unread/unreadPmsReducer.js b/src/unread/unreadPmsReducer.js index ab3426cc1bb..9ab7d9b84cb 100644 --- a/src/unread/unreadPmsReducer.js +++ b/src/unread/unreadPmsReducer.js @@ -1,5 +1,6 @@ /* @flow strict-local */ -import type { UnreadPmsState, Action } from '../types'; +import type { Action } from '../types'; +import type { UnreadPmsState } from './unreadModelTypes'; import { REALM_INIT, LOGOUT, diff --git a/src/unread/unreadSelectors.js b/src/unread/unreadSelectors.js index 2939672a71d..1e524c82864 100644 --- a/src/unread/unreadSelectors.js +++ b/src/unread/unreadSelectors.js @@ -3,20 +3,14 @@ import { createSelector } from 'reselect'; import type { Narrow, Selector, UnreadStreamItem } from '../types'; import { caseInsensitiveCompareFunc } from '../utils/misc'; -import { - getMute, - getStreams, - getUnreadStreams, - getUnreadPms, - getUnreadHuddles, - getUnreadMentions, -} from '../directSelectors'; +import { getMute, getStreams } from '../directSelectors'; import { getOwnUserId } from '../users/userSelectors'; import { getSubscriptionsById } from '../subscriptions/subscriptionSelectors'; import { isTopicMuted } from '../utils/message'; import { caseNarrow } from '../utils/narrow'; import { NULL_SUBSCRIPTION } from '../nullObjects'; import { pmUnreadsKeyFromPmKeyIds } from '../utils/recipient'; +import { getUnreadStreams, getUnreadPms, getUnreadHuddles, getUnreadMentions } from './unreadModel'; /** The number of unreads in each stream, excluding muted topics, by stream ID. */ export const getUnreadByStream: Selector<{ [number]: number }> = createSelector( diff --git a/src/unread/unreadStreamsReducer.js b/src/unread/unreadStreamsReducer.js index 1a34ace84d8..35036c7283c 100644 --- a/src/unread/unreadStreamsReducer.js +++ b/src/unread/unreadStreamsReducer.js @@ -1,5 +1,6 @@ /* @flow strict-local */ -import type { UnreadStreamsState, Action } from '../types'; +import type { Action } from '../types'; +import type { UnreadStreamsState } from './unreadModelTypes'; import { REALM_INIT, LOGOUT,