Outbox type: Make sender_id required; add optional stream_id.

src/__tests__/lib/exampleData.js

@@ -18,7 +18,7 @@ import type {
 } from '../../api/modelTypes';
 import { makeUserId } from '../../api/idTypes';
 import type { Action, GlobalState, MessagesState, RealmState } from '../../reduxTypes';
-import type { Auth, Account, Outbox } from '../../types';
+import type { Auth, Account, OutboxBase, StreamOutbox } from '../../types';
 import { UploadedAvatarURL } from '../../utils/avatar';
 import { ZulipVersion } from '../../utils/zulipVersion';
 import {
@@ -415,8 +415,11 @@ export const makeMessagesState = (messages: Message[]): MessagesState =>
  * (Only stream messages for now. Feel free to add PMs, if you need them.)
  */
 
-/** An outbox message with no interesting data. */
-const outboxMessageBase: $Diff<Outbox, {| id: mixed, timestamp: mixed |}> = deepFreeze({
+/**
+ * Properties in common among PM and stream outbox messages, with no
+ *   interesting data.
+ */
+const outboxMessageBase: $Diff<OutboxBase, {| id: mixed, timestamp: mixed |}> = deepFreeze({
   isOutbox: true,
   isSent: false,
   avatar_url: selfUser.avatar_url,
@@ -434,11 +437,12 @@ const outboxMessageBase: $Diff<Outbox, {| id: mixed, timestamp: mixed |}> = deep
 });
 
 /**
- * Create an outbox message from an interesting subset of its data.
+ * Create a stream outbox message from an interesting subset of its
+ *   data.
  *
  * `.id` is always identical to `.timestamp` and should not be supplied.
  */
-export const makeOutboxMessage = (data: $Shape<$Diff<Outbox, {| id: mixed |}>>): Outbox => {
+export const streamOutbox = (data: $Shape<$Diff<StreamOutbox, {| id: mixed |}>>): StreamOutbox => {
   const { timestamp } = data;
 
   const outputTimestamp = timestamp ?? makeTime() / 1000;

src/boot/store.js

@@ -283,6 +283,7 @@ const migrations: {| [string]: (GlobalState) => GlobalState |} = {
     accounts: state.accounts.filter(a => a.email !== ''),
   }),
 
+  // Add "open links with in-app browser" setting.
   '28': state => ({
     ...state,
     settings: {
@@ -291,6 +292,12 @@ const migrations: {| [string]: (GlobalState) => GlobalState |} = {
     },
   }),
 
+  // Make `sender_id` on `Outbox` required.
+  '29': state => ({
+    ...state,
+    outbox: state.outbox.filter(o => o.sender_id !== undefined),
+  }),
+
   // TIP: When adding a migration, consider just using `dropCache`.
 };
 

src/chat/__tests__/narrowsSelectors-test.js

@@ -24,7 +24,7 @@ import * as eg from '../../__tests__/lib/exampleData';
 describe('getMessagesForNarrow', () => {
   const message = eg.streamMessage({ id: 123 });
   const messages = eg.makeMessagesState([message]);
-  const outboxMessage = eg.makeOutboxMessage({});
+  const outboxMessage = eg.streamOutbox({});
 
   test('if no outbox messages returns messages with no change', () => {
     const state = eg.reduxState({

src/message/__tests__/getHtmlPieceDescriptors-test.js

@@ -1,5 +1,8 @@
+/* @flow strict-local */
 import deepFreeze from 'deep-freeze';
+import invariant from 'invariant';
 
+import * as eg from '../../__tests__/lib/exampleData';
 import { HOME_NARROW } from '../../utils/narrow';
 import getHtmlPieceDescriptors from '../getHtmlPieceDescriptors';
 
@@ -12,13 +15,7 @@ describe('getHtmlPieceDescriptors', () => {
   });
 
   test('renders time, header and message for a single input', () => {
-    const messages = deepFreeze([
-      {
-        timestamp: 123,
-        avatar_url: '',
-        id: 12345,
-      },
-    ]);
+    const messages = deepFreeze([{ ...eg.streamMessage({ id: 12345 }), timestamp: 123 }]);
 
     const htmlPieceDescriptors = getHtmlPieceDescriptors(messages, narrow);
 
@@ -28,115 +25,59 @@ describe('getHtmlPieceDescriptors', () => {
   });
 
   test('several messages in same stream, from same person result in time row, header for the stream, three messages, only first of which is full detail', () => {
+    const stream = eg.stream;
+    const sender = eg.otherUser;
+
     const messages = deepFreeze([
-      {
-        timestamp: 123,
-        type: 'stream',
-        id: 1,
-        sender_email: '[email protected]',
-        sender_full_name: 'John Doe',
-        display_recipient: 'general',
-        subject: '',
-        avatar_url: '',
-      },
-      {
-        timestamp: 124,
-        type: 'stream',
-        id: 2,
-        sender_email: '[email protected]',
-        sender_full_name: 'John Doe',
-        display_recipient: 'general',
-        subject: '',
-        avatar_url: '',
-      },
-      {
-        timestamp: 125,
-        type: 'stream',
-        id: 3,
-        sender_email: '[email protected]',
-        sender_full_name: 'John Doe',
-        display_recipient: 'general',
-        subject: '',
-        avatar_url: '',
-      },
+      eg.streamMessage({ stream, sender, id: 1 }),
+      eg.streamMessage({ stream, sender, id: 2 }),
+      eg.streamMessage({ stream, sender, id: 3 }),
     ]);
 
     const htmlPieceDescriptors = getHtmlPieceDescriptors(messages, narrow);
 
     const messageKeys = htmlPieceDescriptors[1].data.map(x => x.key);
-    const messageBriefs = htmlPieceDescriptors[1].data.map(x => x.isBrief);
+    const messageBriefs = htmlPieceDescriptors[1].data.map(x => {
+      invariant(x.type === 'message', 'expected message item');
+      return x.isBrief;
+    });
     expect(messageKeys).toEqual([1, 2, 3]);
     expect(messageBriefs).toEqual([false, true, true]);
   });
 
   test('several messages in same stream, from different people result in time row, header for the stream, three messages, only all full detail', () => {
+    const stream = eg.stream;
+
     const messages = deepFreeze([
-      {
-        timestamp: 123,
-        type: 'stream',
-        id: 1,
-        sender_email: '[email protected]',
-        sender_full_name: 'John',
-        display_recipient: 'general',
-        subject: '',
-        avatar_url: '',
-      },
-      {
-        timestamp: 124,
-        type: 'stream',
-        id: 2,
-        sender_email: '[email protected]',
-        sender_full_name: 'Mark',
-        display_recipient: 'general',
-        subject: '',
-        avatar_url: '',
-      },
-      {
-        timestamp: 125,
-        type: 'stream',
-        id: 3,
-        sender_email: '[email protected]',
-        sender_full_name: 'Peter',
-        display_recipient: 'general',
-        subject: '',
-        avatar_url: '',
-      },
+      eg.streamMessage({ stream, sender: eg.selfUser, id: 1 }),
+      eg.streamMessage({ stream, sender: eg.otherUser, id: 2 }),
+      eg.streamMessage({ stream, sender: eg.thirdUser, id: 3 }),
     ]);
 
     const htmlPieceDescriptors = getHtmlPieceDescriptors(messages, narrow);
 
     const messageKeys = htmlPieceDescriptors[1].data.map(x => x.key);
-    const messageBriefs = htmlPieceDescriptors[1].data.map(x => x.isBrief);
+    const messageBriefs = htmlPieceDescriptors[1].data.map(x => {
+      invariant(x.type === 'message', 'expected message item');
+      return x.isBrief;
+    });
     expect(messageKeys).toEqual([1, 2, 3]);
     expect(messageBriefs).toEqual([false, false, false]);
   });
 
   test('private messages between two people, results in time row, header and two full messages', () => {
     const messages = deepFreeze([
-      {
-        timestamp: 123,
-        type: 'private',
-        id: 1,
-        sender_email: '[email protected]',
-        sender_full_name: 'John',
-        avatar_url: '',
-        display_recipient: [{ email: '[email protected]' }, { email: '[email protected]' }],
-      },
-      {
-        timestamp: 123,
-        type: 'private',
-        id: 2,
-        sender_email: '[email protected]',
-        sender_full_name: 'Mark',
-        avatar_url: '',
-        display_recipient: [{ email: '[email protected]' }, { email: '[email protected]' }],
-      },
+      eg.pmMessage({ sender: eg.selfUser, recipients: [eg.selfUser, eg.otherUser], id: 1 }),
+      eg.pmMessage({ sender: eg.otherUser, recipients: [eg.selfUser, eg.otherUser], id: 2 }),
     ]);
 
     const htmlPieceDescriptors = getHtmlPieceDescriptors(messages, narrow);
 
     const messageKeys = htmlPieceDescriptors[1].data.map(x => x.key);
-    const messageBriefs = htmlPieceDescriptors[1].data.map(x => x.isBrief);
+    const messageBriefs = htmlPieceDescriptors[1].data.map(x => {
+      invariant(x.type === 'message', 'expected message item');
+      return x.isBrief;
+    });
     expect(messageKeys).toEqual([1, 2]);
     expect(messageBriefs).toEqual([false, false]);
   });

src/message/getHtmlPieceDescriptors.js

@@ -34,15 +34,8 @@ export default (
       });
     }
 
-    // TODO(#3764): Use sender_id, not sender_email.  Needs making
-    //   Outbox#sender_id required; which needs a migration to drop Outbox
-    //   values that lack it; which is fine once the release that adds it
-    //   has been out for a few weeks.
     const shouldGroupWithPrev =
-      !diffRecipient
-      && !diffDays
-      && !!prevMessage
-      && prevMessage.sender_email === message.sender_email;
+      !diffRecipient && !diffDays && !!prevMessage && prevMessage.sender_id === message.sender_id;
 
     sections[sections.length - 1].data.push({
       key: message.id,

src/outbox/__tests__/outboxReducer-test.js

@@ -9,9 +9,9 @@ import * as eg from '../../__tests__/lib/exampleData';
 describe('outboxReducer', () => {
   describe('INITIAL_FETCH_COMPLETE', () => {
     test('filters out isSent', () => {
-      const message1 = eg.makeOutboxMessage({ content: 'New one' });
-      const message2 = eg.makeOutboxMessage({ content: 'Another one' });
-      const message3 = eg.makeOutboxMessage({ content: 'Message already sent', isSent: true });
+      const message1 = eg.streamOutbox({ content: 'New one' });
+      const message2 = eg.streamOutbox({ content: 'Another one' });
+      const message3 = eg.streamOutbox({ content: 'Message already sent', isSent: true });
       const initialState = deepFreeze([message1, message2, message3]);
 
       const action = deepFreeze({
@@ -28,7 +28,7 @@ describe('outboxReducer', () => {
 
   describe('MESSAGE_SEND_START', () => {
     test('add a new message to the outbox', () => {
-      const message = eg.makeOutboxMessage({ content: 'New one' });
+      const message = eg.streamOutbox({ content: 'New one' });
 
       const initialState = deepFreeze([]);
 
@@ -45,8 +45,8 @@ describe('outboxReducer', () => {
     });
 
     test('do not add a message with a duplicate timestamp to the outbox', () => {
-      const message1 = eg.makeOutboxMessage({ content: 'hello', timestamp: 123 });
-      const message2 = eg.makeOutboxMessage({ content: 'hello twice', timestamp: 123 });
+      const message1 = eg.streamOutbox({ content: 'hello', timestamp: 123 });
+      const message2 = eg.streamOutbox({ content: 'hello twice', timestamp: 123 });
 
       const initialState = deepFreeze([message1]);
 
@@ -63,7 +63,7 @@ describe('outboxReducer', () => {
 
   describe('EVENT_NEW_MESSAGE', () => {
     test('do not mutate state if a message is not removed', () => {
-      const initialState = deepFreeze([eg.makeOutboxMessage({ timestamp: 546 })]);
+      const initialState = deepFreeze([eg.streamOutbox({ timestamp: 546 })]);
 
       const message = eg.streamMessage({ timestamp: 123 });
 
@@ -78,9 +78,9 @@ describe('outboxReducer', () => {
     });
 
     test('remove message if local_message_id matches', () => {
-      const message1 = eg.makeOutboxMessage({ timestamp: 546 });
-      const message2 = eg.makeOutboxMessage({ timestamp: 150238512430 });
-      const message3 = eg.makeOutboxMessage({ timestamp: 150238594540 });
+      const message1 = eg.streamOutbox({ timestamp: 546 });
+      const message2 = eg.streamOutbox({ timestamp: 150238512430 });
+      const message3 = eg.streamOutbox({ timestamp: 150238594540 });
       const initialState = deepFreeze([message1, message2, message3]);
 
       const action = deepFreeze({
@@ -97,9 +97,9 @@ describe('outboxReducer', () => {
     });
 
     test("remove nothing if local_message_id doesn't match", () => {
-      const message1 = eg.makeOutboxMessage({ timestamp: 546 });
-      const message2 = eg.makeOutboxMessage({ timestamp: 150238512430 });
-      const message3 = eg.makeOutboxMessage({ timestamp: 150238594540 });
+      const message1 = eg.streamOutbox({ timestamp: 546 });
+      const message2 = eg.streamOutbox({ timestamp: 150238512430 });
+      const message3 = eg.streamOutbox({ timestamp: 150238594540 });
       const initialState = deepFreeze([message1, message2, message3]);
 
       const action = deepFreeze({