actions: Refactor do_update_outgoing_webhook_service.

This updates `do_update_outgoing_webhook_service` to use
`BotServicesOutgoing` as the schema for the updated service data we send
back to client because it's the schema `bot_data.update` expects.

The function is also refactored to allow updating specific fields of the
Service row instead of requiring all value for the Service fields to be
passed.

This is a prep commit for #34524, it adds another field to the Service
field.

zerver/actions/users.py

@@ -22,6 +22,7 @@ from zerver.lib.avatar import get_avatar_field
 from zerver.lib.bot_config import ConfigError, get_bot_config, get_bot_configs, set_bot_config
 from zerver.lib.cache import bot_dict_fields
 from zerver.lib.create_user import create_user
+from zerver.lib.event_types import BotServicesOutgoing
 from zerver.lib.invites import revoke_invites_generated_by_user
 from zerver.lib.remote_server import maybe_enqueue_audit_log_upload
 from zerver.lib.send_email import (
@@ -781,14 +782,41 @@ def do_change_can_change_user_emails(user_profile: UserProfile, value: bool) ->
 
 @transaction.atomic(durable=True)
 def do_update_outgoing_webhook_service(
-    bot_profile: UserProfile, service_interface: int, service_payload_url: str
+    bot_profile: UserProfile,
+    *,
+    interface: int | None = None,
+    base_url: str | None = None,
 ) -> None:
-    # TODO: First service is chosen because currently one bot can only have one service.
+    update_fields: dict[str, str | int] = {}
-    # Update this once multiple services are supported.
+    if interface is not None:
+        update_fields["interface"] = interface
+    if base_url is not None:
+        update_fields["base_url"] = base_url
+
+    if len(update_fields) < 1:
+        return
+
+    # TODO: First service is chosen because currently one bot can only
+    # have one service. Update this once multiple services are supported.
     service = get_bot_services(bot_profile.id)[0]
-    service.base_url = service_payload_url
+    updated_fields = []
-    service.interface = service_interface
+    for field, new_value in update_fields.items():
-    service.save()
+        if getattr(service, field) != new_value:
+            setattr(service, field, new_value)
+            updated_fields.append(field)
+
+    if len(updated_fields) < 1:
+        return
+
+    service.save(update_fields=updated_fields)
+
+    # Keep the event payload of the updated bot service in sync with the
+    # schema expected by `bot_data.update()` method.
+    updated_service: dict[str, str | int] = BotServicesOutgoing(
+        base_url=service.base_url,
+        interface=service.interface,
+        token=service.token,
+    ).model_dump()
     send_event_on_commit(
         bot_profile.realm,
         dict(
@@ -796,11 +824,7 @@ def do_update_outgoing_webhook_service(
             op="update",
             bot=dict(
                 user_id=bot_profile.id,
-                services=[
+                services=[updated_service],
-                    dict(
-                        base_url=service.base_url, interface=service.interface, token=service.token
-                    )
-                ],
             ),
         ),
         bot_owner_user_ids(bot_profile),

zerver/tests/test_events.py

@@ -297,6 +297,7 @@ from zerver.models import (
     UserStatus,
     UserTopic,
 )
+from zerver.models.bots import get_bot_services
 from zerver.models.clients import get_client
 from zerver.models.groups import SystemGroups
 from zerver.models.realm_audit_logs import AuditLogEventType
@@ -3517,9 +3518,33 @@ class NormalActionsTest(BaseAction):
             interface_type=Service.GENERIC,
         )
         with self.verify_action() as events:
-            do_update_outgoing_webhook_service(bot, 2, "http://hostname.domain2.com")
+            do_update_outgoing_webhook_service(
+                bot, interface=2, base_url="http://hostname.domain2.com"
+            )
+
         check_realm_bot_update("events[0]", events[0], "services")
 
+        # Check the updated Service data we send as event on commit.
+        bot_service = get_bot_services(bot.id)[0]
+        event_data_service = events[0]["bot"]["services"][0]
+        self.assertEqual(
+            {
+                "base_url": bot_service.base_url,
+                "interface": bot_service.interface,
+                "token": bot_service.token,
+            },
+            event_data_service,
+        )
+
+        with self.verify_action(num_events=0, state_change_expected=False) as events:
+            do_update_outgoing_webhook_service(bot)
+
+        # Trying to update with the same value as existing value results in no op.
+        with self.verify_action(num_events=0, state_change_expected=False) as events:
+            do_update_outgoing_webhook_service(
+                bot, interface=2, base_url="http://hostname.domain2.com"
+            )
+
     def test_do_deactivate_bot(self) -> None:
         bot = self.create_bot("test")
         with self.verify_action(num_events=2) as events:

zerver/views/users.py

@@ -499,7 +499,11 @@ def patch_bot_backend(
     if service_payload_url is not None:
         check_valid_interface_type(service_interface)
         assert service_interface is not None
-        do_update_outgoing_webhook_service(bot, service_interface, service_payload_url)
+        do_update_outgoing_webhook_service(
+            bot,
+            interface=service_interface,
+            base_url=service_payload_url,
+        )
 
     if config_data is not None:
         do_update_bot_config_data(bot, config_data)