bugfix: model/boxes/buttons: Fully handle both server color formats.

The zulip server used to store some colors in the format #xxx and others
in the format #xxxxxx. The terminal client did not handle the #xxx
format correctly in all cases until this commit.

This commit:
   * removes an instance of on-the-fly color canonicalization in
     `ui_tools/boxes.py` (in `StreamButton.__init__`)
   * adds a color canonicalization step to `model.py` (in
     `Model._stream_info_from_subscriptions`)
   * removes integration tests that verify that `StreamButton`
     canonicalizes its colors.
   * modifies integration tests to verify that the model canonicalizes
     its colors.

Author: alex-ozdemir

tests/conftest.py

@@ -104,7 +104,7 @@ def logged_on_user():
 general_stream = {
     'name': 'Some general stream',
     'invite_only': False,
-    'color': '#b0a5fd',
+    'color': '#b0a5fd',                   # Color in '#xxxxxx' format
     'pin_to_top': False,
     'stream_id': 1000,
     'in_home_view': True,
@@ -126,7 +126,7 @@ def logged_on_user():
     'invite_only': True,
     'name': 'Secret stream',
     'email_address': '[email protected]',
-    'color': '#c3c3c3',
+    'color': '#ccc',                      # Color in '#xxx' format
     'in_home_view': True,
     'audible_notifications': False,
     'is_old_stream': True,
@@ -662,10 +662,10 @@ def streams():
     List of streams created corresponding to
     `initial_data` fixture.
     """
-    return [['Secret stream', 99, '#c3c3c3', True],
-            ['Some general stream', 1000, '#b0a5fd', False],
-            ['Stream 1', 1, '#b0a5fd', False],
-            ['Stream 2', 2, '#b0a5fd', False]]
+    return [['Secret stream', 99, '#ccc', True],
+            ['Some general stream', 1000, '#baf', False],
+            ['Stream 1', 1, '#baf', False],
+            ['Stream 2', 2, '#baf', False]]
 
 
 @pytest.fixture

tests/ui/test_ui_tools.py

@@ -1217,7 +1217,7 @@ def test_soup2markup(self, content, markup):
                        timestamp=99, reactions=[])
         self.model.stream_dict = {
             5: {  # matches stream_id above
-                'color': '#bfd56f',
+                'color': '#bd6',
             },
         }
         self.model.server_url = "SOME_BASE_URL"
@@ -1288,7 +1288,7 @@ def test_soup2markup(self, content, markup):
     def test_main_view(self, mocker, message, last_message):
         self.model.stream_dict = {
             5: {
-                'color': '#bfd56f',
+                'color': '#bd6',
             },
         }
         msg_box = MessageBox(message, self.model, last_message)
@@ -1317,7 +1317,7 @@ def test_main_view_generates_stream_header(self, mocker, message,
         mocker.patch(VIEWS + ".urwid.Text")
         self.model.stream_dict = {
             5: {
-                'color': '#bfd56f',
+                'color': '#bd6',
             },
         }
         last_message = dict(message, **to_vary_in_last_message)
@@ -1406,7 +1406,7 @@ def test_msg_generates_search_and_header_bar(self, mocker,
                                                  assert_search_bar):
         self.model.stream_dict = {
             205: {
-                'color': '#bfd56f',
+                'color': '#bd6',
             },
         }
         self.model.narrow = msg_narrow
@@ -1735,48 +1735,6 @@ def test_text_content(self, mocker,
         assert len(text[0]) == len(expected_text) == (width - 1)
         assert text[0] == expected_text
 
-    @pytest.mark.parametrize('color', [
-        '#ffffff', '#f0f0f0', '#f0f1f2', '#fff'
-    ])
-    def test_color_formats(self, mocker, color):
-        mocker.patch(STREAMBUTTON + ".mark_muted")
-        controller = mocker.Mock()
-        controller.model.muted_streams = {}
-        properties = ["", 1, color, False]  # only color is important
-        view_mock = mocker.Mock()
-        background = (None, 'white', 'black')
-        view_mock.palette = [background]
-
-        stream_button = StreamButton(properties,
-                                     controller=controller,
-                                     view=view_mock,
-                                     width=10,
-                                     count=5)
-
-        expected_palette = ([background] +
-                            [('#fff', '', '', '', '#fff, bold', 'black')] +
-                            [('s#fff', '', '', '', 'black', '#fff')])
-        assert view_mock.palette == expected_palette
-
-    @pytest.mark.parametrize('color', [
-        '#', '#f', '#ff', '#ffff', '#fffff', '#fffffff'
-    ])
-    def test_invalid_color_format(self, mocker, color):
-        properties = ["", 1, color, False]  # only color is important
-        view_mock = mocker.Mock()
-        controller = mocker.Mock()
-        controller.model.muted_streams = {}
-        background = (None, 'white', 'black')
-        view_mock.palette = [background]
-
-        with pytest.raises(RuntimeError) as e:
-            StreamButton(properties,
-                         controller=controller,
-                         view=view_mock,
-                         width=10,
-                         count=5)
-        assert str(e.value) == "Unknown color format: '{}'".format(color)
-
     @pytest.mark.parametrize('stream_id, muted_streams, called_value,\
                              is_action_muting, updated_all_msgs', [
         (86, {}, 204, False, 380),

zulipterminal/model.py

@@ -16,6 +16,7 @@
 from zulipterminal.helper import (
     asynch,
     classify_unread_counts,
+    canonicalize_color,
     index_messages,
     set_count,
     initial_index,
@@ -523,6 +524,12 @@ def _stream_info_from_subscriptions(
             subscriptions: List[Dict[str, Any]]
     ) -> Tuple[Dict[int, Any], Set[int], List[List[str]], List[List[str]]]:
         stream_keys = ('name', 'stream_id', 'color', 'invite_only')
+
+        # Canonicalize color formats, since zulip server versions may use
+        # different formats
+        for subscription in subscriptions:
+            subscription['color'] = canonicalize_color(subscription['color'])
+
         # Mapping of stream-id to all available stream info
         # Stream IDs for muted streams
         # Limited stream info sorted by name (used in display)

zulipterminal/ui_tools/boxes.py

@@ -218,7 +218,7 @@ def _is_private_message_to_self(self) -> bool:
 
     def stream_header(self) -> Any:
         bar_color = self.model.stream_dict[self.stream_id]['color']
-        bar_color = 's' + bar_color[:2] + bar_color[3] + bar_color[5]
+        bar_color = 's' + bar_color
         stream_title_markup = ('bar', [
             (bar_color, '{} >'.format(self.stream_name)),
             ('title', ' {} '.format(self.topic_name))
@@ -257,7 +257,7 @@ def top_search_bar(self) -> Any:
             text_to_fill = 'Starred messages'
         elif self.message['type'] == 'stream':
             bar_color = self.model.stream_dict[self.stream_id]['color']
-            bar_color = 's' + bar_color[:2] + bar_color[3] + bar_color[5]
+            bar_color = 's' + bar_color
             if len(curr_narrow) == 2 and curr_narrow[1][0] == 'topic':
                 text_to_fill = ('bar', [  # type: ignore
                     (bar_color, '{}'.format(self.stream_name)),

zulipterminal/ui_tools/buttons.py

@@ -126,20 +126,11 @@ def __init__(self, properties: List[Any],
                  count: int=0) -> None:
         # FIXME Is having self.stream_id the best way to do this?
         # (self.stream_id is used elsewhere)
-        self.stream_name, self.stream_id, orig_color, is_private = properties
+        self.stream_name, self.stream_id, color, is_private = properties
         self.model = controller.model
         self.count = count
         self.view = view
 
-        # Simplify the color from the original version & add to palette
-        # TODO Should this occur elsewhere and more intelligently?
-        if len(orig_color) == 7:  # modern zulip server format
-            color = ''.join(orig_color[i] for i in (0, 1, 3, 5))
-        elif len(orig_color) == 4:  # possible with zulip servers <=1.9.0 ?
-            color = orig_color
-        else:
-            raise RuntimeError("Unknown color format: '{}'".format(orig_color))
-
         for entry in view.palette:
             if entry[0] is None:
                 background = entry[5] if len(entry) > 4 else entry[2]