Mostly handle topic edits / stream moves

src/__tests__/lib/exampleData.js

@@ -18,6 +18,7 @@ import type {
 } from '../../api/modelTypes';
 import { makeUserId } from '../../api/idTypes';
 import type { InitialData } from '../../api/apiTypes';
+import { EventTypes, type UpdateMessageEvent } from '../../api/eventTypes';
 import type {
   AccountSwitchAction,
   LoginSuccessAction,
@@ -44,13 +45,15 @@ import {
   EVENT_NEW_MESSAGE,
   MESSAGE_FETCH_START,
   MESSAGE_FETCH_COMPLETE,
+  EVENT_UPDATE_MESSAGE,
 } from '../../actionConstants';
 import rootReducer from '../../boot/reducers';
 import { authOfAccount } from '../../account/accountMisc';
 import { HOME_NARROW } from '../../utils/narrow';
 import type { BackgroundData } from '../../webview/MessageList';
 import { getSettings, getStreamsById, getSubscriptionsById } from '../../selectors';
 import { getGlobalSettings } from '../../directSelectors';
+import { messageMoved } from '../../api/misc';
 
 /* ========================================================================
  * Utilities
@@ -464,7 +467,7 @@ export const streamMessage = (args?: {|
     content_type: 'text/html',
     id: randMessageId(),
     subject: 'example topic',
-    subject_links: [],
+    topic_links: ([]: {| text: string, url: string |}[]),
     timestamp: 1556579727,
     type: 'stream',
   };
@@ -900,6 +903,25 @@ export const mkActionEventNewMessage = (
     message: { ...message, flags: message.flags ?? [] },
   });
 
+/**
+ * An EVENT_UPDATE_MESSAGE action.
+ */
+export const mkActionEventUpdateMessage = (args: {|
+  ...$Rest<UpdateMessageEvent, { id: mixed, type: mixed, flags?: mixed }>,
+|}): PerAccountAction => {
+  const event = {
+    id: 1,
+    type: EventTypes.update_message,
+    flags: [],
+    ...args,
+  };
+  return {
+    type: EVENT_UPDATE_MESSAGE,
+    event,
+    move: messageMoved(event),
+  };
+};
+
 // If a given action is only relevant to a single test file, no need to
 // provide a generic factory for it here; just define the test data there.
 

src/actionTypes.js

@@ -69,8 +69,10 @@ import type {
   RealmUpdateDictEvent,
   SubmessageEvent,
   RestartEvent,
+  UpdateMessageEvent,
 } from './api/eventTypes';
 import type { MutedTopicTuple, PresenceSnapshot } from './api/apiTypes';
+import type { MessageMove } from './api/misc';
 
 import type {
   Orientation,
@@ -342,45 +344,11 @@ type EventMessageDeleteAction = $ReadOnly<{|
   messageIds: $ReadOnlyArray<number>,
 |}>;
 
-// This is current to feature level 109:
-//   https://zulip.com/api/get-events#update_message
 type EventUpdateMessageAction = $ReadOnly<{|
-  ...ServerEvent,
   type: typeof EVENT_UPDATE_MESSAGE,
-
-  // Future servers might send `null` here:
-  //   https://chat.zulip.org/#narrow/stream/378-api-design/topic/.60update_message.60.20event/near/1309241
-  // TODO(server-5.0): Update this and/or simplify.
-  user_id?: UserId | null,
-
-  // Any content changes apply to just message_id.
-  message_id: number,
-
-  // Any stream/topic changes apply to all of message_ids, which is
-  //   guaranteed to include message_id.
-  message_ids: $ReadOnlyArray<number>,
-
-  flags: $ReadOnlyArray<string>,
-  edit_timestamp?: number,
-  stream_name?: string,
-  stream_id?: number,
-  new_stream_id?: number,
-  propagate_mode?: 'change_one' | 'change_later' | 'change_all',
-  orig_subject?: string,
-  subject?: string,
-
-  // TODO(server-4.0): Changed in feat. 46 to array-of-objects shape, from $ReadOnlyArray<string>
-  topic_links?: $ReadOnlyArray<{| +text: string, +url: string |}> | $ReadOnlyArray<string>,
-
-  // TODO(server-3.0): Replaced in feat. 1 by topic_links
-  subject_links?: $ReadOnlyArray<string>,
-
-  orig_content?: string,
-  orig_rendered_content?: string,
-  prev_rendered_content_version?: number,
-  content?: string,
-  rendered_content?: string,
-  is_me_message?: boolean,
+  /** Here `message_ids` is sorted.  (This isn't guaranteed in the server event.) */
+  event: UpdateMessageEvent,
+  move: null | MessageMove,
 |}>;
 
 type EventReactionCommon = $ReadOnly<{|

src/api/eventTypes.js

@@ -207,3 +207,52 @@ export type RealmUpdateDictEvent = $ReadOnly<{|
   property: 'default',
   data: $Rest<RealmDataForUpdate, { ... }>,
 |}>;
+
+/**
+ * https://zulip.com/api/get-events#update_message
+ *
+ * See also `messageMoved` in `misc.js`.
+ */
+// This is current to feature level 109.
+// NB if this ever gains a feature of moving PMs, `messageMoved` needs updating.
+export type UpdateMessageEvent = $ReadOnly<{|
+  ...EventCommon,
+  type: typeof EventTypes.update_message,
+
+  // Future servers might send `null` here:
+  //   https://chat.zulip.org/#narrow/stream/378-api-design/topic/.60update_message.60.20event/near/1309241
+  // TODO(server-5.0): Update this and/or simplify.
+  user_id?: UserId | null,
+
+  // Any content changes apply to just message_id.
+  message_id: number,
+
+  // Any stream/topic changes apply to all of message_ids, which is
+  //   guaranteed to include message_id.
+  // TODO(server-future): It'd be nice for these to be sorted; NB they may
+  //   not be.  As of server 5.0-dev-3868-gca17a452f (2022-02), there's no
+  //   guarantee of that in the API nor, apparently, in the implementation.
+  message_ids: $ReadOnlyArray<number>,
+
+  flags: $ReadOnlyArray<string>,
+  edit_timestamp?: number,
+  stream_name?: string,
+  stream_id?: number,
+  new_stream_id?: number,
+  propagate_mode?: 'change_one' | 'change_later' | 'change_all',
+  orig_subject?: string,
+  subject?: string,
+
+  // TODO(server-4.0): Changed in feat. 46 to array-of-objects shape, from $ReadOnlyArray<string>
+  topic_links?: $ReadOnlyArray<{| +text: string, +url: string |}> | $ReadOnlyArray<string>,
+
+  // TODO(server-3.0): Replaced in feat. 1 by topic_links
+  subject_links?: $ReadOnlyArray<string>,
+
+  orig_content?: string,
+  orig_rendered_content?: string,
+  prev_rendered_content_version?: number,
+  content?: string,
+  rendered_content?: string,
+  is_me_message?: boolean,
+|}>;

src/api/misc.js

@@ -0,0 +1,54 @@
+/**
+ * Functions to interpret some data from the API.
+ *
+ * @flow strict-local
+ */
+import invariant from 'invariant';
+
+import type { UpdateMessageEvent } from './eventTypes';
+import * as logging from '../utils/logging';
+
+export type MessageMove = $ReadOnly<{|
+  orig_stream_id: number,
+  new_stream_id: number,
+  orig_topic: string,
+  new_topic: string,
+|}>;
+
+/**
+ * Determine if and where this edit moved the message.
+ *
+ * If a move did occur, returns the relevant information in a form with a
+ * consistent naming convention.
+ *
+ * Returns null if the message stayed at the same conversation.
+ */
+export function messageMoved(event: UpdateMessageEvent): null | MessageMove {
+  // The API uses "new" for the stream IDs and "orig" for the topics.
+  // Put them both in a consistent naming convention.
+  const orig_stream_id = event.stream_id;
+  if (orig_stream_id == null) {
+    // Not stream messages, or else a pure content edit (no stream/topic change.)
+    // TODO(server-5.0): Simplify comment: since FL 112 this means it's
+    //   just not a stream message.
+    return null;
+  }
+  const new_stream_id = event.new_stream_id ?? orig_stream_id;
+  const orig_topic = event.orig_subject;
+  const new_topic = event.subject ?? orig_topic;
+
+  if (new_topic === orig_topic && new_stream_id === orig_stream_id) {
+    // Stream and topic didn't change.
+    return null;
+  }
+
+  if (orig_topic == null) {
+    // `orig_subject` is documented to be present when either the
+    // stream or topic changed.
+    logging.warn('Got update_message event with stream/topic change and no orig_subject');
+    return null;
+  }
+  invariant(new_topic != null, 'new_topic must be non-nullish when orig_topic is, by `??`');
+
+  return { orig_stream_id, new_stream_id, orig_topic, new_topic };
+}

src/api/modelTypes.js

@@ -675,7 +675,15 @@ export type StreamMessage = $ReadOnly<{|
    * (see point 4). We assume this has always been the case.
    */
   subject: string,
-  subject_links: $ReadOnlyArray<string>,
+
+  // We don't actually use this property.  If and when we do, we'll probably
+  // want in our internal model to canonicalize it to the newest form (with
+  // the name topic_links rather than subject_links, and newest type.)
+  // TODO(server-4.0): Changed in feat. 46 to array-of-objects shape, from $ReadOnlyArray<string>
+  topic_links?: $ReadOnlyArray<{| +text: string, +url: string |}> | $ReadOnlyArray<string>,
+
+  // TODO(server-3.0): Replaced in feat. 1 by topic_links
+  subject_links?: $ReadOnlyArray<string>,
 |}>;
 
 /**

src/caughtup/__tests__/caughtUpReducer-test.js

@@ -4,7 +4,15 @@ import deepFreeze from 'deep-freeze';
 import * as eg from '../../__tests__/lib/exampleData';
 import caughtUpReducer from '../caughtUpReducer';
 import { MESSAGE_FETCH_ERROR } from '../../actionConstants';
-import { HOME_NARROW, HOME_NARROW_STR, SEARCH_NARROW } from '../../utils/narrow';
+import {
+  HOME_NARROW,
+  HOME_NARROW_STR,
+  keyFromNarrow,
+  SEARCH_NARROW,
+  streamNarrow,
+  topicNarrow,
+} from '../../utils/narrow';
+import { objectFromEntries } from '../../jsBackport';
 
 describe('caughtUpReducer', () => {
   describe('MESSAGE_FETCH_START', () => {
@@ -140,4 +148,76 @@ describe('caughtUpReducer', () => {
 
     expect(newState).toEqual(expectedState);
   });
+
+  describe('EVENT_UPDATE_MESSAGE', () => {
+    const mkAction = args => {
+      const { messages, ...restArgs } = args;
+      const message = messages[0];
+      return eg.mkActionEventUpdateMessage({
+        message_id: message.id,
+        message_ids: messages.map(m => m.id),
+        stream_id: message.stream_id,
+        orig_subject: message.subject,
+        ...restArgs,
+      });
+    };
+
+    const mkKey = (stream, topic) =>
+      topic !== undefined
+        ? keyFromNarrow(topicNarrow(stream.stream_id, topic))
+        : keyFromNarrow(streamNarrow(stream.stream_id));
+
+    const topic1 = 'topic foo';
+    const topic2 = 'topic bar';
+    // const message1a = eg.streamMessage({ subject: topic1, id: 1 });
+    const message1b = eg.streamMessage({ subject: topic1, id: 2 });
+    // const message1c = eg.streamMessage({ subject: topic1, id: 3 });
+    // const message2a = eg.streamMessage({ subject: topic2, id: 4 });
+
+    test('new topic, same stream', () => {
+      expect(
+        caughtUpReducer(
+          objectFromEntries([
+            [mkKey(eg.stream, topic1), { older: true, newer: true }],
+            [mkKey(eg.stream, topic2), { older: true, newer: true }],
+            [mkKey(eg.stream), { older: true, newer: true }],
+          ]),
+          mkAction({ messages: [message1b], subject: topic2 }),
+        ),
+      ).toEqual(
+        objectFromEntries([
+          // old topic narrow remains caught up:
+          [mkKey(eg.stream, topic1), { older: true, newer: true }],
+          // new topic narrow gets cleared
+          // stream narrow unchanged:
+          [mkKey(eg.stream), { older: true, newer: true }],
+        ]),
+      );
+    });
+
+    test('same topic, new stream', () => {
+      expect(
+        caughtUpReducer(
+          objectFromEntries([
+            [mkKey(eg.stream, topic1), { older: true, newer: true }],
+            [mkKey(eg.stream), { older: true, newer: true }],
+            [mkKey(eg.otherStream, topic1), { older: true, newer: true }],
+            [mkKey(eg.otherStream), { older: true, newer: true }],
+          ]),
+          mkAction({ messages: [message1b], new_stream_id: eg.otherStream.stream_id }),
+        ),
+      ).toEqual(
+        objectFromEntries([
+          // old topic and stream narrows remain caught up:
+          [mkKey(eg.stream, topic1), { older: true, newer: true }],
+          [mkKey(eg.stream), { older: true, newer: true }],
+          // new topic and stream narrows both cleared
+        ]),
+      );
+    });
+
+    // Try to keep these tests corresponding closely to those for the
+    // narrows reducer.  (In the future these should really be a single
+    // sub-reducer.)
+  });
 });