From 5fad600054ddeec57b7c3244962c3578c10687de Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Wed, 16 Sep 2026 13:08:08 +0100
Subject: [PATCH 01/14] Add Zetkin people listing and person-tag helpers

Bulk maintenance jobs need to walk the membership and adjust tags, which
the service could not do: addTag and removeTag work one email at a time
and re-authenticate, re-search and re-fetch the full tag list on every
call.

Adds listPeople, getPersonTags, findOrCreateTagByTitle, addTagToPerson
and removeTagFromPerson, all built on the existing getZetkinContext.

The PUT and DELETE calls that apply and remove a person's tag now have a
single implementation each, which signup, addTag and removeTag all share.
Their log wording is unchanged, including treating a 404 on delete as
"tag does not exist" rather than an error.
---
 .../join-block/src/Services/ZetkinService.php | 238 ++++++++++++++----
 1 file changed, 190 insertions(+), 48 deletions(-)

diff --git a/packages/join-block/src/Services/ZetkinService.php b/packages/join-block/src/Services/ZetkinService.php
index b05c583..4d20947 100644
--- a/packages/join-block/src/Services/ZetkinService.php
+++ b/packages/join-block/src/Services/ZetkinService.php
@@ -175,32 +175,14 @@ private static function addPerson($baseUrl, $orgId, $clientId, $clientSecret, $j
             }
 
             foreach ($addTagIds as $tagId) {
-                $response = $client->request("PUT", "$baseUrl/orgs/$orgId/people/$personId/tags/$tagId", [
-                    "headers" => [
-                        "Authorization" => "Bearer {$accessToken}",
-                        "Content-type" => "application/json",
-                    ],
-                ]);
-                $responseData = json_decode($response->getBody()->getContents(), true);
-                if (!empty($responseData["error"])) {
-                    $joinBlockLog->error("Could not tag person: " . json_encode($responseData["error"]));
+                if (self::putPersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId) !== 'ok') {
+                    $joinBlockLog->error("Could not tag person $personId with tag $tagId");
                 }
             }
 
             foreach ($removeTagIds as $tagId) {
-                $response = $client->request("DELETE", "$baseUrl/orgs/$orgId/people/$personId/tags/$tagId", [
-                    "headers" => [
-                        "Authorization" => "Bearer {$accessToken}",
-                        "Content-type" => "application/json",
-                    ],
-                    "http_errors" => false
-                ]);
-                $responseData = json_decode($response->getBody()->getContents(), true);
-                if (!empty($responseData["error"])) {
-                    $msg = $responseData["error"]["title"] ?? "";
-                    if ($msg !== "404 Not Found") {
-                        $joinBlockLog->error("Could not untag person: " . json_encode($responseData["error"]));
-                    }
+                if (self::deletePersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId) === 'error') {
+                    $joinBlockLog->error("Could not untag person $personId of tag $tagId");
                 }
             }
         } catch (\GuzzleHttp\Exception\RequestException $e) {
@@ -425,6 +407,184 @@ private static function getZetkinContext()
         ];
     }
 
+    /**
+     * List people in the organisation, one page at a time.
+     *
+     * Intended for bulk maintenance jobs that need to walk the whole
+     * membership, rather than the per-signup path. Zetkin paginates with `p`
+     * (zero-indexed page) and `pp` (page size); an empty array means the end
+     * of the list has been reached.
+     *
+     * Note that each call opens its own Zetkin context, so a walk over the
+     * full membership costs one OAuth exchange per page. That is deliberate:
+     * it keeps this consistent with the other standalone helpers below, and
+     * bulk jobs are expected to be occasional.
+     *
+     * Only available when OAuth credentials (CLIENT_ID, CLIENT_SECRET, JWT)
+     * are configured.
+     *
+     * @param int $page Zero-indexed page number.
+     * @param int $perPage Records per page.
+     * @return array List of person records, empty when exhausted or unconfigured.
+     */
+    public static function listPeople($page = 0, $perPage = 100)
+    {
+        $zetkinContext = self::getZetkinContext();
+        if (!$zetkinContext) {
+            return [];
+        }
+
+        ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken, 'client' => $client] = $zetkinContext;
+
+        $response = $client->request("GET", "$baseUrl/orgs/$orgId/people?p=$page&pp=$perPage", [
+            "headers" => [
+                "Authorization" => "Bearer {$accessToken}",
+                "Content-type" => "application/json",
+            ]
+        ]);
+        $responseData = json_decode($response->getBody()->getContents(), true);
+
+        if (!empty($responseData["error"])) {
+            throw new \Exception("Could not list people: " . json_encode($responseData["error"]));
+        }
+
+        return $responseData["data"] ?? [];
+    }
+
+    /**
+     * Get the tags currently applied to one person.
+     *
+     * @param int|string $personId
+     * @return array List of tag records, each with at least id and title.
+     */
+    public static function getPersonTags($personId)
+    {
+        $zetkinContext = self::getZetkinContext();
+        if (!$zetkinContext) {
+            return [];
+        }
+
+        ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken, 'client' => $client] = $zetkinContext;
+
+        $response = $client->request("GET", "$baseUrl/orgs/$orgId/people/$personId/tags", [
+            "headers" => [
+                "Authorization" => "Bearer {$accessToken}",
+                "Content-type" => "application/json",
+            ]
+        ]);
+        $responseData = json_decode($response->getBody()->getContents(), true);
+
+        if (!empty($responseData["error"])) {
+            throw new \Exception("Could not get tags for person $personId: " . json_encode($responseData["error"]));
+        }
+
+        return $responseData["data"] ?? [];
+    }
+
+    /**
+     * Look up a tag by title, creating it if it does not exist yet.
+     *
+     * Public wrapper over the same find-or-create the signup path uses, so
+     * bulk jobs tag people with exactly the same tags a signup would.
+     *
+     * @param string $title
+     * @return array|null The tag record, or null if Zetkin is not configured.
+     */
+    public static function findOrCreateTagByTitle($title)
+    {
+        $zetkinContext = self::getZetkinContext();
+        if (!$zetkinContext) {
+            return null;
+        }
+
+        ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken] = $zetkinContext;
+
+        $existingTags = self::getTags($baseUrl, $orgId, $accessToken);
+
+        return self::findOrCreateTag($baseUrl, $orgId, $existingTags, $title, $accessToken);
+    }
+
+    /**
+     * Apply an already-resolved tag to an already-resolved person.
+     *
+     * @param int|string $personId
+     * @param int|string $tagId
+     * @return bool True if the tag was applied.
+     */
+    public static function addTagToPerson($personId, $tagId)
+    {
+        $zetkinContext = self::getZetkinContext();
+        if (!$zetkinContext) {
+            return false;
+        }
+
+        ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken, 'client' => $client] = $zetkinContext;
+
+        return self::putPersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId) === 'ok';
+    }
+
+    /**
+     * Remove an already-resolved tag from an already-resolved person.
+     *
+     * A tag the person does not have is treated as success, not an error.
+     *
+     * @param int|string $personId
+     * @param int|string $tagId
+     * @return bool True if the person no longer has the tag.
+     */
+    public static function removeTagFromPerson($personId, $tagId)
+    {
+        $zetkinContext = self::getZetkinContext();
+        if (!$zetkinContext) {
+            return false;
+        }
+
+        ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken, 'client' => $client] = $zetkinContext;
+
+        return self::deletePersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId) !== 'error';
+    }
+
+    /**
+     * Single implementation of "apply this tag to this person".
+     *
+     * @return string 'ok' or 'error'. Callers add their own context to the log.
+     */
+    private static function putPersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId)
+    {
+        $response = $client->request("PUT", "$baseUrl/orgs/$orgId/people/$personId/tags/$tagId", [
+            "headers" => [
+                "Authorization" => "Bearer {$accessToken}",
+                "Content-type" => "application/json",
+            ]
+        ]);
+        $responseData = json_decode($response->getBody()->getContents(), true);
+
+        return empty($responseData["error"]) ? 'ok' : 'error';
+    }
+
+    /**
+     * Single implementation of "take this tag off this person".
+     *
+     * @return string 'ok', 'missing' when the person did not have the tag, or 'error'.
+     */
+    private static function deletePersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId)
+    {
+        $response = $client->request("DELETE", "$baseUrl/orgs/$orgId/people/$personId/tags/$tagId", [
+            "headers" => [
+                "Authorization" => "Bearer {$accessToken}",
+                "Content-type" => "application/json",
+            ],
+            "http_errors" => false
+        ]);
+        $statusCode = $response->getStatusCode();
+
+        if ($statusCode === 404) {
+            return 'missing';
+        }
+
+        return $statusCode >= 400 ? 'error' : 'ok';
+    }
+
     /**
      * Standalone function to find a person by email and apply a tag (string)
      */
