Skip to content

Commit fb2e6bc

Browse files
committed
model: Add MessageEditState to Message.
This new field is computed from edit_history provided by the API. We create a readValue function that processes the list of edits and determine if message has been edited or moved. Some special handling was needed because topic being marked as resolved should not be considered "moved". Signed-off-by: Zixuan James Li <[email protected]>
1 parent 7dce35f commit fb2e6bc

File tree

4 files changed

+210
-0
lines changed

4 files changed

+210
-0
lines changed

lib/api/model/model.dart

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,9 @@ sealed class Message {
464464
final String contentType;
465465

466466
// final List<MessageEditHistory> editHistory; // TODO handle
467+
@JsonKey(readValue: MessageEditState._readFromMessage, fromJson: Message._messageEditStateFromJson)
468+
MessageEditState editState;
469+
467470
final int id;
468471
bool isMeMessage;
469472
int? lastEditTimestamp;
@@ -490,6 +493,12 @@ sealed class Message {
490493
@JsonKey(name: 'match_subject')
491494
final String? matchTopic;
492495

496+
static MessageEditState _messageEditStateFromJson(dynamic json) {
497+
// This is a no-op so that [MessageEditState._readFromMessage]
498+
// can return the enum value directly.
499+
return json as MessageEditState;
500+
}
501+
493502
static Reactions? _reactionsFromJson(dynamic json) {
494503
final list = (json as List<dynamic>);
495504
return list.isNotEmpty ? Reactions.fromJson(list) : null;
@@ -508,6 +517,7 @@ sealed class Message {
508517
required this.client,
509518
required this.content,
510519
required this.contentType,
520+
required this.editState,
511521
required this.id,
512522
required this.isMeMessage,
513523
required this.lastEditTimestamp,
@@ -573,6 +583,7 @@ class StreamMessage extends Message {
573583
required super.client,
574584
required super.content,
575585
required super.contentType,
586+
required super.editState,
576587
required super.id,
577588
required super.isMeMessage,
578589
required super.lastEditTimestamp,
@@ -675,6 +686,7 @@ class DmMessage extends Message {
675686
required super.client,
676687
required super.content,
677688
required super.contentType,
689+
required super.editState,
678690
required super.id,
679691
required super.isMeMessage,
680692
required super.lastEditTimestamp,
@@ -698,3 +710,79 @@ class DmMessage extends Message {
698710
@override
699711
Map<String, dynamic> toJson() => _$DmMessageToJson(this);
700712
}
713+
714+
enum MessageEditState {
715+
none,
716+
edited,
717+
moved;
718+
719+
// Adapted from the shared code:
720+
// https://github.com/zulip/zulip/blob/1fac99733/web/shared/src/resolved_topic.ts
721+
// The canonical resolved-topic prefix.
722+
static const String _resolvedTopicPrefix = '✔ ';
723+
724+
/// Whether the two topics differs and only differs by whether one of them is
725+
/// resolved or not.
726+
///
727+
/// When a topic is resolved, the clients agree on adding a ✔ prefix to the
728+
/// topic string. Topics whose only difference is the ✔ prefix are considered
729+
/// the same. This helper can be helpful when checking if a message has been
730+
/// moved.
731+
static bool topicsDifferByResolvednessOnly(String topic, String prevTopic) {
732+
if (topic.startsWith(_resolvedTopicPrefix)
733+
&& topic.substring(_resolvedTopicPrefix.length) == prevTopic) {
734+
return true;
735+
}
736+
737+
if (prevTopic.startsWith(_resolvedTopicPrefix)
738+
&& prevTopic.substring(_resolvedTopicPrefix.length) == topic) {
739+
return true;
740+
}
741+
742+
return false;
743+
}
744+
745+
static MessageEditState _readFromMessage(Map<dynamic, dynamic> json, String key) {
746+
// Adapted from `analyze_edit_history` in the web app:
747+
// https://github.com/zulip/zulip/blob/c31cebbf68a93927d41e9947427c2dd4d46503e3/web/src/message_list_view.js#L68-L118
748+
final editHistory = json['edit_history'] as List<dynamic>?;
749+
final lastEditTimestamp = json['last_edit_timestamp'] as int?;
750+
if (editHistory == null) {
751+
return (lastEditTimestamp != null)
752+
? MessageEditState.edited
753+
: MessageEditState.none;
754+
}
755+
756+
// Edit history should never be empty whenever it is present
757+
assert(editHistory.isNotEmpty);
758+
759+
bool hasMoved = false;
760+
for (final entry in editHistory) {
761+
if (entry['prev_content'] != null) {
762+
return MessageEditState.edited;
763+
}
764+
765+
if (entry['prev_stream'] != null) {
766+
hasMoved = true;
767+
continue;
768+
}
769+
770+
// TODO(server-5) prev_subject was the old name of prev_topic on pre-5.0 servers
771+
final prevTopic = (entry['prev_topic'] ?? entry['prev_subject']) as String?;
772+
final topic = entry['topic'] as String?;
773+
if (prevTopic != null) {
774+
// TODO(server-5) pre-5.0 servers do not have the 'topic' field
775+
if (topic == null) {
776+
hasMoved = true;
777+
} else {
778+
hasMoved |= !topicsDifferByResolvednessOnly(topic, prevTopic);
779+
}
780+
}
781+
}
782+
783+
if (hasMoved) return MessageEditState.moved;
784+
785+
// This can happen when a topic is resolved but nothing else has been edited
786+
return MessageEditState.none;
787+
}
788+
}

lib/api/model/model.g.dart

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/api/model/model_checks.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ extension MessageChecks on Subject<Message> {
3434
Subject<int> get id => has((e) => e.id, 'id');
3535
Subject<bool> get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage');
3636
Subject<int?> get lastEditTimestamp => has((e) => e.lastEditTimestamp, 'lastEditTimestamp');
37+
Subject<MessageEditState> get editState => has((e) => e.editState, 'editState');
3738
Subject<Reactions?> get reactions => has((e) => e.reactions, 'reactions');
3839
Subject<int> get recipientId => has((e) => e.recipientId, 'recipientId');
3940
Subject<String> get senderEmail => has((e) => e.senderEmail, 'senderEmail');

test/api/model/model_test.dart

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,9 @@ void main() {
148148
);
149149
check(m2).flags.deepEquals([MessageFlag.read, MessageFlag.unknown]);
150150
});
151+
152+
// Code relevant to messageEditState is tested separately in the
153+
// MessageEditState group.
151154
});
152155

153156
group('DmMessage', () {
@@ -212,4 +215,110 @@ void main() {
212215
.deepEquals([2, 3, 11]);
213216
});
214217
});
218+
219+
group('MessageEditState', () {
220+
Map<String, dynamic> baseJson() => deepToJson(eg.streamMessage()) as Map<String, dynamic>;
221+
222+
group('Edit history is absent', () {
223+
test('Message with no evidence of an edit history -> none', () {
224+
check(Message.fromJson(baseJson()..['edit_history'] = null))
225+
.editState.equals(MessageEditState.none);
226+
});
227+
228+
test('Message without edit history has last edit timestamp -> edited', () {
229+
check(Message.fromJson(baseJson()
230+
..['edit_history'] = null
231+
..['last_edit_timestamp'] = 1678139636))
232+
.editState.equals(MessageEditState.edited);
233+
});
234+
});
235+
236+
void checkEditState(MessageEditState editState, List<Map<String, dynamic>> editHistory){
237+
check(Message.fromJson(baseJson()..['edit_history'] = editHistory))
238+
.editState.equals(editState);
239+
}
240+
241+
group('edit history exists', () {
242+
test('Moved message has last edit timestamp but no actual edits -> moved', () {
243+
check(Message.fromJson(baseJson()
244+
..['edit_history'] = [{'prev_stream': 5, 'stream': 7}]
245+
..['last_edit_timestamp'] = 1678139636))
246+
.editState.equals(MessageEditState.moved);
247+
});
248+
249+
test('Channel change only -> moved', () {
250+
checkEditState(MessageEditState.moved,
251+
[{'prev_stream': 5, 'stream': 7}]);
252+
});
253+
254+
test('Topic name change only -> moved', () {
255+
checkEditState(MessageEditState.moved,
256+
[{'prev_topic': 'old_topic', 'topic': 'new_topic'}]);
257+
});
258+
259+
test('Both topic and content changed -> edited', () {
260+
checkEditState(MessageEditState.edited, [
261+
{'prev_topic': 'old_topic', 'topic': 'new_topic'},
262+
{'prev_content': 'old_content'},
263+
]);
264+
checkEditState(MessageEditState.edited, [
265+
{'prev_content': 'old_content'},
266+
{'prev_topic': 'old_topic', 'topic': 'new_topic'},
267+
]);
268+
});
269+
270+
test('Both topic and content changed in a single edit -> edited', () {
271+
checkEditState(MessageEditState.edited,
272+
[{'prev_topic': 'old_topic', 'topic': 'new_topic', 'prev_content': 'old_content'}]);
273+
});
274+
275+
test('Content change only -> edited', () {
276+
checkEditState(MessageEditState.edited,
277+
[{'prev_content': 'old_content'}]);
278+
});
279+
280+
test("'prev_topic' present without the 'topic' field -> moved", () {
281+
checkEditState(MessageEditState.moved,
282+
[{'prev_topic': 'old_topic'}]);
283+
});
284+
285+
test("'prev_subject' present from a pre-5.0 server -> moved", () {
286+
checkEditState(MessageEditState.moved,
287+
[{'prev_subject': 'old_topic'}]);
288+
});
289+
});
290+
291+
group('topic resolved in edit history', () {
292+
test('Topic was only resolved -> none', () {
293+
checkEditState(MessageEditState.none,
294+
[{'prev_topic': 'old_topic', 'topic': '✔ old_topic'}]);
295+
});
296+
297+
test('Topic was resolved but the content changed in the history -> edited', () {
298+
checkEditState(MessageEditState.edited, [
299+
{'prev_topic': 'old_topic', 'topic': '✔ old_topic'},
300+
{'prev_content': 'old_content'},
301+
]);
302+
});
303+
304+
test('Topic was resolved but it also moved in the history -> moved', () {
305+
checkEditState(MessageEditState.moved, [
306+
{'prev_topic': 'old_topic', 'topic': 'new_topic'},
307+
{'prev_topic': '✔ old_topic', 'topic': 'old_topic'},
308+
]);
309+
});
310+
311+
test('Topic was moved but it also was resolved in the history -> moved', () {
312+
checkEditState(MessageEditState.moved, [
313+
{'prev_topic': '✔ old_topic', 'topic': 'old_topic'},
314+
{'prev_topic': 'old_topic', 'topic': 'new_topic'},
315+
]);
316+
});
317+
318+
test('Unresolving topic with a weird prefix -> moved', () {
319+
checkEditState(MessageEditState.moved,
320+
[{'prev_topic': '✔ ✔old_topic', 'topic': 'old_topic'}]);
321+
});
322+
});
323+
});
215324
}

0 commit comments

Comments
 (0)