api [nfc]: Extract and parse UpdateMessageMoveData from UpdateMessageEvent

PIG208 · PIG208 · commit 28895d4b514f · 2025-02-20T15:51:51.000-05:00
While this says [nfc], there is actually a change: `toJson` no longer
includes these message move related fields.  However, this seems to have nearly
no impact on the tests, and clearly does not affect the app.

Signed-off-by: Zixuan James Li &lt;zixuan@zulip.com&gt;
diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart
@@ -701,6 +701,52 @@ class MessageEvent extends Event {
   }
 }
 
+/// Data structure representing a message move.
+class UpdateMessageMoveData {
+  final int? origStreamId;
+  final int? newStreamId;
+
+  final PropagateMode? propagateMode;
+
+  final TopicName? origTopic;
+  final TopicName? newTopic;
+
+  UpdateMessageMoveData({
+    required this.origStreamId,
+    required this.newStreamId,
+    required this.propagateMode,
+    required this.origTopic,
+    required this.newTopic,
+  });
+
+  /// Extract [UpdateMessageMoveData] from the JSON object for an
+  /// [UpdateMessageEvent].
+  ///
+  /// Throws an error if the data is malformed.
+  factory UpdateMessageMoveData.fromJson(Object? json) {
+    json as Map<String, Object?>;
+    final origStreamId = (json['stream_id'] as num?)?.toInt();
+    final newStreamId = (json['new_stream_id'] as num?)?.toInt();
+    final propagateModeString = json['propagate_mode'] as String?;
+    final propagateMode = propagateModeString == null ? null
+      : PropagateMode.fromRawString(propagateModeString);
+    final origTopic = json['orig_subject'] == null ? null
+      : TopicName.fromJson(json['orig_subject'] as String);
+    final newTopic = json['subject'] == null ? null
+      : TopicName.fromJson(json['subject'] as String);
+
+    return UpdateMessageMoveData(
+      origStreamId: origStreamId,
+      newStreamId: newStreamId,
+      propagateMode: propagateMode,
+      origTopic: origTopic,
+      newTopic: newTopic,
+    );
+  }
+
+  Object? toJson() => null;
+}
+
 /// A Zulip event of type `update_message`: https://zulip.com/api/get-events#update_message
 @JsonSerializable(fieldRename: FieldRename.snake)
 class UpdateMessageEvent extends Event {
@@ -718,16 +764,8 @@ class UpdateMessageEvent extends Event {
 
   // final String? streamName; // ignore
 
-  @JsonKey(name: 'stream_id')
-  final int? origStreamId;
-  final int? newStreamId;
-
-  final PropagateMode? propagateMode;
-
-  @JsonKey(name: 'orig_subject')
-  final TopicName? origTopic;
-  @JsonKey(name: 'subject')
-  final TopicName? newTopic;
+  @JsonKey(readValue: _readMoveData)
+  final UpdateMessageMoveData moveData;
 
   // final List<TopicLink> topicLinks; // TODO handle
 
@@ -747,18 +785,19 @@ class UpdateMessageEvent extends Event {
     required this.messageIds,
     required this.flags,
     required this.editTimestamp,
-    required this.origStreamId,
-    required this.newStreamId,
-    required this.propagateMode,
-    required this.origTopic,
-    required this.newTopic,
+    required this.moveData,
     required this.origContent,
     required this.origRenderedContent,
     required this.content,
     required this.renderedContent,
     required this.isMeMessage,
   });
 
+  static Object? _readMoveData(Map<Object?, Object?> json, String key) {
+    // Parsing [UpdateMessageMoveData] requires `json`, not the default `json[key]`.
+    return json as Object?;
+  }
+
   factory UpdateMessageEvent.fromJson(Map<String, dynamic> json) =>
     _$UpdateMessageEventFromJson(json);
 
diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart
diff --git a/lib/model/message.dart b/lib/model/message.dart
@@ -172,11 +172,12 @@ class MessageStoreImpl with MessageStore {
     // The interaction between the fields of these events are a bit tricky.
     // For reference, see: https://zulip.com/api/get-events#update_message
 
-    final origStreamId = event.origStreamId;
-    final newStreamId = event.newStreamId; // null if topic-only move
-    final origTopic = event.origTopic;
-    final newTopic = event.newTopic;
-    final propagateMode = event.propagateMode;
+    final moveData = event.moveData;
+    final origStreamId = moveData.origStreamId;
+    final newStreamId = moveData.newStreamId; // null if topic-only move
+    final origTopic = moveData.origTopic;
+    final newTopic = moveData.newTopic;
+    final propagateMode = moveData.propagateMode;
 
     if (origTopic == null) {
       // There was no move.
diff --git a/test/api/model/events_checks.dart b/test/api/model/events_checks.dart
@@ -48,18 +48,23 @@ extension UpdateMessageEventChecks on Subject<UpdateMessageEvent> {
   Subject<List<int>> get messageIds => has((e) => e.messageIds, 'messageIds');
   Subject<List<MessageFlag>> get flags => has((e) => e.flags, 'flags');
   Subject<int?> get editTimestamp => has((e) => e.editTimestamp, 'editTimestamp');
-  Subject<int?> get origStreamId => has((e) => e.origStreamId, 'origStreamId');
-  Subject<int?> get newStreamId => has((e) => e.newStreamId, 'newStreamId');
-  Subject<PropagateMode?> get propagateMode => has((e) => e.propagateMode, 'propagateMode');
-  Subject<TopicName?> get origTopic => has((e) => e.origTopic, 'origTopic');
-  Subject<TopicName?> get newTopic => has((e) => e.newTopic, 'newTopic');
+  Subject<UpdateMessageMoveData?> get moveData => has((e) => e.moveData, 'moveData');
   Subject<String?> get origContent => has((e) => e.origContent, 'origContent');
   Subject<String?> get origRenderedContent => has((e) => e.origRenderedContent, 'origRenderedContent');
   Subject<String?> get content => has((e) => e.content, 'content');
   Subject<String?> get renderedContent => has((e) => e.renderedContent, 'renderedContent');
   Subject<bool?> get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage');
 }
 
+
+extension UpdateMessageMoveDataChecks on Subject<UpdateMessageMoveData> {
+  Subject<int?> get origStreamId => has((e) => e.origStreamId, 'origStreamId');
+  Subject<int?> get newStreamId => has((e) => e.newStreamId, 'newStreamId');
+  Subject<PropagateMode?> get propagateMode => has((e) => e.propagateMode, 'propagateMode');
+  Subject<TopicName?> get origTopic => has((e) => e.origTopic, 'origTopic');
+  Subject<TopicName?> get newTopic => has((e) => e.newTopic, 'newTopic');
+}
+
 extension DeleteMessageEventChecks on Subject<DeleteMessageEvent> {
   Subject<MessageType?> get messageType => has((e) => e.messageType, 'messageType');
 }
diff --git a/test/api/model/events_test.dart b/test/api/model/events_test.dart
@@ -101,14 +101,29 @@ void main() {
       'edit_timestamp': 1718741351,
     };
 
+    test('smoke moveData', () {
+      check(Event.fromJson({ ...baseJson,
+        'stream_id': 1,
+        'new_stream_id': 2,
+        'orig_subject': 'foo',
+        'subject': 'bar',
+        'propagate_mode': 'change_all',
+      })).isA<UpdateMessageEvent>().moveData.isNotNull()
+        ..origStreamId.equals(1)
+        ..newStreamId.equals(2)
+        ..origTopic.equals(TopicName('foo'))
+        ..newTopic.equals(TopicName('bar'))
+        ..propagateMode.equals(PropagateMode.changeAll);
+    });
+
     test('stream_id -> origStreamId, subject = orig_subject', () {
       check(Event.fromJson({ ...baseJson,
         'stream_id': 1,
         'new_stream_id': 2,
         'orig_subject': 'foo',
         'subject': null,
         'propagate_mode': 'change_all',
-      })).isA<UpdateMessageEvent>()
+      })).isA<UpdateMessageEvent>().moveData.isNotNull()
         ..origStreamId.equals(1)
         ..newStreamId.equals(2)
         ..origTopic.equals(TopicName('foo'))
@@ -122,12 +137,26 @@ void main() {
         'orig_subject': 'foo',
         'subject': 'bar',
         'propagate_mode': 'change_all',
-      })).isA<UpdateMessageEvent>()
+      })).isA<UpdateMessageEvent>().moveData.isNotNull()
         ..origStreamId.equals(1)
         ..newStreamId.isNull()
         ..origTopic.equals(const TopicName('foo'))
         ..newTopic.equals(const TopicName('bar'));
     });
