diff --git a/src/unread/__tests__/unreadHuddlesReducer-test.js b/src/unread/__tests__/unreadHuddlesReducer-test.js index 6d2fa1042fd..0f9a28e6172 100644 --- a/src/unread/__tests__/unreadHuddlesReducer-test.js +++ b/src/unread/__tests__/unreadHuddlesReducer-test.js @@ -118,7 +118,7 @@ describe('unreadHuddlesReducer', () => { expect(actualState).toBe(initialState); }); - test('if message is sent by self, do not mutate state', () => { + test('if message has "read" flag, do not mutate state', () => { const selfUser = { ...eg.selfUser, user_id: makeUserId(1) }; const user2 = { ...eg.otherUser, user_id: makeUserId(2) }; const user3 = { ...eg.thirdUser, user_id: makeUserId(3) }; @@ -126,8 +126,9 @@ describe('unreadHuddlesReducer', () => { const initialState = deepFreeze([]); const message2 = eg.pmMessage({ - sender: selfUser, + sender: user2, recipients: [selfUser, user2, user3], + flags: ['read'], }); const action = deepFreeze({ diff --git a/src/unread/__tests__/unreadMentionsReducer-test.js b/src/unread/__tests__/unreadMentionsReducer-test.js index 239ea6d7cef..7ce7311591a 100644 --- a/src/unread/__tests__/unreadMentionsReducer-test.js +++ b/src/unread/__tests__/unreadMentionsReducer-test.js @@ -67,6 +67,22 @@ describe('unreadMentionsReducer', () => { expect(actualState).toBe(initialState); }); + test('if message has "read" flag, do not mutate state', () => { + const initialState = deepFreeze([]); + + const action = deepFreeze({ + type: EVENT_NEW_MESSAGE, + message: { + id: 2, + flags: ['mentioned', 'read'], + }, + }); + + const actualState = unreadMentionsReducer(initialState, action); + + expect(actualState).toBe(initialState); + }); + test('if message id already exists, do not mutate state', () => { const initialState = deepFreeze([1, 2]); diff --git a/src/unread/__tests__/unreadModel-test.js b/src/unread/__tests__/unreadModel-test.js index 0cc91bad201..9aa3227ba4b 100644 --- a/src/unread/__tests__/unreadModel-test.js +++ b/src/unread/__tests__/unreadModel-test.js @@ -84,10 +84,10 @@ describe('stream substate', () => { expect(state.streams).toBe(baseState.streams); }); - test('if message is sent by self, do not mutate state', () => { + test('if message has "read" flag, do not mutate state', () => { const state = reducer( baseState, - action(eg.streamMessage({ sender: eg.selfUser })), + action(eg.streamMessage({ sender: eg.selfUser, flags: ['read'] })), eg.plusReduxState, ); expect(state).toBe(baseState); diff --git a/src/unread/__tests__/unreadPmsReducer-test.js b/src/unread/__tests__/unreadPmsReducer-test.js index a571c0c27c6..16f14edfc27 100644 --- a/src/unread/__tests__/unreadPmsReducer-test.js +++ b/src/unread/__tests__/unreadPmsReducer-test.js @@ -111,11 +111,12 @@ describe('unreadPmsReducer', () => { expect(actualState).toBe(initialState); }); - test('if message is sent by self, do not mutate state', () => { + test('if message is marked read, do not mutate state', () => { const initialState = deepFreeze([]); const message1 = eg.pmMessage({ - sender: eg.selfUser, + sender: eg.otherUser, recipients: [eg.otherUser, eg.selfUser], + flags: ['read'], }); const action = deepFreeze({ diff --git a/src/unread/unreadHuddlesReducer.js b/src/unread/unreadHuddlesReducer.js index efa06de2820..c0f3799c00b 100644 --- a/src/unread/unreadHuddlesReducer.js +++ b/src/unread/unreadHuddlesReducer.js @@ -24,7 +24,14 @@ const eventNewMessage = (state, action) => { return state; } - if (action.ownUserId === action.message.sender_id) { + // TODO: In reality, we should error if `flags` is undefined, since it's + // always supposed to be set. However, our tests currently don't pass flags + // into these events, making it annoying to fix this. We should fix the + // tests, then change this to error if `flags` is undefined. See [1] for + // details. + // + // [1]: https://github.com/zulip/zulip-mobile/pull/4710/files#r627850775 + if (action.message.flags?.includes('read')) { return state; } diff --git a/src/unread/unreadMentionsReducer.js b/src/unread/unreadMentionsReducer.js index 60024d1b027..37bdf29cdcd 100644 --- a/src/unread/unreadMentionsReducer.js +++ b/src/unread/unreadMentionsReducer.js @@ -46,6 +46,9 @@ export default (state: UnreadMentionsState = initialState, action: Action): Unre if (!flags) { throw new Error('action.message.flags should be defined.'); } + if (flags.includes('read')) { + return state; + } return (flags.includes('mentioned') || flags.includes('wildcard_mentioned')) && !state.includes(action.message.id) ? addItemsToArray(state, [action.message.id]) diff --git a/src/unread/unreadModel.js b/src/unread/unreadModel.js index f8c4deac1e0..59ac8be670b 100644 --- a/src/unread/unreadModel.js +++ b/src/unread/unreadModel.js @@ -22,7 +22,6 @@ import { MESSAGE_FETCH_COMPLETE, REALM_INIT, } from '../actionConstants'; -import { getOwnUserId } from '../users/userSelectors'; // // @@ -146,7 +145,14 @@ function streamsReducer( return state; } - if (message.sender_id === getOwnUserId(globalState)) { + // TODO: In reality, we should error if `flags` is undefined, since it's + // always supposed to be set. However, our tests currently don't pass flags + // into these events, making it annoying to fix this. We should fix the + // tests, then change this to error if `flags` is undefined. See [1] for + // details. + // + // [1]: https://github.com/zulip/zulip-mobile/pull/4710/files#r627850775 + if (message.flags?.includes('read')) { return state; } diff --git a/src/unread/unreadPmsReducer.js b/src/unread/unreadPmsReducer.js index 6562789fc7a..2d4567a61d5 100644 --- a/src/unread/unreadPmsReducer.js +++ b/src/unread/unreadPmsReducer.js @@ -24,7 +24,14 @@ const eventNewMessage = (state, action) => { return state; } - if (action.ownUserId === action.message.sender_id) { + // TODO: In reality, we should error if `flags` is undefined, since it's + // always supposed to be set. However, our tests currently don't pass flags + // into these events, making it annoying to fix this. We should fix the + // tests, then change this to error if `flags` is undefined. See [1] for + // details. + // + // [1]: https://github.com/zulip/zulip-mobile/pull/4710/files#r627850775 + if (action.message.flags?.includes('read')) { return state; }