Stop sending branch as a Zetkin person field

README.md

@@ -27,6 +27,26 @@ The parent plugin fires hooks at each stage of member registration and membershi
 | 8 | `ck_join_flow_should_lapse_member` (filter) | `LapsingOverride.php` | Override lapse decision using GMTU standing rules (see below) |
 | 9 | `ck_join_flow_should_unlapse_member` (filter) | `LapsingOverride.php` | Override unlapse decision using GMTU standing rules (see below) |
 
+## Branch tagging
+
+### Why this exists
+
+Branch organisers use the CRM tools (Mailchimp, Action Network, Zetkin) to segment and contact their members. For that to work, every new signup must arrive in the CRM already tagged with the branch that was assigned from their postcode.
+
+### How it works
+
+The parent plugin fires the `ck_join_flow_add_tags` filter once per integration service during a successful signup, passing the current tag array, the member data, and the service name. This plugin hooks that filter in `Tagging.php` and appends `$data["branch"]` — which was populated earlier by the `ck_join_flow_pre_handle_join` filter in `BranchAssignment.php` — onto the tag array. The parent plugin then sends the combined tags to each service through its normal tagging path.
+
+Because the filter fires per service, every configured integration receives the same branch tag. Nothing in this plugin talks to the CRMs directly; the parent plugin owns transport.
+
+### Zetkin: tagging only, never a custom field
+
+In Zetkin specifically, branch information **must** flow via tags and **never** as a custom person field. The core join-block plugin forwards `customFields` as direct fields on Zetkin's People API, and Zetkin rejects `branch` as an unrecognised field — the entire signup request fails with an "invalid parameter" error. An earlier version of this plugin injected `branch` into `customFields` and `customFieldsConfig`; that caused every Zetkin signup to fail before the tag filter ever ran, so members ended up with no branch recorded. See PR #9 for the fix. The regression tests in `tests/BranchAssignmentTest.php` pin this: branch must never appear in either `customFields` or `customFieldsConfig`.
+
+### No-branch case
+
+If `$data["branch"]` is empty — either because the postcode outcode is deliberately mapped to `null` in `Branch.php` (e.g. Stockport-area postcodes that currently have no branch), or because branch assignment failed upstream — the filter logs a warning naming the service and the member email, and returns the tag array untouched. The signup still completes; it just arrives in the CRM without a branch tag.
+
 ## Membership lapsing override
 
 ### Why this exists

src/BranchAssignment.php

@@ -2,8 +2,7 @@
 /**
  * Branch assignment functionality.
  *
- * Automatically assigns branches to members based on their postcode
- * and ensures branch data is properly stored in custom fields.
+ * Automatically assigns branches to members based on their postcode.
  *
  * @package CommonKnowledge\JoinBlock\Organisation\GMTU
  */
@@ -51,34 +50,9 @@ function register_branch_assignment() {
             log_warning("Outcode $outcode not found in branch map for postcode $postcode");
         }
 
-        // Ensure "branch" custom field exists in config
-        $customFields = $data["customFieldsConfig"] ?? [];
-        $customFieldExists = false;
-        foreach ($customFields as $field) {
-            if ($field["id"] === "branch") {
-                $customFieldExists = true;
-                break;
-            }
-        }
-        if (!$customFieldExists) {
-            $customFields[] = [
-                "id" => "branch",
-                "field_type" => "text"
-            ];
-        }
-        $data["customFieldsConfig"] = $customFields;
-
-        // Also set the branch value in the custom fields data
-        if (!isset($data["customFields"])) {
-            $data["customFields"] = [];
-        }
-        $data["customFields"]["branch"] = $branch;
-
         log_info("=== ck_join_flow_pre_handle_join FILTER END ===");
         log_info("Branch set to: " . ($branch ?? "NULL"));
         log_info("data['branch']: " . ($data["branch"] ?? "NOT SET"));
-        log_info("data['customFields']['branch']: " . ($data["customFields"]["branch"] ?? "NOT SET"));
-        log_info("MembershipPlan still present: " . (isset($data["membershipPlan"]) ? "YES - " . json_encode($data["membershipPlan"]) : "NO"));
         log_info("Full outgoing data: " . json_encode($data));
 
         return $data;

src/Member.php

