example data: Better support EVENT_NEW_MESSAGE.

src/__tests__/lib/exampleData.js

@@ -17,7 +17,13 @@ import type {
   UserId,
 } from '../../api/modelTypes';
 import { makeUserId } from '../../api/idTypes';
-import type { Action, GlobalState, MessagesState, RealmState } from '../../reduxTypes';
+import type {
+  Action,
+  GlobalState,
+  CaughtUpState,
+  MessagesState,
+  RealmState,
+} from '../../reduxTypes';
 import type { Auth, Account, OutboxBase, StreamOutbox } from '../../types';
 import { UploadedAvatarURL } from '../../utils/avatar';
 import { ZulipVersion } from '../../utils/zulipVersion';
@@ -323,6 +329,11 @@ const randMessageId: () => number = makeUniqueRandInt('message ID', 10000000);
  * A PM, by default a 1:1 from eg.otherUser to eg.selfUser.
  *
  * Beware! These values may not be representative.
+ *
+ * NB that the resulting value has no `flags` property.  This matches what
+ * we expect in `state.messages`, but not some other contexts; see comment
+ * on the `flags` property of `Message`.  For use in an `EVENT_NEW_MESSAGE`
+ * action, pass to `mkActionEventNewMessage`.
  */
 export const pmMessage = (args?: {|
   ...$Rest<PmMessage, { ... }>,
@@ -378,6 +389,11 @@ const messagePropertiesFromStream = (stream1: Stream) => {
  * A stream message, by default in eg.stream sent by eg.otherUser.
  *
  * Beware! These values may not be representative.
+ *
+ * NB that the resulting value has no `flags` property.  This matches what
+ * we expect in `state.messages`, but not some other contexts; see comment
+ * on the `flags` property of `Message`.  For use in an `EVENT_NEW_MESSAGE`
+ * action, pass to `mkActionEventNewMessage`.
  */
 export const streamMessage = (args?: {|
   ...$Rest<StreamMessage, { ... }>,
@@ -687,29 +703,40 @@ export const action = deepFreeze({
 (action: {| [string]: Action |});
 
 /* ========================================================================
- * Action fragments
+ * Action factories
  *
- * Partial actions, for those action types where (a) there's some
- * boilerplate data that's useful to supply here, but (b) there's some other
- * places where a given test will almost always need to fill in specific
- * data of its own.
+ * Useful for action types where a static object of boilerplate data doesn't
+ * suffice.  Generally this is true where (a) there's some boilerplate data
+ * that's useful to supply here, but (b) there's some other places where a
+ * given test will almost always need to fill in specific data of its own.
  *
- * The properties where each test will want to fill in its own specific data
- * should be left out of these fragments.  That way, Flow can ensure the
- * test explicitly supplies the data.
+ * For action types without (b), a static example value `eg.action.foo` is
+ * enough.  For action types without (a), even that isn't necessary, because
+ * each test might as well define the action values it needs directly.
  */
 
-export const eventNewMessageActionBase /* \: $Diff<EventNewMessageAction, {| message: Message |}> */ = {
-  type: EVENT_NEW_MESSAGE,
-  // These properties are boring for most or all tests.
-  id: 1001,
-  caughtUp: {},
-  ownUserId: selfUser.user_id,
+/**
+ * An EVENT_NEW_MESSAGE action.
+ *
+ * The message argument can either have or omit a `flags` property; if
+ * omitted, it defaults to empty.  (The `message` property on an
+ * `EVENT_NEW_MESSAGE` action must have `flags`, while `Message` objects in
+ * some other contexts must not.  See comments on `Message` for details.)
+ */
+export const mkActionEventNewMessage = (
+  message: Message,
+  args?: {| caughtUp?: CaughtUpState, local_message_id?: number, ownUserId?: UserId |},
+): Action =>
+  deepFreeze({
+    type: EVENT_NEW_MESSAGE,
+    id: 1001,
+    caughtUp: {},
+    ownUserId: selfUser.user_id,
 
-  // The details of this property are typically important to what a test is
-  // testing, so we provide it explicitly in each test.
-  // message: Message,
-};
+    ...args,
+
+    message: { ...message, flags: message.flags ?? [] },
+  });
 
 // If a given action is only relevant to a single test file, no need to
-// provide a generic fragment for it here; just define the test data there.
+// provide a generic factory for it here; just define the test data there.

src/chat/__tests__/flagsReducer-test.js

@@ -76,21 +76,6 @@ describe('flagsReducer', () => {
   });
 
   describe('EVENT_NEW_MESSAGE', () => {
-    test('when no flags key is passed, do not fail, do nothing', () => {
-      const prevState = NULL_OBJECT;
-
-      const action = deepFreeze({
-        type: EVENT_NEW_MESSAGE,
-        message: { id: 2 },
-      });
-
-      const expectedState = {};
-
-      const actualState = flagsReducer(prevState, action);
-
-      expect(actualState).toEqual(expectedState);
-    });
-
     test('adds to store flags from new message', () => {
       const prevState = NULL_OBJECT;
 

src/chat/__tests__/narrowsReducer-test.js

@@ -37,10 +37,7 @@ describe('narrowsReducer', () => {
 
       const message = eg.streamMessage({ id: 3, flags: [] });
 
-      const action = deepFreeze({
-        ...eg.eventNewMessageActionBase,
-        message,
-      });
+      const action = eg.mkActionEventNewMessage(message);
 
       const newState = narrowsReducer(initialState, action);
 
@@ -54,9 +51,7 @@ describe('narrowsReducer', () => {
 
       const message = eg.streamMessage({ id: 3, flags: [] });
 
-      const action = deepFreeze({
-        ...eg.eventNewMessageActionBase,
-        message,
+      const action = eg.mkActionEventNewMessage(message, {
         caughtUp: {
           [HOME_NARROW_STR]: {
             older: false,
@@ -80,10 +75,7 @@ describe('narrowsReducer', () => {
 
       const message = eg.streamMessage({ id: 3, flags: [], stream: eg.makeStream() });
 
-      const action = deepFreeze({
-        ...eg.eventNewMessageActionBase,
-        message,
-      });
+      const action = eg.mkActionEventNewMessage(message);
 
       const newState = narrowsReducer(initialState, action);
 
@@ -101,9 +93,7 @@ describe('narrowsReducer', () => {
       recipients: [eg.selfUser, eg.otherUser, eg.thirdUser],
       flags: [],
     });
-    const action = deepFreeze({
-      ...eg.eventNewMessageActionBase,
-      message,
+    const action = eg.mkActionEventNewMessage(message, {
       caughtUp: {
         [ALL_PRIVATE_NARROW_STR]: { older: true, newer: true },
       },
@@ -122,9 +112,7 @@ describe('narrowsReducer', () => {
 
     const message = eg.streamMessage({ id: 3, flags: [] });
 
-    const action = deepFreeze({
-      ...eg.eventNewMessageActionBase,
-      message,
+    const action = eg.mkActionEventNewMessage(message, {
       caughtUp: {
         [HOME_NARROW_STR]: {
           older: false,
@@ -160,9 +148,7 @@ describe('narrowsReducer', () => {
       flags: [],
     });
 
-    const action = deepFreeze({
-      ...eg.eventNewMessageActionBase,
-      message,
+    const action = eg.mkActionEventNewMessage(message, {
       caughtUp: {
         [HOME_NARROW_STR]: { older: false, newer: true },
         [narrowWithSelfStr]: { older: false, newer: true },
@@ -192,9 +178,7 @@ describe('narrowsReducer', () => {
 
     const message = eg.streamMessage({ id: 5, flags: [] });
 
-    const action = deepFreeze({
-      ...eg.eventNewMessageActionBase,
-      message,
+    const action = eg.mkActionEventNewMessage(message, {
       caughtUp: {
         [HOME_NARROW_STR]: { older: false, newer: true },
         [streamNarrowStr]: { older: false, newer: true },
@@ -222,9 +206,7 @@ describe('narrowsReducer', () => {
 
     const message = eg.streamMessage({ id: 3, flags: [] });
 
-    const action = deepFreeze({
-      ...eg.eventNewMessageActionBase,
-      message,
+    const action = eg.mkActionEventNewMessage(message, {
       caughtUp: {
         [HOME_NARROW_STR]: { older: false, newer: true },
       },
@@ -257,9 +239,7 @@ describe('narrowsReducer', () => {
       recipients: [eg.selfUser, eg.otherUser],
     });
 
-    const action = deepFreeze({
-      ...eg.eventNewMessageActionBase,
-      message,
+    const action = eg.mkActionEventNewMessage(message, {
       caughtUp: initialState.map(_ => ({ older: false, newer: true })).toObject(),
     });
 

src/chat/flagsReducer.js

@@ -1,4 +1,6 @@
 /* @flow strict-local */
+import invariant from 'invariant';
+
 import type { Action, FlagsState, Message } from '../types';
 import {
   REALM_INIT,
@@ -29,9 +31,9 @@ const initialState = {
 const addFlagsForMessages = (
   state: FlagsState,
   messages: $ReadOnlyArray<number>,
-  flags?: $ReadOnlyArray<string>,
+  flags: $ReadOnlyArray<string>,
 ): FlagsState => {
-  if (!messages || messages.length === 0 || !flags || flags.length === 0) {
+  if (messages.length === 0 || flags.length === 0) {
     return state;
   }
 
@@ -111,8 +113,10 @@ export default (state: FlagsState = initialState, action: Action): FlagsState =>
     case MESSAGE_FETCH_COMPLETE:
       return processFlagsForMessages(state, action.messages);
 
-    case EVENT_NEW_MESSAGE:
+    case EVENT_NEW_MESSAGE: {
+      invariant(action.message.flags, 'message in EVENT_NEW_MESSAGE must have flags');
       return addFlagsForMessages(state, [action.message.id], action.message.flags);
+    }
 
     case EVENT_UPDATE_MESSAGE_FLAGS:
       return eventUpdateMessageFlags(state, action);

src/message/__tests__/messagesReducer-test.js

@@ -24,16 +24,13 @@ describe('messagesReducer', () => {
       const message3 = eg.streamMessage({ id: 3 });
 
       const prevState = eg.makeMessagesState([message1, message2]);
-      const action = deepFreeze({
-        ...eg.eventNewMessageActionBase,
+      const action = eg.mkActionEventNewMessage(message3, {
         caughtUp: {
           [HOME_NARROW_STR]: {
             older: true,
             newer: true,
           },
         },
-        // `flags` is present in EVENT_NEW_MESSAGE; see note at Message.
-        message: { ...message3, flags: [] },
       });
       const expectedState = eg.makeMessagesState([message1, message2, message3]);
       const newState = messagesReducer(prevState, action);
@@ -46,16 +43,13 @@ describe('messagesReducer', () => {
       const message2 = eg.streamMessage({ id: 2 });
       const message3 = eg.streamMessage({ id: 3 });
       const prevState = eg.makeMessagesState([message1, message2]);
-      const action = deepFreeze({
-        ...eg.eventNewMessageActionBase,
+      const action = eg.mkActionEventNewMessage(message3, {
         caughtUp: {
           [HOME_NARROW_STR]: {
             older: true,
             newer: false,
           },
         },
-        // `flags` is present in EVENT_NEW_MESSAGE; see note at Message.
-        message: { ...message3, flags: [] },
       });
       const newState = messagesReducer(prevState, action);
       expect(newState).toEqual(prevState);

src/outbox/__tests__/outboxReducer-test.js

@@ -67,9 +67,7 @@ describe('outboxReducer', () => {
 
       const message = eg.streamMessage({ timestamp: 123 });
 
-      const action = deepFreeze({
-        ...eg.eventNewMessageActionBase,
-        message,
+      const action = eg.mkActionEventNewMessage(message, {
         local_message_id: message.timestamp,
       });
 
@@ -83,9 +81,7 @@ describe('outboxReducer', () => {
       const message3 = eg.streamOutbox({ timestamp: 150238594540 });
       const initialState = deepFreeze([message1, message2, message3]);
 
-      const action = deepFreeze({
-        ...eg.eventNewMessageActionBase,
-        message: eg.streamMessage(),
+      const action = eg.mkActionEventNewMessage(eg.streamMessage(), {
         local_message_id: 546,
       });
 
@@ -102,9 +98,7 @@ describe('outboxReducer', () => {
       const message3 = eg.streamOutbox({ timestamp: 150238594540 });
       const initialState = deepFreeze([message1, message2, message3]);
 
-      const action = deepFreeze({
-        ...eg.eventNewMessageActionBase,
-        message: eg.streamMessage(),
+      const action = eg.mkActionEventNewMessage(eg.streamMessage(), {
         local_message_id: 15023859,
       });
 

src/pm-conversations/__tests__/pmConversationsModel-test.js

@@ -86,7 +86,7 @@ describe('reducer', () => {
   });
 
   describe('EVENT_NEW_MESSAGE', () => {
-    const actionGeneral = message => ({ ...eg.eventNewMessageActionBase, message });
+    const actionGeneral = eg.mkActionEventNewMessage;
     const [user1, user2] = [eg.makeUser({ user_id: 1 }), eg.makeUser({ user_id: 2 })];
     const action = (id, otherUsers) =>
       actionGeneral(eg.pmMessageFromTo(eg.selfUser, otherUsers, { id }));

src/topics/__tests__/topicsSelectors-test.js

@@ -5,7 +5,6 @@ import { getTopicsForNarrow, getLastMessageTopic, getTopicsForStream } from '../
 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', () => {
@@ -113,7 +112,7 @@ describe('getTopicsForStream', () => {
         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.plusReduxState),
+        (st, message) => unreadReducer(st, eg.mkActionEventNewMessage(message), eg.plusReduxState),
         eg.plusReduxState.unread,
       ),
     });

src/unread/__tests__/unread-testlib.js

@@ -1,6 +1,5 @@
 /* @flow strict-local */
 
-import type { Message } from '../../types';
 import { reducer } from '../unreadModel';
 import * as eg from '../../__tests__/lib/exampleData';
 
@@ -10,11 +9,6 @@ export const initialState = reducer(
   eg.baseReduxState,
 );
 
-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 };
 
@@ -53,7 +47,7 @@ export const selectorBaseState = (() => {
     eg.pmMessageFromTo(user4, [user1, user5], { id: 24 }),
     eg.pmMessageFromTo(user4, [user1, user5], { id: 25 }),
   ]) {
-    state = reducer(state, mkMessageAction(message), globalState);
+    state = reducer(state, eg.mkActionEventNewMessage(message), globalState);
   }
   return state;
 })();