Make new mentioned/starred messages appear in mentions/starred narrows

src/chat/__tests__/narrowsReducer-test.js

@@ -37,7 +37,7 @@ describe('narrowsReducer', () => {
 
       const action = deepFreeze({
         type: EVENT_NEW_MESSAGE,
-        message: { id: 3, display_recipient: 'some stream', subject: 'some topic' },
+        message: { id: 3, display_recipient: 'some stream', subject: 'some topic', flags: [] },
         caughtUp: {},
       });
 
@@ -57,7 +57,7 @@ describe('narrowsReducer', () => {
 
       const action = deepFreeze({
         type: EVENT_NEW_MESSAGE,
-        message: { id: 3 },
+        message: { id: 3, flags: [] },
         caughtUp: {
           [HOME_NARROW_STR]: {
             older: false,
@@ -83,7 +83,13 @@ describe('narrowsReducer', () => {
 
       const action = deepFreeze({
         type: EVENT_NEW_MESSAGE,
-        message: { id: 3, type: 'stream', display_recipient: 'stream', subject: 'topic' },
+        message: {
+          id: 3,
+          type: 'stream',
+          display_recipient: 'stream',
+          subject: 'topic',
+          flags: [],
+        },
         caughtUp: {},
       });
 
@@ -104,6 +110,7 @@ describe('narrowsReducer', () => {
       id: 1,
       type: 'private',
       display_recipient: [{ email: 'me@example.com' }, { email: 'a@a.com' }, { email: 'b@b.com' }],
+      flags: [],
     };
     const action = deepFreeze({
       type: EVENT_NEW_MESSAGE,
@@ -132,6 +139,7 @@ describe('narrowsReducer', () => {
       type: 'stream',
       display_recipient: 'some stream',
       subject: 'some topic',
+      flags: [],
     };
     const action = deepFreeze({
       type: EVENT_NEW_MESSAGE,
@@ -167,6 +175,7 @@ describe('narrowsReducer', () => {
     const message = {
       id: 1,
       display_recipient: [{ email: 'me@example.com' }],
+      flags: [],
     };
     const action = deepFreeze({
       type: EVENT_NEW_MESSAGE,
@@ -204,6 +213,7 @@ describe('narrowsReducer', () => {
       type: 'stream',
       display_recipient: 'some stream',
       subject: 'some topic',
+      flags: [],
     };
     const action = deepFreeze({
       type: EVENT_NEW_MESSAGE,
@@ -240,6 +250,7 @@ describe('narrowsReducer', () => {
       type: 'stream',
       display_recipient: 'stream name',
       subject: 'some topic',
+      flags: [],
     };
     const action = deepFreeze({
       type: EVENT_NEW_MESSAGE,
@@ -274,6 +285,7 @@ describe('narrowsReducer', () => {
       type: 'private',
       sender_email: 'someone@example.com',
       display_recipient: [{ email: 'me@example.com' }, { email: 'mark@example.com' }],
+      flags: [],
     };
     const action = deepFreeze({
       type: EVENT_NEW_MESSAGE,

src/chat/narrowsReducer.js

@@ -2,6 +2,7 @@
 import union from 'lodash.union';
 
 import type { NarrowsState, Action } from '../types';
+import { ensureUnreachable } from '../types';
 import {
   DEAD_QUEUE,
   LOGOUT,
@@ -13,7 +14,7 @@ import {
   EVENT_UPDATE_MESSAGE_FLAGS,
 } from '../actionConstants';
 import { LAST_MESSAGE_ANCHOR, FIRST_UNREAD_ANCHOR } from '../constants';
-import { isMessageInNarrow, STARRED_NARROW_STR } from '../utils/narrow';
+import { isMessageInNarrow, MENTIONED_NARROW_STR, STARRED_NARROW_STR } from '../utils/narrow';
 import { NULL_OBJECT } from '../nullObjects';
 
 const initialState: NarrowsState = NULL_OBJECT;
@@ -64,17 +65,31 @@ const eventUpdateMessageFlags = (state, action) => {
   const messagesSet = new Set(messages);
   const updates = [];
 
-  if (flag === 'starred' && state[STARRED_NARROW_STR]) {
-    if (operation === 'add') {
-      updates.push({
-        [STARRED_NARROW_STR]: [...state[STARRED_NARROW_STR], ...messages].sort(),
-      });
-    } else {
-      // operation === 'remove'
-      updates.push({
-        [STARRED_NARROW_STR]: state[STARRED_NARROW_STR].filter(id => !messagesSet.has(id)),
-      });
+  const updateFlagNarrow = narrowStr => {
+    if (state[narrowStr]) {
+      if (operation === 'add') {
+        updates.push({
+          [narrowStr]: [...state[narrowStr], ...messages].sort(),
+        });
+      } else if (operation === 'remove') {
+        updates.push({
+          [narrowStr]: state[narrowStr].filter(id => !messagesSet.has(id)),
+        });
+      } else {
+        ensureUnreachable(operation);
+        throw new Error(
+          `Unexpected operation ${operation} in an EVENT_UPDATE_MESSAGE_FLAGS action`,
+        );
+      }
     }
+
+    return updates;
+  };
+
+  if (flag === 'starred') {
+    updateFlagNarrow(STARRED_NARROW_STR);
+  } else if (['mentioned', 'wildcard_mentioned'].includes(flag)) {
+    updateFlagNarrow(MENTIONED_NARROW_STR);
   }
 
   if (!updates.length) {

src/events/__tests__/eventMiddleware-test.js

@@ -1,9 +1,8 @@
-import deepFreeze from 'deep-freeze';
-
 import eventMiddleware from '../eventMiddleware';
+import { NULL_ARRAY } from '../../nullObjects';
 
 describe('eventMiddleware', () => {
-  test('if `event.flags` key exist, move it to `event.message.flags`', () => {
+  test('if `event.flags` key exists, move it to `event.message.flags`', () => {
     const state = { session: {} };
     const event = {
       type: 'message',
@@ -16,12 +15,13 @@ describe('eventMiddleware', () => {
     expect(event.message.flags).toEqual(['mentioned']);
   });
 
-  test('if `event.flags` do not exist, do not mutate event', () => {
+  test('if `event.flags` does not exist, set message.flags to an empty array', () => {
     const state = { session: {} };
-    const event = deepFreeze({
+    const event = {
       type: 'message',
       message: {},
-    });
-    expect(() => eventMiddleware(state, event)).not.toThrow();
+    };
+    eventMiddleware(state, event);
+    expect(event.message.flags).toEqual(NULL_ARRAY);
   });
 });

src/events/eventMiddleware.js

@@ -5,23 +5,30 @@ import type { GeneralEvent, GlobalState, MessageEvent } from '../types';
 import { isHomeNarrow, isMessageInNarrow } from '../utils/narrow';
 import { getActiveAccount, getChatScreenParams, getOwnEmail, getIsActive } from '../selectors';
 import { playMessageSound } from '../utils/sound';
+import { NULL_ARRAY } from '../nullObjects';
 
 export default (state: GlobalState, event_: GeneralEvent) => {
   switch (event_.type) {
     case 'message': {
       // $FlowFixMe This expresses our unchecked assumptions about `message` events.
       const event = (event_: MessageEvent);
 
-      // move `flags` key from `event` to `event.message` for consistency
-      if (event.flags && !event.message.flags) {
+      // move `flags` key from `event` to `event.message` for consistency, and
+      // default to an empty array if event.flags is not set.
+      if (!event.message.flags) {
         // $FlowFixMe Message is readonly to serve our use of it in Redux.
-        event.message.flags = event.flags;
+        event.message.flags = event.flags || NULL_ARRAY;
         delete event.flags;
       }
 
       const isActive = getIsActive(state);
       const isPrivateMessage = Array.isArray(event.message.display_recipient);
-      const isMentioned = event.message.flags && event.message.flags.includes('mentioned');
+
+      const { flags } = event.message;
+      if (!flags) {
+        throw new Error('event.message.flags should be defined');
+      }
+      const isMentioned = flags.includes('mentioned') || flags.includes('wildcard_mentioned');
       if (!isActive || !(isPrivateMessage || isMentioned)) {
         break;
       }

src/unread/unreadMentionsReducer.js

@@ -40,12 +40,16 @@ export default (state: UnreadMentionsState = initialState, action: Action): Unre
     case REALM_INIT:
       return (action.data.unread_msgs && action.data.unread_msgs.mentions) || initialState;
 
-    case EVENT_NEW_MESSAGE:
-      return action.message.flags
-        && action.message.flags.includes('mentioned')
+    case EVENT_NEW_MESSAGE: {
+      const { flags } = action.message;
+      if (!flags) {
+        throw new Error('action.message.flags should be defined.');
+      }
+      return (flags.includes('mentioned') || flags.includes('wildcard_mentioned'))
         && !state.includes(action.message.id)
         ? addItemsToArray(state, [action.message.id])
         : state;
+    }
 
     case EVENT_MESSAGE_DELETE:
       return removeItemsFromArray(state, [action.messageId]);

src/utils/__tests__/narrow-test.js

@@ -21,6 +21,7 @@ import {
   getNarrowFromMessage,
   parseNarrowString,
   STARRED_NARROW,
+  MENTIONED_NARROW,
 } from '../narrow';
 
 describe('HOME_NARROW', () => {
@@ -219,13 +220,16 @@ describe('SEARCH_NARROW', () => {
 
 describe('isMessageInNarrow', () => {
   test('any message is in "Home"', () => {
-    const message = {};
+    const message = {
+      flags: [],
+    };
     const narrow = HOME_NARROW;
     expect(isMessageInNarrow(message, narrow)).toBe(true);
   });
 
   test('message with type "private" is in private narrow if recipient matches', () => {
     const message = {
+      flags: [],
       type: 'private',
       display_recipient: [{ email: 'me@example.com' }, { email: 'john@example.com' }],
     };
@@ -236,6 +240,7 @@ describe('isMessageInNarrow', () => {
 
   test('message to self is in "private" narrow with self', () => {
     const message = {
+      flags: [],
       type: 'private',
       display_recipient: [{ email: 'me@example.com' }],
     };
@@ -247,6 +252,7 @@ describe('isMessageInNarrow', () => {
   test('message with type "private" is in group narrow if all recipients match ', () => {
     const message = {
       type: 'private',
+      flags: [],
       display_recipient: [
         { email: 'me@example.com' },
         { email: 'john@example.com' },
@@ -261,6 +267,7 @@ describe('isMessageInNarrow', () => {
 
   test('message with type "private" is always in "private messages" narrow', () => {
     const message = {
+      flags: [],
       type: 'private',
       display_recipient: [{ email: 'me@example.com' }, { email: 'john@example.com' }],
     };
@@ -269,6 +276,7 @@ describe('isMessageInNarrow', () => {
 
   test('message with type "stream" is in narrow if recipient and current stream match', () => {
     const message = {
+      flags: [],
       type: 'stream',
       display_recipient: 'some stream',
     };
@@ -277,11 +285,53 @@ describe('isMessageInNarrow', () => {
     expect(isMessageInNarrow(message, narrow)).toBe(true);
   });
 
+  test('message with flags undefined throws an error', () => {
+    const message = {
+      // no flags key
+    };
+    expect(() => isMessageInNarrow(message, MENTIONED_NARROW)).toThrow();
+  });
+
+  test('message with flag "mentioned" is in is:mentioned narrow', () => {
+    const message = {
+      flags: ['mentioned'],
+    };
+    expect(isMessageInNarrow(message, MENTIONED_NARROW)).toBe(true);
+  });
+
+  test('message with flag "wildcard_mentioned" is in is:mentioned narrow', () => {
+    const message = {
+      flags: ['wildcard_mentioned'],
+    };
+    expect(isMessageInNarrow(message, MENTIONED_NARROW)).toBe(true);
+  });
+
+  test('message without flag "mentioned" or "wildcard_mentioned" is not in is:mentioned narrow', () => {
+    const message = {
+      flags: [],
+    };
+    expect(isMessageInNarrow(message, MENTIONED_NARROW)).toBe(false);
+  });
+
+  test('message with flag "starred" is in is:starred narrow', () => {
+    const message = {
+      flags: ['starred'],
+    };
+    expect(isMessageInNarrow(message, STARRED_NARROW)).toBe(true);
+  });
+  test('message without flag "starred" is not in is:starred narrow', () => {
+    const message = {
+      flags: [],
+    };
+    expect(isMessageInNarrow(message, STARRED_NARROW)).toBe(false);
+  });
+
   test('message with type stream is in topic narrow if current stream and topic match with its own', () => {
     const message = {
       type: 'stream',
       subject: 'some topic',
       display_recipient: 'some stream',
+      flags: [],
     };
     const narrow = topicNarrow('some stream', 'some topic');
 

src/utils/narrow.js

@@ -41,6 +41,8 @@ export const STARRED_NARROW_STR = JSON.stringify(STARRED_NARROW);
 
 export const MENTIONED_NARROW = specialNarrow('mentioned');
 
+export const MENTIONED_NARROW_STR = JSON.stringify(MENTIONED_NARROW);
+
 export const ALL_PRIVATE_NARROW = specialNarrow('private');
 
 export const ALL_PRIVATE_NARROW_STR = JSON.stringify(ALL_PRIVATE_NARROW);
@@ -214,15 +216,20 @@ export const isMessageInNarrow = (message: Message, narrow: Narrow, ownEmail: st
     return normalizedRecipients === ownEmail || normalizedRecipients === normalizedNarrow;
   };
 
+  const { flags } = message;
+  if (!flags) {
+    throw new Error('`message.flags` should be defined.');
+  }
+
   return caseNarrow(narrow, {
     home: () => true,
     stream: name => name === message.display_recipient,
     topic: (streamName, topic) =>
       streamName === message.display_recipient && topic === message.subject,
     pm: email => matchRecipients([email]),
     groupPm: matchRecipients,
-    starred: () => message.type === 'starred',
-    mentioned: () => message.type === 'mentioned',
+    starred: () => flags.includes('starred'),
+    mentioned: () => flags.includes('mentioned') || flags.includes('wildcard_mentioned'),
     allPrivate: () => message.type === 'private',
     search: () => false,
   });