BugFix: Comparing users based on user_id than user_name.

Fixes #1151

Added a function named user_name_count in model.py  and a variable user_name_dict_count that would keep count of all the usernames in zulip-terminal in a dict with key as the username and value as the count of people having the same username.

Adding a check in the boxes.py to compare based on user_id instead of username so that users with same names are shown with different headers.

Also added a check if someone else with same username already exists so common usernames could be displayed with their user_id appended to the name.

Changing label name from "name" to "msg_sender" in every theme

test_ui_tools: changing test cases and parameterized cases to add sender_id

model.py: Changing the function to return just if a user with same name exists or not instead of returning the number of users with same name so that it passed the written test cases

boxes.py: Adding a style class so the id appended to the name doesn't look like it's a part of the name. Changing the type of "textType" so that it supports the test type of Tuple[Optional[str], str].

Author: Sushmey

tests/ui/test_ui_tools.py

@@ -1983,6 +1983,7 @@ def test_main_view(self, mocker, message, last_message):
         [
             {
                 "id": 4,
+                "sender_id": 4,
                 "type": "stream",
                 "display_recipient": "Verona",
                 "stream_id": 5,
@@ -2021,6 +2022,7 @@ def test_main_view_renders_slash_me(self, mocker, message, content, is_me_messag
         [
             {
                 "id": 4,
+                "sender_id": 4,
                 "type": "stream",
                 "display_recipient": "Verona",
                 "stream_id": 5,
@@ -2196,6 +2198,7 @@ def test_msg_generates_search_and_header_bar(
         [
             {
                 "id": 4,
+                "sender_id": 4,
                 "type": "stream",
                 "display_recipient": "Verona",
                 "stream_id": 5,
@@ -2221,11 +2224,11 @@ def test_msg_generates_search_and_header_bar(
         "expected_header, to_vary_in_last_message",
         [
             (
-                [STATUS_INACTIVE, "alice", " ", "DAYDATETIME"],
+                [STATUS_INACTIVE, "alice(4)", " ", "DAYDATETIME"],
                 {"sender_full_name": "bob"},
             ),
             ([" ", " ", " ", "DAYDATETIME"], {"timestamp": 1532103779}),
-            ([STATUS_INACTIVE, "alice", " ", "DAYDATETIME"], {"timestamp": 0}),
+            ([STATUS_INACTIVE, "alice(4)", " ", "DAYDATETIME"], {"timestamp": 0}),
         ],
         ids=[
             "show_author_as_authors_different",

zulipterminal/config/themes.py

@@ -29,7 +29,7 @@
     'header'          : 'bold',
     'general_narrow'  : 'standout',
     'general_bar'     : '',
-    'name'            : '',
+    'msg_sender'      : '',
     'unread'          : 'strikethrough',
     'user_active'     : 'bold',
     'user_idle'       : '',

zulipterminal/model.py

@@ -4,7 +4,7 @@
 
 import json
 import time
-from collections import OrderedDict, defaultdict
+from collections import Counter, OrderedDict, defaultdict
 from concurrent.futures import Future, ThreadPoolExecutor, wait
 from copy import deepcopy
 from datetime import datetime
@@ -1018,6 +1018,7 @@ def get_all_users(self) -> List[Dict[str, Any]]:
         # and a user-id to email mapping
         self.user_dict: Dict[str, Dict[str, Any]] = dict()
         self.user_id_email_dict: Dict[int, str] = dict()
+        self.user_name_dict_count: Dict[str, int] = dict()
         for user in self.initial_data["realm_users"]:
             if self.user_id == user["user_id"]:
                 self._all_users_by_id[self.user_id] = user
@@ -1130,7 +1131,9 @@ def get_all_users(self) -> List[Dict[str, Any]]:
         user_list.insert(0, current_user)
         self.user_dict[current_user["email"]] = current_user
         self.user_id_email_dict[self.user_id] = current_user["email"]
-
+        self.user_name_dict_count = Counter(
+            user["full_name"] for user in user_list
+        )  # Counting number of users having same name
         return user_list
 
     def user_name_from_id(self, user_id: int) -> str:
@@ -1144,6 +1147,12 @@ def user_name_from_id(self, user_id: int) -> str:
 
         return self.user_dict[user_email]["full_name"]
 
+    def user_name_count_compare(self, user_name: str) -> int:
+        """
+        Returns if the count of the users with the same name is more than 1.
+        """
+        return True if self.user_name_dict_count.get(user_name, -1) > 1 else False
+
     def _subscribe_to_streams(self, subscriptions: List[Subscription]) -> None:
         def make_reduced_stream_data(stream: Subscription) -> StreamData:
             # stream_id has been changed to id.

zulipterminal/themes/gruvbox_dark.py

@@ -24,7 +24,7 @@
     'header'           : (Color.NEUTRAL_BLUE,          Color.BRIGHT_BLUE),
     'general_narrow'   : (Color.DARK0_HARD,            Color.BRIGHT_BLUE),
     'general_bar'      : (Color.LIGHT2,                Color.DARK0_HARD),
-    'name'             : (Color.NEUTRAL_YELLOW__BOLD,  Color.DARK0_HARD),
+    'msg_sender'       : (Color.NEUTRAL_YELLOW__BOLD,  Color.DARK0_HARD),
     'unread'           : (Color.NEUTRAL_PURPLE,        Color.DARK0_HARD),
     'user_active'      : (Color.BRIGHT_GREEN,          Color.DARK0_HARD),
     'user_idle'        : (Color.NEUTRAL_YELLOW,        Color.DARK0_HARD),

zulipterminal/themes/gruvbox_light.py

@@ -23,7 +23,7 @@
     'header'           : (Color.NEUTRAL_BLUE,           Color.FADED_BLUE),
     'general_narrow'   : (Color.LIGHT0_HARD,            Color.FADED_BLUE),
     'general_bar'      : (Color.DARK2,                  Color.LIGHT0_HARD),
-    'name'             : (Color.NEUTRAL_YELLOW,         Color.LIGHT0_HARD),
+    'msg_sender'       : (Color.NEUTRAL_YELLOW,         Color.LIGHT0_HARD),
     'unread'           : (Color.NEUTRAL_PURPLE,         Color.LIGHT0_HARD),
     'user_active'      : (Color.FADED_GREEN,            Color.LIGHT0_HARD),
     'user_idle'        : (Color.NEUTRAL_YELLOW,         Color.LIGHT0_HARD),

zulipterminal/themes/zt_blue.py

@@ -18,7 +18,7 @@
     'header'          : (Color.BLACK,               Color.DARK_BLUE),
     'general_narrow'  : (Color.WHITE,               Color.DARK_BLUE),
     'general_bar'     : (Color.DARK_BLUE,           Color.LIGHT_BLUE),
-    'name'            : (Color.DARK_RED,            Color.LIGHT_BLUE),
+    'msg_sender'     : (Color.DARK_RED,            Color.LIGHT_BLUE),
     'unread'          : (Color.LIGHT_GRAY,          Color.LIGHT_BLUE),
     'user_active'     : (Color.LIGHT_GREEN__BOLD,   Color.LIGHT_BLUE),
     'user_idle'       : (Color.DARK_GRAY,           Color.LIGHT_BLUE),

zulipterminal/themes/zt_dark.py

@@ -18,7 +18,7 @@
     'header'          : (Color.DARK_CYAN,           Color.DARK_BLUE),
     'general_narrow'  : (Color.WHITE,               Color.DARK_BLUE),
     'general_bar'     : (Color.WHITE,               Color.BLACK),
-    'name'            : (Color.YELLOW__BOLD,        Color.BLACK),
+    'msg_sender'      : (Color.YELLOW__BOLD,        Color.BLACK),
     'unread'          : (Color.DARK_BLUE,           Color.BLACK),
     'user_active'     : (Color.LIGHT_GREEN,         Color.BLACK),
     'user_idle'       : (Color.YELLOW,              Color.BLACK),

zulipterminal/themes/zt_light.py

@@ -18,7 +18,7 @@
     'header'          : (Color.WHITE,               Color.DARK_BLUE),
     'general_narrow'  : (Color.WHITE,               Color.DARK_BLUE),
     'general_bar'     : (Color.DARK_BLUE,           Color.WHITE),
-    'name'            : (Color.DARK_GREEN,          Color.WHITE),
+    'msg_sender'      : (Color.DARK_GREEN,          Color.WHITE),
     'unread'          : (Color.DARK_GRAY,           Color.LIGHT_GRAY),
     'user_active'     : (Color.DARK_GREEN,          Color.WHITE),
     'user_idle'       : (Color.DARK_BLUE,           Color.WHITE),

zulipterminal/ui_tools/boxes.py

@@ -76,7 +76,6 @@ def __init__(self, view: Any) -> None:
         super().__init__(self.main_view(True))
         self.model = view.model
         self.view = view
-
         # Used to indicate user's compose status, "closed" by default
         self.compose_box_status: Literal[
             "open_with_private", "open_with_stream", "closed"
@@ -575,7 +574,9 @@ def autocomplete_mentions(
         return combined_typeahead, combined_names
 
     def autocomplete_users(
-        self, text: str, prefix_string: str
+        self,
+        text: str,
+        prefix_string: str,
     ) -> Tuple[List[str], List[str]]:
         users_list = self.view.users
         matching_users = [
@@ -1530,9 +1531,7 @@ def main_view(self) -> List[Any]:
                 "author": (
                     msg["sender_full_name"] if "sender_full_name" in msg else None
                 ),
-                "author_id":(
-                    msg["sender_id"] if "sender_id" in msg else None
-                ),
+                "author_id": (msg["sender_id"] if "sender_id" in msg else None),
                 "time": (
                     self.model.formatted_local_time(
                         msg["timestamp"], show_seconds=False
@@ -1550,7 +1549,8 @@ def main_view(self) -> List[Any]:
         }
         different = {  # How this message differs from the previous one
             "recipients": recipient_header is not None,
-            "author": message["this"]["author_id"] != message["last"]["author_id"],
+            "author": message["this"]["author"] != message["last"]["author"],
+            "author_id": message["this"]["author_id"] != message["last"]["author_id"],
             "24h": (
                 message["last"]["datetime"] is not None
                 and ((message["this"]["datetime"] - message["last"]["datetime"]).days)
@@ -1566,12 +1566,19 @@ def main_view(self) -> List[Any]:
         any_differences = any(different.values())
 
         if any_differences:  # Construct content_header, if needed
-            TextType = Dict[str, urwid_MarkupTuple]
-            text_keys = ("author", "star", "time", "status")
-            text: TextType = {key: (None, " ") for key in text_keys}
-
-            if any(different[key] for key in ("recipients", "author", "24h")):
-                text["author"] = ("name", message["this"]["author"])
+            TextType = Dict[str, List[urwid_MarkupTuple]]
+            text_keys = ("author", "star", "time", "status", "author_id")
+            text: TextType = {key: [(None, " ")] for key in text_keys}
+            if any(
+                different[key] for key in ("recipients", "author", "24h", "author_id")
+            ):
+                if self.model.user_name_count_compare(message["this"]["author"]):
+                    text["author"] = [
+                        ("msg_sender", message["this"]["author"]),
+                        ("msg_mention", "(" + str(message["this"]["author_id"]) + ")"),
+                    ]
+                else:
+                    text["author"] = [("msg_sender", message["this"]["author"])]
 
                 # TODO: Refactor to use user ids for look up instead of emails.
                 email = self.message.get("sender_email", "")
@@ -1582,17 +1589,17 @@ def main_view(self) -> List[Any]:
 
                 # The default text['status'] value is (None, ' ')
                 if status in STATE_ICON:
-                    text["status"] = (f"user_{status}", STATE_ICON[status])
+                    text["status"] = [(f"user_{status}", STATE_ICON[status])]
 
             if message["this"]["is_starred"]:
-                text["star"] = ("starred", "*")
+                text["star"] = [("starred", "*")]
             if any(different[key] for key in ("recipients", "author", "timestamp")):
                 this_year = date.today().year
                 msg_year = message["this"]["datetime"].year
                 if this_year != msg_year:
-                    text["time"] = ("time", f"{msg_year} - {message['this']['time']}")
+                    text["time"] = [("time", f"{msg_year} - {message['this']['time']}")]
                 else:
-                    text["time"] = ("time", message["this"]["time"])
+                    text["time"] = [("time", message["this"]["time"])]
 
             content_header = urwid.Columns(
                 [