Skip to content

Commit d41bb8d

Browse files
committed
api [nfc]: Rationalize "orig"/"new" field names on UpdateMessageEvent
In the server API here, "stream_id" is the old value and contrasts with "new_stream_id", but "subject" is the new value and contrasts with "orig_subject". This makes them confusing to read, and is the way it is only for historical reasons: I believe the story is that before Zulip Server 3.0 added support for moving topics between streams, three of those properties were already present and then "new_stream_id" was squeezed in at the best name still available. Fixing that in the API will be a pain because it'll need a migration path for compatibility. But we can fix it here within our code.
1 parent 40c29ae commit d41bb8d

File tree

5 files changed

+30
-14
lines changed

5 files changed

+30
-14
lines changed

lib/api/model/events.dart

+11-4
Original file line numberDiff line numberDiff line change
@@ -617,24 +617,31 @@ class UpdateMessageEvent extends Event {
617617
final bool? renderingOnly; // TODO(server-5)
618618
final int messageId;
619619
final List<int> messageIds;
620+
620621
final List<MessageFlag> flags;
621622
final int? editTimestamp; // TODO(server-5)
623+
622624
// final String? streamName; // ignore
623-
final int? streamId;
625+
626+
@JsonKey(name: 'stream_id')
627+
final int? origStreamId;
624628
final int? newStreamId;
629+
625630
final PropagateMode? propagateMode;
626631

627632
@JsonKey(name: 'orig_subject')
628633
final String? origTopic;
629634
@JsonKey(name: 'subject')
630-
final String? topic;
635+
final String? newTopic;
631636

632637
// final List<TopicLink> topicLinks; // TODO handle
638+
633639
final String? origContent;
634640
final String? origRenderedContent;
635641
// final int? prevRenderedContentVersion; // deprecated
636642
final String? content;
637643
final String? renderedContent;
644+
638645
final bool? isMeMessage;
639646

640647
UpdateMessageEvent({
@@ -645,11 +652,11 @@ class UpdateMessageEvent extends Event {
645652
required this.messageIds,
646653
required this.flags,
647654
required this.editTimestamp,
648-
required this.streamId,
655+
required this.origStreamId,
649656
required this.newStreamId,
650657
required this.propagateMode,
651658
required this.origTopic,
652-
required this.topic,
659+
required this.newTopic,
653660
required this.origContent,
654661
required this.origRenderedContent,
655662
required this.content,

lib/api/model/events.g.dart

+4-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/api/model/events_checks.dart

+2-2
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ extension UpdateMessageEventChecks on Subject<UpdateMessageEvent> {
3434
Subject<List<int>> get messageIds => has((e) => e.messageIds, 'messageIds');
3535
Subject<List<MessageFlag>> get flags => has((e) => e.flags, 'flags');
3636
Subject<int?> get editTimestamp => has((e) => e.editTimestamp, 'editTimestamp');
37-
Subject<int?> get streamId => has((e) => e.streamId, 'streamId');
37+
Subject<int?> get origStreamId => has((e) => e.origStreamId, 'origStreamId');
3838
Subject<int?> get newStreamId => has((e) => e.newStreamId, 'newStreamId');
3939
Subject<PropagateMode?> get propagateMode => has((e) => e.propagateMode, 'propagateMode');
4040
Subject<String?> get origTopic => has((e) => e.origTopic, 'origTopic');
41-
Subject<String?> get topic => has((e) => e.topic, 'topic');
41+
Subject<String?> get newTopic => has((e) => e.newTopic, 'newTopic');
4242
Subject<String?> get origContent => has((e) => e.origContent, 'origContent');
4343
Subject<String?> get origRenderedContent => has((e) => e.origRenderedContent, 'origRenderedContent');
4444
Subject<String?> get content => has((e) => e.content, 'content');

test/api/model/events_test.dart

+11-2
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,22 @@ void main() {
8080
'stream_id': eg.stream().streamId,
8181
};
8282

83-
test('orig_subject -> origTopic, subject -> topic', () {
83+
test('stream_id -> origStreamId', () {
84+
check(Event.fromJson({ ...baseJson,
85+
'stream_id': 1,
86+
'new_stream_id': 2,
87+
}) as UpdateMessageEvent)
88+
..origStreamId.equals(1)
89+
..newStreamId.equals(2);
90+
});
91+
92+
test('orig_subject -> origTopic, subject -> newTopic', () {
8493
check(Event.fromJson({ ...baseJson,
8594
'orig_subject': 'foo',
8695
'subject': 'bar',
8796
}) as UpdateMessageEvent)
8897
..origTopic.equals('foo')
89-
..topic.equals('bar');
98+
..newTopic.equals('bar');
9099
});
91100
});
92101

test/example_data.dart

+2-2
Original file line numberDiff line numberDiff line change
@@ -407,11 +407,11 @@ UpdateMessageEvent updateMessageEditEvent(
407407
messageIds: [messageId],
408408
flags: flags ?? origMessage.flags,
409409
editTimestamp: editTimestamp ?? 1234567890, // TODO generate timestamp
410-
streamId: origMessage is StreamMessage ? origMessage.streamId : null,
410+
origStreamId: origMessage is StreamMessage ? origMessage.streamId : null,
411411
newStreamId: null,
412412
propagateMode: null,
413413
origTopic: null,
414-
topic: null,
414+
newTopic: null,
415415
origContent: 'some probably-mismatched old Markdown',
416416
origRenderedContent: origMessage.content,
417417
content: 'some probably-mismatched new Markdown',

0 commit comments

Comments
 (0)