refactor: model/api_types/views/version: Simplify feature-level typing.

When the server originally did not provide `zulip_feature_level`, this
was previously represented by a `None` value in
`model.server_feature_level`.

This replaces the current potential `None` value with a zero value,
allowing a pure integer representation in the model. This enables
simplification of conditional statements when handling different server
versions.

The feature-level is represented by various strings in the repository,
including:
- server_feature_level
- zulip_feature_level
- ZFL
- feature_level

Tests updated, including test ids.
Additional test case added for realm retention.

Fixes #1364.

Co-authored-by: Lucas.C.B <[email protected]>

Author: 2 people

tests/conftest.py

@@ -559,7 +559,7 @@ def messages_successful_response(
     params=SUPPORTED_SERVER_VERSIONS,
     ids=(lambda param: "server_version:{}-server_feature_level:{}".format(*param)),
 )
-def zulip_version(request: Any) -> Tuple[str, Optional[int]]:
+def zulip_version(request: Any) -> Tuple[str, int]:
     """
     Fixture to test different components based on the server version and the
     feature level.
@@ -1449,7 +1449,7 @@ def stream_dict(streams_fixture: List[Dict[str, Any]]) -> Dict[int, Any]:
         },
     ],
     ids=[
-        "zulip_feature_level:None",
+        "zulip_feature_level:0",
         "zulip_feature_level:1",
     ],
 )

tests/model/test_model.py

@@ -81,7 +81,7 @@ def test_init(
         )
         assert model.initial_data == initial_data
         assert model.server_version == initial_data["zulip_version"]
-        assert model.server_feature_level == initial_data.get("zulip_feature_level")
+        assert model.server_feature_level == initial_data.get("zulip_feature_level", 0)
         assert model.user_id == user_profile["user_id"]
         assert model.user_full_name == user_profile["full_name"]
         assert model.user_email == user_profile["email"]
@@ -157,7 +157,7 @@ def test_user_settings_expected_contents(self, model):
             (
                 [["Stream 1", "muted stream muted topic"]],
                 {("Stream 1", "muted stream muted topic"): None},
-                None,
+                0,
             ),
             (
                 [["Stream 2", "muted topic", 1530129122]],
@@ -166,7 +166,7 @@ def test_user_settings_expected_contents(self, model):
             ),
         ],
         ids=[
-            "zulip_feature_level:None",
+            "zulip_feature_level:0",
             "zulip_feature_level:1",
         ],
     )
@@ -276,12 +276,19 @@ def test_register_initial_desired_events(self, mocker, initial_data):
             "expect_msg_retention_text",
         ],
         [
+            case(
+                {1: {}},
+                None,
+                0,
+                {1: "Indefinite [Organization default]"},
+                id="ZFL=0_no_stream_retention_realm_retention=None",
+            ),
             case(
                 {1: {}},
                 None,
                 10,
                 {1: "Indefinite [Organization default]"},
-                id="ZFL=None_no_stream_retention_realm_retention=None",
+                id="ZFL=10_no_stream_retention_realm_retention=None",
             ),
             case(
                 {1: {}, 2: {}},
@@ -973,7 +980,7 @@ def test_update_stream_message(
     @pytest.mark.parametrize(
         "ZFL, expect_API_notify_args",
         [
-            (None, False),
+            (0, False),
             (8, False),
             (9, True),
             (152, True),
@@ -1351,7 +1358,7 @@ def test_modernize_message_response(
     @pytest.mark.parametrize(
         "feature_level, to_vary_in_initial_data",
         [
-            (None, {}),
+            (0, {}),
             (27, {}),
             (52, {}),
             (
@@ -1364,7 +1371,7 @@ def test_modernize_message_response(
             ),
         ],
         ids=[
-            "Zulip_2.1.x_ZFL_None_no_restrictions",
+            "Zulip_2.1.x_ZFL_0_no_restrictions",
             "Zulip_3.1.x_ZFL_27_no_restrictions",
             "Zulip_4.0.x_ZFL_52_no_restrictions",
             "Zulip_4.0.x_ZFL_53_with_restrictions",
@@ -2777,7 +2784,7 @@ def test__handle_reaction_event_for_msg_in_index(
         params=[
             ("op", 32),  # At server feature level 32, event uses standard field
             ("operation", 31),
-            ("operation", None),
+            ("operation", 0),
         ]
     )
     def update_message_flags_operation(self, request):
@@ -3297,11 +3304,11 @@ def test__handle_typing_event(
             ),
         ],
         ids=[
-            "remove_18_in_home_view:already_unmuted:ZFLNone",
-            "remove_19_in_home_view:muted:ZFLNone",
-            "add_19_in_home_view:already_muted:ZFLNone",
-            "add_30_in_home_view:unmuted:ZFLNone",
-            "remove_30_is_muted:already_unmutedZFL139",
+            "remove_18_in_home_view:already_unmuted:ZFL0",
+            "remove_19_in_home_view:muted:ZFL0",
+            "add_19_in_home_view:already_muted:ZFL0",
+            "add_30_in_home_view:unmuted:ZFL0",
+            "remove_30_is_muted:already_unmuted:ZFL139",
             "remove_19_is_muted:muted:ZFL139",
             "add_15_is_muted:already_muted:ZFL139",
             "add_30_is_muted:unmuted:ZFL139",
@@ -3469,7 +3476,7 @@ def test__handle_subscription_event_visual_notifications(
         [
             (
                 {"op": "peer_add", "stream_id": 99, "user_id": 12},
-                None,
+                0,
                 99,
                 [1001, 11, 12],
             ),
@@ -3491,7 +3498,7 @@ def test__handle_subscription_event_visual_notifications(
                 99,
                 [1001, 11, 12],
             ),
-            ({"op": "peer_remove", "stream_id": 2, "user_id": 12}, None, 2, [1001, 11]),
+            ({"op": "peer_remove", "stream_id": 2, "user_id": 12}, 0, 2, [1001, 11]),
             ({"op": "peer_remove", "stream_id": 2, "user_id": 12}, 34, 2, [1001, 11]),
             (
                 {"op": "peer_remove", "stream_ids": [2], "user_ids": [12]},
@@ -3507,11 +3514,11 @@ def test__handle_subscription_event_visual_notifications(
             ),
         ],
         ids=[
-            "user_subscribed_to_stream:ZFLNone",
+            "user_subscribed_to_stream:ZFL0",
             "user_subscribed_to_stream:ZFL34",
             "user_subscribed_to_stream:ZFL34_should_be_35",
             "user_subscribed_to_stream:ZFL35",
-            "user_unsubscribed_from_stream:ZFLNone",
+            "user_unsubscribed_from_stream:ZFL0",
             "user_unsubscribed_from_stream:ZFL34",
             "user_unsubscribed_from_stream:ZFL34_should_be_35",
             "user_unsubscribed_from_stream:ZFL35",

tests/ui_tools/test_popups.py

@@ -222,7 +222,7 @@ def test_keypress_exit_popup_invalid_key(
         assert not self.controller.exit_popup.called
 
     def test_feature_level_content(
-        self, mocker: MockerFixture, zulip_version: Tuple[str, Optional[int]]
+        self, mocker: MockerFixture, zulip_version: Tuple[str, int]
     ) -> None:
         self.controller = mocker.Mock()
         mocker.patch.object(
@@ -1282,16 +1282,16 @@ def test_keypress_stream_members(
             case(
                 {"date_created": None, "is_announcement_only": True},
                 "74 [Organization default]",
-                None,
+                0,
                 17,
-                id="ZFL=None_no_date_created__no_retention_days__admins_only",
+                id="ZFL=0_no_date_created__no_retention_days__admins_only",
             ),
             case(
                 {"date_created": None, "is_announcement_only": False},
                 "74 [Organization default]",
-                None,
+                0,
                 16,
-                id="ZFL=None_no_date_created__no_retention_days__anyone_can_type",
+                id="ZFL=0_no_date_created__no_retention_days__anyone_can_type",
             ),
             case(
                 {"date_created": None, "stream_post_policy": 1},
@@ -1363,7 +1363,7 @@ def test_popup_height(
         general_stream: Dict[str, Any],
         to_vary_in_stream_data: Dict[str, Optional[int]],
         cached_message_retention_text: str,
-        server_feature_level: Optional[int],
+        server_feature_level: int,
         expected_height: int,
     ) -> None:
         model = self.controller.model

zulipterminal/api_types.py

@@ -655,7 +655,6 @@ class ServerSettings(TypedDict):
     # Added in Zulip 2.1.0
     external_authentication_methods: List[ExternalAuthenticationMethod]
 
-    # TODO Refactor ZFL to default to zero
     zulip_feature_level: NotRequired[int]  # New in Zulip 3.0, ZFL 1
     zulip_version: str
     zulip_merge_base: NotRequired[str]  # New in Zulip 5.0, ZFL 88

zulipterminal/model.py

@@ -170,7 +170,7 @@ def __init__(self, controller: Any) -> None:
         self._fetch_initial_data()
 
         self.server_version = self.initial_data["zulip_version"]
-        self.server_feature_level = self.initial_data.get("zulip_feature_level")
+        self.server_feature_level: int = self.initial_data.get("zulip_feature_level", 0)
 
         self.user_dict: Dict[str, MinimalUserData] = {}
         self.user_id_email_dict: Dict[int, str] = {}
@@ -189,8 +189,8 @@ def __init__(self, controller: Any) -> None:
 
         # NOTE: The date_created field of stream has been added in feature
         # level 30, server version 4. For consistency we add this field
-        # on server iterations even before this with value of None.
-        if self.server_feature_level is None or self.server_feature_level < 30:
+        # on earlier server iterations with the value of None.
+        if self.server_feature_level < 30:
             for stream in self.stream_dict.values():
                 stream["date_created"] = None
 
@@ -203,7 +203,7 @@ def __init__(self, controller: Any) -> None:
         assert set(map(len, muted_topics)) in (set(), {2}, {3})
         self._muted_topics: Dict[Tuple[str, str], Optional[int]] = {
             (stream_name, topic): (
-                None if self.server_feature_level is None else date_muted[0]
+                None if self.server_feature_level == 0 else date_muted[0]
             )
             for stream_name, topic, *date_muted in muted_topics
         }
@@ -259,7 +259,7 @@ def normalize_and_cache_message_retention_text(self) -> None:
         # sream_id in model.cached_retention_text. This will be displayed in the UI.
         self.cached_retention_text: Dict[int, str] = {}
         realm_message_retention_days = self.initial_data["realm_message_retention_days"]
-        if self.server_feature_level is None or self.server_feature_level < 17:
+        if self.server_feature_level < 17:
             for stream in self.stream_dict.values():
                 stream["message_retention_days"] = None
 
@@ -590,7 +590,7 @@ def update_stream_message(
         if content is not None:
             request["content"] = content
 
-        if self.server_feature_level is not None and self.server_feature_level >= 9:
+        if self.server_feature_level >= 9:
             request["send_notification_to_old_thread"] = notify_old
             request["send_notification_to_new_thread"] = notify_new
 
@@ -1822,7 +1822,7 @@ def _handle_update_message_flags_event(self, event: Event) -> None:
         Handle change to message flags (eg. starred, read)
         """
         assert event["type"] == "update_message_flags"
-        if self.server_feature_level is None or self.server_feature_level < 32:
+        if self.server_feature_level < 32:
             operation = event["operation"]
         else:
             operation = event["op"]

zulipterminal/ui_tools/views.py

@@ -1083,7 +1083,7 @@ def __init__(
         *,
         zt_version: str,
         server_version: str,
-        server_feature_level: Optional[int],
+        server_feature_level: int,
         theme_name: str,
         color_depth: int,
         autohide_enabled: bool,

zulipterminal/version.py

@@ -5,7 +5,7 @@
 ZT_VERSION = "0.7.0+git"
 
 SUPPORTED_SERVER_VERSIONS = [
-    ("2.1", None),
+    ("2.1", 0),
     ("3.0", 25),
 ]