+
+    test('no message move', () {
+      check(Event.fromJson({...baseJson,
+        'stream_id': 1,
+        'orig_content': 'foo',
+        'orig_rendered_content': 'foo',
+        'content': 'bar',
+        'rendered_content': 'bar',
+      })).isA<UpdateMessageEvent>().moveData.isNotNull()
+        ..origStreamId.equals(1)
+        ..newStreamId.isNull()
+        ..origTopic.isNull()
+        ..newTopic.isNull();
+    });
   });
 
   test('delete_message: require streamId and topic for stream messages', () {
diff --git a/test/example_data.dart b/test/example_data.dart
@@ -653,11 +653,13 @@ UpdateMessageEvent updateMessageEditEvent(
     messageIds: [messageId],
     flags: flags ?? origMessage.flags,
     editTimestamp: editTimestamp ?? 1234567890, // TODO generate timestamp
-    origStreamId: origMessage is StreamMessage ? origMessage.streamId : null,
-    newStreamId: null,
-    propagateMode: null,
-    origTopic: null,
-    newTopic: null,
+    moveData: UpdateMessageMoveData(
+      origStreamId: origMessage is StreamMessage ? origMessage.streamId : null,
+      newStreamId: null,
+      propagateMode: null,
+      origTopic: null,
+      newTopic: null,
+    ),
     origContent: 'some probably-mismatched old Markdown',
     origRenderedContent: origMessage.content,
     content: 'some probably-mismatched new Markdown',
@@ -690,11 +692,13 @@ UpdateMessageEvent _updateMessageMoveEvent(
     messageIds: messageIds,
     flags: flags,
     editTimestamp: 1234567890, // TODO generate timestamp
-    origStreamId: origStreamId,
-    newStreamId: newStreamId,
-    propagateMode: propagateMode,
-    origTopic: origTopic,
-    newTopic: newTopic,
+    moveData: UpdateMessageMoveData(
+      origStreamId: origStreamId,
+      newStreamId: newStreamId,
+      propagateMode: propagateMode,
+      origTopic: origTopic,
+      newTopic: newTopic,
+    ),
     origContent: origContent,
     origRenderedContent: origContent,
     content: newContent,
