diff --git a/packages/join-block/join.php b/packages/join-block/join.php index cc58662..d64e1fb 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.28 + * Version: 1.4.29 * Author: Common Knowledge * Text Domain: common-knowledge-join-flow * License: GPLv2 or later @@ -396,7 +396,12 @@ // Make stage = "confirm" to match the normal flow // (the user is redirected back from GoCardless and submits their data to the /join endpoint) $data['stage'] = "confirm"; - update_option("JOIN_FORM_UNPROCESSED_GOCARDLESS_REQUEST_{$billingRequest['id']}", wp_json_encode($data)); + // autoload disabled: full join payload, see the Stripe equivalent below. + update_option( + "JOIN_FORM_UNPROCESSED_GOCARDLESS_REQUEST_{$billingRequest['id']}", + wp_json_encode($data), + false + ); return ["href" => $authLink, "gcBillingRequestId" => $billingRequest["id"]]; } @@ -458,7 +463,12 @@ // Make stage = "confirm" to match the normal flow // (the user is redirected back from ChargeBee and submits their data to the /join endpoint) $data['stage'] = "confirm"; - update_option("JOIN_FORM_UNPROCESSED_CHARGEBEE_REQUEST_{$hostedPageId}", wp_json_encode($data)); + // autoload disabled: full join payload, see the Stripe equivalent below. + update_option( + "JOIN_FORM_UNPROCESSED_CHARGEBEE_REQUEST_{$hostedPageId}", + wp_json_encode($data), + false + ); return ["href" => $hostedPageUrl, "cbHostedPageId" => $hostedPageId]; } catch (\Exception $e) { @@ -509,7 +519,8 @@ $data["customMembershipAmount"] ?? null, $data["donationAmount"] ?? null, $data["recurDonation"] ?? false, - !empty($data["donationSupporterMode"]) + !empty($data["donationSupporterMode"]), + StripeService::buildSubscriptionIdempotencyKey($data, $plan) ); // Save this data in the database so if the user pays but never returns to @@ -521,7 +532,14 @@ // Make stage = "confirm" to match the normal flow // (the user returns from the payment step and submits their data to the /join endpoint) $data['stage'] = "confirm"; - update_option("JOIN_FORM_UNPROCESSED_STRIPE_REQUEST_{$subscription->id}", wp_json_encode($data)); + // autoload disabled: this holds the full join payload, and a + // backlog of unprocessed requests would otherwise be read into + // memory on every page request. + update_option( + "JOIN_FORM_UNPROCESSED_STRIPE_REQUEST_{$subscription->id}", + wp_json_encode($data), + false + ); return $subscription; } catch (\Exception $e) { @@ -747,6 +765,8 @@ continue; } + // Replay: complete the join without touching payment state. + $data['isRecoveryReplay'] = true; JoinService::handleJoin($data); delete_option($result->option_name); $joinBlockLog->info("ensureSubscriptionsCreated: success, deleting option {$result->option_name}"); @@ -769,6 +789,83 @@ wp_schedule_event(time(), 'hourly', 'ck_join_block_stripe_cron_hook'); } +// Finish CRM pushes for members whose payment succeeded but whose CRM record +// was never written. Complements the reconciliation cron below, which can spot +// such a member but holds no copy of their details to fix it with. Reads Stripe +// only to confirm the payment still stands; writes exclusively to CRMs. +add_action('ck_join_block_crm_retry_cron_hook', function () { + JoinService::ensureCrmPushesCompleted(); +}); + +if (!wp_next_scheduled('ck_join_block_crm_retry_cron_hook')) { + wp_schedule_event(time(), 'hourly', 'ck_join_block_crm_retry_cron_hook'); +} + +// Level-triggered safety net: converge Action Network state with Stripe for +// recent subscriptions, catching webhooks that were missed, suppressed or +// arrived out of order. Alerts (error-level logs, which reach Sentry) +// whenever it finds a divergence it cannot fix itself. +add_action('ck_join_block_reconciliation_cron_hook', function () { + StripeService::initialise(); + StripeService::reconcileRecentMemberships(); +}); + +if (!wp_next_scheduled('ck_join_block_reconciliation_cron_hook')) { + wp_schedule_event(time(), 'daily', 'ck_join_block_reconciliation_cron_hook'); +} + +// Heartbeat for the downstream CRM pipeline (Action Network -> Make -> +// Airtable, etc.), which the join flow cannot observe directly. Completed +// joins are recorded on success and a daily digest is logged and emailed, so +// a silent downstream outage shows up within a day instead of surfacing weeks +// later as missing records. +add_action('ck_join_flow_success', function ($data) { + $joins = get_option('CK_JOIN_FLOW_COMPLETED_JOINS', []); + if (!is_array($joins)) { + $joins = []; + } + $joins[] = [ + 'email' => $data['email'] ?? '(unknown)', + 'time' => gmdate('c'), + ]; + $cutoff = time() - 30 * 86400; + $joins = array_values(array_filter($joins, function ($join) use ($cutoff) { + return strtotime($join['time'] ?? '') >= $cutoff; + })); + update_option('CK_JOIN_FLOW_COMPLETED_JOINS', $joins, false); +}); + +add_action('ck_join_block_heartbeat_cron_hook', function () { + global $joinBlockLog; + + $joins = get_option('CK_JOIN_FLOW_COMPLETED_JOINS', []); + $dayAgo = time() - 86400; + $recent = array_values(array_filter(is_array($joins) ? $joins : [], function ($join) use ($dayAgo) { + return strtotime($join['time'] ?? '') >= $dayAgo; + })); + + $count = count($recent); + $joinBlockLog->info("Join heartbeat: $count completed join(s) in the last 24 hours"); + + $recipient = apply_filters('ck_join_flow_heartbeat_recipient', get_option('admin_email')); + if (!$recipient) { + return; + } + + $lines = array_map(function ($join) { + return ($join['time'] ?? '(unknown time)') . ' ' . ($join['email'] ?? '(unknown)'); + }, $recent); + $body = "Completed joins in the last 24 hours: $count\n\n" + . ($lines ? implode("\n", $lines) . "\n\n" : "") + . "Compare against the CRM pipeline (e.g. Action Network -> Make -> Airtable). " + . "If joins are listed here but missing downstream, the pipeline is broken.\n"; + wp_mail($recipient, "[CK Join Flow] Daily join heartbeat: $count join(s)", $body); +}); + +if (!wp_next_scheduled('ck_join_block_heartbeat_cron_hook')) { + wp_schedule_event(time(), 'daily', 'ck_join_block_heartbeat_cron_hook'); +} + if (defined('WP_CLI') && WP_CLI) { WP_CLI::add_command('join update_an_from_stripe', function () { StripeService::initialise(); diff --git a/packages/join-block/readme.txt b/packages/join-block/readme.txt index 93bb149..0f41dfa 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: 6.8 -Stable tag: 1.4.28 +Stable tag: 1.4.29 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.29 = +* Improve push-to-CRM robustness (add retry queue) = 1.4.28 = * Add incomplete stripe journey handling = 1.4.27 = diff --git a/packages/join-block/src/Services/ActionNetworkService.php b/packages/join-block/src/Services/ActionNetworkService.php index 2e58a66..bd84cf3 100644 --- a/packages/join-block/src/Services/ActionNetworkService.php +++ b/packages/join-block/src/Services/ActionNetworkService.php @@ -140,6 +140,42 @@ public static function personExists($email) return self::getPerson($email) !== null; } + /** + * Read-only view of a person for reconciliation. Returns null if the + * person does not exist, otherwise: + * 'email_status' — the primary email address status (e.g. 'subscribed', + * 'bouncing'), or null if unavailable; + * 'tags' — the person's tag names, or null when enumeration was + * aborted (treat as unknown, not as zero tags); + * 'has_name' — whether the person has a given or family name. The + * join flow always collects a name, while newsletter and + * petition signups create a person from an email alone, + * so this distinguishes "joined" from "merely exists". + */ + public static function getPersonSnapshot($email) + { + $person = self::getPerson($email); + if ($person === null) { + return null; + } + + $emailStatus = null; + foreach ($person['email_addresses'] ?? [] as $address) { + if (!empty($address['primary'])) { + $emailStatus = $address['status'] ?? null; + break; + } + } + + $tagNames = self::getPersonTagNames($email); + + return [ + 'email_status' => $emailStatus, + 'tags' => is_array($tagNames) ? $tagNames : null, + 'has_name' => !empty($person['given_name']) || !empty($person['family_name']), + ]; + } + private static function getPerson($email) { global $joinBlockLog; diff --git a/packages/join-block/src/Services/JoinService.php b/packages/join-block/src/Services/JoinService.php index 4912f64..118dc0c 100644 --- a/packages/join-block/src/Services/JoinService.php +++ b/packages/join-block/src/Services/JoinService.php @@ -10,6 +10,22 @@ class JoinService { + // Membership amounts are money in major units, held as floats. Compare with + // a tolerance rather than == / === so that a value which has been through + // Stripe's minor-unit integers and back cannot fail on representation + // alone. Half a penny is far below the smallest real price difference. + public const AMOUNT_EPSILON = 0.005; + + // Join data whose CRM push failed after the payment succeeded. The member + // has paid and is a member; only the CRM write is outstanding, so this is + // kept for retry rather than surfaced as a failure to them. + public const CRM_RETRY_OPTION_PREFIX = 'JOIN_FORM_PENDING_CRM_'; + + // After this many failed attempts the record is parked for a human rather + // than retried forever. It is never deleted: parking keeps the member's + // submitted details, which exist nowhere else. + public const CRM_RETRY_MAX_ATTEMPTS = 12; + // According to error messages from Chargebee, dates should be sent as the format yyyy-MM-dd. // Meaning 2021-12-25 for Christmas Day, 25th of December 2021. private static function formatDob($day, $month, $year) @@ -20,6 +36,180 @@ private static function formatDob($day, $month, $year) return $date->format('Y-m-d'); } + /** + * Option name for a pending CRM push. Keyed on the Stripe subscription + * where there is one, falling back to the email so a GoCardless or + * Chargebee join still gets exactly one record per member. + * + * Each fallback must be non-empty before it is used. handleJoin() can be + * called without an email — it falls back to sessionToken for its lock — + * and hashing an empty string would give every such record the same option + * name, so each would silently overwrite the last. + */ + public static function crmRetryKey($data) + { + $subscriptionId = trim((string) ($data['stripeSubscriptionId'] ?? '')); + if ($subscriptionId !== '') { + return self::CRM_RETRY_OPTION_PREFIX . $subscriptionId; + } + + $email = strtolower(trim((string) ($data['email'] ?? ''))); + if ($email !== '') { + return self::CRM_RETRY_OPTION_PREFIX . 'email_' . sha1($email); + } + + $sessionToken = trim((string) ($data['sessionToken'] ?? '')); + if ($sessionToken !== '') { + return self::CRM_RETRY_OPTION_PREFIX . 'session_' . sha1($sessionToken); + } + + // Last resort: the payload itself. Two identical payloads collapsing to + // one record is correct de-duplication; two different ones must not + // collide, which hashing a missing identifier would guarantee. + return self::CRM_RETRY_OPTION_PREFIX . 'payload_' . sha1((string) wp_json_encode($data)); + } + + /** + * Durably record that a CRM push is still outstanding for a member whose + * payment has already succeeded. + * + * This record is the only copy of what the member submitted — the join + * form data is not otherwise persisted — so losing it means their details + * can only be recovered by asking them again. + * + * @param array $data The join payload. + * @param array|null $services Service keys still outstanding, or null for + * "every configured CRM", used when we do not + * know how far the original join got. + */ + public static function queueCrmRetry($data, $services, $reason) + { + global $joinBlockLog; + + $optionName = self::crmRetryKey($data); + + $attempts = 1; + $existing = get_option($optionName); + if ($existing) { + $decoded = json_decode($existing, true); + $attempts = (int) ($decoded['attempts'] ?? 0) + 1; + } + + // autoload disabled: each record carries the full join payload, and a + // backlog of them would otherwise be read into memory on every single + // page request. + update_option($optionName, wp_json_encode([ + 'data' => $data, + 'services' => $services, + 'reason' => $reason, + 'attempts' => $attempts, + 'lastAttemptAt' => time(), + ]), false); + + $email = $data['email'] ?? 'unknown'; + $label = $services ? implode(', ', $services) : 'all configured CRMs'; + $joinBlockLog->error("Queued CRM retry for $email ($label, attempt $attempts): $reason"); + } + + /** + * Push a member to every configured CRM, returning the service keys that + * failed. + * + * Contains no payment code at all — no subscription creation, cancellation + * or Chargebee/GoCardless/Auth0 calls. The retry worker depends on that + * structurally: replaying a CRM push must never be able to touch a payment + * that is working fine, and from here it cannot reach the code that would. + * + * Each service is independent — one failing must not stop the others. + * + * @param array $data The join payload. + * @param array|null $only Restrict to these service keys, or null for every + * configured service. + * @return string[] Service keys that failed. + */ + public static function pushToCrms($data, $only = null) + { + global $joinBlockLog; + + $email = $data['email'] ?? 'unknown'; + $wanted = function ($service) use ($only) { + return $only === null || in_array($service, $only, true); + }; + $failed = []; + + if (Settings::get("USE_MAILCHIMP") && $wanted('mailchimp')) { + $joinBlockLog->info("Processing Mailchimp signup request for $email"); + try { + MailchimpService::signup($data); + $joinBlockLog->info("Completed Mailchimp signup request for $email"); + } catch (\Exception $exception) { + $joinBlockLog->error("Mailchimp error for email $email: " . $exception->getMessage()); + $failed[] = 'mailchimp'; + } + } + + if (Settings::get("USE_ACTION_NETWORK") && $wanted('action_network')) { + $joinBlockLog->info("Processing Action Network signup request for $email"); + try { + ActionNetworkService::signup($data); + $joinBlockLog->info("Completed Action Network signup request for $email"); + } catch (\Exception $exception) { + $joinBlockLog->error( + "Action Network error for email $email after successful payment — money taken with no CRM" + . " record yet: " . $exception->getMessage() + ); + $failed[] = 'action_network'; + } + } + + if (Settings::get("USE_ZETKIN") && $wanted('zetkin')) { + $joinBlockLog->info("Processing Zetkin signup request for $email"); + try { + ZetkinService::signup($data); + $joinBlockLog->info("Completed Zetkin signup request for $email"); + } catch (\Exception $exception) { + $joinBlockLog->error("Zetkin error for email $email: " . $exception->getMessage()); + $failed[] = 'zetkin'; + } + } + + return $failed; + } + + /** + * Refresh the recovery snapshot with the payload we were actually handed. + * + * join.php writes this at create-subscription time, before the member has + * finished the form, so the stored copy can be missing fields that the + * final /join request carries. Any replay should use the freshest version. + */ + private static function saveRecoverySnapshot($data) + { + $subscriptionId = $data['stripeSubscriptionId'] ?? ''; + if (!$subscriptionId) { + return; + } + + $optionName = "JOIN_FORM_UNPROCESSED_STRIPE_REQUEST_{$subscriptionId}"; + + $existing = get_option($optionName); + if ($existing) { + // Keep the original createdAt. ensureStripeSubscriptionsCreated() + // uses it to decide when to stop retrying, so refreshing it on + // every attempt would make the record immortal. + $decoded = json_decode($existing, true); + if (!empty($decoded['createdAt'])) { + $data['createdAt'] = $decoded['createdAt']; + } + } + if (empty($data['createdAt'])) { + $data['createdAt'] = time(); + } + + // autoload disabled — see queueCrmRetry(): full join payload. + update_option($optionName, wp_json_encode($data), false); + } + public static function handleJoin($data) { global $joinBlockLog; @@ -34,6 +224,7 @@ public static function handleJoin($data) } } $lockFile = self::acquireLock($lockKey); + self::saveRecoverySnapshot($data); $chargebeeCustomer = self::tryHandleJoin($data); // The join is complete, so it no longer needs to be picked up by // ensureStripeSubscriptionsCreated() (the webhook/cron recovery path) @@ -182,10 +373,10 @@ private static function tryHandleJoin($data) } $data['membershipPlan']['remove_tags'] = Settings::computeTagsToRemove($data['membershipPlan']); - $membershipAmount = (float) $data['membershipPlan']['amount'] ?? 0; + $membershipAmount = (float) ($data['membershipPlan']['amount'] ?? 0); if ($data['membershipPlan']['allow_custom_amount']) { $minimumAmount = $membershipAmount; - $membershipAmount = (float) $data['customMembershipAmount'] ?? 0; + $membershipAmount = (float) ($data['customMembershipAmount'] ?? 0); if ($membershipAmount < $minimumAmount || $membershipAmount > 1000) { $error = "Invalid membership amount: $membershipAmount < $minimumAmount or > 1000"; $joinBlockLog->error($error); @@ -220,13 +411,54 @@ private static function tryHandleJoin($data) if (Settings::get("USE_STRIPE") && !$isOneOffSupporterDonation) { StripeService::initialise(); - $subscriptionInfo = StripeService::cancelPreviousAndGetCurrentSubscription($data["email"], $data["stripeCustomerId"] ?? null, $data["stripeSubscriptionId"] ?? null); - if ($subscriptionInfo["amount"] !== $membershipAmount) { - $email = $data['email']; - $subAmount = $subscriptionInfo["amount"]; - $joinBlockLog->error("Found mismatched subscription amounts for $email - claimed: $membershipAmount, found in stripe: $subAmount"); + $email = $data['email']; + $subscriptionId = $data["stripeSubscriptionId"] ?? null; + + // Verify the amount BEFORE cancelling anything. Cancelling first + // means a failed verification leaves the member with their previous + // subscription cancelled and no new membership to show for it. + try { + $actualAmount = StripeService::getSubscriptionAmount($subscriptionId); + } catch (\Exception $e) { + $joinBlockLog->error( + "Could not read subscription amount for $email from Stripe: " . $e->getMessage() + ); + throw new JoinBlockException("Could not verify subscription amount", 9); + } + + // Distinct from a mismatch: we do not know the amount, so we cannot + // say it is wrong. Treated as retryable rather than telling a member + // who has already paid that their amount is invalid. + if ($actualAmount === null) { + $subscriptionLabel = $subscriptionId ?: 'none'; + $joinBlockLog->error( + "Could not determine subscription amount for $email (subscription: $subscriptionLabel)" + ); + throw new JoinBlockException("Could not verify subscription amount", 9); + } + + if (abs($actualAmount - $membershipAmount) > self::AMOUNT_EPSILON) { + $joinBlockLog->error( + "Found mismatched subscription amounts for $email - claimed: $membershipAmount," + . " found in stripe: $actualAmount" + ); throw new JoinBlockException("Invalid subscription amount", 8); } + + $customerId = StripeService::resolveCustomerId($email, $data["stripeCustomerId"] ?? null); + + // A recovery replay runs on data saved hours or days earlier, by + // which time the member may have joined again. Cancelling + // "previous" subscriptions from that stale view would cancel the + // live one. De-duplication belongs to the live flow, which knows + // the current subscription; a replay only ever reads. + if (empty($data['isRecoveryReplay'])) { + StripeService::cancelPreviousSubscriptions($email, $customerId, $subscriptionId); + } else { + $joinBlockLog->info("Recovery replay for $email: leaving existing subscriptions untouched"); + } + + $subscriptionInfo = StripeService::getSubscriptionDates($email, $customerId); $data["stripeFirstSubscriptionDate"] = $subscriptionInfo["firstSubscription"]; $data["stripeFirstPaymentDate"] = $subscriptionInfo["firstPayment"]; $data["stripeLastPaymentDate"] = $subscriptionInfo["lastPayment"]; @@ -252,45 +484,13 @@ private static function tryHandleJoin($data) } } - if (Settings::get("USE_MAILCHIMP")) { - $email = $data['email']; - $joinBlockLog->info("Processing Mailchimp signup request for $email"); - try { - MailchimpService::signup($data); - $joinBlockLog->info("Completed Mailchimp signup request for $email"); - } catch (\Exception $exception) { - // A Mailchimp failure should not block a successful join. - // The member record can be retro-added to Mailchimp once the underlying issue is resolved. - $joinBlockLog->error("Mailchimp error for email $email: " . $exception->getMessage()); - } - } - - if (Settings::get("USE_ACTION_NETWORK")) { - $email = $data['email']; - $joinBlockLog->info("Processing Action Network signup request for $email"); - try { - ActionNetworkService::signup($data); - $joinBlockLog->info("Completed Action Network signup request for $email"); - } catch (\Exception $exception) { - $joinBlockLog->error("Action Network error for email $email: " . $exception->getMessage()); - throw $exception; - } - } - - if (Settings::get("USE_ZETKIN")) { - $email = $data['email']; - $joinBlockLog->info("Processing Zetkin signup request for $email"); - try { - ZetkinService::signup($data); - $joinBlockLog->info("Completed Zetkin signup request for $email"); - } catch (\Exception $exception) { - // Non-blocking: Zetkin is a secondary integration. The Stripe payment is - // the essential step and has already completed by this point. A Zetkin - // failure (e.g. expired credentials) should not surface an error to the - // member — they have successfully joined. The member record can be - // retro-added to Zetkin once the underlying issue is resolved. - $joinBlockLog->error("Zetkin error for email $email: " . $exception->getMessage()); - } + // The payment has already succeeded by this point, so a CRM failure is + // not something the member can act on. Record what is outstanding and + // let ensureCrmPushesCompleted() finish it, rather than showing them an + // error and inviting the resubmit that creates a duplicate subscription. + $failedCrms = self::pushToCrms($data); + if ($failedCrms) { + self::queueCrmRetry($data, $failedCrms, 'push failed during join'); } $webhookUuid = $data['webhookUuid'] ?? ''; @@ -433,6 +633,8 @@ public static function ensureStripeSubscriptionsCreated($subscriptionId = null) } if ($subscription && in_array($subscription->status, ['active', 'trialing'])) { + // Replay: complete the join without touching payment state. + $data['isRecoveryReplay'] = true; self::handleJoin($data); delete_option($name); $joinBlockLog->info("ensureStripeSubscriptionsCreated: success, deleting option {$name}"); @@ -446,7 +648,40 @@ public static function ensureStripeSubscriptionsCreated($subscriptionId = null) || in_array($subscription->status, ['incomplete_expired', 'canceled']); if ($expired || (time() - $createdAt) > $day) { - $joinBlockLog->info("ensureStripeSubscriptionsCreated: deleting unprocessable {$name}"); + // If money was actually collected (e.g. the subscription was + // cancelled after a successful or still-settling payment), + // discarding the join data silently would strand a paid + // member outside the CRM. + $paidInvoice = false; + if ($subscription && !empty($subscription->latest_invoice)) { + try { + $latestInvoice = \Stripe\Invoice::retrieve($subscription->latest_invoice); + $paidInvoice = $latestInvoice->status === 'paid'; + } catch (\Exception $e) { + $joinBlockLog->warning( + "ensureStripeSubscriptionsCreated: could not inspect latest invoice for {$name}: " + . $e->getMessage() + ); + } + } + if ($paidInvoice) { + $joinBlockLog->error( + "ensureStripeSubscriptionsCreated: subscription {$data['stripeSubscriptionId']} has a" + . " paid invoice — payment collected without a completed join. Preserving the" + . " submitted data for retry." + ); + // Do not discard. This is the only record of what the + // member submitted, so dropping it makes the join + // unrecoverable without asking them for it again. + // Services unknown: the join never ran, so retry them all. + self::queueCrmRetry( + $data, + null, + 'payment collected without a completed join' + ); + } else { + $joinBlockLog->info("ensureStripeSubscriptionsCreated: deleting unprocessable {$name}"); + } delete_option($name); } else { $joinBlockLog->info("ensureStripeSubscriptionsCreated: not yet paid, will retry {$name}"); @@ -460,6 +695,184 @@ public static function ensureStripeSubscriptionsCreated($subscriptionId = null) } } + /** + * Seconds to wait before the next attempt: 1h, 2h, 4h ... capped at 24h. + * + * A CRM that is down is usually down for everyone, so retrying every queued + * member hourly helps nobody and buries the useful log lines. + */ + public static function crmRetryBackoffSeconds($attempts) + { + $hours = min(2 ** max(0, (int) $attempts - 1), 24); + return $hours * 3600; + } + + public static function crmRetryIsDue($record, $now = null) + { + $now = $now ?? time(); + $lastAttemptAt = (int) ($record['lastAttemptAt'] ?? 0); + $backoff = self::crmRetryBackoffSeconds($record['attempts'] ?? 0); + return ($now - $lastAttemptAt) >= $backoff; + } + + /** + * Read-only check that the member's payment still stands. + * + * Deliberately never mutates. This runs long after the join, by which time + * the member may have re-joined or changed plan, so altering a subscription + * from this stale data would damage a payment that is working fine. + * + * @return bool|null true = paid, false = no payment, null = could not tell. + */ + private static function paymentStillStands($data) + { + $subscriptionId = $data['stripeSubscriptionId'] ?? ''; + if (!$subscriptionId) { + // A GoCardless or Chargebee join: there is no Stripe subscription to + // check, and the record only exists because a payment succeeded. + return true; + } + + StripeService::initialise(); + return StripeService::subscriptionWasPaid($subscriptionId); + } + + /** + * Stop retrying a record, without ever deleting it. + * + * Parking is how the worker makes progress on something it cannot finish. + * The record is the only copy of what the member submitted, so it stays for + * a human to pick up — but it is not re-attempted, and not re-logged on + * every subsequent run. + */ + private static function parkCrmRetry($optionName, $record, $reason) + { + global $joinBlockLog; + + $record['parked'] = true; + $record['parkedReason'] = $reason; + $record['parkedAt'] = time(); + + update_option($optionName, wp_json_encode($record), false); + + $joinBlockLog->error( + "ensureCrmPushesCompleted: parked $optionName — $reason. The submitted details are preserved in" + . " that option; complete the CRM record manually." + ); + } + + /** + * Retry outstanding CRM pushes for members whose payment already succeeded. + * + * Complements reconcileRecentMemberships(), which can detect a member who + * paid but never reached the CRM yet has no copy of their submitted details + * to fix it with. This worker holds that copy. + * + * Touches no payment state: it reads Stripe to confirm the payment stands, + * then only writes to CRMs. + */ + public static function ensureCrmPushesCompleted() + { + global $wpdb; + global $joinBlockLog; + + $joinBlockLog->info("Running ensureCrmPushesCompleted"); + + $results = $wpdb->get_results( + $wpdb->prepare( + "SELECT * FROM {$wpdb->prefix}options WHERE option_name LIKE %s", + self::CRM_RETRY_OPTION_PREFIX . '%' + ) + ); + + foreach ($results as $result) { + $name = $result->option_name; + try { + $record = json_decode($result->option_value, true); + + if (!is_array($record)) { + // Keep the raw value: it is still the only copy of whatever + // was written here, and may be recoverable by hand. + self::parkCrmRetry( + $name, + ['raw' => $result->option_value], + 'record could not be decoded' + ); + continue; + } + + if (!empty($record['parked'])) { + continue; + } + + $data = $record['data'] ?? null; + $email = $data['email'] ?? ''; + + if (!$data || !$email) { + // Without an email there is no CRM to push to and no lock to + // take, so this can never succeed. Park it rather than + // re-reading and re-logging it on every run forever. + self::parkCrmRetry($name, $record, 'no usable join data (missing payload or email)'); + continue; + } + + $attempts = (int) ($record['attempts'] ?? 0); + if ($attempts >= self::CRM_RETRY_MAX_ATTEMPTS) { + self::parkCrmRetry($name, $record, "gave up after $attempts attempts"); + continue; + } + + if (!self::crmRetryIsDue($record)) { + continue; + } + + $paymentStands = self::paymentStillStands($data); + if ($paymentStands === null) { + // Could not reach Stripe. Leave the attempt count alone so a + // provider outage does not burn through the retry budget. + $joinBlockLog->warning( + "ensureCrmPushesCompleted: could not confirm payment for $email, will try again later" + ); + continue; + } + if ($paymentStands === false) { + self::parkCrmRetry($name, $record, "no surviving payment for $email — refund or complete by hand"); + continue; + } + + // Same lock the /join endpoint and the webhooks take, so a retry + // cannot interleave with a live write for the same person. + $lockFile = self::acquireLock($email); + try { + $failed = self::pushToCrms($data, $record['services'] ?? null); + } finally { + self::releaseLock($lockFile); + } + + if (!$failed) { + delete_option($name); + // Deliberately does not fire ck_join_flow_success: handleJoin() + // now completes even when a CRM push fails, so the hook has + // already fired for this member and the heartbeat digest would + // count them twice. + $joinBlockLog->info("ensureCrmPushesCompleted: completed CRM push for $email, cleared $name"); + continue; + } + + $record['services'] = $failed; + $record['attempts'] = $attempts + 1; + $record['lastAttemptAt'] = time(); + update_option($name, wp_json_encode($record), false); + $joinBlockLog->warning( + "ensureCrmPushesCompleted: still outstanding for $email (" . implode(', ', $failed) . ")," + . " attempt " . ($attempts + 1) . " of " . self::CRM_RETRY_MAX_ATTEMPTS + ); + } catch (\Exception $e) { + $joinBlockLog->error("ensureCrmPushesCompleted: could not process $name: " . $e->getMessage()); + } + } + } + public static function shouldLapseMember($email, $context = [], $default = true) { return (bool) apply_filters('ck_join_flow_should_lapse_member', $default, $email, $context); diff --git a/packages/join-block/src/Services/StripeService.php b/packages/join-block/src/Services/StripeService.php index 7573f99..bc1a15c 100644 --- a/packages/join-block/src/Services/StripeService.php +++ b/packages/join-block/src/Services/StripeService.php @@ -13,11 +13,49 @@ class StripeService { + /** + * How long after a subscription's first invoice is created its webhooks + * keep deferring to the /join endpoint for Action Network state. Within + * this window first-invoice webhooks race the /join request, and + * concurrent Action Network writes mangle the webhook payloads Action + * Network sends to downstream consumers (e.g. Make). Card payments + * resolve in seconds, so anything older is a delayed-settlement payment + * method (e.g. Bacs, which takes days) or a webhook redelivery — /join + * finished long ago and there is nothing left to race. + */ + public const FIRST_INVOICE_RACE_WINDOW = 3600; + public static function initialise() { Stripe::setApiKey(Settings::get('STRIPE_SECRET_KEY')); } + /** + * Decide how to treat a webhook for a subscription_create (first) invoice. + * + * @param bool $hasUnprocessedJoinData The JOIN_FORM_UNPROCESSED_STRIPE_REQUEST + * option still exists, i.e. /join has not run. + * @param int $invoiceCreated Invoice creation timestamp. + * @param int|null $now Current timestamp, injectable for tests. + * @return string 'recover' — the join never completed: run the saved-data + * recovery path (invoice.paid only); + * 'defer' — /join may still be in flight: leave Action + * Network state alone; + * 'settle' — late settlement: /join is long done, act on + * the member's state directly. + */ + public static function classifyFirstInvoiceEvent($hasUnprocessedJoinData, $invoiceCreated, $now = null) + { + $now = $now ?? time(); + if ($hasUnprocessedJoinData) { + return 'recover'; + } + if (($now - (int) $invoiceCreated) < self::FIRST_INVOICE_RACE_WINDOW) { + return 'defer'; + } + return 'settle'; + } + /** * Validates a one-off donation amount. * Returns null if valid, or an error message string if invalid. @@ -107,7 +145,39 @@ public static function resolveSubscriptionPriceStrategy(array $plan, float $cust return 'default'; } - public static function createSubscription($customer, $plan, $customAmount = null, $donationAmount = null, $recurDonation = false, $isSupporterMode = false) + /** + * Stable key for a subscription-creation request, or null when one cannot + * be derived safely. + * + * Submitting the same choices twice within one form session should return + * the subscription Stripe already created rather than making another one: + * resubmitting after an error is how a member ends up charged twice. + * Changing plan or amount changes the key, as does starting a new session, + * so a deliberate second subscription is still possible. + * + * Without a session token the key would collapse to email + plan, and two + * genuinely separate joins would collide — so we return null and accept the + * old behaviour rather than risk replaying the wrong subscription. + */ + public static function buildSubscriptionIdempotencyKey($data, $plan) + { + $sessionToken = $data['sessionToken'] ?? ''; + if (!$sessionToken) { + return null; + } + + return 'join_sub_' . sha1(implode('|', [ + $sessionToken, + strtolower(trim((string) ($data['email'] ?? ''))), + Settings::getMembershipPlanId($plan), + (string) ($data['customMembershipAmount'] ?? ''), + (string) ($data['donationAmount'] ?? ''), + !empty($data['recurDonation']) ? '1' : '0', + !empty($data['donationSupporterMode']) ? '1' : '0', + ])); + } + + public static function createSubscription($customer, $plan, $customAmount = null, $donationAmount = null, $recurDonation = false, $isSupporterMode = false, $idempotencyKey = null) { $priceId = $plan["stripe_price_id"]; $customAmount = (float) $customAmount; @@ -151,7 +221,8 @@ public static function createSubscription($customer, $plan, $customAmount = null $subscriptionPayload['add_invoice_items'] = $addInvoiceItems; } - $subscription = Subscription::create($subscriptionPayload); + $options = $idempotencyKey ? ['idempotency_key' => $idempotencyKey] : []; + $subscription = Subscription::create($subscriptionPayload, $options); return $subscription; } @@ -524,29 +595,119 @@ public static function getOrCreatePriceForProduct($product, $amount, $currency, } /** - * Cancels the customer's existing subscriptions, except for $subscriptionId, which is the - * subscription that was just created via the browser Stripe client during this join flow. - * That current subscription is left in place, and its amount is returned so the caller can - * verify it against the amount claimed in the join request. + * The recurring amount of one specific subscription, in major units + * (e.g. 3.30 for £3.30), or null when it cannot be determined. + * + * Fetches by ID rather than scanning the customer's subscription list, so + * there is no "wasn't in the page we looked at" failure mode. Callers MUST + * treat null as *unknown* and never as zero: a null that gets coerced to + * 0.0 looks exactly like a genuine amount mismatch, which is how a paying + * member can be rejected at the amount check. + * + * API errors propagate rather than being swallowed, so a transport failure + * (retryable) stays distinguishable from a subscription that really has no + * priced item (not retryable). + * + * @throws ApiErrorException */ - public static function cancelPreviousAndGetCurrentSubscription($email, $customerId, $subscriptionId) + public static function getSubscriptionAmount($subscriptionId) { global $joinBlockLog; - $joinBlockLog->info("Removing previous subscriptions for user " . $email . ", customer: " . $customerId); + if (!$subscriptionId) { + $joinBlockLog->warning("getSubscriptionAmount: called without a subscription ID"); + return null; + } + + $subscription = Subscription::retrieve($subscriptionId); + $item = $subscription->items->first(); + + if (!$item || !isset($item->price->unit_amount)) { + $joinBlockLog->warning("getSubscriptionAmount: $subscriptionId has no priced item"); + return null; + } + + return round($item->price->unit_amount / 100, 2); + } + + /** + * Whether a subscription represents a payment that actually went through. + * + * Strictly read-only — used by the CRM retry worker to confirm a member + * really paid before writing them to a CRM, long after the join. Nothing + * here may mutate the subscription: by now it may be a live membership that + * has moved on from the data the retry record was written with. + * + * @return bool|null true = paid, false = not paid, null = could not tell + * (transient failure — the caller should try again later + * rather than conclude anything). + */ + public static function subscriptionWasPaid($subscriptionId) + { + global $joinBlockLog; + + try { + $subscription = Subscription::retrieve($subscriptionId); + } catch (\Stripe\Exception\InvalidRequestException $e) { + // The subscription does not exist; a definite answer, not an outage. + $joinBlockLog->warning("subscriptionWasPaid($subscriptionId): not found — " . $e->getMessage()); + return false; + } catch (\Exception $e) { + $joinBlockLog->warning("subscriptionWasPaid($subscriptionId): could not check — " . $e->getMessage()); + return null; + } + + if (in_array($subscription->status, ['active', 'trialing'], true)) { + return true; + } + + // A cancelled subscription still counts if money changed hands: the + // member paid, so they belong in the CRM regardless of its state now. + if (empty($subscription->latest_invoice)) { + return false; + } + + try { + $invoice = \Stripe\Invoice::retrieve($subscription->latest_invoice); + } catch (\Exception $e) { + $joinBlockLog->warning( + "subscriptionWasPaid($subscriptionId): could not inspect invoice — " . $e->getMessage() + ); + return null; + } + + return $invoice->status === 'paid'; + } + + /** + * Resolve the member's Stripe customer, creating one if we do not have it. + */ + public static function resolveCustomerId($email, $customerId) + { + if ($customerId) { + return $customerId; + } + [$customer,] = self::upsertCustomer($email); + return $customer->id; + } + + /** + * Read-only: the member's earliest subscription date, and their first and + * last payment dates, for the CRM custom fields. + * + * Best-effort — a transient Stripe error must not fail an otherwise good + * join, so on failure the caller gets today's date and nulls rather than an + * exception. + */ + public static function getSubscriptionDates($email, $customerId) + { + global $joinBlockLog; $firstSubscriptionDate = date('Y-m-d'); $firstPayment = null; $lastPayment = null; - $amount = 0; - - if (!$customerId) { - [$customer,] = self::upsertCustomer($email); - $customerId = $customer->id; - } try { - // Fetch all subscriptions for date calculation $subscriptions = \Stripe\Subscription::all([ 'customer' => $customerId, 'status' => 'all', @@ -554,40 +715,12 @@ public static function cancelPreviousAndGetCurrentSubscription($email, $customer ]); foreach ($subscriptions->autoPagingIterator() as $sub) { - // Track earliest subscription date $createdDate = date('Y-m-d', $sub->created); - if (is_null($firstSubscriptionDate) || $createdDate < $firstSubscriptionDate) { + if ($createdDate < $firstSubscriptionDate) { $firstSubscriptionDate = $createdDate; } - - // Cancel and void if it's an "active-ish" subscription and not the current one - if ($sub->id !== $subscriptionId && in_array($sub->status, ['active', 'trialing', 'past_due'])) { - $joinBlockLog->info("Canceling subscription " . $sub->id . " for user " . $email); - $sub->cancel(); - - // Find and void open invoices for this subscription - $invoices = \Stripe\Invoice::all([ - 'customer' => $customerId, - 'subscription' => $sub->id, - 'status' => 'open', - 'limit' => 100, - ]); - - foreach ($invoices->autoPagingIterator() as $invoice) { - $joinBlockLog->info("Voiding invoice " . $invoice->id . " for canceled subscription " . $sub->id); - $invoice->voidInvoice(); - } - } - - if ($sub->id === $subscriptionId) { - $subItem = $sub->items->first(); - if ($subItem) { - $amount = $subItem->price->unit_amount; - } - } } - // Fetch all paid invoices for first/last payment dates $paidInvoices = \Stripe\Invoice::all([ 'customer' => $customerId, 'status' => 'paid', @@ -604,17 +737,71 @@ public static function cancelPreviousAndGetCurrentSubscription($email, $customer } } } catch (\Exception $e) { - $joinBlockLog->error("Error removing subscriptions for user " . $email . ": " . $e->getMessage()); + $joinBlockLog->error("Error reading subscription dates for " . $email . ": " . $e->getMessage()); } return [ "firstSubscription" => $firstSubscriptionDate, "firstPayment" => $firstPayment, "lastPayment" => $lastPayment, - "amount" => round($amount / 100, 2), ]; } + /** + * Cancel every *other* active-ish subscription for the customer, removing + * the duplicates a member creates by submitting the payment step twice. + * $subscriptionId is the one to keep: the subscription the browser Stripe + * client just created. + * + * Only safe to call while $subscriptionId is genuinely the member's current + * subscription. Calling it from stale data — a recovery replay hours later, + * say — would cancel a live subscription the member has since created. See + * ensureStripeSubscriptionsCreated(), which deliberately does not. + */ + public static function cancelPreviousSubscriptions($email, $customerId, $subscriptionId) + { + global $joinBlockLog; + + $joinBlockLog->info("Removing previous subscriptions for user " . $email . ", customer: " . $customerId); + + try { + $subscriptions = \Stripe\Subscription::all([ + 'customer' => $customerId, + 'status' => 'all', + 'limit' => 100, + ]); + + foreach ($subscriptions->autoPagingIterator() as $sub) { + if ($sub->id === $subscriptionId) { + continue; + } + if (!in_array($sub->status, ['active', 'trialing', 'past_due'])) { + continue; + } + + $joinBlockLog->info("Canceling subscription " . $sub->id . " for user " . $email); + $sub->cancel(); + + // Find and void open invoices for this subscription + $invoices = \Stripe\Invoice::all([ + 'customer' => $customerId, + 'subscription' => $sub->id, + 'status' => 'open', + 'limit' => 100, + ]); + + foreach ($invoices->autoPagingIterator() as $invoice) { + $joinBlockLog->info( + "Voiding invoice " . $invoice->id . " for canceled subscription " . $sub->id + ); + $invoice->voidInvoice(); + } + } + } catch (\Exception $e) { + $joinBlockLog->error("Error removing subscriptions for user " . $email . ": " . $e->getMessage()); + } + } + public static function getSubscriptionHistory($customerId) { global $joinBlockLog; @@ -776,10 +963,46 @@ public static function handleWebhook($event) $customerId = $subscription['customer'] ?? '(unknown)'; $joinBlockLog->info("Subscription cancelled for Stripe customer $customerId"); - if (!empty($subscription['customer'])) { - $customerLapsed = true; - $lapseTrigger = 'subscription_deleted'; + if (empty($subscription['customer'])) { + break; } + + // Cancelling a subscription does not stop a first payment + // that is still settling (delayed methods such as Bacs): + // it can land days later against the dead subscription. + $latestInvoiceId = $subscription['latest_invoice'] ?? null; + if ($latestInvoiceId) { + try { + $latestInvoice = \Stripe\Invoice::retrieve($latestInvoiceId); + if ($latestInvoice->status === 'open') { + $joinBlockLog->error( + "Subscription {$subscription['id']} for Stripe customer $customerId was cancelled" + . " with open invoice {$latestInvoice->id}. If its payment is still processing it" + . " will settle against the dead subscription — void the invoice or refund the" + . " payment." + ); + } + } catch (\Exception $e) { + $joinBlockLog->warning( + "Could not inspect latest invoice for cancelled subscription {$subscription['id']}: " + . $e->getMessage() + ); + } + } + + // Cancelling one subscription must not lapse a member who + // still has another live one — e.g. /join cancels the + // previous subscription moments after creating its + // replacement during a re-join or tier change. + if (self::customerHasActiveSubscription($customerId, $subscription['id'] ?? null)) { + $joinBlockLog->info( + "Skipping lapsing for Stripe customer $customerId: another active subscription exists." + ); + break; + } + + $customerLapsed = true; + $lapseTrigger = 'subscription_deleted'; break; case 'invoice.payment_failed': @@ -787,8 +1010,25 @@ public static function handleWebhook($event) $customerId = $invoice['customer'] ?? '(unknown)'; if (($invoice['billing_reason'] ?? null) === 'subscription_create') { - $joinBlockLog->info("Skipping invoice.payment_failed lapsing for Stripe customer $customerId: subscription_create invoice, /join endpoint will handle Action Network state."); - break; + $subscriptionId = $invoice['subscription'] + ?? $invoice['parent']['subscription_details']['subscription'] + ?? null; + $hasUnprocessedJoinData = $subscriptionId + && get_option("JOIN_FORM_UNPROCESSED_STRIPE_REQUEST_{$subscriptionId}"); + + if (self::classifyFirstInvoiceEvent($hasUnprocessedJoinData, $invoice['created'] ?? 0) !== 'settle') { + $joinBlockLog->info("Skipping invoice.payment_failed lapsing for Stripe customer $customerId: subscription_create invoice, /join endpoint will handle Action Network state."); + break; + } + + // A first invoice failing this long after signup means a + // delayed-settlement payment (e.g. Bacs) bounced after the + // join completed: the member is tagged but never paid. + // Fall through to the normal failed-payment handling. + $joinBlockLog->warning( + "Late first-invoice payment failure for Stripe customer $customerId" + . " (invoice {$invoice['id']}): the join completed but the payment has now failed." + ); } if (empty($invoice['next_payment_attempt'])) { @@ -817,18 +1057,31 @@ public static function handleWebhook($event) $customerId = $invoice['customer'] ?? '(unknown)'; $joinBlockLog->info("Invoice paid for Stripe customer $customerId"); if (($invoice['billing_reason'] ?? null) === 'subscription_create') { - $joinBlockLog->info("Skipping invoice.paid un-lapsing for Stripe customer $customerId: subscription_create invoice, /join endpoint will handle Action Network state."); - // If the member paid but never returned to the site, the /join - // endpoint is never hit, so complete the join from the saved - // form data instead. NOTE: this must run without acquiring the - // per-email lock here — handleJoin() acquires it itself, and - // flock blocks on a second handle even within one process. $subscriptionId = $invoice['subscription'] ?? $invoice['parent']['subscription_details']['subscription'] ?? null; - if ($subscriptionId) { + $hasUnprocessedJoinData = $subscriptionId + && get_option("JOIN_FORM_UNPROCESSED_STRIPE_REQUEST_{$subscriptionId}"); + $classification = self::classifyFirstInvoiceEvent($hasUnprocessedJoinData, $invoice['created'] ?? 0); + + if ($classification === 'recover') { + // The member paid but never returned to the site, so the + // /join endpoint was never hit: complete the join from the + // saved form data instead. NOTE: this must run without + // acquiring the per-email lock here — handleJoin() acquires + // it itself, and flock blocks on a second handle even + // within one process. + $joinBlockLog->info("Skipping invoice.paid un-lapsing for Stripe customer $customerId: subscription_create invoice, /join endpoint will handle Action Network state."); JoinService::ensureStripeSubscriptionsCreated($subscriptionId); + break; + } + + if ($classification === 'defer') { + $joinBlockLog->info("Skipping invoice.paid un-lapsing for Stripe customer $customerId: subscription_create invoice, /join endpoint will handle Action Network state."); + break; } + + self::handleLateFirstInvoicePaid($invoice, $subscriptionId, $event); break; } if (!empty($invoice['customer'])) { @@ -891,8 +1144,20 @@ public static function handleWebhook($event) $currentStatus = $subscription['status'] ?? null; $email = null; - // Only act on status changes not already covered by invoice events - if ($previousStatus && $previousStatus !== $currentStatus) { + // A new subscription's incomplete -> active flip happens + // seconds before /join writes to Action Network: acting on + // it here recreates exactly the concurrent-write problem + // the subscription_create invoice guard exists to prevent. + $subscriptionAge = time() - (int) ($subscription['created'] ?? 0); + if ( + $previousStatus && $previousStatus !== $currentStatus + && $subscriptionAge < self::FIRST_INVOICE_RACE_WINDOW + ) { + $joinBlockLog->info( + "Skipping status change handling for new subscription {$subscription['id']}" + . " ($previousStatus -> $currentStatus): /join endpoint will handle Action Network state." + ); + } elseif ($previousStatus && $previousStatus !== $currentStatus) { $cid = $subscription['customer']; $email = self::getEmailForCustomer($cid); @@ -1023,6 +1288,224 @@ private static function getEmailForCustomer($customerId) return $customer->email; } + private static function customerHasActiveSubscription($customerId, $excludeSubscriptionId = null) + { + global $joinBlockLog; + + try { + foreach (['active', 'trialing'] as $status) { + $subscriptions = Subscription::all([ + 'customer' => $customerId, + 'status' => $status, + 'limit' => 10, + ]); + foreach ($subscriptions->data as $subscription) { + if ($subscription->id !== $excludeSubscriptionId) { + return true; + } + } + } + } catch (\Exception $e) { + $joinBlockLog->warning( + "Could not list subscriptions for Stripe customer $customerId: " . $e->getMessage() + ); + } + return false; + } + + /** + * A subscription_create invoice was paid long after it was created — a + * delayed-settlement payment method (e.g. Bacs direct debit, ~6 working + * days) or a webhook redelivery. Unlike fresh first invoices this cannot + * race the /join endpoint, so act on the member's state directly: un-lapse + * them if their subscription is live, or raise an alert if money has been + * collected for a subscription that no longer exists. + */ + private static function handleLateFirstInvoicePaid($invoice, $subscriptionId, $event) + { + global $joinBlockLog; + + $customerId = $invoice['customer']; + + $subscription = null; + if ($subscriptionId) { + try { + $subscription = Subscription::retrieve($subscriptionId); + } catch (\Stripe\Exception\InvalidRequestException $e) { + $joinBlockLog->warning( + "Late first-invoice settlement: could not retrieve subscription $subscriptionId: " + . $e->getMessage() + ); + } + } + $status = $subscription->status ?? 'missing'; + + if (!in_array($status, ['active', 'trialing'])) { + $joinBlockLog->error( + "Payment collected for dead subscription: first invoice {$invoice['id']} of Stripe customer" + . " $customerId settled late, but subscription " . ($subscriptionId ?: '(unknown)') . " is $status." + . " Refund the payment or reinstate the membership manually." + ); + return; + } + + $email = self::getEmailForCustomer($customerId); + if (!$email) { + $joinBlockLog->error( + "Late first-invoice settlement for Stripe customer $customerId: could not resolve an email" + . " address, cannot reconcile Action Network state." + ); + return; + } + + $joinBlockLog->info( + "Late first-invoice settlement for $email: subscription $subscriptionId is $status," + . " ensuring the member is not marked lapsed." + ); + + $lockFile = JoinService::acquireLock($email); + try { + $context = ['provider' => 'stripe', 'trigger' => 'late_first_invoice_paid', 'event' => $event]; + if (JoinService::shouldUnlapseMember($email, $context)) { + JoinService::toggleMemberLapsed($email, false, null, $context); + } + } finally { + JoinService::releaseLock($lockFile); + } + } + + /** + * Level-triggered safety net, run daily by cron: converge Action Network + * state with Stripe for customers with recent subscription activity, + * catching webhooks that were missed, suppressed or arrived out of order. + * + * Only the unambiguous divergence is fixed automatically (a lapsed tag on + * a paying member with a live subscription, at most one Action Network + * write per person, under the per-email lock). Everything else — money + * collected with no CRM record, money collected with no live subscription, + * a member whose email is bouncing — is alerted via error/warning-level + * logs, which reach Sentry, because fixing those requires either full join + * data or a human decision (refund vs reinstate). + */ + public static function reconcileRecentMemberships($sinceDays = 7) + { + global $joinBlockLog; + + $joinBlockLog->info("Running reconcileRecentMemberships"); + + if (!Settings::get("USE_ACTION_NETWORK")) { + $joinBlockLog->info("reconcileRecentMemberships: Action Network integration disabled, nothing to do"); + return; + } + + $subscriptions = Subscription::all([ + 'created' => ['gte' => time() - $sinceDays * 86400], + 'status' => 'all', + 'limit' => 100, + ]); + + // Group by customer so a re-join sequence (cancelled subscription + // followed by its live replacement) is judged as one membership. + $customerIds = []; + foreach ($subscriptions->autoPagingIterator() as $subscription) { + $customerIds[$subscription->customer] = true; + } + + foreach (array_keys($customerIds) as $customerId) { + try { + self::reconcileCustomerMembership($customerId); + } catch (\Exception $e) { + $joinBlockLog->error( + "reconcileRecentMemberships: failed for Stripe customer $customerId: " . $e->getMessage() + ); + } + } + } + + private static function reconcileCustomerMembership($customerId) + { + global $joinBlockLog; + + $paidInvoices = \Stripe\Invoice::all([ + 'customer' => $customerId, + 'status' => 'paid', + 'limit' => 1, + ]); + if (count($paidInvoices->data) === 0) { + // Abandoned signup (incomplete payment, no charge): correctly has + // no membership, nothing to reconcile. + return; + } + + $email = self::getEmailForCustomer($customerId); + if (!$email) { + $joinBlockLog->warning( + "reconcileRecentMemberships: could not resolve email for Stripe customer $customerId" + ); + return; + } + + $hasLiveSubscription = self::customerHasActiveSubscription($customerId); + $lapsedTag = Settings::get("LAPSED_TAG"); + + $person = ActionNetworkService::getPersonSnapshot($email); + $isLapsed = $person !== null + && $lapsedTag + && is_array($person['tags']) + && in_array($lapsedTag, $person['tags'], true); + + if ($hasLiveSubscription) { + if ($person === null) { + $joinBlockLog->error( + "reconcileRecentMemberships: $email has a paid, active Stripe subscription but no Action" + . " Network record — the join never completed. Investigate and backfill." + ); + return; + } + + // A person record on its own is not evidence the join completed: a + // newsletter or petition signup creates one from an email alone, + // and that record then masks the missing join. The join flow always + // collects a name, so its absence means the demographic push never + // ran. (Update-flow joins deliberately send no name, but those are + // existing members who already have one.) + if (empty($person['has_name'])) { + $joinBlockLog->error( + "reconcileRecentMemberships: $email has a paid, active Stripe subscription but no name in" + . " Action Network — the join's demographic push never completed. Investigate and backfill." + ); + } + + if ($isLapsed) { + $joinBlockLog->error( + "reconcileRecentMemberships: $email has an active Stripe subscription but carries the" + . " '$lapsedTag' tag — removing it." + ); + $lockFile = JoinService::acquireLock($email); + try { + $context = ['provider' => 'stripe', 'trigger' => 'reconciliation', 'event' => null]; + if (JoinService::shouldUnlapseMember($email, $context)) { + JoinService::toggleMemberLapsed($email, false, null, $context); + } + } finally { + JoinService::releaseLock($lockFile); + } + } + + if (($person['email_status'] ?? null) === 'bouncing') { + $joinBlockLog->warning( + "reconcileRecentMemberships: paying member $email has a bouncing email address in Action" + . " Network — they are not receiving membership emails. Possible typo at signup." + ); + } + } elseif ($person !== null && !$isLapsed && is_array($person['tags'])) { + $joinBlockLog->error( + "reconcileRecentMemberships: $email has paid Stripe invoices but no live subscription, and is" + . " not marked lapsed in Action Network. Refund or reinstate manually." + ); + } + } + private static function resolveTierTagChanges(array $newPlan, ?array $oldPlan): array { $parseTags = fn($str) => array_filter(array_map('trim', explode(',', $str ?? '')), fn($t) => $t !== ''); diff --git a/packages/join-block/tests/CrmRetryBackoffTest.php b/packages/join-block/tests/CrmRetryBackoffTest.php new file mode 100644 index 0000000..577c183 --- /dev/null +++ b/packages/join-block/tests/CrmRetryBackoffTest.php @@ -0,0 +1,95 @@ +assertSame( + $expectedHours * self::HOUR, + JoinService::crmRetryBackoffSeconds($attempts) + ); + } + + public function backoffSchedule(): array + { + return [ + 'never attempted' => [0, 1], + 'after 1 failure' => [1, 1], + 'after 2 failures' => [2, 2], + 'after 3 failures' => [3, 4], + 'after 4 failures' => [4, 8], + 'after 5 failures' => [5, 16], + 'capped at 24h' => [6, 24], + 'still capped' => [12, 24], + ]; + } + + /** + * A negative or nonsense attempt count must not produce a negative or zero + * delay, which would spin the worker. + */ + public function testMalformedAttemptCountStillYieldsAPositiveDelay(): void + { + $this->assertSame(self::HOUR, JoinService::crmRetryBackoffSeconds(-5)); + } + + // ------------------------------------------------------------------------- + // Due-ness + // ------------------------------------------------------------------------- + + public function testRecordIsDueOnceTheBackoffHasElapsed(): void + { + $now = 1_700_000_000; + $record = ['attempts' => 3, 'lastAttemptAt' => $now - (4 * self::HOUR)]; + + $this->assertTrue(JoinService::crmRetryIsDue($record, $now)); + } + + public function testRecordIsNotDueBeforeTheBackoffHasElapsed(): void + { + $now = 1_700_000_000; + $record = ['attempts' => 3, 'lastAttemptAt' => $now - (4 * self::HOUR) + 1]; + + $this->assertFalse(JoinService::crmRetryIsDue($record, $now)); + } + + /** + * A freshly queued record has no lastAttemptAt, and must be picked up on + * the next run rather than waiting out a backoff it never earned. + */ + public function testFreshlyQueuedRecordIsDueImmediately(): void + { + $this->assertTrue(JoinService::crmRetryIsDue([], 1_700_000_000)); + } + + public function testLongStalledRecordIsDue(): void + { + $now = 1_700_000_000; + $record = ['attempts' => 11, 'lastAttemptAt' => $now - (30 * 24 * self::HOUR)]; + + $this->assertTrue(JoinService::crmRetryIsDue($record, $now)); + } +} diff --git a/packages/join-block/tests/CrmRetryKeyTest.php b/packages/join-block/tests/CrmRetryKeyTest.php new file mode 100644 index 0000000..bcd6198 --- /dev/null +++ b/packages/join-block/tests/CrmRetryKeyTest.php @@ -0,0 +1,133 @@ +alias('json_encode'); + } + + protected function tearDown(): void + { + Monkey\tearDown(); + parent::tearDown(); + } + + // ------------------------------------------------------------------------- + // Preferred identifiers + // ------------------------------------------------------------------------- + + public function testPrefersTheStripeSubscriptionId(): void + { + $key = JoinService::crmRetryKey([ + 'stripeSubscriptionId' => 'sub_123', + 'email' => 'member@example.com', + ]); + + $this->assertSame(JoinService::CRM_RETRY_OPTION_PREFIX . 'sub_123', $key); + } + + public function testFallsBackToEmailWhenThereIsNoSubscription(): void + { + $withEmail = JoinService::crmRetryKey(['email' => 'member@example.com']); + $other = JoinService::crmRetryKey(['email' => 'someone-else@example.com']); + + $this->assertStringStartsWith(JoinService::CRM_RETRY_OPTION_PREFIX . 'email_', $withEmail); + $this->assertNotSame($withEmail, $other); + } + + public function testEmailIsNormalisedSoOneMemberGetsOneRecord(): void + { + $this->assertSame( + JoinService::crmRetryKey(['email' => 'member@example.com']), + JoinService::crmRetryKey(['email' => ' Member@Example.COM ']) + ); + } + + // ------------------------------------------------------------------------- + // Collision resistance — the reason the fallback chain exists + // ------------------------------------------------------------------------- + + /** + * handleJoin() can be called without an email (it falls back to + * sessionToken for its lock), so this is reachable. Hashing an empty string + * would give every such record the same option name, and each queued member + * would overwrite the last. + */ + public function testTwoRecordsWithNoSubscriptionOrEmailDoNotCollide(): void + { + $first = JoinService::crmRetryKey(['sessionToken' => 'session-aaa']); + $second = JoinService::crmRetryKey(['sessionToken' => 'session-bbb']); + + $this->assertNotSame($first, $second); + $this->assertStringStartsWith(JoinService::CRM_RETRY_OPTION_PREFIX . 'session_', $first); + } + + public function testBlankIdentifiersAreTreatedAsAbsentNotHashed(): void + { + $first = JoinService::crmRetryKey([ + 'stripeSubscriptionId' => '', + 'email' => ' ', + 'sessionToken' => 'session-aaa', + ]); + $second = JoinService::crmRetryKey([ + 'stripeSubscriptionId' => '', + 'email' => '', + 'sessionToken' => 'session-bbb', + ]); + + $this->assertNotSame($first, $second); + } + + /** + * With no identifier at all, distinct payloads must still get distinct + * records. Identical payloads collapsing to one is correct de-duplication. + */ + public function testWithNoIdentifiersAtAllDistinctPayloadsStillDiffer(): void + { + $first = JoinService::crmRetryKey(['firstName' => 'Ada']); + $second = JoinService::crmRetryKey(['firstName' => 'Grace']); + + $this->assertNotSame($first, $second); + $this->assertStringStartsWith(JoinService::CRM_RETRY_OPTION_PREFIX . 'payload_', $first); + $this->assertSame($first, JoinService::crmRetryKey(['firstName' => 'Ada'])); + } + + /** + * The degenerate case the fallback chain was added for: two entirely empty + * payloads are indistinguishable, but must not take a key that a real + * member's record could also land on. + */ + public function testEmptyPayloadDoesNotShareAKeyWithAnIdentifiedMember(): void + { + $empty = JoinService::crmRetryKey([]); + $identified = JoinService::crmRetryKey(['email' => 'member@example.com']); + + $this->assertNotSame($empty, $identified); + } + + public function testKeyFitsWithinTheWordPressOptionNameColumn(): void + { + $key = JoinService::crmRetryKey(['sessionToken' => str_repeat('x', 500)]); + + $this->assertLessThanOrEqual(191, strlen($key)); + } +} diff --git a/packages/join-block/tests/FirstInvoiceClassifierTest.php b/packages/join-block/tests/FirstInvoiceClassifierTest.php new file mode 100644 index 0000000..c9d7ee6 --- /dev/null +++ b/packages/join-block/tests/FirstInvoiceClassifierTest.php @@ -0,0 +1,70 @@ +assertSame( + 'recover', + StripeService::classifyFirstInvoiceEvent(true, self::NOW - 5, self::NOW) + ); + $this->assertSame( + 'recover', + StripeService::classifyFirstInvoiceEvent(true, self::NOW - 10 * 86400, self::NOW) + ); + } + + public function testDeferWhenInvoiceIsFresh(): void + { + // A card-paid first invoice settles seconds after creation, while + // /join is still in flight: leave Action Network state alone. + $this->assertSame( + 'defer', + StripeService::classifyFirstInvoiceEvent(false, self::NOW - 5, self::NOW) + ); + $this->assertSame( + 'defer', + StripeService::classifyFirstInvoiceEvent( + false, + self::NOW - (StripeService::FIRST_INVOICE_RACE_WINDOW - 1), + self::NOW + ) + ); + } + + public function testSettleWhenInvoiceIsOld(): void + { + // A first invoice settling days later (e.g. Bacs) cannot race /join. + $this->assertSame( + 'settle', + StripeService::classifyFirstInvoiceEvent( + false, + self::NOW - StripeService::FIRST_INVOICE_RACE_WINDOW, + self::NOW + ) + ); + $this->assertSame( + 'settle', + StripeService::classifyFirstInvoiceEvent(false, self::NOW - 8 * 86400, self::NOW) + ); + } + + public function testMissingInvoiceCreatedTimestampSettles(): void + { + // A zero/absent created timestamp must not be mistaken for a fresh + // invoice, or the event would be dropped forever. + $this->assertSame( + 'settle', + StripeService::classifyFirstInvoiceEvent(false, 0, self::NOW) + ); + } +} diff --git a/packages/join-block/tests/JoinServiceMailchimpTest.php b/packages/join-block/tests/JoinServiceMailchimpTest.php index f959b1b..43755bc 100644 --- a/packages/join-block/tests/JoinServiceMailchimpTest.php +++ b/packages/join-block/tests/JoinServiceMailchimpTest.php @@ -55,11 +55,16 @@ class JoinServiceMailchimpTest extends TestCase 'add_tags' => '', ]; + /** Options written during a test, so we can assert on the CRM retry record. */ + private array $savedOptions = []; + protected function setUp(): void { parent::setUp(); Monkey\setUp(); + $this->savedOptions = []; + // WordPress functions used in the join path. Monkey\Functions\when('wp_json_encode')->alias('json_encode'); Monkey\Functions\when('esc_html')->returnArg(); @@ -92,7 +97,19 @@ public function get_results(string $query, $output = null): array if ($key === 'ck_join_flow_membership_plan_standard') { return $this->membershipPlan; } - return false; + return $this->savedOptions[$key] ?? false; + }); + + Monkey\Functions\when('update_option') + ->alias(function (string $key, $value) { + $this->savedOptions[$key] = $value; + return true; + }); + + Monkey\Functions\when('delete_option') + ->alias(function (string $key) { + unset($this->savedOptions[$key]); + return true; }); // All plugin settings default to false/empty — no payment provider, @@ -156,6 +173,21 @@ public function testMailchimpFailureDoesNotBlockJoin(): void global $joinBlockLog; $this->assertNotEmpty($joinBlockLog->errors, 'Expected Mailchimp error to be logged'); $this->assertStringContainsString('Mailchimp', $joinBlockLog->errors[0]); + + // ...and the outstanding push recorded, so the retry worker can finish + // it. Previously the failure was swallowed and the member was simply + // never added to Mailchimp. + $queued = array_filter( + $this->savedOptions, + fn($key) => str_starts_with($key, JoinService::CRM_RETRY_OPTION_PREFIX), + ARRAY_FILTER_USE_KEY + ); + $this->assertCount(1, $queued, 'Expected the failed Mailchimp push to be queued for retry'); + + $record = json_decode((string) reset($queued), true); + $this->assertSame(['mailchimp'], $record['services']); + $this->assertSame($this->joinData['email'], $record['data']['email']); + $this->assertSame(1, $record['attempts']); } /** diff --git a/packages/join-block/tests/SubscriptionIdempotencyKeyTest.php b/packages/join-block/tests/SubscriptionIdempotencyKeyTest.php new file mode 100644 index 0000000..6a56010 --- /dev/null +++ b/packages/join-block/tests/SubscriptionIdempotencyKeyTest.php @@ -0,0 +1,172 @@ + 'Cost of a cup of coffee in your country', + 'frequency' => 'monthly', + 'currency' => 'USD', + ]; + + private array $data = [ + 'sessionToken' => '11111111-2222-3333-4444-555555555555', + 'email' => 'member@example.com', + 'customMembershipAmount' => null, + 'donationAmount' => null, + 'recurDonation' => false, + 'donationSupporterMode' => false, + ]; + + protected function setUp(): void + { + parent::setUp(); + Monkey\setUp(); + + Monkey\Functions\when('sanitize_title')->alias(function ($title) { + $title = strtolower((string) $title); + $title = preg_replace('/[^a-z0-9]+/', '-', $title); + return trim($title, '-'); + }); + } + + protected function tearDown(): void + { + Monkey\tearDown(); + parent::tearDown(); + } + + private function key(array $overrides = []): ?string + { + return StripeService::buildSubscriptionIdempotencyKey( + array_merge($this->data, $overrides), + $this->plan + ); + } + + // ------------------------------------------------------------------------- + // Stability — the double-charge case + // ------------------------------------------------------------------------- + + public function testResubmittingTheSameChoicesProducesTheSameKey(): void + { + $this->assertSame($this->key(), $this->key()); + } + + public function testEmailIsNormalisedSoCasingDoesNotSplitTheKey(): void + { + $this->assertSame( + $this->key(), + $this->key(['email' => ' Member@Example.COM ']) + ); + } + + // ------------------------------------------------------------------------- + // Distinctness — a deliberate second subscription must still be possible + // ------------------------------------------------------------------------- + + public function testANewFormSessionProducesADifferentKey(): void + { + $this->assertNotSame( + $this->key(), + $this->key(['sessionToken' => '99999999-8888-7777-6666-555555555555']) + ); + } + + public function testChangingTheCustomAmountProducesADifferentKey(): void + { + $this->assertNotSame( + $this->key(['customMembershipAmount' => 5]), + $this->key(['customMembershipAmount' => 10]) + ); + } + + public function testChangingTheDonationAmountProducesADifferentKey(): void + { + $this->assertNotSame( + $this->key(['donationAmount' => 5]), + $this->key(['donationAmount' => 20]) + ); + } + + public function testTogglingRecurringDonationProducesADifferentKey(): void + { + $this->assertNotSame( + $this->key(['donationAmount' => 5, 'recurDonation' => false]), + $this->key(['donationAmount' => 5, 'recurDonation' => true]) + ); + } + + public function testTogglingSupporterModeProducesADifferentKey(): void + { + $this->assertNotSame( + $this->key(['donationSupporterMode' => false]), + $this->key(['donationSupporterMode' => true]) + ); + } + + public function testADifferentPlanProducesADifferentKey(): void + { + $usdKey = StripeService::buildSubscriptionIdempotencyKey($this->data, $this->plan); + $gbpKey = StripeService::buildSubscriptionIdempotencyKey( + $this->data, + array_merge($this->plan, ['currency' => 'GBP']) + ); + + $this->assertNotSame($usdKey, $gbpKey); + } + + public function testADifferentMemberProducesADifferentKey(): void + { + $this->assertNotSame( + $this->key(), + $this->key(['email' => 'someone-else@example.com']) + ); + } + + // ------------------------------------------------------------------------- + // Safety valve + // ------------------------------------------------------------------------- + + /** + * Without a session token the key would collapse to email + plan, so two + * genuinely separate joins by the same person would collide and the second + * would silently receive the first subscription. Returning null keeps the + * old (duplicate-prone) behaviour, which is the lesser failure. + */ + public function testMissingSessionTokenYieldsNoKeyRatherThanAWeakOne(): void + { + $this->assertNull($this->key(['sessionToken' => ''])); + + $withoutToken = $this->data; + unset($withoutToken['sessionToken']); + $this->assertNull( + StripeService::buildSubscriptionIdempotencyKey($withoutToken, $this->plan) + ); + } + + public function testKeyFitsWithinStripesIdempotencyKeyLimit(): void + { + $this->assertLessThanOrEqual(255, strlen((string) $this->key())); + } +} diff --git a/packages/join-flow/src/index.tsx b/packages/join-flow/src/index.tsx index 2c32106..6e52694 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.28" + release: "1.4.29" }); if (getEnv('USE_CHARGEBEE')) { diff --git a/packages/join-flow/src/pages/payment-details.page.tsx b/packages/join-flow/src/pages/payment-details.page.tsx index c91a794..1f34081 100644 --- a/packages/join-flow/src/pages/payment-details.page.tsx +++ b/packages/join-flow/src/pages/payment-details.page.tsx @@ -406,7 +406,8 @@ const StripeForm = ({ ); } else { const subscription = await createSubscription({ ...data }); - clientSecret = subscription.latest_invoice.payment_intent.client_secret; + const paymentIntent = subscription.latest_invoice.payment_intent; + clientSecret = paymentIntent.client_secret; sessionStorage.setItem( SAVED_STATE_KEY, @@ -414,9 +415,21 @@ const StripeForm = ({ ...data, stripeCustomerId: subscription.customer, stripeSubscriptionId: subscription.id, - stripePaymentIntentId: subscription.latest_invoice.payment_intent.id + stripePaymentIntentId: paymentIntent.id }) ); + + // Resubmitting within one session returns the subscription Stripe + // already created (see buildSubscriptionIdempotencyKey server-side). + // If that payment has already gone through, confirming it again would + // error — and taking a second payment would be worse. Carry straight + // on to the confirm step instead. + if (paymentIntent.status === "succeeded") { + const succeededUrl = new URL(window.location.href); + succeededUrl.searchParams.set("stripe_success", "true"); + window.location.assign(succeededUrl.toString()); + return; + } } const returnUrl = new URL(window.location.href);