Repository: zulip/zulip-terminal
Pull request #1556 (2024-11-18 05:01:06)

Preparation for deprecating reactions' user metadata
<!-- See README for documentation, or ask in #zulip-terminal if unclear -->
### What does this PR do, and why?
- Prepares for the deprecation of user metadata from messages' reactions and reaction events
    - Supports both `user_id` (ZFL>=2) and `user["id"]` for messages' reactions (with added test cases)
    - Supports both `user_id` (ZFL>=2) and `user["user_id"]` for reaction events (with added test cases)
    - Introduces a common `get_user_id_from_reaction()` for handling the different possible fields
    - Switches to using `_all_users_by_id["user_id"]` to remove dependency on `reaction["user"]["full_name"]
    - Improves the `ReactionEvent` schema to support both versions
- Fixes a bug in reaction event handling (remove op), and adds tests covering the missing cases
- Refactors the reactions' functions a bit before the above changes

### External discussion & connections
<!-- [x] all that apply, specifying topic and adding numbers after # for issues/PRs -->
- [x] Discussed in **#zulip-terminal** in `reaction metadata`
- [ ] Fully fixes #
- [ ] Partially fixes issue #
- [ ] Builds upon previous unmerged work in PR #
- [ ] Is a follow-up to work in PR #
- [ ] Requires merge of PR #
- [ ] Merge will enable work on #

### How did you test this?
<!-- [x] all that apply -->
- [x] Manually - Behavioral changes
- [ ] Manually - Visual changes
- [x] Adapting existing automated tests
- [x] Adding automated tests for new behavior (or missing tests)
- [ ] Existing automated tests should already cover this (*only a refactor of tested code*)

### Self-review checklist for each commit
- [x] It is a [minimal coherent idea](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development)
- [x] It has a commit summary following the [documented style](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development) (title & body)
- [x] It has a commit summary describing the  motivation and reasoning for the change
- [x] It individually passes linting and tests
- [x] It contains test additions for any new behavior
- [x] It flows clearly from a previous branch commit, and/or prepares for the next commit

### Visual changes    <!-- DELETE SECTION IF NO VISUAL CHANGE -->
<!-- Zulip tips at https://zulip.readthedocs.io/en/latest/tutorials/screenshot-and-gif-software.html -->
<!-- For video, try asciinema; after uploading, embed using
[![yourtitle](https://asciinema.org/a/<id>.png)](https://asciinema.org/a/<id>)
-->
<!-- NOTE: Attached videos/images will be clearer from smaller terminal windows -->



Patch:
diff --git a/zulipterminal/api_types.py b/zulipterminal/api_types.py
index b7474be..b6afa2b 100644
--- a/zulipterminal/api_types.py
+++ b/zulipterminal/api_types.py
@@ -408,7 +408,8 @@ class UpdateMessagesLocationEvent(BaseUpdateMessageEvent):
 class ReactionEvent(TypedDict):
     type: Literal["reaction"]
     op: str
-    user: Dict[str, Any]  # 'email', 'user_id', 'full_name'
+    user_id: NotRequired[int]  # Added in Zulip v3.0, ZFL 2 replacing 'user'
+    user: NotRequired[Dict[str, Any]]  # 'email', 'user_id', 'full_name'
     reaction_type: EmojiType
     emoji_code: str
     emoji_name: str
diff --git a/zulipterminal/model.py b/zulipterminal/model.py
index b3840a5..1b8064f 100644
--- a/zulipterminal/model.py
+++ b/zulipterminal/model.py
@@ -48,6 +48,7 @@ from zulipterminal.api_types import (
     MessagesFlagChange,
     PrivateComposition,
     PrivateMessageUpdateRequest,
+    ReactionEvent,
     RealmEmojiData,
     RealmUser,
     StreamComposition,
@@ -453,17 +454,11 @@ class Model:
     def toggle_message_reaction(
         self, message: Message, reaction_to_toggle: str
     ) -> None:
-        # Check if reaction_to_toggle is a valid original/alias
         assert reaction_to_toggle in self.all_emoji_names
 
         for emoji_name, emoji_data in self.active_emoji_data.items():
-            if (
-                reaction_to_toggle == emoji_name
-                or reaction_to_toggle in emoji_data["aliases"]
-            ):
-                # Found the emoji to toggle. Store its code/type and dont check further
-                emoji_code = emoji_data["code"]
-                emoji_type = emoji_data["type"]
+            if reaction_to_toggle in (emoji_name, *emoji_data["aliases"]):
+                emoji_code, emoji_type = emoji_data["code"], emoji_data["type"]
                 break
 
         reaction_to_toggle_spec = dict(
@@ -485,18 +480,22 @@ class Model:
         for reaction in message["reactions"]:
             if reaction["emoji_code"] != emoji_code:
                 continue
-            # The reaction.user_id field was added in Zulip v3.0, ZFL 2 so we need to
-            # check both the reaction.user.{user_id/id} fields too for pre v3 support.
-            user = reaction.get("user", {})
-            has_user_reacted = (
-                user.get("user_id", None) == self.user_id
-                or user.get("id", None) == self.user_id
-                or reaction.get("user_id", None) == self.user_id
-            )
-            if has_user_reacted:
+            user_id = self.get_user_id_from_reaction(reaction)
+            if user_id == self.user_id:
                 return True
         return False
 
+    def get_user_id_from_reaction(
+        self, reaction: Union[Dict[str, Any], ReactionEvent]
+    ) -> int:
+        # The reaction.user_id field was added in Zulip v3.0, ZFL 2 so we need to
+        # check both the reaction.user.{user_id/id} fields too for pre v3 support.
+        user = reaction.get("user", {})
+        assert isinstance(user, dict)
+        user_id = user.get("id") or user.get("user_id") or reaction.get("user_id")
+        assert isinstance(user_id, int)
+        return user_id
+
     def session_draft_message(self) -> Optional[Composition]:
         return deepcopy(self._draft)
 
@@ -1838,25 +1837,41 @@ class Model:
         """
         assert event["type"] == "reaction"
         message_id = event["message_id"]
-        # If the message is indexed
         if message_id in self.index["messages"]:
             message = self.index["messages"][message_id]
             if event["op"] == "add":
-                message["reactions"].append(
-                    {
-                        "user": event["user"],
-                        "reaction_type": event["reaction_type"],
-                        "emoji_code": event["emoji_code"],
-                        "emoji_name": event["emoji_name"],
+                reactions_entry = {
+                    key: event.get(key)
+                    for key in [
+                        "user",
+                        "reaction_type",
+                        "emoji_code",
+                        "emoji_name",
+                        "user_id",
+                    ]
+                }
+
+                # Convert from reaction event schema to message reactions schema
+                if isinstance(event.get("user"), dict) and isinstance(
+                    reactions_entry["user"], dict
+                ):
+                    reactions_entry["user"] = {
+                        **event["user"],
+                        "id": event["user"].get("user_id"),
                     }
-                )
+                    reactions_entry["user"].pop("user_id", None)
+
+                message["reactions"].append(reactions_entry)
             else:
-                emoji_code = event["emoji_code"]
                 for reaction in message["reactions"]:
-                    # Since Who reacted is not displayed,
-                    # remove the first one encountered
-                    if reaction["emoji_code"] == emoji_code:
+                    reaction_user_id = self.get_user_id_from_reaction(reaction)
+                    event_user_id = self.get_user_id_from_reaction(event)
+                    if (
+                        reaction["emoji_code"] == event["emoji_code"]
+                        and reaction_user_id == event_user_id
+                    ):
                         message["reactions"].remove(reaction)
+                        break
 
             self.index["messages"][message_id] = message
             self._update_rendered_view(message_id)
diff --git a/zulipterminal/ui_tools/messages.py b/zulipterminal/ui_tools/messages.py
index 2b67ae6..76e76a7 100644
--- a/zulipterminal/ui_tools/messages.py
+++ b/zulipterminal/ui_tools/messages.py
@@ -272,10 +272,8 @@ class MessageBox(urwid.Pile):
             my_user_id = self.model.user_id
             reaction_stats = defaultdict(list)
             for reaction in reactions:
-                user_id = int(reaction["user"].get("id", -1))
-                if user_id == -1:
-                    user_id = int(reaction["user"]["user_id"])
-                user_name = reaction["user"]["full_name"]
+                user_id = self.model.get_user_id_from_reaction(reaction)
+                user_name = self.model._all_users_by_id[user_id]["full_name"]
                 if user_id == my_user_id:
                     user_name = "You"
                 reaction_stats[reaction["emoji_name"]].append((user_id, user_name))
diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py
index 00158f8..02b3afb 100644
--- a/zulipterminal/ui_tools/views.py
+++ b/zulipterminal/ui_tools/views.py
@@ -241,11 +241,10 @@ class MessageView(urwid.ListBox):
         elif is_command_key("REACTION_AGREEMENT", key) and self.focus is not None:
             message = self.focus.original_widget.message
             message_reactions = message["reactions"]
-            if len(message_reactions) > 0:
-                for reaction in message_reactions:
-                    emoji = reaction["emoji_name"]
-                    self.model.toggle_message_reaction(message, emoji)
-                    break
+            if message_reactions:
+                self.model.toggle_message_reaction(
+                    message, message_reactions[0]["emoji_name"]
+                )
 
         key = super().keypress(size, key)
         return key
@@ -1658,7 +1657,12 @@ class MsgInfoView(PopUpView):
             msg_info.append(("Time mentions", time_mentions))
         if msg["reactions"]:
             reactions = sorted(
-                (reaction["emoji_name"], reaction["user"]["full_name"])
+                (
+                    reaction["emoji_name"],
+                    controller.model._all_users_by_id[
+                        controller.model.get_user_id_from_reaction(reaction)
+                    ]["full_name"],
+                )
                 for reaction in msg["reactions"]
             )
             grouped_reactions: Dict[str, str] = dict()


Test patch:
diff --git a/tests/model/test_model.py b/tests/model/test_model.py
index 2cd811a..81bb574 100644
--- a/tests/model/test_model.py
+++ b/tests/model/test_model.py
@@ -790,6 +790,75 @@ class TestModel:
 
         assert has_reacted == expected_has_user_reacted
 
+    @pytest.mark.parametrize(
+        "user_data, expected_user_id",
+        [
+            case({"user_id": 456}, 456, id="reaction_event_user_id"),
+            case(
+                {"user": {"user_id": 123}},
+                123,
+                id="reaction_event_user",
+            ),
+            case(
+                {"user_id": 101},
+                101,
+                id="reaction_user_id",
+            ),
+            case(
+                {"user": {"id": 202}},
+                202,
+                id="reaction_user",
+            ),
+        ],
+    )
+    def test_get_user_id_from_reaction_success(
+        self,
+        model: Model,
+        user_data: Dict[str, Any],
+        expected_user_id: int,
+    ):
+        reaction = {
+            "type": "reaction",
+            "op": "add",
+            "reaction_type": "unicode_emoji",
+            "emoji_code": "1f44d",
+            "emoji_name": "thumbs_up",
+            "message_id": "24757",
+            **user_data,
+        }
+        assert model.get_user_id_from_reaction(reaction) == expected_user_id
+
+    @pytest.mark.parametrize(
+        "user_data",
+        [
+            case({}, id="reaction_event_missing_user_id_user"),
+            case(
+                {"user": {"id": "not_an_int"}},
+                id="invalid_user",
+            ),
+            case(
+                {"user_id": "not_an_int"},
+                id="invalid_user_id",
+            ),
+        ],
+    )
+    def test_get_user_id_from_reaction_failure(
+        self,
+        model: Model,
+        user_data: Dict[str, Any],
+    ):
+        reaction = {
+            "type": "reaction",
+            "op": "add",
+            "reaction_type": "unicode_emoji",
+            "emoji_code": "1f44d",
+            "emoji_name": "thumbs_up",
+            "message_id": "24757",
+            **user_data,
+        }
+        with pytest.raises((AssertionError, AttributeError)):
+            model.get_user_id_from_reaction(reaction)
+
     @pytest.mark.parametrize("recipient_user_ids", [[5140], [5140, 5179]])
     @pytest.mark.parametrize("status", ["start", "stop"])
     def test_send_typing_status_by_user_ids(
@@ -2793,21 +2862,25 @@ class TestModel:
 
     @pytest.fixture
     def reaction_event_factory(self):
-        def _factory(*, op: str, message_id: int):
-            return {
-                "emoji_code": "1f44d",
+        def _factory(*, op: str, message_id: int, user=None, user_id=None):
+            base_event = {
                 "id": 2,
-                "user": {
-                    "email": "Foo@zulip.com",
-                    "user_id": 5140,
-                    "full_name": "Foo Boo",
-                },
+                "emoji_code": "1f44d",
                 "reaction_type": "unicode_emoji",
                 "message_id": message_id,
                 "emoji_name": "thumbs_up",
                 "type": "reaction",
                 "op": op,
             }
+            if user is not None:
+                base_event["user"] = {
+                    "email": "Foo@zulip.com",
+                    "user_id": 5140,
+                    "full_name": "Foo Boo",
+                }
+            if user_id is not None:
+                base_event["user_id"] = user_id
+            return base_event
 
         return _factory
 
@@ -2816,28 +2889,45 @@ class TestModel:
         """
         Generate index for reaction tests based on minimal specification
 
-        Input is a list of pairs, of a message-id and a list of reaction tuples
+        Input:
+        - msgs: A list of tuples where each tuple contains:
+            - A message ID
+            - A list of reaction tuples, where each reaction includes:
+                - user_id
+                - reaction_type
+                - emoji_code
+                - emoji_name
+        - schema: Defines the fields present in the reaction
+        (e.g., "with_user", "with_user_id", "with_both")
+
         NOTE: reactions as None indicate not indexed, [] indicates no reaction
         """
         MsgsType = List[Tuple[int, Optional[List[Tuple[int, str, str, str]]]]]
 
-        def _factory(msgs: MsgsType):
+        def _factory(msgs: MsgsType, schema="with_user"):
+            def build_reaction(user_id, type, code, name):
+                reaction = {
+                    "reaction_type": type,
+                    "emoji_code": code,
+                    "emoji_name": name,
+                }
+                if schema in {"with_user", "with_both"}:
+                    reaction["user"] = {
+                        "email": f"User email #{user_id}",
+                        "full_name": f"User #{user_id}",
+                        "id": user_id,
+                    }
+                if schema in {"with_user_id", "with_both"}:
+                    reaction["user_id"] = user_id
+                return reaction
+
             return {
                 "messages": {
                     message_id: {
                         "id": message_id,
                         "content": f"message content {message_id}",
                         "reactions": [
-                            {
-                                "user": {
-                                    "email": f"User email #{user_id}",
-                                    "full_name": f"User #{user_id}",
-                                    "user_id": user_id,
-                                },
-                                "reaction_type": type,
-                                "emoji_code": code,
-                                "emoji_name": name,
-                            }
+                            build_reaction(user_id, type, code, name)
                             for user_id, type, code, name in reactions
                         ],
                     }
@@ -2849,6 +2939,12 @@ class TestModel:
         return _factory
 
     @pytest.mark.parametrize("op", ["add", "remove"])
+    @pytest.mark.parametrize(
+        "reaction_event_schema", ["with_user", "with_user_id", "with_both"]
+    )
+    @pytest.mark.parametrize(
+        "reaction_schema", ["with_user", "with_user_id", "with_both"]
+    )
     def test__handle_reaction_event_not_in_index(
         self,
         mocker,
@@ -2856,18 +2952,30 @@ class TestModel:
         reaction_event_factory,
         reaction_event_index_factory,
         op,
+        reaction_event_schema,
+        reaction_schema,
         unindexed_message_id=1,
     ):
-        reaction_event = reaction_event_factory(
-            op=op,
-            message_id=unindexed_message_id,
-        )
+        common_args = {
+            "op": op,
+            "message_id": unindexed_message_id,
+        }
+        if reaction_event_schema == "with_user":
+            reaction_event = reaction_event_factory(**common_args, user=True)
+        elif reaction_event_schema == "with_user_id":
+            reaction_event = reaction_event_factory(**common_args, user_id=5140)
+        else:
+            reaction_event = reaction_event_factory(
+                **common_args, user=True, user_id=5140
+            )
+
         model.index = reaction_event_index_factory(
             [
                 (unindexed_message_id, None),  # explicitly exclude
                 (2, [(1, "unicode_emoji", "1232", "thumbs_up")]),
                 (3, []),
-            ]
+            ],
+            reaction_schema,
         )
         model._update_rendered_view = mocker.Mock()
         previous_index = deepcopy(model.index)
@@ -2879,10 +2987,116 @@ class TestModel:
         assert not model._update_rendered_view.called
 
     @pytest.mark.parametrize(
-        "op, expected_number_after",
+        "reaction_event_schema", ["with_user", "with_user_id", "with_both"]
+    )
+    @pytest.mark.parametrize(
+        "reaction_schema", ["with_user", "with_user_id", "with_both"]
+    )
+    @pytest.mark.parametrize(
+        "msgs, op, expected_number_after",
         [
-            ("add", 2),
-            ("remove", 1),  # Removed emoji doesn't match, so length remains 1
+            case(
+                [
+                    (2, [(5140, "unicode_emoji", "2764", "heart")]),
+                    (1, []),
+                ],
+                "add",
+                2,
+                id="single_reaction_add",
+            ),
+            case(
+                [
+                    (2, [(5140, "unicode_emoji", "1f44d", "thumbs_up")]),
+                    (1, []),
+                ],
+                "remove",
+                0,
+                id="single_reaction_remove",
+            ),
+            case(
+                [
+                    (
+                        2,
+                        [
+                            (3478, "unicode_emoji", "1f44d", "thumbs_up"),
+                            (3479, "unicode_emoji", "1f44d", "thumbs_up"),
+                        ],
+                    ),
+                ],
+                "add",
+                3,
+                id="same_emoji_different_users_add",
+            ),
+            case(
+                [
+                    (
+                        2,
+                        [
+                            (3478, "unicode_emoji", "1f44d", "thumbs_up"),
+                            (5140, "unicode_emoji", "1f44d", "thumbs_up"),
+                        ],
+                    ),
+                ],
+                "remove",
+                1,
+                id="same_emoji_different_users_remove",
+            ),
+            case(
+                [
+                    (
+                        2,
+                        [
+                            (5140, "zulip_extra_emoji", "zulip", "zulip"),
+                            (3478, "unicode_emoji", "2764", "heart"),
+                        ],
+                    ),
+                ],
+                "add",
+                3,
+                id="different_emoji_different_users_add",
+            ),
+            case(
+                [
+                    (
+                        2,
+                        [
+                            (5140, "unicode_emoji", "1f44d", "thumbs_up"),
+                            (3478, "unicode_emoji", "2764", "heart"),
+                        ],
+                    ),
+                ],
+                "remove",
+                1,
+                id="different_emoji_different_users_remove",
+            ),
+            case(
+                [
+                    (
+                        2,
+                        [
+                            (5140, "zulip_extra_emoji", "zulip", "zulip"),
+                            (5140, "unicode_emoji", "2764", "heart"),
+                        ],
+                    ),
+                ],
+                "add",
+                3,
+                id="different_emoji_same_user_add",
+            ),
+            case(
+                [
+                    (
+                        2,
+                        [
+                            (5140, "unicode_emoji", "1f44d", "thumbs_up"),
+                            (5140, "unicode_emoji", "5678", "heart"),
+                        ],
+                    ),
+                ],
+                "remove",
+                1,
+                id="different_emoji_same_user_remove",
+            ),
         ],
     )
     def test__handle_reaction_event_for_msg_in_index(
@@ -2892,19 +3106,26 @@ class TestModel:
         reaction_event_factory,
         reaction_event_index_factory,
         op,
+        reaction_event_schema,
+        reaction_schema,
+        msgs,
         expected_number_after,
-        event_message_id=1,
+        event_message_id=2,
     ):
-        reaction_event = reaction_event_factory(
-            op=op,
-            message_id=event_message_id,
-        )
-        model.index = reaction_event_index_factory(
-            [
-                (1, [(1, "unicode_emoji", "1232", "thumbs_up")]),
-                (2, []),
-            ]
-        )
+        common_args = {
+            "op": op,
+            "message_id": event_message_id,
+        }
+        if reaction_event_schema == "with_user":
+            reaction_event = reaction_event_factory(**common_args, user=True)
+        elif reaction_event_schema == "with_user_id":
+            reaction_event = reaction_event_factory(**common_args, user_id=5140)
+        else:
+            reaction_event = reaction_event_factory(
+                **common_args, user=True, user_id=5140
+            )
+
+        model.index = reaction_event_index_factory(msgs, reaction_schema)
         model._update_rendered_view = mocker.Mock()
 
         model._handle_reaction_event(reaction_event)
diff --git a/tests/ui_tools/test_messages.py b/tests/ui_tools/test_messages.py
index 08ba7d0..09a68bc 100644
--- a/tests/ui_tools/test_messages.py
+++ b/tests/ui_tools/test_messages.py
@@ -1,5 +1,6 @@
 from collections import OrderedDict, defaultdict
 from datetime import date
+from unittest.mock import patch
 
 import pytest
 import pytz
@@ -1597,60 +1598,6 @@ class TestMessageBox:
     @pytest.mark.parametrize(
         "to_vary_in_each_message, expected_text, expected_attributes",
         [
-            case(
-                {
-                    "reactions": [
-                        {
-                            "emoji_name": "thumbs_up",
-                            "emoji_code": "1f44d",
-                            "user": {
-                                "email": "iago@zulip.com",
-                                "full_name": "Iago",
-                                "id": 5,
-                            },
-                            "reaction_type": "unicode_emoji",
-                        },
-                        {
-                            "emoji_name": "zulip",
-                            "emoji_code": "zulip",
-                            "user": {
-                                "email": "iago@zulip.com",
-                                "full_name": "Iago",
-                                "id": 5,
-                            },
-                            "reaction_type": "zulip_extra_emoji",
-                        },
-                        {
-                            "emoji_name": "zulip",
-                            "emoji_code": "zulip",
-                            "user": {
-                                "email": "AARON@zulip.com",
-                                "full_name": "aaron",
-                                "id": 1,
-                            },
-                            "reaction_type": "zulip_extra_emoji",
-                        },
-                        {
-                            "emoji_name": "heart",
-                            "emoji_code": "2764",
-                            "user": {
-                                "email": "iago@zulip.com",
-                                "full_name": "Iago",
-                                "id": 5,
-                            },
-                            "reaction_type": "unicode_emoji",
-                        },
-                    ],
-                },
-                " :thumbs_up: 1   :zulip: 2   :heart: 1  ",
-                [
-                    ("reaction", 15),
-                    (None, 1),
-                    ("reaction_mine", 11),
-                    (None, 1),
-                    ("reaction", 11),
-                ],
-            ),
             case(
                 {
                     "reactions": [
@@ -1701,31 +1648,19 @@ class TestMessageBox:
                         {
                             "emoji_name": "zulip",
                             "emoji_code": "zulip",
-                            "user": {
-                                "email": "iago@zulip.com",
-                                "full_name": "Iago",
-                                "id": 5,
-                            },
+                            "user_id": 5,
                             "reaction_type": "unicode_emoji",
                         },
                         {
                             "emoji_name": "zulip",
                             "emoji_code": "zulip",
-                            "user": {
-                                "email": "AARON@zulip.com",
-                                "full_name": "aaron",
-                                "id": 1,
-                            },
+                            "user_id": 1,
                             "reaction_type": "zulip_extra_emoji",
                         },
                         {
                             "emoji_name": "zulip",
                             "emoji_code": "zulip",
-                            "user": {
-                                "email": "shivam@zulip.com",
-                                "full_name": "Shivam",
-                                "id": 6,
-                            },
+                            "user_id": 6,
                             "reaction_type": "unicode_emoji",
                         },
                     ],
@@ -1741,31 +1676,19 @@ class TestMessageBox:
                         {
                             "emoji_name": "heart",
                             "emoji_code": "2764",
-                            "user": {
-                                "email": "iago@zulip.com",
-                                "full_name": "Iago",
-                                "id": 5,
-                            },
+                            "user_id": 5,
                             "reaction_type": "unicode_emoji",
                         },
                         {
                             "emoji_name": "zulip",
                             "emoji_code": "zulip",
-                            "user": {
-                                "email": "AARON@zulip.com",
-                                "full_name": "aaron",
-                                "id": 1,
-                            },
+                            "user_id": 1,
                             "reaction_type": "zulip_extra_emoji",
                         },
                         {
                             "emoji_name": "zulip",
                             "emoji_code": "zulip",
-                            "user": {
-                                "email": "shivam@zulip.com",
-                                "full_name": "Shivam",
-                                "id": 6,
-                            },
+                            "user_id": 6,
                             "reaction_type": "unicode_emoji",
                         },
                     ],
@@ -1821,10 +1744,26 @@ class TestMessageBox:
         msg_box = MessageBox(varied_message, self.model, None)
         reactions = to_vary_in_each_message["reactions"]
 
-        reactions_view = msg_box.reactions_view(reactions)
+        mock_all_users_by_id = {
+            1: {"full_name": "aaron"},
+            5: {"full_name": "Iago"},
+            6: {"full_name": "Shivam"},
+        }
+
+        def mock_get_user_id(reaction):
+            if "user_id" in reaction:
+                return reaction["user_id"]
+            return reaction["user"]["id"]
+
+        with patch.object(
+            self.model,
+            "get_user_id_from_reaction",
+            side_effect=mock_get_user_id,
+        ), patch.object(self.model, "_all_users_by_id", mock_all_users_by_id):
+            reactions_view = msg_box.reactions_view(reactions)
 
-        assert reactions_view.original_widget.text == expected_text
-        assert reactions_view.original_widget.attrib == expected_attributes
+            assert reactions_view.original_widget.text == expected_text
+            assert reactions_view.original_widget.attrib == expected_attributes
 
     @pytest.mark.parametrize(
         "message_links, expected_text, expected_attrib, expected_footlinks_width",
diff --git a/tests/ui_tools/test_popups.py b/tests/ui_tools/test_popups.py
index 878be17..47b3ff1 100644
--- a/tests/ui_tools/test_popups.py
+++ b/tests/ui_tools/test_popups.py
@@ -1,5 +1,6 @@
 from collections import OrderedDict
 from typing import Any, Callable, Dict, List, Optional, Tuple
+from unittest.mock import patch
 
 import pytest
 from pytest import param as case
@@ -1169,21 +1170,13 @@ class TestMsgInfoView:
                     {
                         "emoji_name": "zulip",
                         "emoji_code": "zulip",
-                        "user": {
-                            "email": "iago@zulip.com",
-                            "full_name": "Iago",
-                            "id": 5,
-                        },
+                        "user_id": 5,
                         "reaction_type": "zulip_extra_emoji",
                     },
                     {
                         "emoji_name": "zulip",
                         "emoji_code": "zulip",
-                        "user": {
-                            "email": "AARON@zulip.com",
-                            "full_name": "aaron",
-                            "id": 1,
-                        },
+                        "user_id": 1,
                         "reaction_type": "zulip_extra_emoji",
                     },
                     {
@@ -1207,18 +1200,37 @@ class TestMsgInfoView:
     ) -> None:
         varied_message = message_fixture
         varied_message.update(to_vary_in_each_message)
-        self.msg_info_view = MsgInfoView(
-            self.controller,
-            varied_message,
-            "Message Information",
-            OrderedDict(),
-            OrderedDict(),
-            list(),
-        )
-        # 12 = 7 labels + 2 blank lines + 1 'Reactions' (category)
-        # + 4 reactions (excluding 'Message Links').
-        expected_height = 14
-        assert self.msg_info_view.height == expected_height
+
+        mock_all_users_by_id = {
+            1: {"full_name": "aaron"},
+            5: {"full_name": "Iago"},
+        }
+
+        def mock_get_user_id(reaction: Dict[str, Any]) -> int:
+            if "user_id" in reaction:
+                return reaction["user_id"]
+            return reaction["user"]["id"]
+
+        with patch.object(
+            self.controller.model,
+            "get_user_id_from_reaction",
+            side_effect=mock_get_user_id,
+        ), patch.object(
+            self.controller.model, "_all_users_by_id", mock_all_users_by_id
+        ):
+            self.msg_info_view = MsgInfoView(
+                self.controller,
+                varied_message,
+                "Message Information",
+                OrderedDict(),
+                OrderedDict(),
+                list(),
+            )
+
+            # 12 = 7 labels + 2 blank lines + 1 'Reactions' (category)
+            # + 4 reactions (excluding 'Message Links').
+            expected_height = 14
+            assert self.msg_info_view.height == expected_height
 
     @pytest.mark.parametrize(
         [