unreads: Don't add messages with "read" flag to unreads.

Previously, the main case where we wouldn't add a message to unreads was
if we were the one who sent it. This check was not correct - instead, we
want to check to see if the message has the "read" flag, which is set by
the server.

In the past, these checks were very close to the same (with the
exception of "Notification Bot" announcing a stream that you created,
which would be "read" for you), but with the addition of muted users,
the check is now incorrect.

This commit removes the logic to not mark a message as read when we are
the one who sent it, and instead just looks at the server flag, since
that should be the canonical place for that information to be.

src/unread/__tests__/unreadHuddlesReducer-test.js

@@ -118,16 +118,17 @@ 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) };
 
       const initialState = deepFreeze([]);
 
       const message2 = eg.pmMessage({
-        sender: selfUser,
+        sender: user2,
         recipients: [selfUser, user2, user3],
+        flags: ['read'],
       });
 
       const action = deepFreeze({

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]);
 

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);

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({

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;
   }
 

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])

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;
       }
 

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;
   }