@@ -20,9 +20,7 @@ function get_member_details($data) {
 
     log_info("=== get_member_details FUNCTION START ===");
     log_info("Data keys: " . implode(", ", array_keys($data)));
-    log_info("Checking branch in multiple locations:");
-    log_info("  - data['branch']: " . ($data['branch'] ?? "NOT SET"));
-    log_info("  - data['customFields']['branch']: " . ($data['customFields']['branch'] ?? "NOT SET"));
+    log_info("data['branch']: " . ($data['branch'] ?? "NOT SET"));
     log_info("Checking membershipPlan:");
     log_info("  - data['membershipPlan'] exists: " . (isset($data['membershipPlan']) ? "YES" : "NO"));
     if (isset($data['membershipPlan'])) {
@@ -31,9 +29,8 @@ function get_member_details($data) {
     log_info("Available plan data: planId=" . ($data['planId'] ?? "NOT SET") . ", membership=" . ($data['membership'] ?? "NOT SET"));
     log_info("Full data structure: " . json_encode($data));
 
-    // Try to get branch from multiple possible locations
-    $branch = $data['branch'] ?? $data['customFields']['branch'] ?? null;
-
+    $branch = $data['branch'] ?? null;
+
     // If branch not found, recalculate from postcode
     if (empty($branch) && !empty($data['addressPostcode'])) {
         log_info("Branch not found in data, recalculating from postcode: " . $data['addressPostcode']);

src/Notifications.php

@@ -24,7 +24,6 @@ function register_notifications($config) {
         log_info("=== ck_join_flow_success ACTION (priority 10) START ===");
         log_info("Data keys: " . implode(", ", array_keys($data)));
         log_info("Branch in data['branch']: " . ($data['branch'] ?? "NOT SET"));
-        log_info("Branch in data['customFields']['branch']: " . ($data['customFields']['branch'] ?? "NOT SET"));
         log_info("MembershipPlan in data: " . (isset($data['membershipPlan']) ? json_encode($data['membershipPlan']) : "NOT SET"));
         log_info("Full data received: " . json_encode($data));
 
@@ -37,7 +36,6 @@ function register_notifications($config) {
         log_info("=== ck_join_flow_success ACTION (priority 20) START ===");
         log_info("Data keys: " . implode(", ", array_keys($data)));
         log_info("Branch in data['branch']: " . ($data['branch'] ?? "NOT SET"));
-        log_info("Branch in data['customFields']['branch']: " . ($data['customFields']['branch'] ?? "NOT SET"));
         log_info("MembershipPlan in data: " . (isset($data['membershipPlan']) ? json_encode($data['membershipPlan']) : "NOT SET"));
         log_info("Full data received: " . json_encode($data));
 

src/Tagging.php

@@ -23,7 +23,6 @@ function register_tagging() {
         log_info("=== ck_join_flow_add_tags FILTER for $service ===");
         log_info("Data keys: " . implode(", ", array_keys($data)));
         log_info("Branch in data['branch']: " . ($data['branch'] ?? "NOT SET"));
-        log_info("Branch in data['customFields']['branch']: " . ($data['customFields']['branch'] ?? "NOT SET"));
         log_info("Full data structure: " . json_encode($data));
 
         $branch = $data['branch'] ?? null;

tests/BranchAssignmentTest.php

@@ -65,50 +65,6 @@ public function test_assigns_branch_from_postcode()
         $this->assertSame('Moss Side', $result['branch']);
     }
 
-    public function test_sets_branch_in_custom_fields()
-    {
-        $handler = $this->registerBranchAssignmentAndCaptureHandler();
-
-        Functions\when('get_transient')->justReturn('M14');
-
-        $data = ['addressPostcode' => 'M14 5RQ'];
-        $result = $handler($data);
-        $this->assertSame('Moss Side', $result['customFields']['branch']);
-    }
-
-    public function test_adds_branch_to_custom_fields_config()
-    {
-        $handler = $this->registerBranchAssignmentAndCaptureHandler();
-
-        Functions\when('get_transient')->justReturn('M14');
-
-        $data = ['addressPostcode' => 'M14 5RQ'];
-        $result = $handler($data);
-
-        $branchField = array_filter($result['customFieldsConfig'], function ($f) {
-            return $f['id'] === 'branch';
-        });
-        $this->assertCount(1, $branchField);
-    }
-
-    public function test_does_not_duplicate_branch_in_config()
-    {
-        $handler = $this->registerBranchAssignmentAndCaptureHandler();
-
-        Functions\when('get_transient')->justReturn('M14');
-
-        $data = [
-            'addressPostcode' => 'M14 5RQ',
-            'customFieldsConfig' => [['id' => 'branch', 'field_type' => 'text']],
-        ];
-        $result = $handler($data);
-
-        $branchFields = array_filter($result['customFieldsConfig'], function ($f) {
-            return $f['id'] === 'branch';
-        });
-        $this->assertCount(1, $branchFields);
-    }
-
     public function test_sets_null_branch_for_null_mapping()
     {
         $handler = $this->registerBranchAssignmentAndCaptureHandler();
@@ -130,32 +86,57 @@ public function test_assigns_null_branch_for_known_no_branch_postcode()
         $data = ['addressPostcode' => 'BL8 1AA'];
         $result = $handler($data);
         $this->assertNull($result['branch']);
-        $this->assertNull($result['customFields']['branch']);
     }
 
-    public function test_sets_custom_fields_config_for_known_no_branch_postcode()
+    public function test_returns_data_unchanged_when_no_postcode()
+    {
+        $handler = $this->registerBranchAssignmentAndCaptureHandler();
+
+        $data = ['firstName' => 'Jane'];
+        $result = $handler($data);
+        $this->assertSame($data, $result);
+    }
+
+    /*
+     * Regression tests for the Zetkin "invalid parameter" bug (fixed in PR #9).
+     *
+     * Prior versions of this filter injected `branch` into both
+     * `customFieldsConfig` and `customFields`. The core join-block plugin
+     * forwards `customFields` as direct fields on Zetkin's People API call —
+     * Zetkin rejects `branch` as an unrecognised person field and the entire
+     * signup request fails. Because the Zetkin call failed, the downstream
+     * `ck_join_flow_add_tags` filter (which correctly adds branch as a tag)
+     * never ran, so new members ended up with no branch assigned at all.
+     *
+     * The two tests below pin the fix: branch information must flow to Zetkin
+     * only via the tag filter, never as a custom person field. If someone
+     * reintroduces a `customFields['branch']` or `customFieldsConfig` entry
+     * for branch, these tests will fail loudly.
+     */
+    public function test_does_not_add_branch_to_custom_fields_config()
     {
         $handler = $this->registerBranchAssignmentAndCaptureHandler();
 
-        // WA14 is in the branch map but deliberately has no branch
-        Functions\when('get_transient')->justReturn('WA14');
+        Functions\when('get_transient')->justReturn('M14');
 
-        $data = ['addressPostcode' => 'WA14 1AA'];
+        $data = ['addressPostcode' => 'M14 5RQ'];
         $result = $handler($data);
-        $this->assertNull($result['branch']);
 
-        $branchField = array_filter($result['customFieldsConfig'], function ($f) {
+        $branchField = array_filter($result['customFieldsConfig'] ?? [], function ($f) {
             return $f['id'] === 'branch';
         });
-        $this->assertCount(1, $branchField);
+        $this->assertCount(0, $branchField, 'branch must not be sent as a custom field — Zetkin rejects it as an invalid person field');
     }
 
-    public function test_returns_data_unchanged_when_no_postcode()
+    public function test_does_not_set_branch_in_custom_fields()
     {
         $handler = $this->registerBranchAssignmentAndCaptureHandler();
 
-        $data = ['firstName' => 'Jane'];
+        Functions\when('get_transient')->justReturn('M14');
+
+        $data = ['addressPostcode' => 'M14 5RQ'];
         $result = $handler($data);
-        $this->assertSame($data, $result);
+
+        $this->assertArrayNotHasKey('branch', $result['customFields'] ?? [], 'branch must not be sent as a custom field — Zetkin rejects it as an invalid person field');
     }
 }

tests/MemberTest.php

@@ -78,14 +78,6 @@ public function test_uses_branch_from_data()
         $this->assertSame('Moss Side', $result['branch']);
     }
 
-    public function test_uses_branch_from_custom_fields_fallback()
-    {
-        $data = $this->makeSampleRegistrationData();
-        unset($data['branch']);
-        $result = get_member_details($data);
-        $this->assertSame('Moss Side', $result['branch']);
-    }
-
     public function test_recalculates_branch_from_postcode_when_missing()
     {
         $data = $this->makeSampleRegistrationData([