From 4e40d5e0ada88f09e88c78610347d331a3efc04a Mon Sep 17 00:00:00 2001 From: Floke Date: Wed, 25 Feb 2026 17:21:36 +0000 Subject: [PATCH] Refactor API calls to use PATCH instead of PUT for cleaner updates (SuperOffice Certification) --- connector-superoffice/README.md | 7 ++- connector-superoffice/superoffice_client.py | 65 ++++++++++----------- connector-superoffice/worker.py | 39 ++++++------- 3 files changed, 52 insertions(+), 59 deletions(-) diff --git a/connector-superoffice/README.md b/connector-superoffice/README.md index 9aaf0bd6..70c802b4 100644 --- a/connector-superoffice/README.md +++ b/connector-superoffice/README.md @@ -196,9 +196,10 @@ Um die Zertifizierung für den SuperOffice App Store zu erhalten, mussten kritis * **Lösung:** Implementierung von **OData `$select`**. Wir fordern nun strikt nur die Felder an, die wir wirklich benötigen (z.B. `get_person(id, select=['JobTitle', 'UserDefinedFields'])`). * **Wichtig:** Niemals pauschal `get_person()` aufrufen, wenn nur die Rolle geprüft werden soll. -2. **NullReference bei `$select`:** - * **Problem:** Wenn `$select` genutzt wird, gibt SuperOffice nicht angeforderte komplexe Objekte (wie `Contact` in `Person`) als `null` zurück. Der Zugriff `person['Contact']['ContactId']` führt dann zum Crash. - * **Lösung:** Robuste Checks (`if contact_obj and isinstance(contact_obj, dict)`) und primäre Nutzung der IDs direkt aus dem Webhook-Payload (`FieldValues`), um API-Calls komplett zu vermeiden. +3. **PUT vs. PATCH (Safe Updates):** + * **Problem:** Die Verwendung von `PUT` zum Aktualisieren von Entitäten (Person/Contact) erfordert, dass das *gesamte* Objekt gesendet wird. Dies birgt das Risiko, Felder zu überschreiben, die zwischenzeitlich von anderen Benutzern geändert wurden ("Race Condition"), und verursacht unnötigen Traffic. + * **Lösung:** Umstellung auf **`PATCH`**. Wir senden nun nur noch die *tatsächlich geänderten Felder* (Delta). + * **Implementierung:** Der Worker baut nun ein `patch_payload` (z.B. `{'Position': {'Id': 123}}`) und nutzt den dedizierten PATCH-Endpunkt. Dies wurde explizit von SuperOffice für die Zertifizierung gefordert. ## Appendix: The "First Sentence" Prompt This is the core logic used to generate the company-specific opener. diff --git a/connector-superoffice/superoffice_client.py b/connector-superoffice/superoffice_client.py index 3844491e..3f481c44 100644 --- a/connector-superoffice/superoffice_client.py +++ b/connector-superoffice/superoffice_client.py @@ -80,6 +80,8 @@ class SuperOfficeClient: resp = requests.post(url, headers=self.headers, json=payload) elif method == "PUT": resp = requests.put(url, headers=self.headers, json=payload) + elif method == "PATCH": + resp = requests.patch(url, headers=self.headers, json=payload) # 401 Handling if resp.status_code == 401 and retry: @@ -109,6 +111,9 @@ class SuperOfficeClient: def _put(self, endpoint, payload): return self._request_with_retry("PUT", endpoint, payload) + def _patch(self, endpoint, payload): + return self._request_with_retry("PATCH", endpoint, payload) + def _post(self, endpoint, payload): return self._request_with_retry("POST", endpoint, payload) @@ -196,53 +201,45 @@ class SuperOfficeClient: def update_entity_udfs(self, entity_id: int, entity_type: str, udf_data: dict): """ - Updates UDFs for a given entity (Contact or Person). + Updates UDFs for a given entity (Contact or Person) using PATCH. entity_type: 'Contact' or 'Person' udf_data: {ProgId: Value} """ endpoint = f"{entity_type}/{entity_id}" - # 1. GET existing - existing_entity = self._get(endpoint) - if not existing_entity: - logger.error(f"❌ Failed to retrieve existing {entity_type} {entity_id} for UDF update.") - return False - - if "UserDefinedFields" not in existing_entity: - existing_entity["UserDefinedFields"] = {} + # Construct PATCH payload + payload = { + "UserDefinedFields": udf_data + } - # 2. Merge payload - existing_entity["UserDefinedFields"].update(udf_data) + logger.info(f"Patching {entity_type} {entity_id} UDFs: {udf_data}...") - logger.info(f"Updating {entity_type} {entity_id} UDFs: {udf_data}...") - - # 3. PUT update - result = self._put(endpoint, existing_entity) + # PATCH update + result = self._patch(endpoint, payload) return bool(result) def update_person_position(self, person_id: int, position_id: int): """ - Updates the standard 'Position' field of a Person. + Updates the standard 'Position' field of a Person using PATCH. """ endpoint = f"Person/{person_id}" - # 1. GET existing - existing_person = self._get(endpoint) - if not existing_person: - logger.error(f"❌ Failed to retrieve Person {person_id} for Position update.") - return False - - # 2. Check current value to avoid redundant updates - current_pos = existing_person.get("Position", {}) - if current_pos and str(current_pos.get("Id")) == str(position_id): - logger.info(f"Person {person_id} Position already set to {position_id}. Skipping.") - return True + # Construct PATCH payload + payload = { + "Position": {"Id": int(position_id)} + } + + logger.info(f"Patching Person {person_id} Position to ID {position_id}...") + + # PATCH update + result = self._patch(endpoint, payload) + return bool(result) - # 3. Update - existing_person["Position"] = {"Id": int(position_id)} - - logger.info(f"Updating Person {person_id} Position to ID {position_id}...") - - # 4. PUT - result = self._put(endpoint, existing_person) + def patch_contact(self, contact_id: int, patch_data: dict): + """ + Generic PATCH for Contact entity. + """ + endpoint = f"Contact/{contact_id}" + logger.info(f"Patching Contact {contact_id} with data keys: {list(patch_data.keys())}...") + result = self._patch(endpoint, patch_data) return bool(result) \ No newline at end of file diff --git a/connector-superoffice/worker.py b/connector-superoffice/worker.py index eaf9af9b..c00637b2 100644 --- a/connector-superoffice/worker.py +++ b/connector-superoffice/worker.py @@ -214,8 +214,7 @@ def process_job(job, so_client: SuperOfficeClient): logger.error(f"Failed to fetch contact {contact_id}: {e}") return "RETRY" - dirty_standard = False - dirty_udfs = False + contact_patch = {} if "UserDefinedFields" not in contact_data: contact_data["UserDefinedFields"] = {} @@ -233,8 +232,7 @@ def process_job(job, so_client: SuperOfficeClient): if str(current_val) != str(vertical_id): logger.info(f"Change detected: Vertical {current_val} -> {vertical_id}") - contact_data["UserDefinedFields"][udf_key] = str(vertical_id) - dirty_udfs = True + contact_patch.setdefault("UserDefinedFields", {})[udf_key] = str(vertical_id) except Exception as e: logger.error(f"Vertical sync error: {e}") @@ -252,26 +250,23 @@ def process_job(job, so_client: SuperOfficeClient): if "Postal" not in addr_obj or addr_obj["Postal"] is None: addr_obj["Postal"] = {} if "Street" not in addr_obj or addr_obj["Street"] is None: addr_obj["Street"] = {} - def update_addr_field(field_name, new_val, log_name): - nonlocal dirty_standard + def update_addr_patch(field_name, new_val, log_name): if new_val: for type_key in ["Postal", "Street"]: cur = addr_obj[type_key].get(field_name, "") if cur != new_val: logger.info(f"Change detected: {type_key} {log_name} '{cur}' -> '{new_val}'") - addr_obj[type_key][field_name] = new_val - dirty_standard = True + contact_patch.setdefault("Address", {}).setdefault(type_key, {})[field_name] = new_val - update_addr_field("City", ce_city, "City") - update_addr_field("Address1", ce_street, "Street") - update_addr_field("Zipcode", ce_zip, "Zip") + update_addr_patch("City", ce_city, "City") + update_addr_patch("Address1", ce_street, "Street") + update_addr_patch("Zipcode", ce_zip, "Zip") if ce_vat: current_vat = contact_data.get("OrgNr", "") if current_vat != ce_vat: logger.info(f"Change detected: VAT '{current_vat}' -> '{ce_vat}'") - contact_data["OrgNr"] = ce_vat - dirty_standard = True + contact_patch["OrgNr"] = ce_vat # --- C. AI Openers Sync --- ce_opener = provisioning_data.get("opener") @@ -281,25 +276,25 @@ def process_job(job, so_client: SuperOfficeClient): current_opener = contact_data["UserDefinedFields"].get(settings.UDF_OPENER, "") if current_opener != ce_opener: logger.info("Change detected: Primary Opener") - contact_data["UserDefinedFields"][settings.UDF_OPENER] = ce_opener - dirty_udfs = True + contact_patch.setdefault("UserDefinedFields", {})[settings.UDF_OPENER] = ce_opener if ce_opener_secondary and ce_opener_secondary != "null": current_opener_sec = contact_data["UserDefinedFields"].get(settings.UDF_OPENER_SECONDARY, "") if current_opener_sec != ce_opener_secondary: logger.info("Change detected: Secondary Opener") - contact_data["UserDefinedFields"][settings.UDF_OPENER_SECONDARY] = ce_opener_secondary - dirty_udfs = True + contact_patch.setdefault("UserDefinedFields", {})[settings.UDF_OPENER_SECONDARY] = ce_opener_secondary - # --- D. Apply Updates (Single Transaction) --- - if dirty_standard or dirty_udfs: - logger.info(f"Pushing combined updates for Contact {contact_id}...") + # --- D. Apply Updates (Single PATCH Transaction) --- + if contact_patch: + logger.info(f"Pushing combined PATCH updates for Contact {contact_id}...") try: - so_client._put(f"Contact/{contact_id}", contact_data) - logger.info("✅ Contact Update Successful.") + so_client.patch_contact(contact_id, contact_patch) + logger.info("✅ Contact Update Successful (PATCH).") except Exception as e: logger.error(f"Failed to update Contact {contact_id}: {e}") raise + else: + logger.info(f"No changes detected for Contact {contact_id}. Skipping update.") # 2d. Sync Person Position (Role) - if Person exists role_name = provisioning_data.get("role_name")