From 99c17efe1ac93276ebe05b0ca67705fa6fde67ea Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 15 Aug 2023 12:51:47 -0500 Subject: [PATCH 1/3] api: Implement ApiConnection.delete --- lib/api/core.dart | 10 ++++++++++ test/api/core_test.dart | 30 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/lib/api/core.dart b/lib/api/core.dart index 5d792ac272..04616a2e76 100644 --- a/lib/api/core.dart +++ b/lib/api/core.dart @@ -147,6 +147,16 @@ class ApiConnection { ..files.add(http.MultipartFile('file', content, length, filename: filename)); return send(routeName, fromJson, request); } + + Future delete(String routeName, T Function(Map) fromJson, + String path, Map? params) async { + final url = realmUrl.replace(path: "/api/v1/$path"); + final request = http.Request('DELETE', url); + if (params != null) { + request.bodyFields = encodeParameters(params)!; + } + return send(routeName, fromJson, request); + } } ApiRequestException _makeApiException(String routeName, int httpStatus, Map? json) { diff --git a/test/api/core_test.dart b/test/api/core_test.dart index ddc196aa18..9b1cc26b61 100644 --- a/test/api/core_test.dart +++ b/test/api/core_test.dart @@ -104,6 +104,36 @@ void main() { checkRequest(['asdf'.codeUnits], 100, null); }); + test('ApiConnection.delete', () async { + Future checkRequest(Map? params, String expectedBody, {bool expectContentType = true}) { + return FakeApiConnection.with_(account: eg.selfAccount, (connection) async { + connection.prepare(json: {}); + await connection.delete(kExampleRouteName, (json) => json, 'example/route', params); + check(connection.lastRequest!).isA() + ..method.equals('DELETE') + ..url.asString.equals('${eg.realmUrl.origin}/api/v1/example/route') + ..headers.deepEquals({ + ...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey), + if (expectContentType) + 'content-type': 'application/x-www-form-urlencoded; charset=utf-8', + }) + ..body.equals(expectedBody); + }); + } + + checkRequest(null, '', expectContentType: false); + checkRequest({}, ''); + checkRequest({'x': 3}, 'x=3'); + checkRequest({'x': 3, 'y': 4}, 'x=3&y=4'); + checkRequest({'x': null}, 'x=null'); + checkRequest({'x': true}, 'x=true'); + checkRequest({'x': 'foo'}, 'x=%22foo%22'); + checkRequest({'x': [1, 2]}, 'x=%5B1%2C2%5D'); + checkRequest({'x': {'y': 1}}, 'x=%7B%22y%22%3A1%7D'); + checkRequest({'x': RawParameter('foo')}, 'x=foo'); + checkRequest({'x': RawParameter('foo'), 'y': 'bar'}, 'x=foo&y=%22bar%22'); + }); + test('API success result', () async { await FakeApiConnection.with_(account: eg.selfAccount, (connection) async { connection.prepare(json: {'result': 'success', 'x': 3}); From 8aedecca4f029aeef4c8dbdba276c98ceae7657f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 15 Aug 2023 17:32:06 -0500 Subject: [PATCH 2/3] model [nfc]: Implement ReactionType.toJson We'll call this directly soon, when we'll want reusable code to translate an enum value into a 'snake_case' string, for API bindings for the add- and remove-reaction endpoints. With a `toJson` method, instances can now be handled by `encodeJson`. That's nice, but not really a goal here; we don't have an imminent need to pass an instance through `encodeJson`. Anyway, it means that json_serializable's generated _$ReactionToJson and _$ReactionEventToJson functions don't need the enum-to-string conversion on their reactionType fields in order for jsonEncode to accept those functions' output. And it looks like json_serializable has removed that conversion in both functions. So, adjust our code to not depend on it. --- lib/api/model/events.g.dart | 2 +- lib/api/model/model.dart | 2 ++ lib/api/model/model.g.dart | 2 +- test/example_data.dart | 4 +++- test/model/message_list_test.dart | 15 +++++++-------- 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index 0a43c905d9..f892b7eba0 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -251,7 +251,7 @@ Map _$ReactionEventToJson(ReactionEvent instance) => 'op': _$ReactionOpEnumMap[instance.op]!, 'emoji_name': instance.emojiName, 'emoji_code': instance.emojiCode, - 'reaction_type': _$ReactionTypeEnumMap[instance.reactionType]!, + 'reaction_type': instance.reactionType, 'user_id': instance.userId, 'message_id': instance.messageId, }; diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 30b9af5528..3ebd8fee16 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -475,4 +475,6 @@ enum ReactionType { unicodeEmoji, realmEmoji, zulipExtraEmoji; + + String toJson() => _$ReactionTypeEnumMap[this]!; } diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 2597d4556e..452a5a4fc4 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -297,7 +297,7 @@ Reaction _$ReactionFromJson(Map json) => Reaction( Map _$ReactionToJson(Reaction instance) => { 'emoji_name': instance.emojiName, 'emoji_code': instance.emojiCode, - 'reaction_type': _$ReactionTypeEnumMap[instance.reactionType]!, + 'reaction_type': instance.reactionType, 'user_id': instance.userId, }; diff --git a/test/example_data.dart b/test/example_data.dart index 740976a9e9..c18d4a3e0a 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -147,7 +147,9 @@ StreamMessage streamMessage({ ..._messagePropertiesFromContent(content, contentMarkdown), 'display_recipient': effectiveStream.name, 'stream_id': effectiveStream.streamId, - 'reactions': reactions?.map((r) => r.toJson()).toList() ?? [], + 'reactions': reactions?.map( + (r) => r.toJson()..['reaction_type'] = r.reactionType.toJson(), + ).toList() ?? [], 'flags': flags ?? [], 'id': id ?? 1234567, // TODO generate example IDs 'last_edit_timestamp': lastEditTimestamp, diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 3c3f51460e..6dfca6e4de 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -396,23 +396,22 @@ void main() async { test('remove reaction', () async { final eventReaction = Reaction(reactionType: ReactionType.unicodeEmoji, - emojiName: 'wave', emojiCode: '1f44b', userId: 1); + emojiName: 'wave', emojiCode: '1f44b', userId: 1); // Same emoji, different user. Not to be removed. - final reaction2 = Reaction.fromJson(eventReaction.toJson() - ..['user_id'] = 2); + final reaction2 = Reaction(reactionType: ReactionType.unicodeEmoji, + emojiName: 'wave', emojiCode: '1f44b', userId: 2); // Same user, different emoji. Not to be removed. - final reaction3 = Reaction.fromJson(eventReaction.toJson() - ..['emoji_code'] = '1f6e0' - ..['emoji_name'] = 'working_on_it'); + final reaction3 = Reaction(reactionType: ReactionType.unicodeEmoji, + emojiName: 'working_on_it', emojiCode: '1f6e0', userId: 1); // 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 reaction4 = Reaction(reactionType: ReactionType.unicodeEmoji, + emojiName: 'hello', emojiCode: '1f44b', userId: 1); final originalMessage = eg.streamMessage( reactions: [reaction2, reaction3, reaction4]); From 7e798bbbcc8bcd6f89e2e3e5ff6bf37afc79ebac Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 15 Aug 2023 13:50:43 -0500 Subject: [PATCH 3/3] api: Add add-reaction and remove-reaction routes --- lib/api/route/messages.dart | 28 ++++++++ test/api/route/messages_test.dart | 102 ++++++++++++++++++++++++++++++ test/example_data.dart | 14 ++++ 3 files changed, 144 insertions(+) diff --git a/lib/api/route/messages.dart b/lib/api/route/messages.dart index 1a7019b3fc..a82b38159f 100644 --- a/lib/api/route/messages.dart +++ b/lib/api/route/messages.dart @@ -263,3 +263,31 @@ class UploadFileResult { Map toJson() => _$UploadFileResultToJson(this); } + +/// https://zulip.com/api/add-reaction +Future addReaction(ApiConnection connection, { + required int messageId, + required ReactionType reactionType, + required String emojiCode, + required String emojiName, +}) { + return connection.post('addReaction', (_) {}, 'messages/$messageId/reactions', { + 'emoji_name': RawParameter(emojiName), + 'emoji_code': RawParameter(emojiCode), + 'reaction_type': RawParameter(reactionType.toJson()), + }); +} + +/// https://zulip.com/api/remove-reaction +Future removeReaction(ApiConnection connection, { + required int messageId, + required ReactionType reactionType, + required String emojiCode, + required String emojiName, +}) { + return connection.delete('removeReaction', (_) {}, 'messages/$messageId/reactions', { + 'emoji_name': RawParameter(emojiName), + 'emoji_code': RawParameter(emojiCode), + 'reaction_type': RawParameter(reactionType.toJson()), + }); +} diff --git a/test/api/route/messages_test.dart b/test/api/route/messages_test.dart index d9853bb79b..5670bc3118 100644 --- a/test/api/route/messages_test.dart +++ b/test/api/route/messages_test.dart @@ -334,4 +334,106 @@ void main() { }); }); }); + + group('addReaction', () { + Future checkAddReaction(FakeApiConnection connection, { + required int messageId, + required Reaction reaction, + required String expectedReactionType, + }) async { + connection.prepare(json: {}); + await addReaction(connection, + messageId: messageId, + reactionType: reaction.reactionType, + emojiCode: reaction.emojiCode, + emojiName: reaction.emojiName, + ); + check(connection.lastRequest).isNotNull().isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/messages/$messageId/reactions') + ..bodyFields.deepEquals({ + 'reaction_type': expectedReactionType, + 'emoji_code': reaction.emojiCode, + 'emoji_name': reaction.emojiName, + }); + } + + test('unicode emoji', () { + return FakeApiConnection.with_((connection) async { + await checkAddReaction(connection, + messageId: eg.streamMessage().id, + reaction: eg.unicodeEmojiReaction, + expectedReactionType: 'unicode_emoji'); + }); + }); + + test('realm emoji', () { + return FakeApiConnection.with_((connection) async { + await checkAddReaction(connection, + messageId: eg.streamMessage().id, + reaction: eg.realmEmojiReaction, + expectedReactionType: 'realm_emoji'); + }); + }); + + test('Zulip extra emoji', () { + return FakeApiConnection.with_((connection) async { + await checkAddReaction(connection, + messageId: eg.streamMessage().id, + reaction: eg.zulipExtraEmojiReaction, + expectedReactionType: 'zulip_extra_emoji'); + }); + }); + }); + + group('removeReaction', () { + Future checkRemoveReaction(FakeApiConnection connection, { + required int messageId, + required Reaction reaction, + required String expectedReactionType, + }) async { + connection.prepare(json: {}); + await removeReaction(connection, + messageId: messageId, + reactionType: reaction.reactionType, + emojiCode: reaction.emojiCode, + emojiName: reaction.emojiName, + ); + check(connection.lastRequest).isNotNull().isA() + ..method.equals('DELETE') + ..url.path.equals('/api/v1/messages/$messageId/reactions') + ..bodyFields.deepEquals({ + 'reaction_type': expectedReactionType, + 'emoji_code': reaction.emojiCode, + 'emoji_name': reaction.emojiName, + }); + } + + test('unicode emoji', () { + return FakeApiConnection.with_((connection) async { + await checkRemoveReaction(connection, + messageId: eg.streamMessage().id, + reaction: eg.unicodeEmojiReaction, + expectedReactionType: 'unicode_emoji'); + }); + }); + + test('realm emoji', () { + return FakeApiConnection.with_((connection) async { + await checkRemoveReaction(connection, + messageId: eg.streamMessage().id, + reaction: eg.realmEmojiReaction, + expectedReactionType: 'realm_emoji'); + }); + }); + + test('Zulip extra emoji', () { + return FakeApiConnection.with_((connection) async { + await checkRemoveReaction(connection, + messageId: eg.streamMessage().id, + reaction: eg.zulipExtraEmojiReaction, + expectedReactionType: 'zulip_extra_emoji'); + }); + }); + }); } diff --git a/test/example_data.dart b/test/example_data.dart index c18d4a3e0a..d61c84e7c1 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -197,6 +197,20 @@ Reaction unicodeEmojiReaction = Reaction( userId: selfUser.userId, ); +Reaction realmEmojiReaction = Reaction( + emojiName: 'twocents', + emojiCode: '181', + reactionType: ReactionType.realmEmoji, + userId: selfUser.userId, +); + +Reaction zulipExtraEmojiReaction = Reaction( + emojiName: 'zulip', + emojiCode: 'zulip', + reactionType: ReactionType.zulipExtraEmoji, + userId: selfUser.userId, +); + // TODO example data for many more types InitialSnapshot initialSnapshot({