refactor: Clean up args for fetch_initial_state_data.

We now require explicit keywords for all arguments
to fetch_initial_state_data except user_profile.

We provide reasonable defaults to keep the test
code concise.

zerver/lib/events.py

@@ -83,11 +83,12 @@ def always_want(msg_type: str) -> bool:
 
 def fetch_initial_state_data(
     user_profile: Optional[UserProfile],
-    event_types: Optional[Iterable[str]],
+    *,
-    queue_id: Optional[str],
+    realm: Optional[Realm] = None,
-    client_gravatar: bool,
+    event_types: Optional[Iterable[str]] = None,
-    user_avatar_url_field_optional: bool,
+    queue_id: Optional[str] = "",
-    realm: Realm,
+    client_gravatar: bool = False,
+    user_avatar_url_field_optional: bool = False,
     slim_presence: bool = False,
     include_subscribers: bool = True,
     include_streams: bool = True,
@@ -103,6 +104,10 @@ def fetch_initial_state_data(
     corresponding events for changes in the data structures and new
     code to apply_events (and add a test in test_events.py).
     """
+    if realm is None:
+        assert user_profile is not None
+        realm = user_profile.realm
+
     state: Dict[str, Any] = {'queue_id': queue_id}
 
     if event_types is None:
@@ -965,11 +970,10 @@ def do_events_register(
 
     ret = fetch_initial_state_data(
         user_profile,
-        event_types_set,
+        event_types=event_types_set,
-        queue_id,
+        queue_id=queue_id,
         client_gravatar=client_gravatar,
         user_avatar_url_field_optional=user_avatar_url_field_optional,
-        realm=user_profile.realm,
         slim_presence=slim_presence,
         include_subscribers=include_subscribers,
         include_streams=include_streams,

zerver/lib/home.py

@@ -141,15 +141,17 @@ def build_page_params_for_home_page_load(
         # at the time of request and don't register for any events.
         # TODO: Implement events for web_public_visitor.
         from zerver.lib.events import fetch_initial_state_data, post_process_state
-        register_ret = fetch_initial_state_data(user_profile,
+        register_ret = fetch_initial_state_data(
+            user_profile,
+            realm=realm,
             event_types=None,
             queue_id=None,
             client_gravatar=False,
             user_avatar_url_field_optional=client_capabilities['user_avatar_url_field_optional'],
-                                                realm=realm,
             slim_presence=False,
             include_subscribers=False,
-                                                include_streams=False)
+            include_streams=False
+        )
 
         post_process_state(user_profile, register_ret, False)
 

zerver/tests/test_event_system.py

@@ -331,7 +331,7 @@ class FetchInitialStateDataTest(ZulipTestCase):
     def test_realm_bots_non_admin(self) -> None:
         user_profile = self.example_user('cordelia')
         self.assertFalse(user_profile.is_realm_admin)
-        result = fetch_initial_state_data(user_profile, None, "", client_gravatar=False, user_avatar_url_field_optional=False, realm=user_profile.realm)
+        result = fetch_initial_state_data(user_profile)
         self.assert_length(result['realm_bots'], 0)
 
         # additionally the API key for a random bot is not present in the data
@@ -343,14 +343,14 @@ class FetchInitialStateDataTest(ZulipTestCase):
         user_profile = self.example_user('hamlet')
         do_change_user_role(user_profile, UserProfile.ROLE_REALM_ADMINISTRATOR)
         self.assertTrue(user_profile.is_realm_admin)
-        result = fetch_initial_state_data(user_profile, None, "", client_gravatar=False, user_avatar_url_field_optional=False, realm=user_profile.realm)
+        result = fetch_initial_state_data(user_profile)
         self.assertTrue(len(result['realm_bots']) > 2)
 
     def test_max_message_id_with_no_history(self) -> None:
         user_profile = self.example_user('aaron')
         # Delete all historical messages for this user
         UserMessage.objects.filter(user_profile=user_profile).delete()
-        result = fetch_initial_state_data(user_profile, None, "", client_gravatar=False, user_avatar_url_field_optional=False, realm=user_profile.realm)
+        result = fetch_initial_state_data(user_profile)
         self.assertEqual(result['max_message_id'], -1)
 
     def test_delivery_email_presence_for_non_admins(self) -> None:
@@ -359,13 +359,15 @@ class FetchInitialStateDataTest(ZulipTestCase):
 
         do_set_realm_property(user_profile.realm, "email_address_visibility",
                               Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE)
-        result = fetch_initial_state_data(user_profile, None, "", client_gravatar=False, user_avatar_url_field_optional=False, realm=user_profile.realm)
+        result = fetch_initial_state_data(user_profile)
+
         for key, value in result['raw_users'].items():
             self.assertNotIn('delivery_email', value)
 
         do_set_realm_property(user_profile.realm, "email_address_visibility",
                               Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS)
-        result = fetch_initial_state_data(user_profile, None, "", client_gravatar=False, user_avatar_url_field_optional=False, realm=user_profile.realm)
+        result = fetch_initial_state_data(user_profile)
+
         for key, value in result['raw_users'].items():
             self.assertNotIn('delivery_email', value)
 
@@ -375,13 +377,13 @@ class FetchInitialStateDataTest(ZulipTestCase):
 
         do_set_realm_property(user_profile.realm, "email_address_visibility",
                               Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE)
-        result = fetch_initial_state_data(user_profile, None, "", client_gravatar=False, user_avatar_url_field_optional=False, realm=user_profile.realm)
+        result = fetch_initial_state_data(user_profile)
         for key, value in result['raw_users'].items():
             self.assertNotIn('delivery_email', value)
 
         do_set_realm_property(user_profile.realm, "email_address_visibility",
                               Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS)
-        result = fetch_initial_state_data(user_profile, None, "", client_gravatar=False, user_avatar_url_field_optional=False, realm=user_profile.realm)
+        result = fetch_initial_state_data(user_profile)
         for key, value in result['raw_users'].items():
             self.assertIn('delivery_email', value)
 
@@ -400,12 +402,10 @@ class FetchInitialStateDataTest(ZulipTestCase):
 
         long_term_idle_users_ids = [user.id for user in users]
 
-        result = fetch_initial_state_data(user_profile=hamlet,
+        result = fetch_initial_state_data(
-                                          event_types=None,
+            user_profile=hamlet,
-                                          queue_id='',
-                                          client_gravatar=False,
             user_avatar_url_field_optional=True,
-                                          realm=hamlet.realm)
+        )
 
         raw_users = result['raw_users']
 
@@ -419,12 +419,11 @@ class FetchInitialStateDataTest(ZulipTestCase):
                              if 'avatar_url' in user_dict and 'gravatar.com' in user_dict['avatar_url']]
 
         # Test again with client_gravatar = True
-        result = fetch_initial_state_data(user_profile=hamlet,
+        result = fetch_initial_state_data(
-                                          event_types=None,
+            user_profile=hamlet,
-                                          queue_id='',
             client_gravatar=True,
             user_avatar_url_field_optional=True,
-                                          realm=hamlet.realm)
+        )
 
         raw_users = result['raw_users']
 
@@ -725,14 +724,7 @@ class FetchQueriesTest(ZulipTestCase):
         flush_per_request_caches()
         with queries_captured() as queries:
             with mock.patch('zerver.lib.events.always_want') as want_mock:
-                fetch_initial_state_data(
+                fetch_initial_state_data(user)
-                    user_profile=user,
-                    event_types=None,
-                    queue_id='x',
-                    client_gravatar=False,
-                    user_avatar_url_field_optional=False,
-                    realm=user.realm,
-                )
 
         self.assert_length(queries, 29)
 
@@ -782,14 +774,7 @@ class FetchQueriesTest(ZulipTestCase):
                 else:
                     event_types = [event_type]
 
-                fetch_initial_state_data(
+                fetch_initial_state_data(user, event_types=event_types)
-                    user_profile=user,
-                    event_types=event_types,
-                    queue_id='x',
-                    client_gravatar=False,
-                    user_avatar_url_field_optional=False,
-                    realm=user.realm,
-                )
             self.assert_length(queries, count)
 
 

zerver/tests/test_events.py

@@ -237,13 +237,13 @@ class BaseAction(ZulipTestCase):
         # hybrid_state = initial fetch state + re-applying events triggered by our action
         # normal_state = do action then fetch at the end (the "normal" code path)
         hybrid_state = fetch_initial_state_data(
-            self.user_profile, event_types, "",
+            self.user_profile,
+            event_types=event_types,
             client_gravatar=client_gravatar,
             user_avatar_url_field_optional=user_avatar_url_field_optional,
             slim_presence=slim_presence,
             include_subscribers=include_subscribers,
             include_streams=include_streams,
-            realm=self.user_profile.realm,
         )
         action()
         events = client.event_queue.contents()
@@ -279,13 +279,13 @@ class BaseAction(ZulipTestCase):
                 raise AssertionError('Test is invalid--state actually does change here.')
 
         normal_state = fetch_initial_state_data(
-            self.user_profile, event_types, "",
+            self.user_profile,
+            event_types=event_types,
             client_gravatar=client_gravatar,
             user_avatar_url_field_optional=user_avatar_url_field_optional,
             slim_presence=slim_presence,
             include_subscribers=include_subscribers,
             include_streams=include_streams,
-            realm=self.user_profile.realm,
         )
         post_process_state(self.user_profile, normal_state, notification_settings_null)
         self.match_states(hybrid_state, normal_state, events)
@@ -1229,7 +1229,7 @@ class NormalActionsTest(BaseAction):
     def test_realm_update_plan_type(self) -> None:
         realm = self.user_profile.realm
 
-        state_data = fetch_initial_state_data(self.user_profile, None, "", False, False, self.user_profile.realm)
+        state_data = fetch_initial_state_data(self.user_profile)
         self.assertEqual(state_data['realm_plan_type'], Realm.SELF_HOSTED)
         self.assertEqual(state_data['zulip_plan_is_not_limited'], True)
 
@@ -1237,7 +1237,7 @@ class NormalActionsTest(BaseAction):
             lambda: do_change_plan_type(realm, Realm.LIMITED))
         check_realm_update('events[0]', events[0], 'plan_type')
 
-        state_data = fetch_initial_state_data(self.user_profile, None, "", False, False, self.user_profile.realm)
+        state_data = fetch_initial_state_data(self.user_profile)
         self.assertEqual(state_data['realm_plan_type'], Realm.LIMITED)
         self.assertEqual(state_data['zulip_plan_is_not_limited'], False)
 
@@ -1589,7 +1589,7 @@ class NormalActionsTest(BaseAction):
             lambda: do_delete_messages(self.user_profile.realm, [message]),
             state_change_expected=True,
         )
-        result = fetch_initial_state_data(user_profile, None, "", client_gravatar=False, user_avatar_url_field_optional=False, realm=self.user_profile.realm)
+        result = fetch_initial_state_data(user_profile)
         self.assertEqual(result['max_message_id'], -1)
 
     def test_add_attachment(self) -> None: