Skip to content

Commit e5b41f7

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 6e42d34 commit e5b41f7

File tree

4 files changed

+206
-0
lines changed

4 files changed

+206
-0
lines changed

lib/api/model/model.dart

Lines changed: 83 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+
// The value passed here must be a MessageEditState already due to
498+
// processing work done in [MessageEditState.readFromMessage].
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,74 @@ 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+
// Pattern for an arbitrary resolved-topic prefix.
722+
// These always begin with the canonical prefix, but can go on longer.
723+
// It's designed to remove a weird "✔ ✔✔ " prefix, if present.
724+
static RegExp _resolvedTopicPrefixRe = RegExp('^✔ [ ✔]*');
725+
726+
/// Whether two topics are equal, ignoring any resolved-topic prefix.
727+
///
728+
/// When a topic is resolved, the clients agree on adding a ✔ prefix to the
729+
/// topic string. Topics whose only difference is the ✔ prefix are considered
730+
/// the same. This helper can be helpful when checking if a message has been
731+
/// moved.
732+
static bool areSameTopic(String topic, String prevTopic) {
733+
// TODO(#744) Extract this to its own home to support "mark as resolve".
734+
topic = topic.replaceFirst(_resolvedTopicPrefixRe, '');
735+
prevTopic = prevTopic.replaceFirst(_resolvedTopicPrefixRe, '');
736+
737+
return topic == prevTopic;
738+
}
739+
740+
static MessageEditState _readFromMessage(Map<dynamic, dynamic> json, String key) {
741+
// Adapted from the web app:
742+
// https://github.com/zulip/zulip/blob/c31cebbf68a93927d41e9947427c2dd4d46503e3/web/src/message_list_view.js#L68-L118
743+
final editHistory = json['edit_history'] as List<dynamic>?;
744+
final lastEditTimestamp = json['last_edit_timestamp'] as int?;
745+
if (editHistory == null) {
746+
return (lastEditTimestamp != null)
747+
? MessageEditState.edited
748+
: MessageEditState.none;
749+
}
750+
751+
// Edit history should never be empty whenever it is present
752+
assert(editHistory.isNotEmpty);
753+
754+
bool hasMoved = false;
755+
for (final entry in editHistory) {
756+
if (entry['prev_content'] != null) {
757+
return MessageEditState.edited;
758+
}
759+
760+
if (entry['prev_stream'] != null) {
761+
hasMoved = true;
762+
continue;
763+
}
764+
765+
// TODO(server-5) prev_subject was the old name of prev_topic on pre-5.0 servers
766+
final prevTopic = (entry['prev_topic'] ?? entry['prev_subject']) as String?;
767+
final topic = entry['topic'] as String?;
768+
if (prevTopic != null) {
769+
// TODO(server-5) pre-5.0 servers do not have the 'topic' field
770+
if (topic == null) {
771+
hasMoved = true;
772+
} else {
773+
hasMoved |= !areSameTopic(topic, prevTopic);
774+
}
775+
}
776+
}
777+
778+
if (hasMoved) return MessageEditState.moved;
779+
780+
// This can happen when a topic is resolved but nothing else has been edited
781+
return MessageEditState.none;
782+
}
783+
}

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: 110 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,111 @@ 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+
// TODO(server-5) prev_subject was the old name of prev_topic on pre-5.0 servers
286+
test("'prev_subject' present from a pre-5.0 server -> moved", () {
287+
checkEditState(MessageEditState.moved,
288+
[{'prev_subject': 'old_topic'}]);
289+
});
290+
});
291+
292+
group('topic resolved in edit history', () {
293+
test('Topic was only resolved -> none', () {
294+
checkEditState(MessageEditState.none,
295+
[{'prev_topic': 'old_topic', 'topic': '✔ old_topic'}]);
296+
});
297+
298+
test('Topic was resolved but the content changed in the history -> edited', () {
299+
checkEditState(MessageEditState.edited, [
300+
{'prev_topic': 'old_topic', 'topic': '✔ old_topic'},
301+
{'prev_content': 'old_content'},
302+
]);
303+
});
304+
305+
test('Topic was resolved but it also moved in the history -> moved', () {
306+
checkEditState(MessageEditState.moved, [
307+
{'prev_topic': 'old_topic', 'topic': 'new_topic'},
308+
{'prev_topic': '✔ old_topic', 'topic': 'old_topic'},
309+
]);
310+
});
311+
312+
test('Topic was moved but it also was resolved in the history -> moved', () {
313+
checkEditState(MessageEditState.moved, [
314+
{'prev_topic': '✔ old_topic', 'topic': 'old_topic'},
315+
{'prev_topic': 'old_topic', 'topic': 'new_topic'},
316+
]);
317+
});
318+
319+
test('Unresolving topic with a weird prefix -> none', () {
320+
checkEditState(MessageEditState.none,
321+
[{'prev_topic': '✔ ✔old_topic', 'topic': 'old_topic'}]);
322+
});
323+
});
324+
});
215325
}

0 commit comments

Comments
 (0)