Skip to content

Commit 7fc1a18

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 8bfeaff commit 7fc1a18

File tree

4 files changed

+201
-0
lines changed

4 files changed

+201
-0
lines changed

lib/api/model/model.dart

+90
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,9 @@ sealed class Message {
634634
final String contentType;
635635

636636
// final List<MessageEditHistory> editHistory; // TODO handle
637+
@JsonKey(readValue: MessageEditState.readFromMessage, fromJson: Message._messageEditStateFromJson)
638+
MessageEditState editState;
639+
637640
final int id;
638641
bool isMeMessage;
639642
int? lastEditTimestamp;
@@ -660,6 +663,12 @@ sealed class Message {
660663
@JsonKey(name: 'match_subject')
661664
final String? matchTopic;
662665

666+
static MessageEditState _messageEditStateFromJson(dynamic json) {
667+
// The value passed here must be a MessageEditState already due to
668+
// processing work done in [MessageEditState.readFromMessage].
669+
return json as MessageEditState;
670+
}
671+
663672
static Reactions? _reactionsFromJson(dynamic json) {
664673
final list = (json as List<dynamic>);
665674
return list.isNotEmpty ? Reactions.fromJson(list) : null;
@@ -678,6 +687,7 @@ sealed class Message {
678687
required this.client,
679688
required this.content,
680689
required this.contentType,
690+
required this.editState,
681691
required this.id,
682692
required this.isMeMessage,
683693
required this.lastEditTimestamp,
@@ -743,6 +753,7 @@ class StreamMessage extends Message {
743753
required super.client,
744754
required super.content,
745755
required super.contentType,
756+
required super.editState,
746757
required super.id,
747758
required super.isMeMessage,
748759
required super.lastEditTimestamp,
@@ -845,6 +856,7 @@ class DmMessage extends Message {
845856
required super.client,
846857
required super.content,
847858
required super.contentType,
859+
required super.editState,
848860
required super.id,
849861
required super.isMeMessage,
850862
required super.lastEditTimestamp,
@@ -868,3 +880,81 @@ class DmMessage extends Message {
868880
@override
869881
Map<String, dynamic> toJson() => _$DmMessageToJson(this);
870882
}
883+
884+
enum MessageEditState {
885+
none,
886+
edited,
887+
moved;
888+
889+
static bool isTopicMoved(String topic, String prevTopic) {
890+
// TODO(#744) Extract this to its own home to fully support mark as resolve
891+
892+
// Code adapted from the shared code: web/shared/src/resolve_topic.ts
893+
894+
/**
895+
* Pattern for an arbitrary resolved-topic prefix.
896+
*
897+
* These always begin with the canonical prefix, but can go on longer.
898+
*/
899+
// The class has the same characters as RESOLVED_TOPIC_PREFIX.
900+
// It's designed to remove a weird "✔ ✔✔ " prefix, if present.
901+
final RegExp resolvedTopicPrefixRe = RegExp('^✔ [ ✔]*');
902+
903+
// Normalize both topics so the resolved-topic prefix do not interfere.
904+
topic = topic.replaceFirst(resolvedTopicPrefixRe, '');
905+
prevTopic = prevTopic.replaceFirst(resolvedTopicPrefixRe, '');
906+
907+
return topic != prevTopic;
908+
}
909+
910+
static MessageEditState readFromMessage(Map<dynamic, dynamic> json, String key) {
911+
return isMoved(json['edit_history'] as List<dynamic>?, json['last_edit_timestamp'] as int?);
912+
}
913+
914+
static MessageEditState isMoved(List<dynamic>? editHistory, int? lastEditTimeStamp) {
915+
// TODO refactor this into a helper that computes this from the serialized
916+
// MessageEditHistory.
917+
if (editHistory == null) {
918+
return (lastEditTimeStamp != null)
919+
? MessageEditState.edited : MessageEditState.none;
920+
}
921+
922+
// Edit history should never be empty whenever it is present
923+
assert(editHistory.isNotEmpty);
924+
925+
bool hasEditedContent = false;
926+
bool hasMoved = false;
927+
for (final entry in editHistory) {
928+
if (entry['prev_content'] != null) {
929+
hasEditedContent = true;
930+
}
931+
932+
if (entry['prev_stream'] != null) {
933+
hasMoved = true;
934+
}
935+
936+
// TODO(server-5) prev_subject was the old name of prev_topic on pre-5.0 servers
937+
if (entry['prev_topic'] != null || entry['prev_subject'] != null) {
938+
// TODO(server-5) pre-5.0 servers do not have the 'topic' field
939+
if (entry['topic'] == null) {
940+
hasMoved = true;
941+
}
942+
else {
943+
hasMoved = isTopicMoved(
944+
entry['topic'] as String,
945+
// TODO(server-5) prev_subject was the old name of prev_topic on pre-5.0 servers
946+
(entry['prev_topic'] ?? entry['prev_subject']) as String
947+
);
948+
}
949+
}
950+
}
951+
952+
// Prioritize the 'edited' state over 'moved' when they both apply
953+
if (hasEditedContent) return MessageEditState.edited;
954+
955+
if (hasMoved) return MessageEditState.moved;
956+
957+
// This can happen when a topic is resolved but nothing else has been edited
958+
return MessageEditState.none;
959+
}
960+
}

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
@@ -44,6 +44,7 @@ extension MessageChecks on Subject<Message> {
4444
Subject<int> get id => has((e) => e.id, 'id');
4545
Subject<bool> get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage');
4646
Subject<int?> get lastEditTimestamp => has((e) => e.lastEditTimestamp, 'lastEditTimestamp');
47+
Subject<MessageEditState> get editState => has((e) => e.editState, 'editState');
4748
Subject<Reactions?> get reactions => has((e) => e.reactions, 'reactions');
4849
Subject<int> get recipientId => has((e) => e.recipientId, 'recipientId');
4950
Subject<String> get senderEmail => has((e) => e.senderEmail, 'senderEmail');

test/api/model/model_test.dart

+98
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,9 @@ void main() {
581581
);
582582
check(m2).flags.deepEquals([MessageFlag.read, MessageFlag.unknown]);
583583
});
584+
585+
// Code relevant to messageEditState is tested separately in the
586+
// MessageEditState group.
584587
});
585588

586589
group('DmMessage', () {
@@ -645,4 +648,99 @@ void main() {
645648
.deepEquals([2, 3, 11]);
646649
});
647650
});
651+
652+
group('MessageEditState', () {
653+
Map<String, dynamic> baseJson() => deepToJson(eg.streamMessage()) as Map<String, dynamic>;
654+
655+
test('edit history is absent', () {
656+
check(Message.fromJson(baseJson()
657+
..['edit_history'] = null
658+
).editState).equals(MessageEditState.none);
659+
660+
check(Message.fromJson(baseJson()
661+
..['edit_history'] = null
662+
..['last_edit_timestamp'] = 1678139636).editState
663+
).equals(MessageEditState.edited);
664+
});
665+
666+
void checkEditState(MessageEditState editState, List<Map<String, dynamic>> editHistory){
667+
return check(Message.fromJson(baseJson()
668+
..['edit_history'] = editHistory
669+
).editState).equals(editState);
670+
}
671+
672+
group('edit history exists', () {
673+
test('Channel name change only -> moved', () {
674+
checkEditState(MessageEditState.moved, [{
675+
'prev_stream': 'old_stream',
676+
'stream': 'new_stream',
677+
}]);
678+
});
679+
680+
test('Topic name change only -> moved', () {
681+
checkEditState(MessageEditState.moved, [{
682+
'prev_topic': 'old_topic',
683+
'topic': 'new_topic',
684+
}]);
685+
});
686+
687+
test('Both topic and content changed -> edited', () {
688+
checkEditState(MessageEditState.edited, [{
689+
'prev_topic': 'old_topic',
690+
'topic': 'new_topic',
691+
}, {
692+
'prev_content': 'old_content',
693+
}]);
694+
});
695+
696+
test('Content change only -> edited', () {
697+
checkEditState(MessageEditState.edited, [{
698+
'prev_content': 'old_content',
699+
}]);
700+
});
701+
702+
test("'prev_topic' present without the 'topic' field -> moved", () {
703+
checkEditState(MessageEditState.moved, [{
704+
'prev_topic': 'old_topic',
705+
}]);
706+
});
707+
});
708+
709+
group('topic resolved in edit history', () {
710+
test('Topic was only resolved -> none', () {
711+
checkEditState(MessageEditState.none, [{
712+
'prev_topic': 'old_topic',
713+
'topic': '✔ old_topic',
714+
}]);
715+
});
716+
717+
test('Topic was resolved but the content changed in the history -> edited', () {
718+
checkEditState(MessageEditState.edited, [{
719+
'prev_topic': 'old_topic',
720+
'topic': '✔ old_topic',
721+
}, {
722+
'prev_content': 'old_content',
723+
}]);
724+
});
725+
726+
test('Topic was resolved but it also changed in the history -> moved', () {
727+
checkEditState(MessageEditState.moved, [{
728+
'prev_topic': '✔ old_topic',
729+
'topic': 'old_topic',
730+
}, {
731+
'prev_topic': 'old_topic',
732+
'topic': 'new_topic',
733+
}]);
734+
});
735+
736+
737+
test('Unresolving topic with a weird prefix -> none', () {
738+
checkEditState(MessageEditState.none, [{
739+
'prev_topic': '✔ ✔old_topic',
740+
'topic': 'old_topic',
741+
}]);
742+
});
743+
744+
});
745+
});
648746
}

0 commit comments

Comments
 (0)