Address review feedback on webhook update

- Make status_change_reason a getter-only property (server-set field, silent no-op on assignment)
- Add CHANGELOG entry for Webhooks.update() and new fields
- Align _parse_common_tags with UserItem's selective-None merge pattern
- Fix stale _parse_element return-type annotation
- Add -> None to test_request_factory for consistency
- Document which fields Webhooks.update() serializes
- Match create_req's type-validation in update_req

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: jacalata

CHANGELOG.md

@@ -5,6 +5,10 @@
   hierarchy path (e.g. `"Marketing/Q1 Reports"`). The walk is performed level by
   level using the REST API name filter, so a path with *n* components issues *n*
   requests. Returns the matching `ProjectItem` or `None` if no project is found.
+* Added `Webhooks.update()` to modify an existing webhook's name, url, event, or
+  enabled state. Exposed new `WebhookItem` fields: `is_enabled`,
+  `status_change_reason` (read-only, server-set), and `event_tag` (raw wire form
+  of the event tag).
 
 ## 0.18.0 (6 April 2022)    
 * Switched to using defused_xml for xml attack protection

tableauserverclient/models/webhook_item.py

@@ -59,7 +59,7 @@ def __init__(self):
         self._event: str | None = None
         self.owner_id: str | None = None
         self.is_enabled: bool | None = None
-        self.status_change_reason: str | None = None
+        self._status_change_reason: str | None = None
 
     def _set_values(self, id, name, url, event, owner_id, is_enabled=None, status_change_reason=None):
         if id is not None:
@@ -75,12 +75,17 @@ def _set_values(self, id, name, url, event, owner_id, is_enabled=None, status_ch
         if is_enabled is not None:
             self.is_enabled = is_enabled
         if status_change_reason is not None:
-            self.status_change_reason = status_change_reason
+            self._status_change_reason = status_change_reason
 
     @property
     def id(self) -> str | None:
         return self._id
 
+    @property
+    def status_change_reason(self) -> str | None:
+        """Server-set; assignment has no effect on the wire."""
+        return self._status_change_reason
+
     @property
     def event(self) -> str | None:
         if self._event:
@@ -129,12 +134,22 @@ def _parse_common_tags(self, webhook_xml, ns) -> "WebhookItem":
             parsed = fromstring(webhook_xml)
             webhook_xml = parsed.find(".//t:webhook", namespaces=ns)
         if webhook_xml is not None:
-            values = self._parse_element(webhook_xml, ns)
-            self._set_values(*values)
+            (
+                _,
+                name,
+                url,
+                event,
+                _,
+                is_enabled,
+                status_change_reason,
+            ) = self._parse_element(webhook_xml, ns)
+            self._set_values(None, name, url, event, None, is_enabled, status_change_reason)
         return self
 
     @staticmethod
-    def _parse_element(webhook_xml: ET.Element, ns) -> tuple:
+    def _parse_element(
+        webhook_xml: ET.Element, ns
+    ) -> tuple[str | None, str | None, str | None, str | None, str | None, bool | None, str | None]:
         id = webhook_xml.get("id", None)
         name = webhook_xml.get("name", None)
 
@@ -143,9 +158,10 @@ def _parse_element(webhook_xml: ET.Element, ns) -> tuple:
         if url_tag is not None:
             url = url_tag.get("url", None)
 
-        event = webhook_xml.findall(".//t:webhook-source/*", namespaces=ns)
-        if event is not None and len(event) > 0:
-            event = _parse_event(event)
+        event: str | None = None
+        event_elements = webhook_xml.findall(".//t:webhook-source/*", namespaces=ns)
+        if event_elements:
+            event = _parse_event(event_elements)
 
         owner_id = None
         owner_tag = webhook_xml.find(".//t:owner", namespaces=ns)

tableauserverclient/server/endpoint/webhooks_endpoint.py

@@ -125,6 +125,10 @@ def update(self, webhook_item: WebhookItem) -> WebhookItem:
         """
         Modifies an existing webhook.
 
+        Fields serialized to the wire: name, url, event, is_enabled. Other
+        fields on the passed `WebhookItem` (e.g. `status_change_reason`,
+        `owner_id`) are not sent.
+
         REST API: https://help.tableau.com/current/api/rest_api/en-us/REST/rest_api_ref.htm#update_webhook
 
         Parameters

tableauserverclient/server/request_factory.py

@@ -1431,15 +1431,21 @@ def update_req(self, xml_request: ET.Element, webhook_item: "WebhookItem") -> by
 
         webhook = ET.SubElement(xml_request, "webhook")
         if webhook_item.name is not None:
+            if not isinstance(webhook_item.name, str):
+                raise ValueError(f"Name must be a string on {webhook_item}")
             webhook.attrib["name"] = webhook_item.name
         if webhook_item.is_enabled is not None:
             webhook.attrib["isEnabled"] = str(webhook_item.is_enabled).lower()
 
         if webhook_item.event_tag is not None:
+            if not isinstance(webhook_item.event_tag, str):
+                raise ValueError(f"event for Webhook must be a string on {webhook_item}")
             source = ET.SubElement(webhook, "webhook-source")
             ET.SubElement(source, webhook_item.event_tag)
 
         if webhook_item.url is not None:
+            if not isinstance(webhook_item.url, str):
+                raise ValueError(f"URL must be a string on {webhook_item}")
             destination = ET.SubElement(webhook, "webhook-destination")
             post = ET.SubElement(destination, "webhook-destination-http")
             post.attrib["method"] = "POST"

test/test_webhook.py

@@ -81,7 +81,7 @@ def test_create(server: TSC.Server) -> None:
         assert new_webhook.id is not None
 
 
-def test_request_factory():
+def test_request_factory() -> None:
     webhook_request_expected = CREATE_REQUEST_XML.read_text()
 
     webhook_item = WebhookItem()