Skip to content

Commit

Permalink
messages/views: Prepare for the deprecation of user metadata.
Browse files Browse the repository at this point in the history 
The "user" field is to be deprecated from messages' reactions and
reaction events in the API.

Add support for `user_id` in MsgInfoView, by using the recently
introduced `get_user_id_from_reaction()`.

Remove dependence on reaction["user"]["full_name"] by instead using
`_all_users_by_id` with the "user_id".

Tests updated.
  • Loading branch information
@Niloth-p @timabbott
Niloth-p authored and timabbott committed Dec 10, 2024
1 parent ccea7b5 commit 23a773c
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 114 deletions.
113 changes: 26 additions & 87 deletions tests/ui_tools/test_messages.py
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from collections import OrderedDict, defaultdict
from datetime import date
from unittest.mock import patch

import pytest
import pytz
Expand Down Expand Up @@ -1597,60 +1598,6 @@ def test_transform_content(self, mocker, raw_html, expected_content):
@pytest.mark.parametrize(
"to_vary_in_each_message, expected_text, expected_attributes",
[
case(
{
"reactions": [
{
"emoji_name": "thumbs_up",
"emoji_code": "1f44d",
"user": {
"email": "[email protected]",
"full_name": "Iago",
"id": 5,
},
"reaction_type": "unicode_emoji",
},
{
"emoji_name": "zulip",
"emoji_code": "zulip",
"user": {
"email": "[email protected]",
"full_name": "Iago",
"id": 5,
},
"reaction_type": "zulip_extra_emoji",
},
{
"emoji_name": "zulip",
"emoji_code": "zulip",
"user": {
"email": "[email protected]",
"full_name": "aaron",
"id": 1,
},
"reaction_type": "zulip_extra_emoji",
},
{
"emoji_name": "heart",
"emoji_code": "2764",
"user": {
"email": "[email protected]",
"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": [
Expand Down Expand Up @@ -1701,31 +1648,19 @@ def test_transform_content(self, mocker, raw_html, expected_content):
{
"emoji_name": "zulip",
"emoji_code": "zulip",
"user": {
"email": "[email protected]",
"full_name": "Iago",
"id": 5,
},
"user_id": 5,
"reaction_type": "unicode_emoji",
},
{
"emoji_name": "zulip",
"emoji_code": "zulip",
"user": {
"email": "[email protected]",
"full_name": "aaron",
"id": 1,
},
"user_id": 1,
"reaction_type": "zulip_extra_emoji",
},
{
"emoji_name": "zulip",
"emoji_code": "zulip",
"user": {
"email": "[email protected]",
"full_name": "Shivam",
"id": 6,
},
"user_id": 6,
"reaction_type": "unicode_emoji",
},
],
Expand All @@ -1741,31 +1676,19 @@ def test_transform_content(self, mocker, raw_html, expected_content):
{
"emoji_name": "heart",
"emoji_code": "2764",
"user": {
"email": "[email protected]",
"full_name": "Iago",
"id": 5,
},
"user_id": 5,
"reaction_type": "unicode_emoji",
},
{
"emoji_name": "zulip",
"emoji_code": "zulip",
"user": {
"email": "[email protected]",
"full_name": "aaron",
"id": 1,
},
"user_id": 1,
"reaction_type": "zulip_extra_emoji",
},
{
"emoji_name": "zulip",
"emoji_code": "zulip",
"user": {
"email": "[email protected]",
"full_name": "Shivam",
"id": 6,
},
"user_id": 6,
"reaction_type": "unicode_emoji",
},
],
Expand Down Expand Up @@ -1821,10 +1744,26 @@ def test_reactions_view(
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",
Expand Down
56 changes: 34 additions & 22 deletions tests/ui_tools/test_popups.py
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -1169,21 +1170,13 @@ def test_height_noreactions(self) -> None:
{
"emoji_name": "zulip",
"emoji_code": "zulip",
"user": {
"email": "[email protected]",
"full_name": "Iago",
"id": 5,
},
"user_id": 5,
"reaction_type": "zulip_extra_emoji",
},
{
"emoji_name": "zulip",
"emoji_code": "zulip",
"user": {
"email": "[email protected]",
"full_name": "aaron",
"id": 1,
},
"user_id": 1,
"reaction_type": "zulip_extra_emoji",
},
{
Expand All @@ -1207,18 +1200,37 @@ def test_height_reactions(
) -> 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(
[
Expand Down
6 changes: 2 additions & 4 deletions zulipterminal/ui_tools/messages.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,8 @@ def reactions_view(
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))
Expand Down
7 changes: 6 additions & 1 deletion zulipterminal/ui_tools/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1657,7 +1657,12 @@ def __init__(
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()
Expand Down

0 comments on commit 23a773c

Please sign in to comment.