Repository: zulip/zulip-terminal (python)

Issue:
Support server-customized presence parameters
This is a fairly small migration in Zulip 7.0, server feature level 164 (https://zulip.com/api/changelog).

Quoting the changelog entry:
> [POST /register](https://zulip.com/api/register-queue): Added the server_presence_ping_interval_seconds and server_presence_offline_threshold_seconds fields for clients to use when implementing the [presence](https://zulip.com/api/get-presence) system.

Instead of hard-coded constants, we should therefore use these values provided by the server, falling back to the original constants when the server version is lower (when the keys in the response to register-queue are empty).

If tests are added or updated, note that they should likely be parametrized over different feature levels - see other tests which do so for comparisons.

Pull request description:
Support server-customized presence parameters.

<!-- See README for documentation, or ask in #zulip-terminal if unclear -->
### What does this PR do, and why?

Instead of hard-coded constants, uses server provided values, falling back to the original constants when server version is lower.

### External discussion & connections
<!-- [x] all that apply, specifying topic and adding numbers after # for issues/PRs -->
- [x] Discussed in **#zulip-terminal** in `Support server-customized presence parameters #T1421 #T1463`
- [x] Fully fixes #1421 
- [ ] Partially fixes issue #
- [ ] Builds upon previous unmerged work in PR #
- [ ] Is a follow-up to work in PR #
- [ ] Requires merge of PR #
- [ ] Merge will enable work on #

### How did you test this?
<!-- [x] all that apply -->
- [ ] Manually - Behavioral changes
- [ ] Manually - Visual changes
- [x] Adapting existing automated tests
- [x] Adding automated tests for new behavior (or missing tests)
- [ ] Existing automated tests should already cover this (*only a refactor of tested code*)

### Self-review checklist for each commit
- [x] It is a [minimal coherent idea](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development)
- [x] It has a commit summary following the [documented style](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development) (title & body)
- [x] It has a commit summary describing the  motivation and reasoning for the change
- [x] It individually passes linting and tests
- [x] It contains test additions for any new behavior
- [x] It flows clearly from a previous branch commit, and/or prepares for the next commit

Patch:
diff --git a/zulipterminal/api_types.py b/zulipterminal/api_types.py
index 119770e..b341993 100644
--- a/zulipterminal/api_types.py
+++ b/zulipterminal/api_types.py
@@ -34,6 +34,14 @@ MAX_TOPIC_NAME_LENGTH: Final = 60
 MAX_MESSAGE_LENGTH: Final = 10000
 
 
+###############################################################################
+# These values are in the register response from ZFL 164
+# Before this feature level, they had the listed default (fixed) values
+
+PRESENCE_OFFLINE_THRESHOLD_SECS: Final = 140
+PRESENCE_PING_INTERVAL_SECS: Final = 60
+
+
 ###############################################################################
 # Core message types (used in Composition and Message below)
 
diff --git a/zulipterminal/model.py b/zulipterminal/model.py
index af0b846..72215c8 100644
--- a/zulipterminal/model.py
+++ b/zulipterminal/model.py
@@ -34,6 +34,8 @@ from zulipterminal.api_types import (
     MAX_MESSAGE_LENGTH,
     MAX_STREAM_NAME_LENGTH,
     MAX_TOPIC_NAME_LENGTH,
+    PRESENCE_OFFLINE_THRESHOLD_SECS,
+    PRESENCE_PING_INTERVAL_SECS,
     TYPING_STARTED_EXPIRY_PERIOD,
     TYPING_STARTED_WAIT_PERIOD,
     TYPING_STOPPED_WAIT_PERIOD,
@@ -81,9 +83,6 @@ from zulipterminal.platform_code import notify
 from zulipterminal.ui_tools.utils import create_msg_box_list
 
 
-OFFLINE_THRESHOLD_SECS = 140
-
-
 class ServerConnectionFailure(Exception):
     pass
 
@@ -172,6 +171,8 @@ class Model:
         self.server_version = self.initial_data["zulip_version"]
         self.server_feature_level: int = self.initial_data.get("zulip_feature_level", 0)
 
+        self._store_server_presence_intervals()
+
         self.user_dict: Dict[str, MinimalUserData] = {}
         self.user_id_email_dict: Dict[int, str] = {}
         self._all_users_by_id: Dict[int, RealmUser] = {}
@@ -445,7 +446,7 @@ class Model:
                     view = self.controller.view
                     view.users_view.update_user_list(user_list=self.users)
                     view.middle_column.update_message_list_status_markers()
-            time.sleep(60)
+            time.sleep(self.server_presence_ping_interval_secs)
 
     @asynch
     def toggle_message_reaction(
@@ -814,6 +815,18 @@ class Model:
             TYPING_STARTED_EXPIRY_PERIOD,
         )
 
+    def _store_server_presence_intervals(self) -> None:
+        """
+        In ZFL 164, these values were added to the register response.
+        Uses default values if not received.
+        """
+        self.server_presence_offline_threshold_secs = self.initial_data.get(
+            "server_presence_offline_threshold_seconds", PRESENCE_OFFLINE_THRESHOLD_SECS
+        )
+        self.server_presence_ping_interval_secs = self.initial_data.get(
+            "server_presence_ping_interval_seconds", PRESENCE_PING_INTERVAL_SECS
+        )
+
     @staticmethod
     def modernize_message_response(message: Message) -> Message:
         """
@@ -1202,7 +1215,7 @@ class Model:
                 *
                 * Out of the ClientPresence objects found in `presence`, we
                 * consider only those with a timestamp newer than
-                * OFFLINE_THRESHOLD_SECS; then of
+                * self.server_presence_offline_threshold_secs; then of
                 * those, return the one that has the greatest UserStatus, where
                 * `active` > `idle` > `offline`.
                 *
@@ -1216,7 +1229,9 @@ class Model:
                     timestamp = client[1]["timestamp"]
                     if client_name == "aggregated":
                         continue
-                    elif (time.time() - timestamp) < OFFLINE_THRESHOLD_SECS:
+                    elif (
+                        time.time() - timestamp
+                    ) < self.server_presence_offline_threshold_secs:
                         if status == "active":
                             aggregate_status = "active"
                         if status == "idle" and aggregate_status != "active":


Test patch:
diff --git a/tests/model/test_model.py b/tests/model/test_model.py
index 83ce9de..6b5a18e 100644
--- a/tests/model/test_model.py
+++ b/tests/model/test_model.py
@@ -14,6 +14,8 @@ from zulipterminal.model import (
     MAX_MESSAGE_LENGTH,
     MAX_STREAM_NAME_LENGTH,
     MAX_TOPIC_NAME_LENGTH,
+    PRESENCE_OFFLINE_THRESHOLD_SECS,
+    PRESENCE_PING_INTERVAL_SECS,
     TYPING_STARTED_EXPIRY_PERIOD,
     TYPING_STARTED_WAIT_PERIOD,
     TYPING_STOPPED_WAIT_PERIOD,
@@ -1440,6 +1442,60 @@ class TestModel:
         assert model.typing_stopped_wait_period == typing_stopped_wait
         assert model.typing_started_expiry_period == typing_started_expiry
 
+    @pytest.mark.parametrize(
+        "feature_level, to_vary_in_initial_data, "
+        "expected_offline_threshold, expected_presence_ping_interval",
+        [
+            (0, {}, PRESENCE_OFFLINE_THRESHOLD_SECS, PRESENCE_PING_INTERVAL_SECS),
+            (157, {}, PRESENCE_OFFLINE_THRESHOLD_SECS, PRESENCE_PING_INTERVAL_SECS),
+            (
+                164,
+                {
+                    "server_presence_offline_threshold_seconds": 200,
+                    "server_presence_ping_interval_seconds": 100,
+                },
+                200,
+                100,
+            ),
+        ],
+        ids=[
+            "Zulip_2.1_ZFL_0_hard_coded",
+            "Zulip_6.2_ZFL_157_hard_coded",
+            "Zulip_7.0_ZFL_164_server_provided",
+        ],
+    )
+    def test__store_server_presence_intervals(
+        self,
+        model,
+        initial_data,
+        feature_level,
+        to_vary_in_initial_data,
+        expected_offline_threshold,
+        expected_presence_ping_interval,
+    ):
+        # Ensure inputs are not the defaults, to avoid the test accidentally passing
+        assert (
+            to_vary_in_initial_data.get("server_presence_offline_threshold_seconds")
+            != PRESENCE_OFFLINE_THRESHOLD_SECS
+        )
+        assert (
+            to_vary_in_initial_data.get("server_presence_ping_interval_seconds")
+            != PRESENCE_PING_INTERVAL_SECS
+        )
+
+        initial_data.update(to_vary_in_initial_data)
+        model.initial_data = initial_data
+        model.server_feature_level = feature_level
+
+        model._store_server_presence_intervals()
+
+        assert (
+            model.server_presence_offline_threshold_secs == expected_offline_threshold
+        )
+        assert (
+            model.server_presence_ping_interval_secs == expected_presence_ping_interval
+        )
+
     def test_get_message_false_first_anchor(
         self,
         mocker,