@@ -463,19 +623,11 @@ public static function addTag($email, $tag)
             $existingTags = self::getTags($baseUrl, $orgId, $accessToken);
             $existingTag = self::findOrCreateTag($baseUrl, $orgId, $existingTags, $tag, $accessToken);
             foreach ($matched as $person) {
-                $personId = $person["id"];
-                $tagId = $existingTag["id"];
-                $response = $client->request("PUT", "$baseUrl/orgs/$orgId/people/$personId/tags/$tagId", [
-                    "headers" => [
-                        "Authorization" => "Bearer {$accessToken}",
-                        "Content-type" => "application/json",
-                    ]
-                ]);
-                $responseData = json_decode($response->getBody()->getContents(), true);
-                if (!empty($responseData["error"])) {
-                    $joinBlockLog->error("Could not add tag '$tag' to $email in Zetkin: " . json_encode($responseData["error"]));
-                } else {
+                $result = self::putPersonTag($client, $baseUrl, $orgId, $accessToken, $person["id"], $existingTag["id"]);
+                if ($result === 'ok') {
                     $joinBlockLog->info("Added tag '$tag' to $email in Zetkin");
+                } else {
+                    $joinBlockLog->error("Could not add tag '$tag' to $email in Zetkin");
                 }
             }
         } catch (\Exception $e) {
@@ -521,21 +673,11 @@ public static function removeTag($email, $tag)
             $existingTags = self::getTags($baseUrl, $orgId, $accessToken);
             $existingTag = self::findOrCreateTag($baseUrl, $orgId, $existingTags, $tag, $accessToken);
             foreach ($matched as $person) {
-                $personId = $person["id"];
-                $tagId = $existingTag["id"];
-                $response = $client->request("DELETE", "$baseUrl/orgs/$orgId/people/$personId/tags/$tagId", [
-                    "headers" => [
-                        "Authorization" => "Bearer {$accessToken}",
-                        "Content-type" => "application/json",
-                    ],
-                    "http_errors" => false
-                ]);
-                $statusCode = $response->getStatusCode();
-                if ($statusCode === 404) {
+                $result = self::deletePersonTag($client, $baseUrl, $orgId, $accessToken, $person["id"], $existingTag["id"]);
+                if ($result === 'missing') {
                     $joinBlockLog->info("Could not remove tag '$tag' from $email in Zetkin: tag does not exist");
-                } elseif ($statusCode >= 400) {
-                    $responseData = json_decode($response->getBody()->getContents(), true);
-                    $joinBlockLog->error("Could not remove tag '$tag' from $email in Zetkin: " . json_encode($responseData["error"] ?? $statusCode));
+                } elseif ($result === 'error') {
+                    $joinBlockLog->error("Could not remove tag '$tag' from $email in Zetkin");
                 } else {
                     $joinBlockLog->info("Removed tag '$tag' from $email in Zetkin");
                 }

From ed69df36344ad2a7403965b6743a3736efbdd7a5 Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Wed, 16 Sep 2026 13:08:09 +0100
Subject: [PATCH 02/14] Bump version to 1.4.38

---
 packages/join-block/join.php     | 2 +-
 packages/join-block/readme.txt   | 4 +++-
 packages/join-flow/src/index.tsx | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/packages/join-block/join.php b/packages/join-block/join.php
index 56e0540..c28e019 100644
--- a/packages/join-block/join.php
+++ b/packages/join-block/join.php
@@ -3,7 +3,7 @@
 /**
  * Plugin Name:     Common Knowledge Join Flow
  * Description:     Common Knowledge join flow plugin.
- * Version:         1.4.37
+ * Version:         1.4.38
  * Author:          Common Knowledge <hello@commonknowledge.coop>
  * Text Domain:     common-knowledge-join-flow
  * License: GPLv2 or later
diff --git a/packages/join-block/readme.txt b/packages/join-block/readme.txt
index 7e28ef7..e9cae1f 100644
--- a/packages/join-block/readme.txt
+++ b/packages/join-block/readme.txt
@@ -4,7 +4,7 @@ Tags: membership, subscription, join
 Contributors: commonknowledgecoop
 Requires at least: 5.4
 Tested up to: 7.0
-Stable tag: 1.4.37
+Stable tag: 1.4.38
 Requires PHP: 8.1
 License: GPLv2 or later
 License URI: https://www.gnu.org/licenses/gpl-2.0.html
@@ -107,6 +107,8 @@ Need help? Contact us at [hello@commonknowledge.coop](mailto:hello@commonknowled
 
 == Changelog ==
 
+= 1.4.38 =
+* Add Zetkin people listing and person-tag helpers for bulk maintenance jobs
 = 1.4.37 =
 * Add "cancelled" tag option, distinguishing this from lapsed membership
 = 1.4.36 =
diff --git a/packages/join-flow/src/index.tsx b/packages/join-flow/src/index.tsx
index fc695b1..91eeb3c 100644
--- a/packages/join-flow/src/index.tsx
+++ b/packages/join-flow/src/index.tsx
@@ -24,7 +24,7 @@ const init = () => {
   const sentryDsn = getEnvStr("SENTRY_DSN")
   Sentry.init({
     dsn: sentryDsn,
-    release: "1.4.37"
+    release: "1.4.38"
   });
 
   if (getEnv('USE_CHARGEBEE')) {

From 3bf0ab46ed714f1a99386bed958743811b858546 Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 11:17:21 +0100
Subject: [PATCH 03/14] Add Mailchimp tag helpers that report rather than throw

GMTU still use Mailchimp alongside Zetkin, so the branch re-tagging job
has to fix both. The existing addTag and removeTag are built for a single
signup: they return nothing and throw on any API error. In a bulk run
that is unusable, because "this member is not in the audience" has to be
distinguishable from "Mailchimp is broken", and neither should stop the
walk.

addTagToMember and removeTagFromMember return ok, not_found,
not_configured or error. isConfigured lets a job ask once at the start
whether Mailchimp is worth talking to at all.

Not found is read from the 404 rather than pre-checked with
memberExists, which halves the API calls per member. Across a whole
membership walk that is the difference between one round trip and two.

The client is injectable so this is testable without network access.
The old addTag and removeTag are untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../src/Services/MailchimpService.php         |  99 ++++++++
 .../tests/MailchimpServiceTagsTest.php        | 236 ++++++++++++++++++
 2 files changed, 335 insertions(+)
 create mode 100644 packages/join-block/tests/MailchimpServiceTagsTest.php

diff --git a/packages/join-block/src/Services/MailchimpService.php b/packages/join-block/src/Services/MailchimpService.php
index 845edbb..24ace91 100644
--- a/packages/join-block/src/Services/MailchimpService.php
+++ b/packages/join-block/src/Services/MailchimpService.php
@@ -255,6 +255,105 @@ public static function memberExists($email)
         }
     }
 
+    /**
+     * Is Mailchimp worth talking to at all?
+     *
+     * A bulk job asks this once at the start rather than discovering the
+     * answer member by member.
+     *
+     * @since 1.4.39
+     *
+     * @return bool True when both an API key and an audience are configured.
+     */
+    public static function isConfigured()
+    {
+        return !empty(Settings::get("MAILCHIMP_API_KEY"))
+            && !empty(Settings::get("MAILCHIMP_AUDIENCE_ID"));
+    }
+
+    /**
+     * Apply a tag to an audience member, reporting what happened.
+     *
+     * Unlike addTag(), this returns a status rather than throwing, because a
+     * bulk run has to carry on past one bad member and account for it at the
+     * end.
+     *
+     * @since 1.4.39
+     *
+     * @param string      $email
+     * @param string      $tag
+     * @param object|null $client Injected Mailchimp client, for testing.
+     * @return string 'ok', 'not_found', 'not_configured' or 'error'.
+     */
+    public static function addTagToMember($email, $tag, $client = null)
+    {
+        return self::setMemberTagStatus($email, $tag, 'active', $client);
+    }
+
+    /**
+     * Take a tag off an audience member, reporting what happened.
+     *
+     * @since 1.4.39
+     *
+     * @param string      $email
+     * @param string      $tag
+     * @param object|null $client Injected Mailchimp client, for testing.
+     * @return string 'ok', 'not_found', 'not_configured' or 'error'.
+     */
+    public static function removeTagFromMember($email, $tag, $client = null)
+    {
+        return self::setMemberTagStatus($email, $tag, 'inactive', $client);
+    }
+
+    /**
+     * Single implementation of "set this tag to this status on this member".
+     *
+     * Mailchimp has no separate remove call; a tag is switched between active
+     * and inactive. A member who is not in the audience comes back as a 404,
+     * which is a reportable outcome rather than a failure, so this reads the
+     * status out of the exception instead of pre-checking with memberExists().
+     * That also halves the API calls per member, which matters across a
+     * whole-membership walk.
+     *
+     * @since 1.4.39
+     *
+     * @return string 'ok', 'not_found', 'not_configured' or 'error'.
+     */
+    private static function setMemberTagStatus($email, $tag, $status, $client = null)
+    {
+        global $joinBlockLog;
+
+        if (!self::isConfigured()) {
+            return 'not_configured';
+        }
+
+        $client = $client ?? self::getClient();
+        $mailchimp_audience_id = Settings::get("MAILCHIMP_AUDIENCE_ID");
+        $subscriberHash = md5(strtolower($email));
+
+        try {
+            $client->lists->updateListMemberTags(
+                $mailchimp_audience_id,
+                $subscriberHash,
+                ["tags" => [["name" => $tag, "status" => $status]]]
+            );
+            return 'ok';
+        } catch (\GuzzleHttp\Exception\ClientException $e) {
+            $response = $e->getResponse();
+            $body = $response ? $response->getBody()->getContents() : $e->getMessage();
+
+            if (($response && $response->getStatusCode() === 404) || str_contains($body, "Resource Not Found")) {
+                return 'not_found';
+            }
+
+            $joinBlockLog->error("Failed to set tag '$tag' to $status for $email in Mailchimp: " . $body);
+            return 'error';
+        } catch (\Throwable $e) {
+            $joinBlockLog->error("Failed to set tag '$tag' to $status for $email in Mailchimp: " . $e->getMessage());
+            return 'error';
+        }
+    }
+
     public static function addTag($email, $tag)
     {
         global $joinBlockLog;
diff --git a/packages/join-block/tests/MailchimpServiceTagsTest.php b/packages/join-block/tests/MailchimpServiceTagsTest.php
new file mode 100644
index 0000000..7e6e598
--- /dev/null
+++ b/packages/join-block/tests/MailchimpServiceTagsTest.php
@@ -0,0 +1,236 @@
+<?php
+
+namespace CommonKnowledge\JoinBlock\Tests;
+
+use Brain\Monkey;
+use CommonKnowledge\JoinBlock\Services\MailchimpService;
+use GuzzleHttp\Exception\ClientException;
+use GuzzleHttp\Psr7\Request;
+use GuzzleHttp\Psr7\Response;
+use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Unit tests for the bulk-friendly Mailchimp tag helpers.
+ *
+ * These exist so a maintenance job can re-tag many members and report
+ * honestly on what happened to each one. The existing addTag/removeTag
+ * return nothing and throw on an API error, which is fine for a single
+ * signup but useless in a bulk run, where "this member is not in the
+ * audience" has to be distinguishable from "Mailchimp is broken".
+ *
+ * The Mailchimp client is injected so these run without network access.
+ */
+class MailchimpServiceTagsTest extends TestCase
+{
+    use MockeryPHPUnitIntegration;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        Monkey\setUp();
+        Monkey\Functions\when('carbon_get_theme_option')->justReturn(null);
+
+        $_ENV['MAILCHIMP_API_KEY'] = 'test-key-us1';
+        $_ENV['MAILCHIMP_AUDIENCE_ID'] = 'test-audience';
+
+        global $joinBlockLog;
+        $joinBlockLog = new class {
+            public array $errors = [];
+
+            // phpcs:ignore PSR1.Methods.CamelCapsMethodName.NotCamelCaps
+            public function info(string $msg, array $ctx = []): void {}
+
+            // phpcs:ignore PSR1.Methods.CamelCapsMethodName.NotCamelCaps
+            public function warning(string $msg, array $ctx = []): void {}
+
+            // phpcs:ignore PSR1.Methods.CamelCapsMethodName.NotCamelCaps
+            public function error(string $msg, array $ctx = []): void
+            {
+                $this->errors[] = $msg;
+            }
+        };
+    }
+
+    protected function tearDown(): void
+    {
+        global $joinBlockLog;
+        $joinBlockLog = null;
+        unset($_ENV['MAILCHIMP_API_KEY'], $_ENV['MAILCHIMP_AUDIENCE_ID']);
+        Monkey\tearDown();
+        parent::tearDown();
+    }
+
+    /**
+     * A client that records the calls made to it and returns a canned result.
+     */
+    private function fakeClient(?\Throwable $throw = null): object
+    {
+        $lists = new class ($throw) {
+            public array $calls = [];
+            private ?\Throwable $throw;
+
+            public function __construct(?\Throwable $throw)
+            {
+                $this->throw = $throw;
+            }
+
+            public function updateListMemberTags($audienceId, $subscriberHash, $body)
+            {
+                $this->calls[] = compact('audienceId', 'subscriberHash', 'body');
+                if ($this->throw) {
+                    throw $this->throw;
+                }
+                return null;
+            }
+        };
+
+        return new class ($lists) {
+            public $lists;
+
+            public function __construct($lists)
+            {
+                $this->lists = $lists;
+            }
+        };
+    }
+
+    private function clientException(int $status, string $body): ClientException
+    {
+        return new ClientException(
+            "HTTP $status",
+            new Request('POST', '/'),
+            new Response($status, [], $body)
+        );
+    }
+
+    public function testAddTagToMemberReportsOk(): void
+    {
+        $client = $this->fakeClient();
+
+        $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
+
+        $this->assertSame('ok', $result);
+    }
+
+    public function testAddTagToMemberSendsTheTagAsActive(): void
+    {
+        $client = $this->fakeClient();
+
+        MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
+
+        $call = $client->lists->calls[0];
+        $this->assertSame([['name' => 'Bury', 'status' => 'active']], $call['body']['tags']);
+    }
+
+    public function testRemoveTagFromMemberSendsTheTagAsInactive(): void
+    {
+        $client = $this->fakeClient();
+
+        $result = MailchimpService::removeTagFromMember('person@example.com', 'South Manchester', $client);
+
+        $this->assertSame('ok', $result);
+        $call = $client->lists->calls[0];
+        $this->assertSame(
+            [['name' => 'South Manchester', 'status' => 'inactive']],
+            $call['body']['tags']
+        );
+    }
+
+    /**
+     * Mailchimp addresses a member by the md5 of their lowercased email.
+     * Getting this wrong tags the wrong person, or nobody, without erroring.
+     */
+    public function testMemberIsAddressedByLowercasedEmailHash(): void
+    {
+        $client = $this->fakeClient();
+
+        MailchimpService::addTagToMember('Person@Example.COM', 'Bury', $client);
+
+        $call = $client->lists->calls[0];
+        $this->assertSame(md5('person@example.com'), $call['subscriberHash']);
+        $this->assertSame('test-audience', $call['audienceId']);
+    }
+
+    /**
+     * Someone in Zetkin but not in the Mailchimp audience is an expected,
+     * reportable outcome, not a failure of the run.
+     */
+    public function testUnknownMemberIsReportedAsNotFound(): void
+    {
+        $client = $this->fakeClient($this->clientException(404, '{"title":"Resource Not Found"}'));
+
+        $result = MailchimpService::addTagToMember('ghost@example.com', 'Bury', $client);
+
+        $this->assertSame('not_found', $result);
+    }
+
+    public function testUnknownMemberIsReportedAsNotFoundWhenRemoving(): void
+    {
+        $client = $this->fakeClient($this->clientException(404, '{"title":"Resource Not Found"}'));
+
+        $result = MailchimpService::removeTagFromMember('ghost@example.com', 'Bury', $client);
+
+        $this->assertSame('not_found', $result);
+    }
+
+    /**
+     * Anything else from Mailchimp is an error. It must not be swallowed as
+     * success, and it must not be mistaken for a missing member.
+     */
+    public function testOtherClientErrorsAreReportedAsError(): void
+    {
+        $client = $this->fakeClient($this->clientException(403, '{"title":"API Key Invalid"}'));
+
+        $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
+
+        $this->assertSame('error', $result);
+    }
+
+    public function testUnexpectedFailuresAreReportedAsError(): void
+    {
+        $client = $this->fakeClient(new \RuntimeException('connection reset'));
+
+        $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
+
+        $this->assertSame('error', $result);
+    }
+
+    /**
+     * A bulk job must be able to ask whether Mailchimp is worth talking to
+     * before it starts, rather than discovering it per member.
+     */
+    public function testIsConfiguredIsTrueWhenKeyAndAudienceAreSet(): void
+    {
+        $this->assertTrue(MailchimpService::isConfigured());
+    }
+
+    public function testIsConfiguredIsFalseWithoutAnApiKey(): void
+    {
+        unset($_ENV['MAILCHIMP_API_KEY']);
+
+        $this->assertFalse(MailchimpService::isConfigured());
+    }
+
+    public function testIsConfiguredIsFalseWithoutAnAudience(): void
+    {
+        unset($_ENV['MAILCHIMP_AUDIENCE_ID']);
+
+        $this->assertFalse(MailchimpService::isConfigured());
+    }
+
+    /**
+     * With Mailchimp switched off, the helpers say so rather than
+     * constructing a client against an empty key.
+     */
+    public function testTaggingIsSkippedWhenMailchimpIsNotConfigured(): void
+    {
+        unset($_ENV['MAILCHIMP_API_KEY']);
+        $client = $this->fakeClient();
+
+        $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
+
+        $this->assertSame('not_configured', $result);
+        $this->assertSame([], $client->lists->calls);
+    }
+}

From 289b676511ee012e5b66acdc9e3e0dab820ee4e3 Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 11:17:40 +0100
Subject: [PATCH 04/14] Bump version to 1.4.39

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 packages/join-block/join.php     | 2 +-
 packages/join-block/readme.txt   | 4 +++-
 packages/join-flow/src/index.tsx | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/packages/join-block/join.php b/packages/join-block/join.php
index c28e019..ccedbfb 100644
--- a/packages/join-block/join.php
+++ b/packages/join-block/join.php
@@ -3,7 +3,7 @@
 /**
  * Plugin Name:     Common Knowledge Join Flow
  * Description:     Common Knowledge join flow plugin.
- * Version:         1.4.38
+ * Version:         1.4.39
  * Author:          Common Knowledge <hello@commonknowledge.coop>
  * Text Domain:     common-knowledge-join-flow
  * License: GPLv2 or later
diff --git a/packages/join-block/readme.txt b/packages/join-block/readme.txt
index e9cae1f..f25bbba 100644
--- a/packages/join-block/readme.txt
+++ b/packages/join-block/readme.txt
@@ -4,7 +4,7 @@ Tags: membership, subscription, join
 Contributors: commonknowledgecoop
 Requires at least: 5.4
 Tested up to: 7.0
-Stable tag: 1.4.38
+Stable tag: 1.4.39
 Requires PHP: 8.1
 License: GPLv2 or later
 License URI: https://www.gnu.org/licenses/gpl-2.0.html
@@ -107,6 +107,8 @@ Need help? Contact us at [hello@commonknowledge.coop](mailto:hello@commonknowled
 
 == Changelog ==
 
+= 1.4.39 =
+* Add Mailchimp tag helpers that report status instead of throwing, for bulk maintenance jobs
 = 1.4.38 =
 * Add Zetkin people listing and person-tag helpers for bulk maintenance jobs
 = 1.4.37 =
diff --git a/packages/join-flow/src/index.tsx b/packages/join-flow/src/index.tsx
index 91eeb3c..1b228e2 100644
--- a/packages/join-flow/src/index.tsx
+++ b/packages/join-flow/src/index.tsx
@@ -24,7 +24,7 @@ const init = () => {
   const sentryDsn = getEnvStr("SENTRY_DSN")
   Sentry.init({
     dsn: sentryDsn,
-    release: "1.4.38"
+    release: "1.4.39"
   });
 
   if (getEnv('USE_CHARGEBEE')) {

From e14d108b0c4587f22689a6ade364ca88e3a44588 Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 11:53:35 +0100
Subject: [PATCH 05/14] Follow the codebase's own commenting and logging
 conventions

Review feedback on #118.

Drops the @since tags and the @param/@return docblocks from the new
helpers. Nothing else in this repo documents that way: @since appears
nowhere on master and the services carry only a handful of @param lines
between them. Introducing the convention in one PR makes the codebase
less consistent, not more. The GMTU add-on is the opposite case, it uses
@since almost everywhere, so its docblocks stay as they are.

The substance of those docblocks is kept as plain comments, because the
non-obvious parts still need saying: that listPeople costs one OAuth
exchange per page, that a tag the person does not have counts as
success, and that Mailchimp's 404 is read from the exception rather than
pre-checked.

Log messages now name the service that failed, matching how the rest of
ZetkinService already words them. "Could not tag person 42 with tag 7"
becomes "... in Zetkin".

The two Mailchimp failure paths were worded identically, so a log could
not tell them apart. One is now "Mailchimp rejected ..." for an API
rejection carrying a response body, the other "Could not reach Mailchimp
..." for a call that never completed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../src/Services/MailchimpService.php         | 70 +++++-----------
 .../join-block/src/Services/ZetkinService.php | 81 ++++---------------
 2 files changed, 36 insertions(+), 115 deletions(-)

diff --git a/packages/join-block/src/Services/MailchimpService.php b/packages/join-block/src/Services/MailchimpService.php
index 24ace91..d8e57c6 100644
--- a/packages/join-block/src/Services/MailchimpService.php
+++ b/packages/join-block/src/Services/MailchimpService.php
@@ -255,70 +255,33 @@ public static function memberExists($email)
         }
     }
 
-    /**
-     * Is Mailchimp worth talking to at all?
-     *
-     * A bulk job asks this once at the start rather than discovering the
-     * answer member by member.
-     *
-     * @since 1.4.39
-     *
-     * @return bool True when both an API key and an audience are configured.
-     */
+    // A bulk job asks this once at the start rather than discovering the
+    // answer member by member.
     public static function isConfigured()
     {
         return !empty(Settings::get("MAILCHIMP_API_KEY"))
             && !empty(Settings::get("MAILCHIMP_AUDIENCE_ID"));
     }
 
-    /**
-     * Apply a tag to an audience member, reporting what happened.
-     *
-     * Unlike addTag(), this returns a status rather than throwing, because a
-     * bulk run has to carry on past one bad member and account for it at the
-     * end.
-     *
-     * @since 1.4.39
-     *
-     * @param string      $email
-     * @param string      $tag
-     * @param object|null $client Injected Mailchimp client, for testing.
-     * @return string 'ok', 'not_found', 'not_configured' or 'error'.
-     */
+    // Unlike addTag(), returns a status rather than throwing, because a bulk
+    // run has to carry on past one bad member and account for it at the end.
+    // Returns 'ok', 'not_found', 'not_configured' or 'error'.
+    // $client is injectable so this is testable without network access.
     public static function addTagToMember($email, $tag, $client = null)
     {
         return self::setMemberTagStatus($email, $tag, 'active', $client);
     }
 
-    /**
-     * Take a tag off an audience member, reporting what happened.
-     *
-     * @since 1.4.39
-     *
-     * @param string      $email
-     * @param string      $tag
-     * @param object|null $client Injected Mailchimp client, for testing.
-     * @return string 'ok', 'not_found', 'not_configured' or 'error'.
-     */
     public static function removeTagFromMember($email, $tag, $client = null)
     {
         return self::setMemberTagStatus($email, $tag, 'inactive', $client);
     }
 
-    /**
-     * Single implementation of "set this tag to this status on this member".
-     *
-     * Mailchimp has no separate remove call; a tag is switched between active
-     * and inactive. A member who is not in the audience comes back as a 404,
-     * which is a reportable outcome rather than a failure, so this reads the
-     * status out of the exception instead of pre-checking with memberExists().
-     * That also halves the API calls per member, which matters across a
-     * whole-membership walk.
-     *
-     * @since 1.4.39
-     *
-     * @return string 'ok', 'not_found', 'not_configured' or 'error'.
-     */
+    // Mailchimp has no separate remove call; a tag is switched between active
+    // and inactive. A member who is not in the audience comes back as a 404,
+    // which is a reportable outcome rather than a failure, so read the status
+    // out of the exception instead of pre-checking with memberExists(). That
+    // also halves the API calls per member across a whole-membership walk.
     private static function setMemberTagStatus($email, $tag, $status, $client = null)
     {
         global $joinBlockLog;
@@ -346,10 +309,17 @@ private static function setMemberTagStatus($email, $tag, $status, $client = null
                 return 'not_found';
             }
 
-            $joinBlockLog->error("Failed to set tag '$tag' to $status for $email in Mailchimp: " . $body);
+            $joinBlockLog->error(
+                "Mailchimp rejected setting tag '$tag' to $status for $email: " . $body
+            );
             return 'error';
         } catch (\Throwable $e) {
-            $joinBlockLog->error("Failed to set tag '$tag' to $status for $email in Mailchimp: " . $e->getMessage());
+            // Not a Mailchimp API rejection: the call never completed. Worded
+            // differently from the branch above so the two are distinguishable
+            // in the logs.
+            $joinBlockLog->error(
+                "Could not reach Mailchimp to set tag '$tag' to $status for $email: " . $e->getMessage()
+            );
             return 'error';
         }
     }
diff --git a/packages/join-block/src/Services/ZetkinService.php b/packages/join-block/src/Services/ZetkinService.php
index 4d20947..7844058 100644
--- a/packages/join-block/src/Services/ZetkinService.php
+++ b/packages/join-block/src/Services/ZetkinService.php
@@ -176,13 +176,13 @@ private static function addPerson($baseUrl, $orgId, $clientId, $clientSecret, $j
 
             foreach ($addTagIds as $tagId) {
                 if (self::putPersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId) !== 'ok') {
-                    $joinBlockLog->error("Could not tag person $personId with tag $tagId");
+                    $joinBlockLog->error("Could not tag person $personId with tag $tagId in Zetkin");
                 }
             }
 
             foreach ($removeTagIds as $tagId) {
                 if (self::deletePersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId) === 'error') {
-                    $joinBlockLog->error("Could not untag person $personId of tag $tagId");
+                    $joinBlockLog->error("Could not remove tag $tagId from person $personId in Zetkin");
                 }
             }
         } catch (\GuzzleHttp\Exception\RequestException $e) {
@@ -407,26 +407,11 @@ private static function getZetkinContext()
         ];
     }
 
