fix(active-campaign): keep signups working after field tag edits

[wil-gerken]

All Submissions:

• Have you followed the Newspack Contributing guidelines?
• Does your code follow the WordPress coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Reference: NPPM-283

When a custom field's personalization tag (perstag) is renamed in ActiveCampaign, newsletter signups that sync reader data start failing with "There is already a field with this title". Because Newspack attaches signup metadata to every signup, on an affected site that means all signups fail.

What this does

Signup field-matching is now resilient to field edits made on the ActiveCampaign side:

• Fields are matched by their tag first (as before), and now fall back to matching by title — case-insensitively, since ActiveCampaign treats titles as duplicates regardless of case.
• When a field is matched by title, the contact data is written to that field's actual tag, so the value lands in the right place.
• If a field can't be registered at all, the signup no longer fails: the problem is logged, that one field is skipped, and the reader is still subscribed.

How to test the changes in this Pull Request:

Prerequisite: a site with ActiveCampaign connected. Run at least one signup first so the Newspack-generated contact fields exist.

Reproduce the problem (on release without this fix):

1. In ActiveCampaign, under Contacts → Fields → Manage, change the personalization tag (perstag) of a Newspack-generated field (for example NEWSLETTER_SUBSCRIPTION_METHOD → NEWSLETTERSSUBSCRIPTIONMETHOD).
2. Register through Reader Activation so the signup syncs reader data. An email-only subscribe doesn't send this field and won't reproduce the problem.
3. The signup fails with "There is already a field with this title", and ActiveCampaign does not add the reader.

Verify the fix:

1. Keep the renamed personalization tag and sign up again on this branch. The signup succeeds, and ActiveCampaign saves the contact's data under the renamed field.
2. Sign up on a site with no renamed tags. The signup succeeds, and the contact data lands in the existing fields.

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully run tests with your changes locally?

Notes for reviewers

• The change is confined to the field-registration block of add_contact() in the AC provider; the method's signature and everything downstream are untouched.
• Matching uses index-preserving loops rather than array_search over array_column — AC field rows can be missing keys, and array_column reindexes around them, which could map a match to the wrong field's perstag. A title match also requires the row to carry a usable perstag; otherwise it falls through to field creation.
• A failed field-create now logs via Newspack_Newsletters_Logger and continues; previously the raw WP_Error aborted the signup before any contact sync ran.
• Four tests using the suite's established pre_http_request mock pattern: renamed perstag matches by title and uses the actual perstag; a failed create doesn't block the signup; the perstag fast path is unchanged; malformed rows (missing title or perstag) and re-cased, whitespace-padded titles are handled. Full newsletters suite green (486 tests).

Release risk

Medium. The change sits on the reader-signup path, but it only executes for publishers using ActiveCampaign as their ESP, and the common case — tag matches, no admin edits — is verified unchanged by tests. One deliberate trade-off: a field that can't be registered is now skipped with a log entry instead of failing the whole signup loudly; keeping signups working outweighs one metadata field. Rollback is a plain revert, which restores the previous hard-fail behavior.

[Copilot]

Pull request overview

Improves the ActiveCampaign provider’s add_contact() metadata-field registration so newsletter signups continue to succeed even when an ActiveCampaign admin edits a custom field’s personalization tag (perstag), which previously caused duplicate-title errors that aborted the entire signup.

Changes:

• Match existing ActiveCampaign fields by perstag first, then fall back to case-insensitive title matching; when matched by title, write using the field’s actual perstag.
• If a field cannot be created, log the error and continue syncing the contact instead of failing the signup.
• Add PHPUnit coverage for perstag match, title fallback (including malformed field rows), and non-blocking behavior on field-create failure.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
plugins/newspack-newsletters/includes/service-providers/active_campaign/class-newspack-newsletters-active-campaign.php	Adds resilient field matching (perstag → title) and makes field-create failures non-fatal with logging.
plugins/newspack-newsletters/tests/test-active-campaign-field-matching.php	Introduces tests covering renamed perstag fallback, malformed field rows, and ensuring signup proceeds when field creation fails.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[dkoo]

Tested end-to-end with a live ActiveCampaign account. Approve with comments. The core logic is a clear improvement over the previous array_search/array_column approach and the tests genuinely cover the claimed behaviors, including malformed rows. No blockers. Two low-severity suggestions before merge: harden the field-match guards against an empty-string perstag (defensive; no data-leak risk), and make the field-create failure log unconditional so a silently-dropped RAS metadata field is diagnosable on a default production site.

Making a failed field-create non-fatal is the correct direction — a single unregisterable field must never block a reader signup. The common real failure (a duplicate-title 422) is now caught by the new title match before any create is attempted, so this silent-drop path is only reached for genuinely-new fields that fail to create for some other reason: a narrow, self-healing window rather than a standing state.

[dkoo]

Suggestion: Field-match guards check isset() not !empty() — an empty perstag can match — The perstag and title-match guards check isset( $existing_field['perstag'] ) rather than ! empty( ... ), so a field row whose perstag is an empty string can still match. When it does, $field_perstag is set to '' (line 2034) and the payload emits a malformed field[%%,0] key — and two such fields would collide on it. There's no cross-field data-leak risk: a value is never routed into a different real named field, and in practice ActiveCampaign auto-assigns perstags, so an empty one is effectively unreachable. Still, tightening both guards to ! empty( $existing_field['perstag'] ) (and skipping the field, or falling through to create, when the generated perstag is empty) is cheap defense and would round out the existing malformed-row test coverage — the current test_title_match_survives_field_rows_without_title covers a missing perstag key but not an empty-string one.

[dkoo]

Suggestion: Field-create failure log is level-gated, so a dropped RAS field is silent on a default site — Making a failed field-create non-fatal is the right call — one unregisterable field should never abort a reader's whole signup. Worth weighing, though: the metadata synced here is Reader Activation contact data (registration, engagement, subscription, donation, content-gate status) that ActiveCampaign segments and automations key on, so a silently dropped field isn't purely cosmetic. The common real failure — the duplicate-title 422 — is now caught by the new title match before any create is attempted, and the remaining path is self-healing (retried on the next signup), which keeps this low-risk. The catch: Newspack_Newsletters_Logger::log() no-ops entirely unless NEWSPACK_LOG_LEVEL is defined (class-newspack-newsletters-logger.php:20), which it isn't on a default production site — so a field that keeps failing to create leaves no trace at all. Consider making just this one failure surface unconditionally (e.g. a direct error_log) so an under-populated segment is diagnosable.

Risk if not addressed: If a field-create keeps failing for a non-title reason on a site that has not defined NEWSPACK_LOG_LEVEL, a load-bearing Reader Activation field (donation, subscription, or membership status) is silently dropped on every signup with no log trace, so readers who should enter an AC segment or automation keyed on that field are never added to it — invisible until a publisher notices the segment is under-populated.