From 2fd0c4cf15cdc7e9fae447f79d621378862680db Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 2 Aug 2023 18:34:17 -0700 Subject: [PATCH 1/2] model: Add [Reaction] class; uncomment [Message.reactions] Related: #121 --- lib/api/model/model.dart | 35 ++++++++++++++++++++++++++++++++++- lib/api/model/model.g.dart | 28 ++++++++++++++++++++++++++++ test/example_data.dart | 3 ++- 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index f962bbcfa9..bb8af8ca45 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -258,7 +258,7 @@ sealed class Message { bool isMeMessage; int? lastEditTimestamp; - // final List reactions; // TODO handle + final List reactions; final int recipientId; final String senderEmail; final String senderFullName; @@ -282,6 +282,7 @@ sealed class Message { required this.id, required this.isMeMessage, this.lastEditTimestamp, + required this.reactions, required this.recipientId, required this.senderEmail, required this.senderFullName, @@ -320,6 +321,7 @@ class StreamMessage extends Message { required super.id, required super.isMeMessage, super.lastEditTimestamp, + required super.reactions, required super.recipientId, required super.senderEmail, required super.senderFullName, @@ -421,6 +423,7 @@ class DmMessage extends Message { required super.id, required super.isMeMessage, super.lastEditTimestamp, + required super.reactions, required super.recipientId, required super.senderEmail, required super.senderFullName, @@ -440,3 +443,33 @@ class DmMessage extends Message { @override Map toJson() => _$DmMessageToJson(this); } + +/// As in [Message.reactions]. +@JsonSerializable(fieldRename: FieldRename.snake) +class Reaction { + final String emojiName; + final String emojiCode; + final ReactionType reactionType; + final int userId; + // final Map user; // deprecated; ignore + + Reaction({ + required this.emojiName, + required this.emojiCode, + required this.reactionType, + required this.userId, + }); + + factory Reaction.fromJson(Map json) => + _$ReactionFromJson(json); + + Map toJson() => _$ReactionToJson(this); +} + +/// As in [Reaction.reactionType]. +@JsonEnum(fieldRename: FieldRename.snake) +enum ReactionType { + unicodeEmoji, + realmEmoji, + zulipExtraEmoji; +} diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 78c9ebd704..2597d4556e 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -186,6 +186,9 @@ StreamMessage _$StreamMessageFromJson(Map json) => id: json['id'] as int, isMeMessage: json['is_me_message'] as bool, lastEditTimestamp: json['last_edit_timestamp'] as int?, + reactions: (json['reactions'] as List) + .map((e) => Reaction.fromJson(e as Map)) + .toList(), recipientId: json['recipient_id'] as int, senderEmail: json['sender_email'] as String, senderFullName: json['sender_full_name'] as String, @@ -208,6 +211,7 @@ Map _$StreamMessageToJson(StreamMessage instance) => 'id': instance.id, 'is_me_message': instance.isMeMessage, 'last_edit_timestamp': instance.lastEditTimestamp, + 'reactions': instance.reactions, 'recipient_id': instance.recipientId, 'sender_email': instance.senderEmail, 'sender_full_name': instance.senderFullName, @@ -243,6 +247,9 @@ DmMessage _$DmMessageFromJson(Map json) => DmMessage( id: json['id'] as int, isMeMessage: json['is_me_message'] as bool, lastEditTimestamp: json['last_edit_timestamp'] as int?, + reactions: (json['reactions'] as List) + .map((e) => Reaction.fromJson(e as Map)) + .toList(), recipientId: json['recipient_id'] as int, senderEmail: json['sender_email'] as String, senderFullName: json['sender_full_name'] as String, @@ -264,6 +271,7 @@ Map _$DmMessageToJson(DmMessage instance) => { 'id': instance.id, 'is_me_message': instance.isMeMessage, 'last_edit_timestamp': instance.lastEditTimestamp, + 'reactions': instance.reactions, 'recipient_id': instance.recipientId, 'sender_email': instance.senderEmail, 'sender_full_name': instance.senderFullName, @@ -278,3 +286,23 @@ Map _$DmMessageToJson(DmMessage instance) => { 'display_recipient': const DmRecipientListConverter().toJson(instance.displayRecipient), }; + +Reaction _$ReactionFromJson(Map json) => Reaction( + emojiName: json['emoji_name'] as String, + emojiCode: json['emoji_code'] as String, + reactionType: $enumDecode(_$ReactionTypeEnumMap, json['reaction_type']), + userId: json['user_id'] as int, + ); + +Map _$ReactionToJson(Reaction instance) => { + 'emoji_name': instance.emojiName, + 'emoji_code': instance.emojiCode, + 'reaction_type': _$ReactionTypeEnumMap[instance.reactionType]!, + 'user_id': instance.userId, + }; + +const _$ReactionTypeEnumMap = { + ReactionType.unicodeEmoji: 'unicode_emoji', + ReactionType.realmEmoji: 'realm_emoji', + ReactionType.zulipExtraEmoji: 'zulip_extra_emoji', +}; diff --git a/test/example_data.dart b/test/example_data.dart index 2e39806cd4..70cebe6536 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -146,6 +146,7 @@ StreamMessage streamMessage({ ..._messagePropertiesFromContent(content, contentMarkdown), 'display_recipient': effectiveStream.name, 'stream_id': effectiveStream.streamId, + 'reactions': [], 'flags': flags ?? [], 'id': id ?? 1234567, // TODO generate example IDs 'last_edit_timestamp': lastEditTimestamp, @@ -176,7 +177,7 @@ DmMessage dmMessage({ 'display_recipient': [from, ...to] .map((u) => {'id': u.userId, 'email': u.email, 'full_name': u.fullName}) .toList(growable: false), - + 'reactions': [], 'flags': flags ?? [], 'id': id ?? 1234567, // TODO generate example IDs 'last_edit_timestamp': lastEditTimestamp, From 15099f4a3a20c94f368d1ae321838c7d1551fdbf Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 2 Aug 2023 18:09:12 -0700 Subject: [PATCH 2/2] api: Add reaction events We don't yet have UI to show the events (#121), but now at least we're keeping our Message objects up-to-date with reactions. Related: #121 --- lib/api/model/events.dart | 45 ++++++++++++++ lib/api/model/events.g.dart | 34 +++++++++++ lib/api/model/model.dart | 3 + lib/model/message_list.dart | 26 ++++++++ lib/model/store.dart | 5 ++ test/api/model/model_checks.dart | 30 +++++++++- test/example_data.dart | 10 +++- test/model/message_list_test.dart | 98 +++++++++++++++++++++++++++++++ 8 files changed, 249 insertions(+), 2 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 47aa24256b..f09f0b24b1 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -33,6 +33,7 @@ sealed class Event { case 'message': return MessageEvent.fromJson(json); case 'update_message': return UpdateMessageEvent.fromJson(json); case 'delete_message': return DeleteMessageEvent.fromJson(json); + case 'reaction': return ReactionEvent.fromJson(json); case 'heartbeat': return HeartbeatEvent.fromJson(json); // TODO add many more event types default: return UnexpectedEvent.fromJson(json); @@ -370,6 +371,50 @@ enum MessageType { private; } +/// A Zulip event of type `reaction`, with op `add` or `remove`. +/// +/// See: +/// https://zulip.com/api/get-events#reaction-add +/// https://zulip.com/api/get-events#reaction-remove +@JsonSerializable(fieldRename: FieldRename.snake) +class ReactionEvent extends Event { + @override + @JsonKey(includeToJson: true) + String get type => 'reaction'; + + final ReactionOp op; + + final String emojiName; + final String emojiCode; + final ReactionType reactionType; + final int userId; + // final Map user; // deprecated; ignore + final int messageId; + + ReactionEvent({ + required super.id, + required this.op, + required this.emojiName, + required this.emojiCode, + required this.reactionType, + required this.userId, + required this.messageId, + }); + + factory ReactionEvent.fromJson(Map json) => + _$ReactionEventFromJson(json); + + @override + Map toJson() => _$ReactionEventToJson(this); +} + +/// The type of [ReactionEvent.op]. +@JsonEnum(fieldRename: FieldRename.snake) +enum ReactionOp { + add, + remove, +} + /// A Zulip event of type `heartbeat`: https://zulip.com/api/get-events#heartbeat @JsonSerializable(fieldRename: FieldRename.snake) class HeartbeatEvent extends Event { diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index 68aa0c4c7a..5183ec3145 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -209,6 +209,40 @@ const _$MessageTypeEnumMap = { MessageType.private: 'private', }; +ReactionEvent _$ReactionEventFromJson(Map json) => + ReactionEvent( + id: json['id'] as int, + op: $enumDecode(_$ReactionOpEnumMap, json['op']), + emojiName: json['emoji_name'] as String, + emojiCode: json['emoji_code'] as String, + reactionType: $enumDecode(_$ReactionTypeEnumMap, json['reaction_type']), + userId: json['user_id'] as int, + messageId: json['message_id'] as int, + ); + +Map _$ReactionEventToJson(ReactionEvent instance) => + { + 'id': instance.id, + 'type': instance.type, + 'op': _$ReactionOpEnumMap[instance.op]!, + 'emoji_name': instance.emojiName, + 'emoji_code': instance.emojiCode, + 'reaction_type': _$ReactionTypeEnumMap[instance.reactionType]!, + 'user_id': instance.userId, + 'message_id': instance.messageId, + }; + +const _$ReactionOpEnumMap = { + ReactionOp.add: 'add', + ReactionOp.remove: 'remove', +}; + +const _$ReactionTypeEnumMap = { + ReactionType.unicodeEmoji: 'unicode_emoji', + ReactionType.realmEmoji: 'realm_emoji', + ReactionType.zulipExtraEmoji: 'zulip_extra_emoji', +}; + HeartbeatEvent _$HeartbeatEventFromJson(Map json) => HeartbeatEvent( id: json['id'] as int, diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index bb8af8ca45..30b9af5528 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -464,6 +464,9 @@ class Reaction { _$ReactionFromJson(json); Map toJson() => _$ReactionToJson(this); + + @override + String toString() => 'Reaction(emojiName: $emojiName, emojiCode: $emojiCode, reactionType: $reactionType, userId: $userId)'; } /// As in [Reaction.reactionType]. diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index 26d01017c2..9ba6d30ab5 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -151,6 +151,32 @@ class MessageListView extends ChangeNotifier { notifyListeners(); } + void maybeUpdateMessageReactions(ReactionEvent event) { + final index = findMessageWithId(event.messageId); + if (index == -1) { + return; + } + + final message = messages[index]; + switch (event.op) { + case ReactionOp.add: + message.reactions.add(Reaction( + emojiName: event.emojiName, + emojiCode: event.emojiCode, + reactionType: event.reactionType, + userId: event.userId, + )); + case ReactionOp.remove: + message.reactions.removeWhere((r) { + return r.emojiCode == event.emojiCode + && r.reactionType == event.reactionType + && r.userId == event.userId; + }); + } + + notifyListeners(); + } + /// Called when the app is reassembled during debugging, e.g. for hot reload. /// /// This will redo from scratch any computations we can, such as parsing diff --git a/lib/model/store.dart b/lib/model/store.dart index f5121f3da8..9132d30c20 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -278,6 +278,11 @@ class PerAccountStore extends ChangeNotifier { } else if (event is DeleteMessageEvent) { assert(debugLog("server event: delete_message ${event.messageIds}")); // TODO handle + } else if (event is ReactionEvent) { + assert(debugLog("server event: reaction/${event.op}")); + for (final view in _messageListViews) { + view.maybeUpdateMessageReactions(event); + } } else if (event is UnexpectedEvent) { assert(debugLog("server event: ${jsonEncode(event.toJson())}")); // TODO log better } else { diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index a049df975a..a1c5c82a3a 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -5,15 +5,43 @@ extension MessageChecks on Subject { Subject> get toJson => has((e) => e.toJson(), 'toJson'); void jsonEquals(Message expected) { - toJson.deepEquals(expected.toJson()); + final expectedJson = expected.toJson(); + expectedJson['reactions'] = it()..isA>().jsonEquals(expected.reactions); + toJson.deepEquals(expectedJson); } Subject get content => has((e) => e.content, 'content'); Subject get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage'); Subject get lastEditTimestamp => has((e) => e.lastEditTimestamp, 'lastEditTimestamp'); + Subject> get reactions => has((e) => e.reactions, 'reactions'); Subject> get flags => has((e) => e.flags, 'flags'); // TODO accessors for other fields } +extension ReactionsChecks on Subject> { + void deepEquals(_) { + throw UnimplementedError('Tried to call [Subject>.deepEquals]. Use jsonEquals instead.'); + } + + void jsonEquals(List expected) { + // (cast, to bypass this extension's deepEquals implementation, which throws) + // ignore: unnecessary_cast + (this as Subject).deepEquals(expected.map((r) => it()..isA().jsonEquals(r))); + } +} + +extension ReactionChecks on Subject { + Subject> get toJson => has((r) => r.toJson(), 'toJson'); + + void jsonEquals(Reaction expected) { + toJson.deepEquals(expected.toJson()); + } + + Subject get emojiName => has((r) => r.emojiName, 'emojiName'); + Subject get emojiCode => has((r) => r.emojiCode, 'emojiCode'); + Subject get reactionType => has((r) => r.reactionType, 'reactionType'); + Subject get userId => has((r) => r.userId, 'userId'); +} + // TODO similar extensions for other types in model diff --git a/test/example_data.dart b/test/example_data.dart index 70cebe6536..460c2981b6 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -132,6 +132,7 @@ StreamMessage streamMessage({ String? content, String? contentMarkdown, int? lastEditTimestamp, + List? reactions, List? flags, }) { final effectiveStream = stream ?? _stream(); @@ -146,7 +147,7 @@ StreamMessage streamMessage({ ..._messagePropertiesFromContent(content, contentMarkdown), 'display_recipient': effectiveStream.name, 'stream_id': effectiveStream.streamId, - 'reactions': [], + 'reactions': reactions?.map((r) => r.toJson()).toList() ?? [], 'flags': flags ?? [], 'id': id ?? 1234567, // TODO generate example IDs 'last_edit_timestamp': lastEditTimestamp, @@ -187,6 +188,13 @@ DmMessage dmMessage({ }); } +Reaction unicodeEmojiReaction = Reaction( + emojiName: 'thumbs_up', + emojiCode: '1f44d', + reactionType: ReactionType.unicodeEmoji, + userId: selfUser.userId, +); + // TODO example data for many more types InitialSnapshot initialSnapshot({ diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 6893938a23..463860b4e8 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -162,5 +162,103 @@ void main() async { test('rendering-only update does not change timestamp (for old server versions)', () async { await checkRenderingOnly(legacy: true); }); + + group('ReactionEvent handling', () { + ReactionEvent mkEvent(Reaction reaction, ReactionOp op, int messageId) { + return ReactionEvent( + id: 1, + op: op, + emojiName: reaction.emojiName, + emojiCode: reaction.emojiCode, + reactionType: reaction.reactionType, + userId: reaction.userId, + messageId: messageId, + ); + } + + test('add reaction', () async { + final originalMessage = eg.streamMessage(stream: stream, reactions: []); + final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); + + final message = messageList.messages.single; + + bool listenersNotified = false; + messageList.addListener(() { listenersNotified = true; }); + + messageList.maybeUpdateMessageReactions( + mkEvent(eg.unicodeEmojiReaction, ReactionOp.add, originalMessage.id)); + + check(listenersNotified).isTrue(); + check(messageList.messages.single) + ..identicalTo(message) + ..reactions.jsonEquals([eg.unicodeEmojiReaction]); + }); + + test('add reaction; message is not in list', () async { + final someMessage = eg.streamMessage(id: 1, reactions: []); + final messageList = await messageListViewWithMessages([someMessage], stream, narrow); + + bool listenersNotified = false; + messageList.addListener(() { listenersNotified = true; }); + + messageList.maybeUpdateMessageReactions( + mkEvent(eg.unicodeEmojiReaction, ReactionOp.add, 1000)); + + check(listenersNotified).isFalse(); + check(messageList.messages.single).reactions.jsonEquals([]); + }); + + test('remove reaction', () async { + final eventReaction = Reaction(reactionType: ReactionType.unicodeEmoji, + emojiName: 'wave', emojiCode: '1f44b', userId: 1); + + // Same emoji, different user. Not to be removed. + final reaction2 = Reaction.fromJson(eventReaction.toJson() + ..['user_id'] = 2); + + // Same user, different emoji. Not to be removed. + final reaction3 = Reaction.fromJson(eventReaction.toJson() + ..['emoji_code'] = '1f6e0' + ..['emoji_name'] = 'working_on_it'); + + // Same user, same emojiCode, different emojiName. To be removed: servers + // key on user, message, reaction type, and emoji code, but not emoji name. + // So we mimic that behavior; see discussion: + // https://github.com/zulip/zulip-flutter/pull/256#discussion_r1284865099 + final reaction4 = Reaction.fromJson(eventReaction.toJson() + ..['emoji_name'] = 'hello'); + + final originalMessage = eg.streamMessage(stream: stream, + reactions: [reaction2, reaction3, reaction4]); + final messageList = await messageListViewWithMessages([originalMessage], stream, narrow); + + final message = messageList.messages.single; + + bool listenersNotified = false; + messageList.addListener(() { listenersNotified = true; }); + + messageList.maybeUpdateMessageReactions( + mkEvent(eventReaction, ReactionOp.remove, originalMessage.id)); + + check(listenersNotified).isTrue(); + check(messageList.messages.single) + ..identicalTo(message) + ..reactions.jsonEquals([reaction2, reaction3]); + }); + + test('remove reaction; message is not in list', () async { + final someMessage = eg.streamMessage(id: 1, reactions: [eg.unicodeEmojiReaction]); + final messageList = await messageListViewWithMessages([someMessage], stream, narrow); + + bool listenersNotified = false; + messageList.addListener(() { listenersNotified = true; }); + + messageList.maybeUpdateMessageReactions( + mkEvent(eg.unicodeEmojiReaction, ReactionOp.remove, 1000)); + + check(listenersNotified).isFalse(); + check(messageList.messages.single).reactions.jsonEquals([eg.unicodeEmojiReaction]); + }); + }); }); }