-    /**
-     * List people in the organisation, one page at a time.
-     *
-     * Intended for bulk maintenance jobs that need to walk the whole
-     * membership, rather than the per-signup path. Zetkin paginates with `p`
-     * (zero-indexed page) and `pp` (page size); an empty array means the end
-     * of the list has been reached.
-     *
-     * Note that each call opens its own Zetkin context, so a walk over the
-     * full membership costs one OAuth exchange per page. That is deliberate:
-     * it keeps this consistent with the other standalone helpers below, and
-     * bulk jobs are expected to be occasional.
-     *
-     * Only available when OAuth credentials (CLIENT_ID, CLIENT_SECRET, JWT)
-     * are configured.
-     *
-     * @param int $page Zero-indexed page number.
-     * @param int $perPage Records per page.
-     * @return array List of person records, empty when exhausted or unconfigured.
-     */
+    // List people one page at a time, for bulk jobs that walk the whole
+    // membership. Zetkin pages with p (zero-indexed) and pp (page size);
+    // an empty array means the end of the list.
+    // Each call opens its own Zetkin context, so a full walk costs one OAuth
+    // exchange per page. Acceptable because bulk jobs are occasional.
     public static function listPeople($page = 0, $perPage = 100)
     {
         $zetkinContext = self::getZetkinContext();
@@ -445,18 +430,12 @@ public static function listPeople($page = 0, $perPage = 100)
         $responseData = json_decode($response->getBody()->getContents(), true);
 
         if (!empty($responseData["error"])) {
-            throw new \Exception("Could not list people: " . json_encode($responseData["error"]));
+            throw new \Exception("Could not list people in Zetkin: " . json_encode($responseData["error"]));
         }
 
         return $responseData["data"] ?? [];
     }
 
-    /**
-     * Get the tags currently applied to one person.
-     *
-     * @param int|string $personId
-     * @return array List of tag records, each with at least id and title.
-     */
     public static function getPersonTags($personId)
     {
         $zetkinContext = self::getZetkinContext();
@@ -475,21 +454,14 @@ public static function getPersonTags($personId)
         $responseData = json_decode($response->getBody()->getContents(), true);
 
         if (!empty($responseData["error"])) {
-            throw new \Exception("Could not get tags for person $personId: " . json_encode($responseData["error"]));
+            throw new \Exception("Could not get tags for person $personId in Zetkin: " . json_encode($responseData["error"]));
         }
 
         return $responseData["data"] ?? [];
     }
 
-    /**
-     * Look up a tag by title, creating it if it does not exist yet.
-     *
-     * Public wrapper over the same find-or-create the signup path uses, so
-     * bulk jobs tag people with exactly the same tags a signup would.
-     *
-     * @param string $title
-     * @return array|null The tag record, or null if Zetkin is not configured.
-     */
+    // Public wrapper over the same find-or-create the signup path uses, so
+    // bulk jobs tag people with exactly the tags a signup would.
     public static function findOrCreateTagByTitle($title)
     {
         $zetkinContext = self::getZetkinContext();
@@ -504,13 +476,6 @@ public static function findOrCreateTagByTitle($title)
         return self::findOrCreateTag($baseUrl, $orgId, $existingTags, $title, $accessToken);
     }
 
-    /**
-     * Apply an already-resolved tag to an already-resolved person.
-     *
-     * @param int|string $personId
-     * @param int|string $tagId
-     * @return bool True if the tag was applied.
-     */
     public static function addTagToPerson($personId, $tagId)
     {
         $zetkinContext = self::getZetkinContext();
@@ -523,15 +488,7 @@ public static function addTagToPerson($personId, $tagId)
         return self::putPersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId) === 'ok';
     }
 
-    /**
-     * Remove an already-resolved tag from an already-resolved person.
-     *
-     * A tag the person does not have is treated as success, not an error.
-     *
-     * @param int|string $personId
-     * @param int|string $tagId
-     * @return bool True if the person no longer has the tag.
-     */
+    // A tag the person does not have is treated as success, not an error.
     public static function removeTagFromPerson($personId, $tagId)
     {
         $zetkinContext = self::getZetkinContext();
@@ -544,11 +501,8 @@ public static function removeTagFromPerson($personId, $tagId)
         return self::deletePersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId) !== 'error';
     }
 
