From 0ca49bc93a8304777259aa038a0b2863fb3a355b Mon Sep 17 00:00:00 2001 From: Nipunn Koorapati Date: Mon, 29 Nov 2021 16:47:09 -0800 Subject: [PATCH] emoji reactions: Order reactions query results by id. Force postgres to give reactions in ID order - which is generally chronological order. Results in frontend displaying reactions in said order. Fixes #20060. --- zerver/models.py | 5 ++++- zerver/tests/test_reactions.py | 29 +++++++++++++++++++---------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/zerver/models.py b/zerver/models.py index 23f97e9cb8..c8e6508c9b 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -2870,7 +2870,10 @@ class Reaction(AbstractReaction): "user_profile_id", "user_profile__full_name", ] - return Reaction.objects.filter(message_id__in=needed_ids).values(*fields) + # The ordering is important here, as it makes it convenient + # for clients to display reactions in order without + # client-side sorting code. + return Reaction.objects.filter(message_id__in=needed_ids).values(*fields).order_by("id") def __str__(self) -> str: return f"{self.user_profile.email} / {self.message.id} / {self.emoji_name}" diff --git a/zerver/tests/test_reactions.py b/zerver/tests/test_reactions.py index e4146675ba..9ca30d1528 100644 --- a/zerver/tests/test_reactions.py +++ b/zerver/tests/test_reactions.py @@ -87,29 +87,38 @@ class ReactionEmojiTest(ZulipTestCase): """ Formatted reactions data is saved in cache. """ - sender = self.example_user("hamlet") - reaction_info = { - "emoji_name": "smile", - } - result = self.api_post(sender, "/api/v1/messages/1/reactions", reaction_info) + senders = [self.example_user("hamlet"), self.example_user("cordelia")] + emojis = ["smile", "tada"] + expected_emoji_codes = ["1f642", "1f389"] + + for sender, emoji in zip(senders, emojis): + reaction_info = { + "emoji_name": emoji, + } + result = self.api_post(sender, "/api/v1/messages/1/reactions", reaction_info) + + self.assert_json_success(result) + self.assertEqual(200, result.status_code) - self.assert_json_success(result) - self.assertEqual(200, result.status_code) key = to_dict_cache_key_id(1) message = extract_message_dict(cache_get(key)[0]) expected_reaction_data = [ { - "emoji_name": "smile", - "emoji_code": "1f642", + "emoji_name": emoji, + "emoji_code": emoji_code, "reaction_type": "unicode_emoji", "user": { "email": f"user{sender.id}@zulip.testserver", "id": sender.id, - "full_name": "King Hamlet", + "full_name": sender.full_name, }, "user_id": sender.id, } + # It's important that we preserve the loop order in this + # test, since this is our test to verify that we're + # returning reactions in chronological order. + for sender, emoji, emoji_code in zip(senders, emojis, expected_emoji_codes) ] self.assertEqual(expected_reaction_data, message["reactions"])