diff options
author | Trygve Aaberge <trygveaa@gmail.com> | 2024-02-03 11:38:33 +0100 |
---|---|---|
committer | Trygve Aaberge <trygveaa@gmail.com> | 2024-02-18 12:57:00 +0100 |
commit | 7639dcb4995b13c2756f9d59e0a67e22cba57a38 (patch) | |
tree | f38f36bfab6ab13fb89497fff1fb5e8d29e787ac | |
parent | 15cabf8ba1b9daa7e28475ea46cc5a8fd478e174 (diff) | |
download | wee-slack-7639dcb4995b13c2756f9d59e0a67e22cba57a38.tar.gz |
Correctly show reactions with more than 50 users in v3.0.0
If a reaction has more than 50 users, the list of users will be cropped
to just 50 users. This made the counts wrong since we used the number of
users rather than the count property. Fix it by using the count property
instead and adding "and others" to the list of users if
show_reaction_nicks is enabled (the same as the Slack client shows when
you hover over the reaction).
This is the same fix as in commit 6b388cd9, but for version 3.0.0.
-rw-r--r-- | mock_data/slack_users_info_person_1.json (renamed from mock_data/slack_users_info_person.json) | 6 | ||||
-rw-r--r-- | mock_data/slack_users_info_person_2.json | 51 | ||||
-rw-r--r-- | slack/slack_message.py | 13 | ||||
-rw-r--r-- | tests/conftest.py | 17 | ||||
-rw-r--r-- | tests/test_render_reactions.py | 73 |
5 files changed, 150 insertions, 10 deletions
diff --git a/mock_data/slack_users_info_person.json b/mock_data/slack_users_info_person_1.json index 14e80f8..60dff7d 100644 --- a/mock_data/slack_users_info_person.json +++ b/mock_data/slack_users_info_person_1.json @@ -18,13 +18,15 @@ "real_name_normalized": "Test 1", "display_name": "Test 1", "display_name_normalized": "Test 1", - "fields": {}, + "fields": null, "status_text": "", "status_emoji": "", "status_emoji_display_info": [], "status_expiration": 0, "avatar_hash": "g57ba37781eb", "email": "trygveaa+wee-slack-test.1@gmail.com", + "first_name": "Test", + "last_name": "1", "image_24": "https://secure.gravatar.com/avatar/57ba37781ebc7fcdf8cc9812dce9c407.jpg?s=24&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0001-24.png", "image_32": "https://secure.gravatar.com/avatar/57ba37781ebc7fcdf8cc9812dce9c407.jpg?s=32&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0001-32.png", "image_48": "https://secure.gravatar.com/avatar/57ba37781ebc7fcdf8cc9812dce9c407.jpg?s=48&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0001-48.png", @@ -41,7 +43,7 @@ "is_ultra_restricted": false, "is_bot": false, "is_app_user": false, - "updated": 1664912039, + "updated": 1703064325, "is_email_confirmed": true, "has_2fa": false, "who_can_share_contact_card": "EVERYONE" diff --git a/mock_data/slack_users_info_person_2.json b/mock_data/slack_users_info_person_2.json new file mode 100644 index 0000000..3927802 --- /dev/null +++ b/mock_data/slack_users_info_person_2.json @@ -0,0 +1,51 @@ +{ + "ok": true, + "user": { + "id": "U0175CGFJRY", + "team_id": "T0FC8BFQR", + "name": "trygveaa_wee-slack195", + "deleted": false, + "color": "5b89d5", + "real_name": "Test 2", + "tz": "Europe/Amsterdam", + "tz_label": "Central European Time", + "tz_offset": 3600, + "profile": { + "title": "", + "phone": "", + "skype": "", + "real_name": "Test 2", + "real_name_normalized": "Test 2", + "display_name": "Test 2", + "display_name_normalized": "Test 2", + "fields": null, + "status_text": "", + "status_emoji": "", + "status_emoji_display_info": [], + "status_expiration": 0, + "avatar_hash": "g7da9d26b2e9", + "email": "trygveaa+wee-slack-test.2@gmail.com", + "first_name": "Test", + "last_name": "2", + "image_24": "https://secure.gravatar.com/avatar/7da9d26b2e952f62a1b1755dbb798975.jpg?s=24&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0006-24.png", + "image_32": "https://secure.gravatar.com/avatar/7da9d26b2e952f62a1b1755dbb798975.jpg?s=32&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0006-32.png", + "image_48": "https://secure.gravatar.com/avatar/7da9d26b2e952f62a1b1755dbb798975.jpg?s=48&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0006-48.png", + "image_72": "https://secure.gravatar.com/avatar/7da9d26b2e952f62a1b1755dbb798975.jpg?s=72&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0006-72.png", + "image_192": "https://secure.gravatar.com/avatar/7da9d26b2e952f62a1b1755dbb798975.jpg?s=192&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0006-192.png", + "image_512": "https://secure.gravatar.com/avatar/7da9d26b2e952f62a1b1755dbb798975.jpg?s=512&d=https%3A%2F%2Fa.slack-edge.com%2Fdf10d%2Fimg%2Favatars%2Fava_0006-512.png", + "status_text_canonical": "", + "team": "T0FC8BFQR" + }, + "is_admin": false, + "is_owner": false, + "is_primary_owner": false, + "is_restricted": false, + "is_ultra_restricted": false, + "is_bot": false, + "is_app_user": false, + "updated": 1664912039, + "is_email_confirmed": true, + "has_2fa": false, + "who_can_share_contact_card": "EVERYONE" + } +} diff --git a/slack/slack_message.py b/slack/slack_message.py index 576629c..30f1ad4 100644 --- a/slack/slack_message.py +++ b/slack/slack_message.py @@ -853,14 +853,15 @@ class SlackMessage: users = await gather( *(self.workspace.users[user_id] for user_id in reaction["users"]) ) - nicks = ",".join(user.nick.format() for user in users) - users_str = f"({nicks})" + nicks = [user.nick.format() for user in users] + nicks_extra = ( + ["and others"] if len(reaction["users"]) < reaction["count"] else [] + ) + users_str = f"({', '.join(nicks + nicks_extra)})" else: users_str = "" - reaction_string = ( - f"{get_emoji(reaction['name'])}{len(reaction['users'])}{users_str}" - ) + reaction_string = f"{get_emoji(reaction['name'])}{reaction['count']}{users_str}" if self.workspace.my_user.id in reaction["users"]: return with_color( @@ -874,7 +875,7 @@ class SlackMessage: async def _create_reactions_string(self) -> str: reactions = self._message_json.get("reactions", []) reactions_with_users = [ - reaction for reaction in reactions if len(reaction["users"]) > 0 + reaction for reaction in reactions if reaction["count"] > 0 ] reaction_strings = await gather( *( diff --git a/tests/conftest.py b/tests/conftest.py index 981faab..cda1833 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -148,13 +148,20 @@ color_reset = "<[color:reset]>" workspace_id = "T0FC8BFQR" -with open("mock_data/slack_users_info_person.json") as f: +with open("mock_data/slack_users_info_person_1.json") as f: user_test1_info_response: SlackUserInfoSuccessResponse[SlackUserInfo] = json.loads( f.read() ) user_test1_info = user_test1_info_response["user"] user_test1_id = user_test1_info["id"] +with open("mock_data/slack_users_info_person_2.json") as f: + user_test2_info_response: SlackUserInfoSuccessResponse[SlackUserInfo] = json.loads( + f.read() + ) + user_test2_info = user_test2_info_response["user"] + user_test2_id = user_test2_info["id"] + with open("mock_data/slack_conversations_info_channel_public.json") as f: channel_public_info_response: SlackConversationsInfoSuccessResponse[ SlackConversationsInfo @@ -172,9 +179,15 @@ def workspace(): user_test1 = SlackUser(w, user_test1_info) user_test1_future = Future[SlackUser]() user_test1_future.set_result(user_test1) - w.my_user = user_test1 w.users[user_test1_id] = user_test1_future + user_test2 = SlackUser(w, user_test2_info) + user_test2_future = Future[SlackUser]() + user_test2_future.set_result(user_test2) + w.users[user_test2_id] = user_test2_future + + w.my_user = user_test1 + channel_public_future = Future[SlackConversation]() w.conversations[channel_public_id] = channel_public_future channel_public = SlackConversation(w, channel_public_info) diff --git a/tests/test_render_reactions.py b/tests/test_render_reactions.py new file mode 100644 index 0000000..dad9696 --- /dev/null +++ b/tests/test_render_reactions.py @@ -0,0 +1,73 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, List + +import pytest + +from slack.shared import shared +from slack.slack_message import SlackMessage +from tests.conftest import ( + color_reset, + user_test1_id, + user_test2_id, +) + +if TYPE_CHECKING: + from slack_api.slack_conversations_history import SlackMessageReaction + from typing_extensions import TypedDict +else: + TypedDict = object + + +class Case(TypedDict): + reactions: List[SlackMessageReaction] + rendered: str + display_reaction_nicks: bool + + +color_reaction_suffix = "<[color:<[config_color:reaction_suffix]>]>" +color_reaction_self_suffix = "<[color:<[config_color:reaction_self_suffix]>]>" + +cases: List[Case] = [ + { + "reactions": [{"name": "custom", "users": [user_test2_id], "count": 1}], + "rendered": f" {color_reaction_suffix}[:custom:1]{color_reset}", + "display_reaction_nicks": False, + }, + { + "reactions": [{"name": "custom", "users": [user_test1_id], "count": 1}], + "rendered": f" {color_reaction_suffix}[{color_reaction_self_suffix}:custom:1{color_reaction_suffix}]{color_reset}", + "display_reaction_nicks": False, + }, + { + "reactions": [{"name": "custom", "users": [user_test2_id], "count": 1}], + "rendered": f" {color_reaction_suffix}[:custom:1(Test_2)]{color_reset}", + "display_reaction_nicks": True, + }, + { + "reactions": [{"name": "custom", "users": [user_test1_id], "count": 1}], + "rendered": f" {color_reaction_suffix}[{color_reaction_self_suffix}:custom:1(Test_1){color_reaction_suffix}]{color_reset}", + "display_reaction_nicks": True, + }, + { + "reactions": [{"name": "custom", "users": [user_test2_id], "count": 2}], + "rendered": f" {color_reaction_suffix}[:custom:2]{color_reset}", + "display_reaction_nicks": False, + }, + { + "reactions": [{"name": "custom", "users": [user_test2_id], "count": 2}], + "rendered": f" {color_reaction_suffix}[:custom:2(Test_2, and others)]{color_reset}", + "display_reaction_nicks": True, + }, +] + + +@pytest.mark.parametrize("case", cases) +def test_create_reactions_string(case: Case, message1_in_channel_public: SlackMessage): + shared.config.look.display_reaction_nicks.value = case["display_reaction_nicks"] + message1_in_channel_public._message_json["reactions"] = case["reactions"] # pyright: ignore [reportPrivateUsage] + + coroutine = message1_in_channel_public._create_reactions_string() # pyright: ignore [reportPrivateUsage] + with pytest.raises(StopIteration) as excinfo: + coroutine.send(None) + assert excinfo.value.value == case["rendered"] |