Skip to content

Commit 95c1619

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 f120857 commit 95c1619

File tree

4 files changed

+184
-0
lines changed

4 files changed

+184
-0
lines changed

lib/api/model/model.dart

+87
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,78 @@ 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+
// Code adapted from the shared code: web/shared/src/resolve_topic.ts
720+
// Pattern for an arbitrary resolved-topic prefix.
721+
// These always begin with the canonical prefix, but can go on longer.
722+
// It's designed to remove a weird "✔ ✔✔ " prefix, if present.
723+
static RegExp resolvedTopicPrefixRe = RegExp('^✔ [ ✔]*');
724+
725+
/// Whether two topics are equal, ignoring any resolved-topic prefix.
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 areSameTopic(String topic, String prevTopic) {
732+
// TODO(#744) Extract this to its own home to support "mark as resolve".
733+
topic = topic.replaceFirst(resolvedTopicPrefixRe, '');
734+
prevTopic = prevTopic.replaceFirst(resolvedTopicPrefixRe, '');
735+
736+
return topic == prevTopic;
737+
}
738+
739+
static MessageEditState readFromMessage(Map<dynamic, dynamic> json, String key) {
740+
// TODO refactor this into a helper that computes this from the serialized
741+
// MessageEditHistory.
742+
final editHistory = json['edit_history'] as List<dynamic>?;
743+
final lastEditTimestamp = json['last_edit_timestamp'] as int?;
744+
if (editHistory == null) {
745+
return (lastEditTimestamp != null)
746+
? MessageEditState.edited
747+
: MessageEditState.none;
748+
}
749+
750+
// Edit history should never be empty whenever it is present
751+
assert(editHistory.isNotEmpty);
752+
753+
bool hasEditedContent = false;
754+
bool hasMoved = false;
755+
for (final entry in editHistory) {
756+
if (entry['prev_content'] != null) {
757+
hasEditedContent = true;
758+
}
759+
760+
if (entry['prev_stream'] != null) {
761+
hasMoved = true;
762+
}
763+
764+
// TODO(server-5) prev_subject was the old name of prev_topic on pre-5.0 servers
765+
if (entry['prev_topic'] != null || entry['prev_subject'] != null) {
766+
// TODO(server-5) pre-5.0 servers do not have the 'topic' field
767+
if (entry['topic'] == null) {
768+
hasMoved = true;
769+
} else {
770+
hasMoved = !areSameTopic(
771+
entry['topic'] as String,
772+
// TODO(server-5) prev_subject was the old name of prev_topic on pre-5.0 servers
773+
(entry['prev_topic'] ?? entry['prev_subject']) as String
774+
);
775+
}
776+
}
777+
}
778+
779+
// Prioritize the 'edited' state over 'moved' when they both apply
780+
if (hasEditedContent) return MessageEditState.edited;
781+
782+
if (hasMoved) return MessageEditState.moved;
783+
784+
// This can happen when a topic is resolved but nothing else has been edited
785+
return MessageEditState.none;
786+
}
787+
}

lib/api/model/model.g.dart

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

test/api/model/model_checks.dart

+1
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

+84
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,85 @@ 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+
test('edit history is absent', () {
223+
check(Message.fromJson(baseJson()..['edit_history'] = null))
224+
.editState.equals(MessageEditState.none);
225+
226+
check(Message.fromJson(baseJson()
227+
..['edit_history'] = null
228+
..['last_edit_timestamp'] = 1678139636))
229+
.editState.equals(MessageEditState.edited);
230+
});
231+
232+
void checkEditState(MessageEditState editState, List<Map<String, dynamic>> editHistory){
233+
check(Message.fromJson(baseJson()..['edit_history'] = editHistory))
234+
.editState.equals(editState);
235+
}
236+
237+
group('edit history exists', () {
238+
test('Channel change only -> moved', () {
239+
checkEditState(MessageEditState.moved,
240+
[{'prev_stream': 5, 'stream': 7}]);
241+
});
242+
243+
test('Topic name change only -> moved', () {
244+
checkEditState(MessageEditState.moved,
245+
[{'prev_topic': 'old_topic', 'topic': 'new_topic'}]);
246+
});
247+
248+
test('Both topic and content changed -> edited', () {
249+
checkEditState(MessageEditState.edited, [
250+
{'prev_topic': 'old_topic', 'topic': 'new_topic'},
251+
{'prev_content': 'old_content'},
252+
]);
253+
});
254+
255+
test('Both topic and content changed in a single edit -> edited', () {
256+
checkEditState(MessageEditState.edited,
257+
[{'prev_topic': 'old_topic', 'topic': 'new_topic', 'prev_content': 'old_content'}]);
258+
});
259+
260+
test('Content change only -> edited', () {
261+
checkEditState(MessageEditState.edited,
262+
[{'prev_content': 'old_content'}]);
263+
});
264+
265+
test("'prev_topic' present without the 'topic' field -> moved", () {
266+
checkEditState(MessageEditState.moved,
267+
[{'prev_topic': 'old_topic'}]);
268+
});
269+
});
270+
271+
group('topic resolved in edit history', () {
272+
test('Topic was only resolved -> none', () {
273+
checkEditState(MessageEditState.none,
274+
[{'prev_topic': 'old_topic', 'topic': '✔ old_topic'}]);
275+
});
276+
277+
test('Topic was resolved but the content changed in the history -> edited', () {
278+
checkEditState(MessageEditState.edited, [
279+
{'prev_topic': 'old_topic', 'topic': '✔ old_topic'},
280+
{'prev_content': 'old_content'},
281+
]);
282+
});
283+
284+
test('Topic was resolved but it also changed in the history -> moved', () {
285+
checkEditState(MessageEditState.moved, [
286+
{'prev_topic': '✔ old_topic', 'topic': 'old_topic'},
287+
{'prev_topic': 'old_topic', 'topic': 'new_topic'},
288+
]);
289+
});
290+
291+
292+
test('Unresolving topic with a weird prefix -> none', () {
293+
checkEditState(MessageEditState.none,
294+
[{'prev_topic': '✔ ✔old_topic', 'topic': 'old_topic'}]);
295+
});
296+
297+
});
298+
});
215299
}

0 commit comments

Comments
 (0)