-    /**
-     * Single implementation of "apply this tag to this person".
-     *
-     * @return string 'ok' or 'error'. Callers add their own context to the log.
-     */
+    // Single implementation of "apply this tag to this person".
+    // Returns 'ok' or 'error'; callers add their own context to the log.
     private static function putPersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId)
     {
         $response = $client->request("PUT", "$baseUrl/orgs/$orgId/people/$personId/tags/$tagId", [
@@ -562,11 +516,8 @@ private static function putPersonTag($client, $baseUrl, $orgId, $accessToken, $p
         return empty($responseData["error"]) ? 'ok' : 'error';
     }
 
-    /**
-     * Single implementation of "take this tag off this person".
-     *
-     * @return string 'ok', 'missing' when the person did not have the tag, or 'error'.
-     */
+    // Single implementation of "take this tag off this person".
+    // Returns 'ok', 'missing' when the person did not have the tag, or 'error'.
     private static function deletePersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId)
     {
         $response = $client->request("DELETE", "$baseUrl/orgs/$orgId/people/$personId/tags/$tagId", [

From dbf81453aba359c96d77e075fa5e3ef4bce5ae21 Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 12:00:58 +0100
Subject: [PATCH 06/14] Name the Mailchimp tag outcomes instead of returning
 loose strings

Review feedback on #118.

I argued against this on the thread on the grounds that it would be a
convention introduced in one PR. That was wrong: I checked for enums and
concluded from their absence, without checking for constants. Both repos
already do exactly this. Settings has GET_ADDRESS_IO and IDEAL_POSTCODES,
JoinService has CRM_RETRY_OPTION_PREFIX, and the GMTU add-on models
membership standing as STANDING_GOOD, STANDING_LAPSING and so on. So this
matches existing practice rather than starting something.

MailchimpService::TAG_OK, TAG_NOT_FOUND, TAG_NOT_CONFIGURED and
TAG_ERROR. The values are unchanged, so nothing on the wire moves.

These cross a plugin boundary into the GMTU add-on, which compares
against them, so a value change there breaks re-tagging silently rather
than loudly. testStatusValuesAreStable pins the values for that reason:
renaming a constant is free, changing what it holds is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../src/Services/MailchimpService.php         | 20 +++++++++-----
 .../tests/MailchimpServiceTagsTest.php        | 27 ++++++++++++++-----
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/packages/join-block/src/Services/MailchimpService.php b/packages/join-block/src/Services/MailchimpService.php
index d8e57c6..a0195cd 100644
--- a/packages/join-block/src/Services/MailchimpService.php
+++ b/packages/join-block/src/Services/MailchimpService.php
@@ -13,6 +13,14 @@
 
 class MailchimpService
 {
+    // Outcomes of a tag write, returned by addTagToMember and
+    // removeTagFromMember. These cross into the GMTU add-on, which compares
+    // against them, so they are named rather than left as loose strings.
+    public const TAG_OK = 'ok';
+    public const TAG_NOT_FOUND = 'not_found';
+    public const TAG_NOT_CONFIGURED = 'not_configured';
+    public const TAG_ERROR = 'error';
+
     public static function buildMergeFields(array $data): array
     {
         if ($data['isUpdateFlow']) {
@@ -265,7 +273,7 @@ public static function isConfigured()
 
     // Unlike addTag(), returns a status rather than throwing, because a bulk
     // run has to carry on past one bad member and account for it at the end.
-    // Returns 'ok', 'not_found', 'not_configured' or 'error'.
+    // Returns one of the TAG_* constants.
     // $client is injectable so this is testable without network access.
     public static function addTagToMember($email, $tag, $client = null)
     {
@@ -287,7 +295,7 @@ private static function setMemberTagStatus($email, $tag, $status, $client = null
         global $joinBlockLog;
 
         if (!self::isConfigured()) {
-            return 'not_configured';
+            return self::TAG_NOT_CONFIGURED;
         }
 
         $client = $client ?? self::getClient();
@@ -300,19 +308,19 @@ private static function setMemberTagStatus($email, $tag, $status, $client = null
                 $subscriberHash,
                 ["tags" => [["name" => $tag, "status" => $status]]]
             );
-            return 'ok';
+            return self::TAG_OK;
         } catch (\GuzzleHttp\Exception\ClientException $e) {
             $response = $e->getResponse();
             $body = $response ? $response->getBody()->getContents() : $e->getMessage();
 
             if (($response && $response->getStatusCode() === 404) || str_contains($body, "Resource Not Found")) {
-                return 'not_found';
+                return self::TAG_NOT_FOUND;
             }
 
             $joinBlockLog->error(
                 "Mailchimp rejected setting tag '$tag' to $status for $email: " . $body
             );
-            return 'error';
+            return self::TAG_ERROR;
         } catch (\Throwable $e) {
             // Not a Mailchimp API rejection: the call never completed. Worded
             // differently from the branch above so the two are distinguishable
@@ -320,7 +328,7 @@ private static function setMemberTagStatus($email, $tag, $status, $client = null
             $joinBlockLog->error(
                 "Could not reach Mailchimp to set tag '$tag' to $status for $email: " . $e->getMessage()
             );
-            return 'error';
+            return self::TAG_ERROR;
         }
     }
 
diff --git a/packages/join-block/tests/MailchimpServiceTagsTest.php b/packages/join-block/tests/MailchimpServiceTagsTest.php
index 7e6e598..4333f1c 100644
--- a/packages/join-block/tests/MailchimpServiceTagsTest.php
+++ b/packages/join-block/tests/MailchimpServiceTagsTest.php
@@ -110,7 +110,7 @@ public function testAddTagToMemberReportsOk(): void
 
         $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
 
-        $this->assertSame('ok', $result);
+        $this->assertSame(MailchimpService::TAG_OK, $result);
     }
 
     public function testAddTagToMemberSendsTheTagAsActive(): void
@@ -129,7 +129,7 @@ public function testRemoveTagFromMemberSendsTheTagAsInactive(): void
 
         $result = MailchimpService::removeTagFromMember('person@example.com', 'South Manchester', $client);
 
-        $this->assertSame('ok', $result);
+        $this->assertSame(MailchimpService::TAG_OK, $result);
         $call = $client->lists->calls[0];
         $this->assertSame(
             [['name' => 'South Manchester', 'status' => 'inactive']],
@@ -162,7 +162,7 @@ public function testUnknownMemberIsReportedAsNotFound(): void
 
         $result = MailchimpService::addTagToMember('ghost@example.com', 'Bury', $client);
 
-        $this->assertSame('not_found', $result);
+        $this->assertSame(MailchimpService::TAG_NOT_FOUND, $result);
     }
 
     public function testUnknownMemberIsReportedAsNotFoundWhenRemoving(): void
@@ -171,7 +171,7 @@ public function testUnknownMemberIsReportedAsNotFoundWhenRemoving(): void
 
         $result = MailchimpService::removeTagFromMember('ghost@example.com', 'Bury', $client);
 
-        $this->assertSame('not_found', $result);
+        $this->assertSame(MailchimpService::TAG_NOT_FOUND, $result);
     }
 
     /**
@@ -184,7 +184,7 @@ public function testOtherClientErrorsAreReportedAsError(): void
 
         $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
 
-        $this->assertSame('error', $result);
+        $this->assertSame(MailchimpService::TAG_ERROR, $result);
     }
 
     public function testUnexpectedFailuresAreReportedAsError(): void
@@ -193,7 +193,7 @@ public function testUnexpectedFailuresAreReportedAsError(): void
 
         $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
 
-        $this->assertSame('error', $result);
+        $this->assertSame(MailchimpService::TAG_ERROR, $result);
     }
 
     /**
@@ -219,6 +219,19 @@ public function testIsConfiguredIsFalseWithoutAnAudience(): void
         $this->assertFalse(MailchimpService::isConfigured());
     }
 
+    /**
+     * The GMTU add-on compares against these values across a plugin boundary,
+     * and its test fakes return them as literals. Renaming a constant is free;
+     * changing its value is not, so pin the wire values here.
+     */
+    public function testStatusValuesAreStable(): void
+    {
+        $this->assertSame('ok', MailchimpService::TAG_OK);
+        $this->assertSame('not_found', MailchimpService::TAG_NOT_FOUND);
+        $this->assertSame('not_configured', MailchimpService::TAG_NOT_CONFIGURED);
+        $this->assertSame('error', MailchimpService::TAG_ERROR);
+    }
+
     /**
      * With Mailchimp switched off, the helpers say so rather than
      * constructing a client against an empty key.
@@ -230,7 +243,7 @@ public function testTaggingIsSkippedWhenMailchimpIsNotConfigured(): void
 
         $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
 
-        $this->assertSame('not_configured', $result);
+        $this->assertSame(MailchimpService::TAG_NOT_CONFIGURED, $result);
         $this->assertSame([], $client->lists->calls);
     }
 }

From 33a675de39fccff7a0918907762cdf633126daaf Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 14:25:56 +0100
Subject: [PATCH 07/14] Build every Mailchimp tag write in one place

Review feedback from @joaquimds on #118, and he is right on both counts.

addTagToMember was not meaningfully distinguishable from addTag: same
operation, same target, same argument list. The axis that actually
differs is error policy, and the name said nothing about it. They are
now tryAddTag and tryRemoveTag, which name the difference.

The duplication was worse than the two methods he flagged.
updateListMemberTags was being constructed in four places in this file,
each repeating the audience lookup, the subscriber hash and the payload
shape. That is the hazard he identified: add a hook for extension
plugins and you have to remember all four, or the bulk path silently
stops honouring a filter the signup path honours.

There is now one private updateMemberTags that builds the call, and it
deliberately does not catch. Callers pick their error policy: addTag and
removeTag log and rethrow exactly as before, the try* pair translates to
a TAG_* status.

That direction matters. Catching in the primitive and returning a status
would force the throwing callers to synthesise a new exception, and any
external code doing catch (ClientException) would stop catching. My
first sketch had it the wrong way round.

No deprecation, no signature change, no behaviour change to anything
that already existed. signup() moves across separately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../src/Services/MailchimpService.php         | 86 ++++++++++---------
 .../tests/MailchimpServiceTagsTest.php        | 18 ++--
 2 files changed, 54 insertions(+), 50 deletions(-)

diff --git a/packages/join-block/src/Services/MailchimpService.php b/packages/join-block/src/Services/MailchimpService.php
index a0195cd..de8d7fa 100644
--- a/packages/join-block/src/Services/MailchimpService.php
+++ b/packages/join-block/src/Services/MailchimpService.php
@@ -13,9 +13,9 @@
 
 class MailchimpService
 {
-    // Outcomes of a tag write, returned by addTagToMember and
-    // removeTagFromMember. These cross into the GMTU add-on, which compares
-    // against them, so they are named rather than left as loose strings.
+    // Outcomes of a tag write, returned by tryAddTag and tryRemoveTag. These
+    // cross into the GMTU add-on, which compares against them, so they are
+    // named rather than left as loose strings.
     public const TAG_OK = 'ok';
     public const TAG_NOT_FOUND = 'not_found';
     public const TAG_NOT_CONFIGURED = 'not_configured';
@@ -271,18 +271,48 @@ public static function isConfigured()
             && !empty(Settings::get("MAILCHIMP_AUDIENCE_ID"));
     }
 
-    // Unlike addTag(), returns a status rather than throwing, because a bulk
-    // run has to carry on past one bad member and account for it at the end.
-    // Returns one of the TAG_* constants.
-    // $client is injectable so this is testable without network access.
-    public static function addTagToMember($email, $tag, $client = null)
+    // The one place a Mailchimp tag write is built. Everything that changes a
+    // member's tags goes through here: signup(), addTag(), removeTag() and the
+    // try* pair below. A hook that needs to see or alter tag writes therefore
+    // has one home rather than four.
+    //
+    // Deliberately does not catch. Callers pick their error policy: addTag and
+    // removeTag log and rethrow as they always have, the try* pair translates
+    // to a TAG_* status. Catching here and returning a status would force the
+    // throwing callers to invent a new exception, and external code catching
+    // ClientException would stop catching.
+    //
+    // $client is injectable so callers that already hold one avoid building a
+    // second, and so this is testable without network access.
+    private static function updateMemberTags($email, array $tagUpdates, $client = null)
     {
-        return self::setMemberTagStatus($email, $tag, 'active', $client);
+        if (empty($tagUpdates)) {
+            return;
+        }
+
+        $client = $client ?? self::getClient();
+        $mailchimp_audience_id = Settings::get("MAILCHIMP_AUDIENCE_ID");
+        $subscriberHash = md5(strtolower($email));
+
+        $client->lists->updateListMemberTags(
+            $mailchimp_audience_id,
+            $subscriberHash,
+            ["tags" => $tagUpdates]
+        );
     }
 
-    public static function removeTagFromMember($email, $tag, $client = null)
+    // Reporting counterparts to addTag and removeTag. Same operation, same
+    // target; the difference is error policy, which is what the name says. A
+    // bulk run has to carry on past one bad member and account for it at the
+    // end, so these return a TAG_* status rather than throwing.
+    public static function tryAddTag($email, $tag, $client = null)
     {
-        return self::setMemberTagStatus($email, $tag, 'inactive', $client);
+        return self::trySetTag($email, $tag, 'active', $client);
+    }
+
+    public static function tryRemoveTag($email, $tag, $client = null)
+    {
+        return self::trySetTag($email, $tag, 'inactive', $client);
     }
 
     // Mailchimp has no separate remove call; a tag is switched between active
@@ -290,7 +320,7 @@ public static function removeTagFromMember($email, $tag, $client = null)
     // which is a reportable outcome rather than a failure, so read the status
     // out of the exception instead of pre-checking with memberExists(). That
     // also halves the API calls per member across a whole-membership walk.
-    private static function setMemberTagStatus($email, $tag, $status, $client = null)
+    private static function trySetTag($email, $tag, $status, $client = null)
     {
         global $joinBlockLog;
 
@@ -298,16 +328,8 @@ private static function setMemberTagStatus($email, $tag, $status, $client = null
             return self::TAG_NOT_CONFIGURED;
         }
 
-        $client = $client ?? self::getClient();
-        $mailchimp_audience_id = Settings::get("MAILCHIMP_AUDIENCE_ID");
-        $subscriberHash = md5(strtolower($email));
-
         try {
-            $client->lists->updateListMemberTags(
-                $mailchimp_audience_id,
-                $subscriberHash,
-                ["tags" => [["name" => $tag, "status" => $status]]]
-            );
+            self::updateMemberTags($email, [["name" => $tag, "status" => $status]], $client);
             return self::TAG_OK;
         } catch (\GuzzleHttp\Exception\ClientException $e) {
             $response = $e->getResponse();
@@ -341,17 +363,8 @@ public static function addTag($email, $tag)
             return;
         }
 
-        $mailchimp = self::getClient();
-        $mailchimp_audience_id = Settings::get("MAILCHIMP_AUDIENCE_ID");
-
-        $subscriberHash = md5(strtolower($email));
-
         try {
-            $mailchimp->lists->updateListMemberTags(
-                $mailchimp_audience_id,
-                $subscriberHash,
-                ["tags" => [["name" => $tag, "status" => "active"]]]
-            );
+            self::updateMemberTags($email, [["name" => $tag, "status" => "active"]]);
             $joinBlockLog->info("Added tag '$tag' to $email in Mailchimp");
         } catch (\GuzzleHttp\Exception\ClientException $e) {
             $joinBlockLog->error("Failed to add tag '$tag' to $email in Mailchimp: " . $e->getMessage());
@@ -368,17 +381,8 @@ public static function removeTag($email, $tag)
             return;
         }
 
-        $mailchimp = self::getClient();
-        $mailchimp_audience_id = Settings::get("MAILCHIMP_AUDIENCE_ID");
-
-        $subscriberHash = md5(strtolower($email));
-
         try {
-            $mailchimp->lists->updateListMemberTags(
-                $mailchimp_audience_id,
-                $subscriberHash,
-                ["tags" => [["name" => $tag, "status" => "inactive"]]]
-            );
+            self::updateMemberTags($email, [["name" => $tag, "status" => "inactive"]]);
             $joinBlockLog->info("Removed tag '$tag' from $email in Mailchimp");
         } catch (\GuzzleHttp\Exception\ClientException $e) {
             $joinBlockLog->error("Failed to remove tag '$tag' from $email in Mailchimp: " . $e->getMessage());
diff --git a/packages/join-block/tests/MailchimpServiceTagsTest.php b/packages/join-block/tests/MailchimpServiceTagsTest.php
index 4333f1c..627ab61 100644
--- a/packages/join-block/tests/MailchimpServiceTagsTest.php
+++ b/packages/join-block/tests/MailchimpServiceTagsTest.php
@@ -108,7 +108,7 @@ public function testAddTagToMemberReportsOk(): void
     {
         $client = $this->fakeClient();
 
-        $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
+        $result = MailchimpService::tryAddTag('person@example.com', 'Bury', $client);
 
         $this->assertSame(MailchimpService::TAG_OK, $result);
     }
@@ -117,7 +117,7 @@ public function testAddTagToMemberSendsTheTagAsActive(): void
     {
         $client = $this->fakeClient();
 
-        MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
+        MailchimpService::tryAddTag('person@example.com', 'Bury', $client);
 
         $call = $client->lists->calls[0];
         $this->assertSame([['name' => 'Bury', 'status' => 'active']], $call['body']['tags']);
@@ -127,7 +127,7 @@ public function testRemoveTagFromMemberSendsTheTagAsInactive(): void
     {
         $client = $this->fakeClient();
 
-        $result = MailchimpService::removeTagFromMember('person@example.com', 'South Manchester', $client);
+        $result = MailchimpService::tryRemoveTag('person@example.com', 'South Manchester', $client);
 
         $this->assertSame(MailchimpService::TAG_OK, $result);
         $call = $client->lists->calls[0];
@@ -145,7 +145,7 @@ public function testMemberIsAddressedByLowercasedEmailHash(): void
     {
         $client = $this->fakeClient();
 
-        MailchimpService::addTagToMember('Person@Example.COM', 'Bury', $client);
+        MailchimpService::tryAddTag('Person@Example.COM', 'Bury', $client);
 
         $call = $client->lists->calls[0];
         $this->assertSame(md5('person@example.com'), $call['subscriberHash']);
@@ -160,7 +160,7 @@ public function testUnknownMemberIsReportedAsNotFound(): void
     {
         $client = $this->fakeClient($this->clientException(404, '{"title":"Resource Not Found"}'));
 
-        $result = MailchimpService::addTagToMember('ghost@example.com', 'Bury', $client);
+        $result = MailchimpService::tryAddTag('ghost@example.com', 'Bury', $client);
 
         $this->assertSame(MailchimpService::TAG_NOT_FOUND, $result);
     }
@@ -169,7 +169,7 @@ public function testUnknownMemberIsReportedAsNotFoundWhenRemoving(): void
     {
         $client = $this->fakeClient($this->clientException(404, '{"title":"Resource Not Found"}'));
 
-        $result = MailchimpService::removeTagFromMember('ghost@example.com', 'Bury', $client);
+        $result = MailchimpService::tryRemoveTag('ghost@example.com', 'Bury', $client);
 
         $this->assertSame(MailchimpService::TAG_NOT_FOUND, $result);
     }
@@ -182,7 +182,7 @@ public function testOtherClientErrorsAreReportedAsError(): void
     {
         $client = $this->fakeClient($this->clientException(403, '{"title":"API Key Invalid"}'));
 
-        $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
+        $result = MailchimpService::tryAddTag('person@example.com', 'Bury', $client);
 
         $this->assertSame(MailchimpService::TAG_ERROR, $result);
     }
@@ -191,7 +191,7 @@ public function testUnexpectedFailuresAreReportedAsError(): void
     {
         $client = $this->fakeClient(new \RuntimeException('connection reset'));
 
-        $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
+        $result = MailchimpService::tryAddTag('person@example.com', 'Bury', $client);
 
         $this->assertSame(MailchimpService::TAG_ERROR, $result);
     }
@@ -241,7 +241,7 @@ public function testTaggingIsSkippedWhenMailchimpIsNotConfigured(): void
         unset($_ENV['MAILCHIMP_API_KEY']);
         $client = $this->fakeClient();
 
-        $result = MailchimpService::addTagToMember('person@example.com', 'Bury', $client);
+        $result = MailchimpService::tryAddTag('person@example.com', 'Bury', $client);
 
         $this->assertSame(MailchimpService::TAG_NOT_CONFIGURED, $result);
         $this->assertSame([], $client->lists->calls);

From 28880192de78bd4baa278cbe9676566f52e918ec Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 14:27:03 +0100
Subject: [PATCH 08/14] Route signup's tag update through the same primitive

The last of the four construction sites. This is the one where behaviour
could have shifted, so it is on its own commit to make the diff easy to
read.

Everything the call depends on is unchanged: the audience comes from the
same Settings lookup, the subscriber hash is the same md5 of the
lowercased email, the payload is the array signup already built, and the
existing client is passed in rather than a second one being made. It
stays inside signup's try/catch, which swallows a ClientException on the
grounds that tag updates are not critical for an existing member.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 packages/join-block/src/Services/MailchimpService.php | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/packages/join-block/src/Services/MailchimpService.php b/packages/join-block/src/Services/MailchimpService.php
index de8d7fa..47179ca 100644
--- a/packages/join-block/src/Services/MailchimpService.php
+++ b/packages/join-block/src/Services/MailchimpService.php
@@ -154,7 +154,6 @@ public static function signup($data)
         // For new members, we need to remove tags via updateListMemberTags (can't do it in addListMember)
         if ($memberExists || !empty($removeTags)) {
             try {
-                $subscriberHash = md5(strtolower($email));
                 $tagUpdates = [];
 
                 // If member exists, add tags that weren't added during creation
@@ -172,11 +171,9 @@ public static function signup($data)
                 }
 
                 if (!empty($tagUpdates)) {
-                    $mailchimp->lists->updateListMemberTags(
-                        $mailchimp_audience_id,
-                        $subscriberHash,
-                        ["tags" => $tagUpdates]
-                    );
+                    // Pass the client we already built rather than letting the
+                    // primitive make a second one.
+                    self::updateMemberTags($email, $tagUpdates, $mailchimp);
                     $joinBlockLog->info("Updated tags for $email in Mailchimp");
                 }
             } catch (\GuzzleHttp\Exception\ClientException $e) {

From 8b0324de40b7e0db861d13434dd5fad4c755d356 Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 14:28:58 +0100
Subject: [PATCH 09/14] Cut the commentary back to what the code needs

The comments on the new helpers had drifted into narrating the review
discussion rather than explaining the code. Why the primitive does not
catch, what would break if it did, why one log message is worded
differently from another: that belongs in the pull request, not in the
file.

What is left is the bits a reader cannot get from the code itself: that
this is the single place a tag write is built, that Mailchimp switches a
tag between active and inactive rather than deleting it, and that
listPeople costs an OAuth exchange per page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../src/Services/MailchimpService.php         | 38 ++++---------------
 .../join-block/src/Services/ZetkinService.php |  8 ++--
 2 files changed, 10 insertions(+), 36 deletions(-)

diff --git a/packages/join-block/src/Services/MailchimpService.php b/packages/join-block/src/Services/MailchimpService.php
index 47179ca..c2ca025 100644
--- a/packages/join-block/src/Services/MailchimpService.php
+++ b/packages/join-block/src/Services/MailchimpService.php
@@ -13,9 +13,7 @@
 
 class MailchimpService
 {
-    // Outcomes of a tag write, returned by tryAddTag and tryRemoveTag. These
-    // cross into the GMTU add-on, which compares against them, so they are
-    // named rather than left as loose strings.
+    // Outcomes of a tag write, returned by tryAddTag and tryRemoveTag.
     public const TAG_OK = 'ok';
     public const TAG_NOT_FOUND = 'not_found';
     public const TAG_NOT_CONFIGURED = 'not_configured';
@@ -171,8 +169,6 @@ public static function signup($data)
                 }
 
                 if (!empty($tagUpdates)) {
-                    // Pass the client we already built rather than letting the
-                    // primitive make a second one.
                     self::updateMemberTags($email, $tagUpdates, $mailchimp);
                     $joinBlockLog->info("Updated tags for $email in Mailchimp");
                 }
@@ -260,27 +256,14 @@ public static function memberExists($email)
         }
     }
 
-    // A bulk job asks this once at the start rather than discovering the
-    // answer member by member.
     public static function isConfigured()
     {
         return !empty(Settings::get("MAILCHIMP_API_KEY"))
             && !empty(Settings::get("MAILCHIMP_AUDIENCE_ID"));
     }
 
-    // The one place a Mailchimp tag write is built. Everything that changes a
-    // member's tags goes through here: signup(), addTag(), removeTag() and the
-    // try* pair below. A hook that needs to see or alter tag writes therefore
-    // has one home rather than four.
-    //
-    // Deliberately does not catch. Callers pick their error policy: addTag and
-    // removeTag log and rethrow as they always have, the try* pair translates
-    // to a TAG_* status. Catching here and returning a status would force the
-    // throwing callers to invent a new exception, and external code catching
-    // ClientException would stop catching.
-    //
-    // $client is injectable so callers that already hold one avoid building a
-    // second, and so this is testable without network access.
+    // The one place a Mailchimp tag write is built. Does not catch: callers
+    // pick their own error policy.
     private static function updateMemberTags($email, array $tagUpdates, $client = null)
     {
         if (empty($tagUpdates)) {
@@ -298,10 +281,8 @@ private static function updateMemberTags($email, array $tagUpdates, $client = nu
         );
     }
 
-    // Reporting counterparts to addTag and removeTag. Same operation, same
-    // target; the difference is error policy, which is what the name says. A
-    // bulk run has to carry on past one bad member and account for it at the
-    // end, so these return a TAG_* status rather than throwing.
+    // Reporting counterparts to addTag and removeTag: return a TAG_* status
+    // rather than throwing, so a bulk run can carry on and account for it.
     public static function tryAddTag($email, $tag, $client = null)
     {
         return self::trySetTag($email, $tag, 'active', $client);
@@ -313,10 +294,8 @@ public static function tryRemoveTag($email, $tag, $client = null)
     }
 
     // Mailchimp has no separate remove call; a tag is switched between active
-    // and inactive. A member who is not in the audience comes back as a 404,
-    // which is a reportable outcome rather than a failure, so read the status
-    // out of the exception instead of pre-checking with memberExists(). That
-    // also halves the API calls per member across a whole-membership walk.
+    // and inactive. A member missing from the audience comes back as a 404,
+    // which is reportable rather than a failure.
     private static function trySetTag($email, $tag, $status, $client = null)
     {
         global $joinBlockLog;
@@ -341,9 +320,6 @@ private static function trySetTag($email, $tag, $status, $client = null)
             );
             return self::TAG_ERROR;
         } catch (\Throwable $e) {
-            // Not a Mailchimp API rejection: the call never completed. Worded
-            // differently from the branch above so the two are distinguishable
-            // in the logs.
             $joinBlockLog->error(
                 "Could not reach Mailchimp to set tag '$tag' to $status for $email: " . $e->getMessage()
             );
diff --git a/packages/join-block/src/Services/ZetkinService.php b/packages/join-block/src/Services/ZetkinService.php
index 7844058..058a84e 100644
--- a/packages/join-block/src/Services/ZetkinService.php
+++ b/packages/join-block/src/Services/ZetkinService.php
@@ -407,11 +407,9 @@ private static function getZetkinContext()
         ];
     }
 
-    // List people one page at a time, for bulk jobs that walk the whole
-    // membership. Zetkin pages with p (zero-indexed) and pp (page size);
-    // an empty array means the end of the list.
-    // Each call opens its own Zetkin context, so a full walk costs one OAuth
-    // exchange per page. Acceptable because bulk jobs are occasional.
+    // Zetkin pages with p (zero-indexed) and pp (page size); an empty array
+    // means the end of the list. Each call opens its own Zetkin context, so a
+    // full walk costs one OAuth exchange per page.
     public static function listPeople($page = 0, $perPage = 100)
     {
         $zetkinContext = self::getZetkinContext();

From 809b59876d9447e3d142638616c603391e0526d8 Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 14:47:31 +0100
Subject: [PATCH 10/14] Collapse addTag and removeTag onto one implementation

The previous commit unified where the API call is built but left the two
throwing methods as near-identical seventeen-line twins, differing only
in active/inactive and the wording of three log lines. That is the
duplication the review actually pointed at, and the fix for that shape
was already sitting ten lines above in trySetTag.

setTagOrThrow now holds it once and addTag and removeTag are two-line
wrappers, matching tryAddTag and tryRemoveTag. Four public methods, two
private error policies, one place the call is built.

Log wording changes, which is the one thing here that is not
behaviour-preserving. The messages were parameterised on the method name
and the preposition ("Added tag 'x' to", "Removed tag 'x' from"), and
threading those through would have cost more clarity than the
duplication did. They now read "Set Mailchimp tag 'x' to active for
<email>". Nothing asserts on these strings, but anyone grepping logs for
the old wording should know.

The exception contract is untouched: the original ClientException is
still rethrown, not re-wrapped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../src/Services/MailchimpService.php         | 31 +++++++------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/packages/join-block/src/Services/MailchimpService.php b/packages/join-block/src/Services/MailchimpService.php
index c2ca025..ff411a2 100644
--- a/packages/join-block/src/Services/MailchimpService.php
+++ b/packages/join-block/src/Services/MailchimpService.php
@@ -327,39 +327,32 @@ private static function trySetTag($email, $tag, $status, $client = null)
         }
     }
 
-    public static function addTag($email, $tag)
+    // Throwing counterpart to trySetTag, for callers that want an exception.
+    private static function setTagOrThrow($email, $tag, $status)
     {
         global $joinBlockLog;
 
         if (!self::memberExists($email)) {
-            $joinBlockLog->warning("Skipping Mailchimp addTag('$tag') for $email: member does not exist");
+            $joinBlockLog->warning("Skipping Mailchimp tag update for $email: member does not exist");
             return;
         }
 
         try {
-            self::updateMemberTags($email, [["name" => $tag, "status" => "active"]]);
-            $joinBlockLog->info("Added tag '$tag' to $email in Mailchimp");
+            self::updateMemberTags($email, [["name" => $tag, "status" => $status]]);
+            $joinBlockLog->info("Set Mailchimp tag '$tag' to $status for $email");
         } catch (\GuzzleHttp\Exception\ClientException $e) {
-            $joinBlockLog->error("Failed to add tag '$tag' to $email in Mailchimp: " . $e->getMessage());
+            $joinBlockLog->error("Failed to set Mailchimp tag '$tag' to $status for $email: " . $e->getMessage());
             throw $e;
         }
     }
 
-    public static function removeTag($email, $tag)
+    public static function addTag($email, $tag)
     {
-        global $joinBlockLog;
-
-        if (!self::memberExists($email)) {
-            $joinBlockLog->warning("Skipping Mailchimp removeTag('$tag') for $email: member does not exist");
-            return;
-        }
+        self::setTagOrThrow($email, $tag, 'active');
+    }
 
-        try {
-            self::updateMemberTags($email, [["name" => $tag, "status" => "inactive"]]);
-            $joinBlockLog->info("Removed tag '$tag' from $email in Mailchimp");
-        } catch (\GuzzleHttp\Exception\ClientException $e) {
-            $joinBlockLog->error("Failed to remove tag '$tag' from $email in Mailchimp: " . $e->getMessage());
-            throw $e;
-        }
+    public static function removeTag($email, $tag)
+    {
+        self::setTagOrThrow($email, $tag, 'inactive');
     }
 }

From 361f4e88ffc8ee75c0957e2ae0bfa72df8c35a33 Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 15:09:16 +0100
Subject: [PATCH 11/14] Apply the same treatment to ZetkinService

The review points about MailchimpService applied here too, and the
duplication was worse.

Zetkin was searched for a person by email in four places:
findPersonByEmail, updatePerson, addTag and removeTag, each repeating
the POST, the error check and the filter down to exact email matches.
addTag and removeTag were otherwise near-identical forty-five line
twins. searchPeopleByEmail now holds the search, and setTagByEmail holds
the shared body, so addTag and removeTag are one-liners.

It returns every exact match rather than the first, because addTag and
removeTag tagged all of them and findPersonByEmail took the first.
Both behaviours are preserved, and nothing stops Zetkin holding
duplicate emails.

addTagToPerson and removeTagFromPerson are now tryAddTagToPerson and
tryRemoveTagFromPerson, matching the Mailchimp naming: the try prefix
marks the pair that reports rather than logging and swallowing. The
older names had the same problem as addTagToMember, in that addTag also
adds a tag to a person.

Log wording changes in the same way it did for Mailchimp: the messages
varied by verb and preposition and parameterising them all cost more
than the duplication did. Nothing asserts on them.

Net 58 lines removed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../join-block/src/Services/ZetkinService.php | 136 +++++-------------
 1 file changed, 39 insertions(+), 97 deletions(-)

diff --git a/packages/join-block/src/Services/ZetkinService.php b/packages/join-block/src/Services/ZetkinService.php
index 058a84e..bbb4316 100644
--- a/packages/join-block/src/Services/ZetkinService.php
+++ b/packages/join-block/src/Services/ZetkinService.php
@@ -265,6 +265,14 @@ public static function findPersonByEmail($email)
             return null;
         }
 
+        return self::searchPeopleByEmail($zetkinContext, $email)[0] ?? null;
+    }
+
+    // The one place Zetkin is searched for a person. Zetkin's search is fuzzy,
+    // so the results are filtered down to exact email matches. More than one
+    // can come back, because nothing stops Zetkin holding duplicates.
+    private static function searchPeopleByEmail($zetkinContext, $email)
+    {
         ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken, 'client' => $client] = $zetkinContext;
 
         $response = $client->request("POST", "$baseUrl/orgs/$orgId/search/person", [
@@ -281,13 +289,8 @@ public static function findPersonByEmail($email)
         }
 
         $people = $responseData["data"] ?? [];
-        foreach ($people as $candidate) {
-            if ($candidate["email"] === $email) {
-                return $candidate;
-            }
-        }
 
-        return null;
+        return array_values(array_filter($people, fn($p) => $p["email"] === $email));
     }
 
     /**
@@ -311,27 +314,7 @@ public static function updatePerson($email, $personData, $previousEmail = null)
 
         try {
             $searchEmail = $previousEmail ?? $email;
-            $response = $client->request("POST", "$baseUrl/orgs/$orgId/search/person", [
-                "headers" => [
-                    "Authorization" => "Bearer {$accessToken}",
-                    "Content-type" => "application/json",
-                ],
-                "json" => ["q" => $searchEmail],
-            ]);
-            $responseData = json_decode($response->getBody()->getContents(), true);
-
-            if (!empty($responseData["error"])) {
-                throw new \Exception(json_encode($responseData["error"]));
-            }
-
-            $people = $responseData["data"] ?? [];
-            $person = null;
-            foreach ($people as $candidate) {
-                if ($candidate["email"] === $searchEmail) {
-                    $person = $candidate;
-                    break;
-                }
-            }
+            $person = self::searchPeopleByEmail($zetkinContext, $searchEmail)[0] ?? null;
 
             if (!$person) {
                 $joinBlockLog->warning("Cannot update person in Zetkin - no person found with email $searchEmail");
@@ -474,7 +457,7 @@ public static function findOrCreateTagByTitle($title)
         return self::findOrCreateTag($baseUrl, $orgId, $existingTags, $title, $accessToken);
     }
 
-    public static function addTagToPerson($personId, $tagId)
+    public static function tryAddTagToPerson($personId, $tagId)
     {
         $zetkinContext = self::getZetkinContext();
         if (!$zetkinContext) {
@@ -487,7 +470,7 @@ public static function addTagToPerson($personId, $tagId)
     }
 
     // A tag the person does not have is treated as success, not an error.
-    public static function removeTagFromPerson($personId, $tagId)
+    public static function tryRemoveTagFromPerson($personId, $tagId)
     {
         $zetkinContext = self::getZetkinContext();
         if (!$zetkinContext) {
@@ -539,57 +522,27 @@ private static function deletePersonTag($client, $baseUrl, $orgId, $accessToken,
      */
     public static function addTag($email, $tag)
     {
-        global $joinBlockLog;
-        try {
-            $zetkinContext = self::getZetkinContext();
-            if (!$zetkinContext) {
-                return;
-            }
-
-            ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken, 'client' => $client] = $zetkinContext;
-
-            $response = $client->request("POST", "$baseUrl/orgs/$orgId/search/person", [
-                "headers" => [
-                    "Authorization" => "Bearer {$accessToken}",
-                    "Content-type" => "application/json",
-                ],
-                "json" => [
-                    "q" => $email,
-                ]
-            ]);
-            $responseData = json_decode($response->getBody()->getContents(), true);
-
-            if (!empty($responseData["error"])) {
-                throw new \Exception(json_encode($responseData["error"]));
-            }
-
-            $people = $responseData["data"] ?? [];
-            $matched = array_filter($people, fn($p) => $p["email"] === $email);
-            if (empty($matched)) {
-                $joinBlockLog->warning("Could not add tag '$tag' in Zetkin: no person found for $email");
-                return;
-            }
-            $existingTags = self::getTags($baseUrl, $orgId, $accessToken);
-            $existingTag = self::findOrCreateTag($baseUrl, $orgId, $existingTags, $tag, $accessToken);
-            foreach ($matched as $person) {
-                $result = self::putPersonTag($client, $baseUrl, $orgId, $accessToken, $person["id"], $existingTag["id"]);
-                if ($result === 'ok') {
-                    $joinBlockLog->info("Added tag '$tag' to $email in Zetkin");
-                } else {
-                    $joinBlockLog->error("Could not add tag '$tag' to $email in Zetkin");
-                }
-            }
-        } catch (\Exception $e) {
-            $joinBlockLog->error("Could not add tag '$tag' to $email in Zetkin: " . $e->getMessage());
-        }
+        self::setTagByEmail($email, $tag, false);
     }
 
     /**
      * Standalone function to find a person by email and remove a tag (string)
      */
     public static function removeTag($email, $tag)
+    {
+        self::setTagByEmail($email, $tag, true);
+    }
+
+    // Resolves the person and the tag title, then applies the change to every
+    // exact email match. Logs and swallows rather than throwing, which is what
+    // both callers of addTag and removeTag relied on.
+    private static function setTagByEmail($email, $tag, $remove)
     {
         global $joinBlockLog;
+
+        $verb = $remove ? 'remove' : 'add';
+        $done = $remove ? 'removed from' : 'added to';
+
         try {
             $zetkinContext = self::getZetkinContext();
             if (!$zetkinContext) {
@@ -598,41 +551,30 @@ public static function removeTag($email, $tag)
 
             ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken, 'client' => $client] = $zetkinContext;
 
-            $response = $client->request("POST", "$baseUrl/orgs/$orgId/search/person", [
-                "headers" => [
-                    "Authorization" => "Bearer {$accessToken}",
-                    "Content-type" => "application/json",
-                ],
-                "json" => [
-                    "q" => $email,
-                ]
-            ]);
-            $responseData = json_decode($response->getBody()->getContents(), true);
-
-            if (!empty($responseData["error"])) {
-                throw new \Exception(json_encode($responseData["error"]));
-            }
-
-            $people = $responseData["data"] ?? [];
-            $matched = array_filter($people, fn($p) => $p["email"] === $email);
+            $matched = self::searchPeopleByEmail($zetkinContext, $email);
             if (empty($matched)) {
-                $joinBlockLog->warning("Could not remove tag '$tag' in Zetkin: no person found for $email");
+                $joinBlockLog->warning("Could not $verb tag '$tag' in Zetkin: no person found for $email");
                 return;
             }
+
             $existingTags = self::getTags($baseUrl, $orgId, $accessToken);
             $existingTag = self::findOrCreateTag($baseUrl, $orgId, $existingTags, $tag, $accessToken);
+
             foreach ($matched as $person) {
-                $result = self::deletePersonTag($client, $baseUrl, $orgId, $accessToken, $person["id"], $existingTag["id"]);
-                if ($result === 'missing') {
-                    $joinBlockLog->info("Could not remove tag '$tag' from $email in Zetkin: tag does not exist");
-                } elseif ($result === 'error') {
-                    $joinBlockLog->error("Could not remove tag '$tag' from $email in Zetkin");
+                $result = $remove
+                    ? self::deletePersonTag($client, $baseUrl, $orgId, $accessToken, $person["id"], $existingTag["id"])
+                    : self::putPersonTag($client, $baseUrl, $orgId, $accessToken, $person["id"], $existingTag["id"]);
+
+                if ($result === 'error') {
+                    $joinBlockLog->error("Could not $verb tag '$tag' for $email in Zetkin");
+                } elseif ($result === 'missing') {
+                    $joinBlockLog->info("Tag '$tag' was not on $email in Zetkin");
                 } else {
-                    $joinBlockLog->info("Removed tag '$tag' from $email in Zetkin");
+                    $joinBlockLog->info("Tag '$tag' $done $email in Zetkin");
                 }
             }
         } catch (\Exception $e) {
-            $joinBlockLog->error("Could not remove tag '$tag' from $email in Zetkin: " . $e->getMessage());
+            $joinBlockLog->error("Could not $verb tag '$tag' for $email in Zetkin: " . $e->getMessage());
         }
     }
 

From e2333320da8a51dc14c008921f23d2600544ef20 Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 15:51:07 +0100
Subject: [PATCH 12/14] Make the tag paths testable and pin their behaviour

The email-based tag paths in both services run during joins and Stripe
webhooks but had no coverage, because getZetkinContext() performs a live
OAuth exchange and getClient() builds a real Mailchimp client. Every
refactor of them so far has been verified by reading the diff.

ZetkinService gains overrideZetkinContext(), a test seam that stands in
for the OAuth exchange. MailchimpService's addTag, removeTag and
memberExists gain the same optional injected client the try* pair
already had. getTags and findOrCreateTag now take the caller's client
instead of constructing a fresh Guzzle client per call, which they did
even when the caller was holding one.

Twenty regression tests pin what the callers rely on: addTag tags every
exact email match, not the first, and ignores Zetkin's fuzzy near
misses; a missing person is a warning and a no-op; an API failure is
logged and swallowed on Zetkin and rethrown as the original exception on
Mailchimp; removing an absent tag is a non-event; listPeople sends p and
pp; findOrCreateTagByTitle reuses an existing tag rather than creating a
duplicate.

The new Zetkin tests immediately caught a bug in this very change:
findOrCreateTagByTitle was not destructuring the client it now passes
along, which would have been a fatal on first use in production. That is
the argument for the seam in one sentence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 .../src/Services/MailchimpService.php         |  18 +-
 .../join-block/src/Services/ZetkinService.php |  36 +-
 .../tests/MailchimpServiceLegacyTagsTest.php  | 192 ++++++++++
 .../tests/ZetkinServiceTagsTest.php           | 345 ++++++++++++++++++
 4 files changed, 570 insertions(+), 21 deletions(-)
 create mode 100644 packages/join-block/tests/MailchimpServiceLegacyTagsTest.php
 create mode 100644 packages/join-block/tests/ZetkinServiceTagsTest.php

diff --git a/packages/join-block/src/Services/MailchimpService.php b/packages/join-block/src/Services/MailchimpService.php
index ff411a2..7d1ee59 100644
--- a/packages/join-block/src/Services/MailchimpService.php
+++ b/packages/join-block/src/Services/MailchimpService.php
@@ -236,11 +236,11 @@ public static function updateMember($email, $mergeFields, $previousEmail = null)
      * @param string $email
      * @return bool
      */
-    public static function memberExists($email)
+    public static function memberExists($email, $client = null)
     {
         global $joinBlockLog;
 
-        $mailchimp = self::getClient();
+        $mailchimp = $client ?? self::getClient();
         $mailchimp_audience_id = Settings::get("MAILCHIMP_AUDIENCE_ID");
         $subscriberHash = md5(strtolower($email));
 
@@ -328,17 +328,17 @@ private static function trySetTag($email, $tag, $status, $client = null)
     }
 
     // Throwing counterpart to trySetTag, for callers that want an exception.
-    private static function setTagOrThrow($email, $tag, $status)
+    private static function setTagOrThrow($email, $tag, $status, $client = null)
     {
         global $joinBlockLog;
 
-        if (!self::memberExists($email)) {
+        if (!self::memberExists($email, $client)) {
             $joinBlockLog->warning("Skipping Mailchimp tag update for $email: member does not exist");
             return;
         }
 
         try {
-            self::updateMemberTags($email, [["name" => $tag, "status" => $status]]);
+            self::updateMemberTags($email, [["name" => $tag, "status" => $status]], $client);
             $joinBlockLog->info("Set Mailchimp tag '$tag' to $status for $email");
         } catch (\GuzzleHttp\Exception\ClientException $e) {
             $joinBlockLog->error("Failed to set Mailchimp tag '$tag' to $status for $email: " . $e->getMessage());
@@ -346,13 +346,13 @@ private static function setTagOrThrow($email, $tag, $status)
         }
     }
 
-    public static function addTag($email, $tag)
+    public static function addTag($email, $tag, $client = null)
     {
-        self::setTagOrThrow($email, $tag, 'active');
+        self::setTagOrThrow($email, $tag, 'active', $client);
     }
 
-    public static function removeTag($email, $tag)
+    public static function removeTag($email, $tag, $client = null)
     {
-        self::setTagOrThrow($email, $tag, 'inactive');
+        self::setTagOrThrow($email, $tag, 'inactive', $client);
     }
 }
diff --git a/packages/join-block/src/Services/ZetkinService.php b/packages/join-block/src/Services/ZetkinService.php
index bbb4316..84f860e 100644
--- a/packages/join-block/src/Services/ZetkinService.php
+++ b/packages/join-block/src/Services/ZetkinService.php
@@ -12,6 +12,8 @@
 
 class ZetkinService
 {
+    private static $zetkinContextOverride = null;
+
     public static function signup($data)
     {
         global $joinBlockLog;
@@ -159,18 +161,18 @@ private static function addPerson($baseUrl, $orgId, $clientId, $clientSecret, $j
 
             $personId = $responseData["data"]["id"];
 
-            $existingTags = self::getTags($baseUrl, $orgId, $accessToken);
+            $existingTags = self::getTags($client, $baseUrl, $orgId, $accessToken);
 
             $addTags[] = "Unconfirmed";
             $addTagIds = [];
             foreach ($addTags as $tag) {
-                $existingTag = self::findOrCreateTag($baseUrl, $orgId, $existingTags, $tag, $accessToken);
+                $existingTag = self::findOrCreateTag($client, $baseUrl, $orgId, $existingTags, $tag, $accessToken);
                 $addTagIds[] = $existingTag["id"];
             }
 
             $removeTagIds = [];
             foreach ($removeTags as $tag) {
-                $existingTag = self::findOrCreateTag($baseUrl, $orgId, $existingTags, $tag, $accessToken);
+                $existingTag = self::findOrCreateTag($client, $baseUrl, $orgId, $existingTags, $tag, $accessToken);
                 $removeTagIds[] = $existingTag["id"];
             }
 
@@ -202,9 +204,8 @@ private static function addPerson($baseUrl, $orgId, $clientId, $clientSecret, $j
         }
     }
 
-    private static function getTags($baseUrl, $orgId, $accessToken)
+    private static function getTags($client, $baseUrl, $orgId, $accessToken)
     {
-        $client = new \GuzzleHttp\Client();
         $response = $client->request("GET", "$baseUrl/orgs/$orgId/people/tags", [
             "headers" => [
                 "Authorization" => "Bearer {$accessToken}",
@@ -219,7 +220,7 @@ private static function getTags($baseUrl, $orgId, $accessToken)
         return $responseData["data"] ?? [];
     }
 
-    private static function findOrCreateTag($baseUrl, $orgId, $tags, $title, $accessToken)
+    private static function findOrCreateTag($client, $baseUrl, $orgId, $tags, $title, $accessToken)
     {
         $matchingTags = array_filter($tags, function ($tag) use ($title) {
             return strtolower($tag['title']) === strtolower($title);
@@ -229,7 +230,6 @@ private static function findOrCreateTag($baseUrl, $orgId, $tags, $title, $access
             return array_values($matchingTags)[0];
         }
 
-        $client = new \GuzzleHttp\Client();
         $response = $client->request("POST", "$baseUrl/orgs/$orgId/people/tags", [
             "headers" => [
                 "Authorization" => "Bearer {$accessToken}",
@@ -362,10 +362,22 @@ public static function updatePerson($email, $personData, $previousEmail = null)
         }
     }
 
+    // Test seam: getZetkinContext() performs a live OAuth exchange, which
+    // makes everything built on it untestable. Pass null to restore normal
+    // behaviour.
+    public static function overrideZetkinContext($zetkinContext)
+    {
+        self::$zetkinContextOverride = $zetkinContext;
+    }
+
     private static function getZetkinContext()
     {
         global $joinBlockLog;
 
+        if (self::$zetkinContextOverride !== null) {
+            return self::$zetkinContextOverride;
+        }
+
         $clientId = Settings::get("ZETKIN_CLIENT_ID");
         $clientSecret = Settings::get("ZETKIN_CLIENT_SECRET");
         $jwt = Settings::get("ZETKIN_JWT");
@@ -450,11 +462,11 @@ public static function findOrCreateTagByTitle($title)
             return null;
         }
 
-        ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken] = $zetkinContext;
+        ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken, 'client' => $client] = $zetkinContext;
 
-        $existingTags = self::getTags($baseUrl, $orgId, $accessToken);
+        $existingTags = self::getTags($client, $baseUrl, $orgId, $accessToken);
 
-        return self::findOrCreateTag($baseUrl, $orgId, $existingTags, $title, $accessToken);
+        return self::findOrCreateTag($client, $baseUrl, $orgId, $existingTags, $title, $accessToken);
     }
 
     public static function tryAddTagToPerson($personId, $tagId)
@@ -557,8 +569,8 @@ private static function setTagByEmail($email, $tag, $remove)
                 return;
             }
 
-            $existingTags = self::getTags($baseUrl, $orgId, $accessToken);
-            $existingTag = self::findOrCreateTag($baseUrl, $orgId, $existingTags, $tag, $accessToken);
+            $existingTags = self::getTags($client, $baseUrl, $orgId, $accessToken);
+            $existingTag = self::findOrCreateTag($client, $baseUrl, $orgId, $existingTags, $tag, $accessToken);
 
             foreach ($matched as $person) {
                 $result = $remove
diff --git a/packages/join-block/tests/MailchimpServiceLegacyTagsTest.php b/packages/join-block/tests/MailchimpServiceLegacyTagsTest.php
new file mode 100644
index 0000000..be8fe96
--- /dev/null
+++ b/packages/join-block/tests/MailchimpServiceLegacyTagsTest.php
@@ -0,0 +1,192 @@
+<?php
+
+namespace CommonKnowledge\JoinBlock\Tests;
+
+use Brain\Monkey;
+use CommonKnowledge\JoinBlock\Services\MailchimpService;
+use GuzzleHttp\Exception\ClientException;
+use GuzzleHttp\Psr7\Request;
+use GuzzleHttp\Psr7\Response;
+use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Regression tests for the throwing Mailchimp tag pair, addTag and removeTag.
+ *
+ * These run in production during joins and Stripe webhooks but had no
+ * coverage until the client became injectable, so the behaviour their
+ * callers rely on is pinned here: the memberExists pre-check that turns a
+ * missing member into a warning rather than an API error, and the rethrow of
+ * the original exception, which both callers catch.
+ */
+class MailchimpServiceLegacyTagsTest extends TestCase
+{
+    use MockeryPHPUnitIntegration;
+
+    private object $logger;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        Monkey\setUp();
+        Monkey\Functions\when('carbon_get_theme_option')->justReturn(null);
+
+        $_ENV['MAILCHIMP_API_KEY'] = 'test-key-us1';
+        $_ENV['MAILCHIMP_AUDIENCE_ID'] = 'test-audience';
+
+        global $joinBlockLog;
+        $this->logger = new class {
+            public array $infos = [];
+            public array $warnings = [];
+            public array $errors = [];
+
+            // phpcs:ignore PSR1.Methods.CamelCapsMethodName.NotCamelCaps
+            public function info(string $msg, array $ctx = []): void
+            {
+                $this->infos[] = $msg;
+            }
+
+            // phpcs:ignore PSR1.Methods.CamelCapsMethodName.NotCamelCaps
+            public function warning(string $msg, array $ctx = []): void
+            {
+                $this->warnings[] = $msg;
+            }
+
+            // phpcs:ignore PSR1.Methods.CamelCapsMethodName.NotCamelCaps
+            public function error(string $msg, array $ctx = []): void
+            {
+                $this->errors[] = $msg;
+            }
+        };
+        $joinBlockLog = $this->logger;
+    }
+
+    protected function tearDown(): void
+    {
+        global $joinBlockLog;
+        $joinBlockLog = null;
+        unset($_ENV['MAILCHIMP_API_KEY'], $_ENV['MAILCHIMP_AUDIENCE_ID']);
+        Monkey\tearDown();
+        parent::tearDown();
+    }
+
+    /**
+     * A client whose member lookup and tag update can each be told to fail.
+     */
+    private function fakeClient(bool $memberExists = true, ?\Throwable $throwOnUpdate = null): object
+    {
+        $lists = new class ($memberExists, $throwOnUpdate) {
+            public array $updates = [];
+            public array $lookups = [];
+
+            public function __construct(private bool $memberExists, private ?\Throwable $throwOnUpdate)
+            {
+            }
+
+            public function getListMember($audienceId, $subscriberHash)
+            {
+                $this->lookups[] = [$audienceId, $subscriberHash];
+                if (!$this->memberExists) {
+                    throw new ClientException(
+                        'HTTP 404',
+                        new Request('GET', '/'),
+                        new Response(404, [], '{"title":"Resource Not Found","status":404}')
+                    );
+                }
+                return ['id' => $subscriberHash];
+            }
+
+            public function updateListMemberTags($audienceId, $subscriberHash, $body)
+            {
+                $this->updates[] = ['audienceId' => $audienceId, 'hash' => $subscriberHash, 'body' => $body];
+                if ($this->throwOnUpdate) {
+                    throw $this->throwOnUpdate;
+                }
+                return null;
+            }
+        };
+
+        return new class ($lists) {
+            public $lists;
+
+            public function __construct($lists)
+            {
+                $this->lists = $lists;
+            }
+        };
+    }
+
+    public function test_add_tag_sends_the_tag_as_active()
+    {
+        $client = $this->fakeClient();
+
+        MailchimpService::addTag('person@example.com', 'Bury', $client);
+
+        $this->assertCount(1, $client->lists->updates);
+        $this->assertSame(
+            [['name' => 'Bury', 'status' => 'active']],
+            $client->lists->updates[0]['body']['tags']
+        );
+        $this->assertSame([], $this->logger->errors);
+    }
+
+    public function test_remove_tag_sends_the_tag_as_inactive()
+    {
+        $client = $this->fakeClient();
+
+        MailchimpService::removeTag('person@example.com', 'Bury', $client);
+
+        $this->assertSame(
+            [['name' => 'Bury', 'status' => 'inactive']],
+            $client->lists->updates[0]['body']['tags']
+        );
+    }
+
+    /**
+     * A member who is not in the audience is a warning and a no-op, never an
+     * API call or an exception. Both callers depend on that.
+     */
+    public function test_add_tag_skips_a_member_who_is_not_in_the_audience()
+    {
+        $client = $this->fakeClient(memberExists: false);
+
+        MailchimpService::addTag('ghost@example.com', 'Bury', $client);
+
+        $this->assertSame([], $client->lists->updates);
+        $this->assertStringContainsString('member does not exist', $this->logger->warnings[0]);
+    }
+
+    /**
+     * The exception that reaches the caller must be the original, not a
+     * re-wrap. External code catching ClientException relies on it.
+     */
+    public function test_add_tag_rethrows_the_original_exception()
+    {
+        $original = new ClientException(
+            'HTTP 403',
+            new Request('POST', '/'),
+            new Response(403, [], '{"title":"Forbidden"}')
+        );
+        $client = $this->fakeClient(throwOnUpdate: $original);
+
+        $caught = null;
+        try {
+            MailchimpService::addTag('person@example.com', 'Bury', $client);
+        } catch (\Throwable $e) {
+            $caught = $e;
+        }
+
+        $this->assertSame($original, $caught);
+        $this->assertNotEmpty($this->logger->errors);
+    }
+
+    public function test_member_exists_is_true_for_a_known_member()
+    {
+        $this->assertTrue(MailchimpService::memberExists('person@example.com', $this->fakeClient()));
+    }
+
+    public function test_member_exists_is_false_for_a_404()
+    {
+        $this->assertFalse(MailchimpService::memberExists('ghost@example.com', $this->fakeClient(memberExists: false)));
+    }
+}
diff --git a/packages/join-block/tests/ZetkinServiceTagsTest.php b/packages/join-block/tests/ZetkinServiceTagsTest.php
new file mode 100644
index 0000000..a768cf3
--- /dev/null
+++ b/packages/join-block/tests/ZetkinServiceTagsTest.php
@@ -0,0 +1,345 @@
+<?php
+
+namespace CommonKnowledge\JoinBlock\Tests;
+
+use Brain\Monkey;
+use CommonKnowledge\JoinBlock\Services\ZetkinService;
+use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Regression tests for the Zetkin tag and person-lookup paths.
+ *
+ * These paths run in production during joins and Stripe webhooks but had no
+ * coverage: getZetkinContext() performs a live OAuth exchange, which made
+ * everything above it untestable. The context override seam replaces that
+ * exchange with a fake client, so the behaviour these paths have always had
+ * is now pinned rather than preserved by hope.
+ */
+class ZetkinServiceTagsTest extends TestCase
+{
+    use MockeryPHPUnitIntegration;
+
+    private object $logger;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        Monkey\setUp();
+        Monkey\Functions\when('carbon_get_theme_option')->justReturn(null);
+
+        global $joinBlockLog;
+        $this->logger = new class {
+            public array $infos = [];
+            public array $warnings = [];
+            public array $errors = [];
+
+            // phpcs:ignore PSR1.Methods.CamelCapsMethodName.NotCamelCaps
+            public function info(string $msg, array $ctx = []): void
+            {
+                $this->infos[] = $msg;
+            }
+
+            // phpcs:ignore PSR1.Methods.CamelCapsMethodName.NotCamelCaps
+            public function warning(string $msg, array $ctx = []): void
+            {
+                $this->warnings[] = $msg;
+            }
+
+            // phpcs:ignore PSR1.Methods.CamelCapsMethodName.NotCamelCaps
+            public function error(string $msg, array $ctx = []): void
+            {
+                $this->errors[] = $msg;
+            }
+        };
+        $joinBlockLog = $this->logger;
+    }
+
+    protected function tearDown(): void
+    {
+        ZetkinService::overrideZetkinContext(null);
+        global $joinBlockLog;
+        $joinBlockLog = null;
+        Monkey\tearDown();
+        parent::tearDown();
+    }
+
+    /**
+     * A client that routes requests by method and URL fragment, recording
+     * everything it is asked for.
+     */
+    private function fakeZetkin(array $routes): object
+    {
+        $client = new class ($routes) {
+            public array $requests = [];
+            private array $routes;
+
+            public function __construct(array $routes)
+            {
+                $this->routes = $routes;
+            }
+
+            public function request($method, $url, $options = [])
+            {
+                $this->requests[] = ['method' => $method, 'url' => $url, 'options' => $options];
+                foreach ($this->routes as [$routeMethod, $needle, $response]) {
+                    if ($routeMethod === $method && str_contains($url, $needle)) {
+                        return $response;
+                    }
+                }
+                throw new \RuntimeException("Unrouted request in test: $method $url");
+            }
+        };
+
+        ZetkinService::overrideZetkinContext([
+            'baseUrl' => 'https://zetkin.test/v1',
+            'orgId' => 99,
+            'accessToken' => 'test-token',
+            'client' => $client,
+        ]);
+
+        return $client;
+    }
+
+    private function response(array $json, int $status = 200): object
+    {
+        $body = new class (json_encode($json)) {
+            public function __construct(private string $contents)
+            {
+            }
+
+            public function getContents(): string
+            {
+                return $this->contents;
+            }
+        };
+
+        return new class ($body, $status) {
+            public function __construct(private object $body, private int $status)
+            {
+            }
+
+            public function getBody(): object
+            {
+                return $this->body;
+            }
+
+            public function getStatusCode(): int
+            {
+                return $this->status;
+            }
+        };
+    }
+
+    private function requestsMatching(object $client, string $method, string $needle): array
+    {
+        return array_values(array_filter(
+            $client->requests,
+            fn($r) => $r['method'] === $method && str_contains($r['url'], $needle)
+        ));
+    }
+
+    // addTag / removeTag, the email-based pair used by joins and webhooks.
+
+    /**
+     * Nothing stops Zetkin holding two people with the same email, and addTag
+     * has always tagged all of them, not the first. Pinned because collapsing
+     * the search into findPersonByEmail (which takes the first) would silently
+     * halve its reach.
+     */
+    public function test_add_tag_tags_every_person_with_that_email()
+    {
+        $client = $this->fakeZetkin([
+            ['POST', 'search/person', $this->response(['data' => [
+                ['id' => 5, 'email' => 'a@example.com'],
+                ['id' => 6, 'email' => 'a@example.com'],
+            ]])],
+            ['GET', 'people/tags', $this->response(['data' => [['id' => 7, 'title' => 'Bury']]])],
+            ['PUT', '/tags/7', $this->response([])],
+        ]);
+
+        ZetkinService::addTag('a@example.com', 'Bury');
+
+        $puts = $this->requestsMatching($client, 'PUT', '/tags/7');
+        $this->assertCount(2, $puts);
+        $this->assertStringContainsString('/people/5/tags/7', $puts[0]['url']);
+        $this->assertStringContainsString('/people/6/tags/7', $puts[1]['url']);
+    }
+
+    /**
+     * Zetkin's search is fuzzy. A near-miss email must never be tagged.
+     */
+    public function test_add_tag_ignores_fuzzy_search_matches()
+    {
+        $client = $this->fakeZetkin([
+            ['POST', 'search/person', $this->response(['data' => [
+                ['id' => 5, 'email' => 'a@example.com'],
+                ['id' => 9, 'email' => 'aa@example.com'],
+            ]])],
+            ['GET', 'people/tags', $this->response(['data' => [['id' => 7, 'title' => 'Bury']]])],
+            ['PUT', '/tags/7', $this->response([])],
+        ]);
+
+        ZetkinService::addTag('a@example.com', 'Bury');
+
+        $puts = $this->requestsMatching($client, 'PUT', '/tags/');
+        $this->assertCount(1, $puts);
+        $this->assertStringContainsString('/people/5/', $puts[0]['url']);
+    }
+
+    public function test_add_tag_warns_and_writes_nothing_when_nobody_matches()
+    {
+        $client = $this->fakeZetkin([
+            ['POST', 'search/person', $this->response(['data' => []])],
+        ]);
+
+        ZetkinService::addTag('ghost@example.com', 'Bury');
+
+        $this->assertCount(1, $client->requests);
+        $this->assertStringContainsString('no person found for ghost@example.com', $this->logger->warnings[0]);
+    }
+
+    /**
+     * addTag has always swallowed API failures: a Zetkin outage must not break
+     * the join or webhook that called it. It logs and returns.
+     */
+    public function test_add_tag_logs_and_does_not_throw_on_an_api_error()
+    {
+        $this->fakeZetkin([
+            ['POST', 'search/person', $this->response(['error' => 'nope'])],
+        ]);
+
+        ZetkinService::addTag('a@example.com', 'Bury');
+
+        $this->assertNotEmpty($this->logger->errors);
+        $this->assertStringContainsString('Could not add tag', $this->logger->errors[0]);
+    }
+
+    public function test_remove_tag_deletes_the_resolved_tag()
+    {
+        $client = $this->fakeZetkin([
+            ['POST', 'search/person', $this->response(['data' => [['id' => 5, 'email' => 'a@example.com']]])],
+            ['GET', 'people/tags', $this->response(['data' => [['id' => 7, 'title' => 'Bury']]])],
+            ['DELETE', '/tags/7', $this->response([])],
+        ]);
+
+        ZetkinService::removeTag('a@example.com', 'Bury');
+
+        $deletes = $this->requestsMatching($client, 'DELETE', '/people/5/tags/7');
+        $this->assertCount(1, $deletes);
+    }
+
+    /**
+     * Removing a tag the person does not have is a non-event, not an error.
+     */
+    public function test_remove_tag_treats_a_missing_tag_as_a_non_event()
+    {
+        $this->fakeZetkin([
+            ['POST', 'search/person', $this->response(['data' => [['id' => 5, 'email' => 'a@example.com']]])],
+            ['GET', 'people/tags', $this->response(['data' => [['id' => 7, 'title' => 'Bury']]])],
+            ['DELETE', '/tags/7', $this->response([], 404)],
+        ]);
+
+        ZetkinService::removeTag('a@example.com', 'Bury');
+
+        $this->assertSame([], $this->logger->errors);
+        $this->assertStringContainsString("was not on", $this->logger->infos[0]);
+    }
+
+    // Person lookup, shared by addTag, removeTag, updatePerson and
+    // findPersonByEmail.
+
+    public function test_find_person_by_email_returns_the_first_exact_match()
+    {
+        $this->fakeZetkin([
+            ['POST', 'search/person', $this->response(['data' => [
+                ['id' => 9, 'email' => 'other@example.com'],
+                ['id' => 5, 'email' => 'a@example.com'],
+                ['id' => 6, 'email' => 'a@example.com'],
+            ]])],
+        ]);
+
+        $person = ZetkinService::findPersonByEmail('a@example.com');
+
+        $this->assertSame(5, $person['id']);
+    }
+
+    public function test_find_person_by_email_returns_null_when_nobody_matches()
+    {
+        $this->fakeZetkin([
+            ['POST', 'search/person', $this->response(['data' => [['id' => 9, 'email' => 'other@example.com']]])],
+        ]);
+
+        $this->assertNull(ZetkinService::findPersonByEmail('a@example.com'));
+    }
+
+    public function test_update_person_patches_the_matched_person()
+    {
+        $client = $this->fakeZetkin([
+            ['POST', 'search/person', $this->response(['data' => [['id' => 5, 'email' => 'a@example.com']]])],
+            ['PATCH', '/people/5', $this->response(['data' => ['id' => 5]])],
+        ]);
+
+        ZetkinService::updatePerson('a@example.com', ['first_name' => 'Test']);
+
+        $patches = $this->requestsMatching($client, 'PATCH', '/people/5');
+        $this->assertCount(1, $patches);
+        $this->assertSame(['first_name' => 'Test'], $patches[0]['options']['json']);
+    }
+
+    public function test_update_person_warns_and_writes_nothing_when_nobody_matches()
+    {
+        $client = $this->fakeZetkin([
+            ['POST', 'search/person', $this->response(['data' => []])],
+        ]);
+
+        ZetkinService::updatePerson('ghost@example.com', ['first_name' => 'Test']);
+
+        $this->assertCount(1, $client->requests);
+        $this->assertStringContainsString('no person found', $this->logger->warnings[0]);
+    }
+
+    // The bulk helpers the re-tag job is built on.
+
+    public function test_list_people_sends_zetkins_paging_parameters()
+    {
+        $client = $this->fakeZetkin([
+            ['GET', '/people?', $this->response(['data' => [['id' => 1]]])],
+        ]);
+
+        $people = ZetkinService::listPeople(2, 50);
+
+        $this->assertSame([['id' => 1]], $people);
+        $this->assertStringContainsString('/people?p=2&pp=50', $client->requests[0]['url']);
+    }
+
+    public function test_list_people_returns_an_empty_array_when_exhausted()
+    {
+        $this->fakeZetkin([
+            ['GET', '/people?', $this->response(['data' => []])],
+        ]);
+
+        $this->assertSame([], ZetkinService::listPeople(7));
+    }
+
+    public function test_get_person_tags_returns_the_tag_records()
+    {
+        $this->fakeZetkin([
+            ['GET', '/people/5/tags', $this->response(['data' => [['id' => 7, 'title' => 'Bury']]])],
+        ]);
+
+        $this->assertSame([['id' => 7, 'title' => 'Bury']], ZetkinService::getPersonTags(5));
+    }
+
+    public function test_find_or_create_tag_by_title_reuses_an_existing_tag()
+    {
+        $client = $this->fakeZetkin([
+            ['GET', 'people/tags', $this->response(['data' => [['id' => 7, 'title' => 'Bury']]])],
+        ]);
+
+        $tag = ZetkinService::findOrCreateTagByTitle('bury');
+
+        $this->assertSame(7, $tag['id']);
+        $this->assertSame([], $this->requestsMatching($client, 'POST', 'people/tags'));
+    }
+}

From 64f1134242ed250bcaf8ccfbf907b173dffd1fe6 Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 15:52:05 +0100
Subject: [PATCH 13/14] Zetkin try helpers report statuses, matching Mailchimp

The two services' try pairs had the same prefix and different contracts:
Mailchimp returned TAG_* strings, Zetkin returned bools. Same word, two
meanings, and the bool could not distinguish "the tag was not there"
from "the delete succeeded", which the re-tag job's reporting cares
about.

Both now report TAG_* constants. Zetkin has TAG_MISSING where Mailchimp
has TAG_NOT_FOUND because they name different facts: a Zetkin 404 on
delete means the person did not carry the tag, a Mailchimp 404 means the
member is not in the audience at all.

The wire values are pinned by a test for the same reason as Mailchimp's:
the GMTU add-on compares against them across a plugin boundary.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 .../join-block/src/Services/ZetkinService.php | 27 +++++--
 .../tests/ZetkinServiceTagsTest.php           | 71 +++++++++++++++++++
 2 files changed, 93 insertions(+), 5 deletions(-)

diff --git a/packages/join-block/src/Services/ZetkinService.php b/packages/join-block/src/Services/ZetkinService.php
index 84f860e..2410839 100644
--- a/packages/join-block/src/Services/ZetkinService.php
+++ b/packages/join-block/src/Services/ZetkinService.php
@@ -12,6 +12,13 @@
 
 class ZetkinService
 {
+    // Outcomes of a tag write, returned by tryAddTagToPerson and
+    // tryRemoveTagFromPerson.
+    public const TAG_OK = 'ok';
+    public const TAG_MISSING = 'missing';
+    public const TAG_NOT_CONFIGURED = 'not_configured';
+    public const TAG_ERROR = 'error';
+
     private static $zetkinContextOverride = null;
 
     public static function signup($data)
@@ -469,29 +476,39 @@ public static function findOrCreateTagByTitle($title)
         return self::findOrCreateTag($client, $baseUrl, $orgId, $existingTags, $title, $accessToken);
     }
 
+    // Both report one of the TAG_* constants, mirroring MailchimpService.
     public static function tryAddTagToPerson($personId, $tagId)
     {
         $zetkinContext = self::getZetkinContext();
         if (!$zetkinContext) {
-            return false;
+            return self::TAG_NOT_CONFIGURED;
         }
 
         ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken, 'client' => $client] = $zetkinContext;
 
-        return self::putPersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId) === 'ok';
+        return self::putPersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId) === 'ok'
+            ? self::TAG_OK
+            : self::TAG_ERROR;
     }
 
-    // A tag the person does not have is treated as success, not an error.
+    // A tag the person does not have comes back as TAG_MISSING, distinct from
+    // both success and failure; callers decide what it means to them.
     public static function tryRemoveTagFromPerson($personId, $tagId)
     {
         $zetkinContext = self::getZetkinContext();
         if (!$zetkinContext) {
-            return false;
+            return self::TAG_NOT_CONFIGURED;
         }
 
         ['baseUrl' => $baseUrl, 'orgId' => $orgId, 'accessToken' => $accessToken, 'client' => $client] = $zetkinContext;
 
-        return self::deletePersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId) !== 'error';
+        $result = self::deletePersonTag($client, $baseUrl, $orgId, $accessToken, $personId, $tagId);
+
+        if ($result === 'missing') {
+            return self::TAG_MISSING;
+        }
+
+        return $result === 'ok' ? self::TAG_OK : self::TAG_ERROR;
     }
 
     // Single implementation of "apply this tag to this person".
diff --git a/packages/join-block/tests/ZetkinServiceTagsTest.php b/packages/join-block/tests/ZetkinServiceTagsTest.php
index a768cf3..295b098 100644
--- a/packages/join-block/tests/ZetkinServiceTagsTest.php
+++ b/packages/join-block/tests/ZetkinServiceTagsTest.php
@@ -331,6 +331,77 @@ public function test_get_person_tags_returns_the_tag_records()
         $this->assertSame([['id' => 7, 'title' => 'Bury']], ZetkinService::getPersonTags(5));
     }
 
+    // The try* pair report one of the TAG_* constants, mirroring
+    // MailchimpService, so the re-tag job reads both services the same way.
+
+    public function test_try_add_tag_to_person_reports_ok()
+    {
+        $this->fakeZetkin([
+            ['PUT', '/people/5/tags/7', $this->response([])],
+        ]);
+
+        $this->assertSame(ZetkinService::TAG_OK, ZetkinService::tryAddTagToPerson(5, 7));
+    }
+
+    public function test_try_add_tag_to_person_reports_an_api_error()
+    {
+        $this->fakeZetkin([
+            ['PUT', '/people/5/tags/7', $this->response(['error' => 'nope'])],
+        ]);
+
+        $this->assertSame(ZetkinService::TAG_ERROR, ZetkinService::tryAddTagToPerson(5, 7));
+    }
+
+    public function test_try_remove_tag_from_person_reports_ok()
+    {
+        $this->fakeZetkin([
+            ['DELETE', '/people/5/tags/7', $this->response([])],
+        ]);
+
+        $this->assertSame(ZetkinService::TAG_OK, ZetkinService::tryRemoveTagFromPerson(5, 7));
+    }
+
+    /**
+     * A tag the person does not have is reported as missing, distinct from
+     * both success and failure, and callers decide what it means to them.
+     */
+    public function test_try_remove_tag_from_person_reports_a_missing_tag()
+    {
+        $this->fakeZetkin([
+            ['DELETE', '/people/5/tags/7', $this->response([], 404)],
+        ]);
+
+        $this->assertSame(ZetkinService::TAG_MISSING, ZetkinService::tryRemoveTagFromPerson(5, 7));
+    }
+
+    public function test_try_remove_tag_from_person_reports_an_api_error()
+    {
+        $this->fakeZetkin([
+            ['DELETE', '/people/5/tags/7', $this->response([], 500)],
+        ]);
+
+        $this->assertSame(ZetkinService::TAG_ERROR, ZetkinService::tryRemoveTagFromPerson(5, 7));
+    }
+
+    public function test_try_helpers_report_not_configured_without_credentials()
+    {
+        $this->assertSame(ZetkinService::TAG_NOT_CONFIGURED, ZetkinService::tryAddTagToPerson(5, 7));
+        $this->assertSame(ZetkinService::TAG_NOT_CONFIGURED, ZetkinService::tryRemoveTagFromPerson(5, 7));
+    }
+
+    /**
+     * The GMTU add-on compares against these values across a plugin boundary,
+     * and its test fakes return them as literals. Renaming a constant is free;
+     * changing its value is not, so pin the wire values here.
+     */
+    public function test_status_values_are_stable()
+    {
+        $this->assertSame('ok', ZetkinService::TAG_OK);
+        $this->assertSame('missing', ZetkinService::TAG_MISSING);
+        $this->assertSame('not_configured', ZetkinService::TAG_NOT_CONFIGURED);
+        $this->assertSame('error', ZetkinService::TAG_ERROR);
+    }
+
     public function test_find_or_create_tag_by_title_reuses_an_existing_tag()
     {
         $client = $this->fakeZetkin([

From c133122b6fffd98da830c7e96e2e7eb572ece050 Mon Sep 17 00:00:00 2001
From: Alex Worrad-Andrews <alex@alexandrews.info>
Date: Mon, 21 Sep 2026 15:52:21 +0100
Subject: [PATCH 14/14] Collapse the changelog into the version that will
 actually ship

1.4.38 was never released on its own; both sets of helpers go out
together as 1.4.39, so a changelog entry for a version nobody can
install is noise.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 packages/join-block/readme.txt | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/packages/join-block/readme.txt b/packages/join-block/readme.txt
index f25bbba..cb5f3ec 100644
--- a/packages/join-block/readme.txt
+++ b/packages/join-block/readme.txt
@@ -108,9 +108,7 @@ Need help? Contact us at [hello@commonknowledge.coop](mailto:hello@commonknowled
 == Changelog ==
 
 = 1.4.39 =
-* Add Mailchimp tag helpers that report status instead of throwing, for bulk maintenance jobs
-= 1.4.38 =
-* Add Zetkin people listing and person-tag helpers for bulk maintenance jobs
+* Add Zetkin people listing and Zetkin and Mailchimp tag helpers that report status instead of throwing, for bulk maintenance jobs
 = 1.4.37 =
 * Add "cancelled" tag option, distinguishing this from lapsed membership
 = 1.4.36 =