refactor(emails): fold registry into newspack_email_configs schema (NPPD-1550)#4762
Closed
kmwilkerson wants to merge 9 commits into
Conversation
…configs Extends the existing config schema with four shared defaults (trigger_description, recipient, recommended, chip) applied at read time in Emails::get_email_configs(). Providers that omit a field get the documented default; providers that declare a field win. Public static helper Emails::apply_config_defaults() makes the merge directly testable and reusable. First step of NPPD-1550 (unifying the email registry into the newspack_email_configs schema). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds trigger_description, recipient, recommended, and chip to all three reader-revenue email configs (receipt, welcome, cancellation). Values mirror the corresponding entries in the soon-to-be-removed Emails_Section::get_email_registry() — all reader-revenue emails are chip='reader-revenue', recipient='reader', recommended=true. Part of NPPD-1550. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds trigger_description, recipient, recommended, and chip to all eight reader-activation email configs. All RA emails are recipient='reader' and chip='auth-account'; recommended is true for the four core sign-in flows (verification, magic link, OTP, password reset) and false for the four account-management flows (delete account, change email × 2, non-reader login reminder), mirroring the soon-to-be-removed registry. Part of NPPD-1550. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nfig Adds trigger_description, recipient, recommended, and chip to the group-subscription invitation email config. chip='reader-revenue' because group subscriptions are a paid product surface, not an auth/account flow — matching the chip mapping in the soon-to-be-removed registry on slice 2. Part of NPPD-1550. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds WooCommerce_Emails::get_email_configs() hooked into the newspack_email_configs filter. Discovery-based: iterates WC()->mailer()->get_emails() and surfaces only the WC email IDs in the curated metadata table (get_surfaced_wc_emails()). Unrecognized IDs and entries whose plugin_dependency isn't active are silently skipped. Each registered config carries the live WC_Email instance as `wc_email_instance` so downstream callers can invoke instance methods directly (per dkoo's Option A + attached-instance direction). The metadata table covers 9 WC emails: 3 from WC core (customer_new_account, customer_refunded_order, new_order) plus 6 from WC Subscriptions (including the WCSG gifting pair bundled into Subs core). All label and trigger_description strings ported from slice 2's registry to preserve UX copy after the registry is removed. Part of NPPD-1550. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds trigger_description, recipient, recommended, chip, and source to the serialized email payload returned by Emails::serialize_email(). The shape change is safe — all callers (lines 297, 509, 544) are internal to class-emails.php, and the downstream consumer (Emails_Section::api_get_email_settings()) is being rewritten in the next commit to read these fields. The null-type fallback (used when serialize_email() is called without a known config type) now runs through Emails::apply_config_defaults() so the returned shape is uniform regardless of which branch built the config. `source` falls back to 'newspack' when the config doesn't declare it — Newspack-side providers don't need to. Part of NPPD-1550. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… configs Replaces the registry join with direct consumption of Emails::get_email_configs() (now public). The response builder branches on each config's `source` field: 'newspack' configs go through Emails::get_emails() (existing path; serialize_email() now forwards the new schema fields), and 'woocommerce' configs build rows directly from the attached `wc_email_instance`. The old registry-lookup table and fallback enrichment branch are gone — every config carries all required fields by construction. Also adds a minimal WC edit-link helper for classic WC settings URLs; slice 2 owns the block-editor branch and preview_post_id resolution when it rebases. `Emails::get_email_configs()` is promoted from private to public so the unified config set is reachable from outside `class-emails.php`. The private modifier was incidental, not load-bearing. Sort behavior preserved: category-first (reader-revenue → reader-activation → woocommerce), with config-registration order as the within-category tiebreaker. Part of NPPD-1550. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_registry filter NPPD-1550 definition-of-done: no parallel email schema. With providers now populating the four new fields on their newspack_email_configs entries (commits 0b9fb6a, d8028ee, 5454d34, 15c7941) and the response builder rewritten to consume Emails::get_email_configs() directly (493b58c), the curated registry array and its filter no longer serve a purpose. They're gone. Removes: - Emails_Section::get_email_registry() (~225 lines) - apply_filters( 'newspack_emails_registry', ... ) call PHP tests in tests/unit-tests/emails-section.php still reference the removed method; they're rewritten in the next commit alongside the new response-shape and schema-completeness assertions. JS code (emails.tsx, emails.test.js) references `registry_slug` as a response field name, not the deleted PHP method — the response builder still emits it (derived from the config key), so the frontend continues to work unchanged. Part of NPPD-1550. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rewrites tests/unit-tests/emails-section.php now that the parallel
registry is gone. Tests are organized into five buckets that each catch
a distinct regression class:
7a Schema-completeness — iterates every Emails::get_email_configs()
entry and asserts the four new fields (trigger_description, recipient,
recommended, chip) are present with valid values. Catches provider
classes that forget to declare the new fields.
7b Per-provider content — asserts each of the three migrated providers
(reader-revenue, reader-activation, group-subscription-invite)
registers entries with the expected chip/recipient/recommended/
trigger_description values. Catches data regressions when providers
are touched.
7c Response shape — verifies api_get_email_settings() returns the
expected top-level keys, that every row carries the new schema
fields, that the dead view_category field doesn't leak through, and
that the reader-revenue → reader-activation → other sort grouping
holds.
7d WC integration — asserts WooCommerce_Emails::get_email_configs()
passes through unchanged when WC isn't loaded, and (when WC IS
loaded) injects recognized WC email IDs with wc_email_instance
attached. Tests gate on class_exists('WooCommerce') so the suite
works in both environments.
7e Default-merge — direct coverage of Emails::apply_config_defaults().
Asserts partial configs get defaults filled in, declared values are
preserved without clobbering, and a third-party provider that
registers a config with no new fields still gets a complete config
back out of the public Emails::get_email_configs() API. This is the
core pattern dkoo asked for, so it gets a dedicated test bucket.
Removed: all tests against Emails_Section::get_email_registry() and the
newspack_emails_registry filter (both deleted in f895105).
Part of NPPD-1550.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
👋 Thanks for your interest in contributing to Newspack! Newspack development has moved to a single monorepo: Automattic/newspack-workspace. This repository is now a read-only mirror, so we're automatically closing new pull requests here. Please reopen your change against the monorepo – |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Eliminates
Emails_Section::get_email_registry()and folds all UI metadata into the existingnewspack_email_configsschema. After this PR, a single config schema covers both Newspack-managed and WooCommerce-source emails. WC emails are discovered viaWC()->mailer()->get_emails()and registered through the standard filter, with their liveWC_Emailinstance attached to each config entry.Addresses dkoo's architectural feedback in #4727 (comment) and resolves NPPD-1550.
Background
Slice 1 (#4727) introduced a parallel registry layered alongside
newspack_email_configsto carry UI metadata (trigger_description,recipient,recommended,chip,source). Slice 2 (#4732) extended it to join against liveWC_Emailresolution. The parallel schema was always intended as a stepping stone; dkoo's review correctly flagged that extending the existing config schema with the new fields is cleaner than maintaining two parallel data shapes.This PR makes that consolidation.
Architecture decision: Option A with
wc_email_instanceattachedPer dkoo's guidance on Slack (May 27), WC emails register through the same
newspack_email_configsfilter as Newspack emails, with the liveWC_Emailinstance attached to each entry aswc_email_instance. Downstream code that needsWC_Emailmethods (e.g.,enable(),disable(),get_recipient()) reaches them through$config['wc_email_instance']rather than re-resolving via the mailer on each call.This preserves the single-schema win without re-implementing WC's write API in Newspack code.
Schema extension
Four fields added to
newspack_email_configs, applied viaEmails::apply_config_defaults()at read time so third-party hooks that omit them keep working:trigger_description''recipient'reader' | 'admin''reader'recommendedtruechip'auth-account' | 'reader-revenue''auth-account'For WC-source entries, the config additionally carries
source: 'woocommerce',category: 'woocommerce', andwc_email_instance.apply_config_defaults()is a public static helper specifically so test bucket 5 (below) can exercise the merge mechanism directly without reflection.Emails::get_email_configs()is also public;api_get_email_settings()needs direct access to iterate the unified config set and branch onsource, and the existing publicEmails::get_emails()only serves Newspack-side rows.Changes by file
includes/emails/class-emails.php— Addsapply_config_defaults()public static helper; promotesget_email_configs()to public and applies defaults to each entry;serialize_email()forwards the four new fields and patches the null-type fallback to carry consistent defaults.includes/wizards/newspack/class-emails-section.php— Deletesget_email_registry()and thenewspack_emails_registryfilter (~233 lines). Rewritesapi_get_email_settings()to consume configs directly, branching onsourceto either resolve Newspack posts (existing path) or buildEmailItemrecords fromwc_email_instancefor WC entries.includes/reader-revenue/class-reader-revenue-emails.php— Extends 3 config entries with the new fields.includes/reader-activation/class-reader-activation-emails.php— Extends 8 config entries with the new fields.includes/plugins/woocommerce-subscriptions/group-subscription/class-group-subscription-invite.php— Extends 1 config entry with the new fields.includes/plugins/woocommerce/class-woocommerce-emails.php— Addsget_email_configs()static method backed by aSURFACED_WC_EMAILSmetadata table. IteratesWC()->mailer()->get_emails()and registers the curated subset, gated onplugin_dependency. Unrecognized WC email IDs are silently skipped.Tests
Five buckets, 11 new test methods, in
tests/unit-tests/emails-section.php:Emails::get_email_configs()has all required fields with valid types after defaults applytrigger_description,recipient,recommended,chipvaluesapi_get_email_settings()returns the new fields, noview_category, no registry join, no count-mirror assertion against the (now-deleted) registry sizeWooCommerce_Emails::get_email_configs()injects entries withwc_email_instanceattached when WC is active; returns input unchanged when WC is inactive; silently skips unrecognized IDs; respectsplugin_dependencygatingVerification
get_email_registry/newspack_emails_registryfully removed fromincludes/,src/, andtests/Post-merge rebase tasks
These belong to descendant slices and get rewritten when they rebase onto the updated
nppd-945head. None of them land in this PR.Slice 2 (#4732):
api_get_email_settings()is no longer needed — already handled here.POST /emails/{id}/toggle) rewrites to validate against the unified config set and call$config['wc_email_instance']->...for writes. Wraps the existingupdate_option( $wc_email->get_option_key(), … )path to also update the in-memory$wc_email->enabledproperty so cached state stays in sync (resolves Copilot's slice 2 staleness finding).recommended === true && source === 'woocommerce', using the same WC option-write helper.preview_post_idresolution for WC entries (currentlyself::get_wc_email_template_post_id( $wc_email_id )on slice 2) must be re-wired against the unified config — either computed insideWooCommerce_Emails::get_email_configs()and surfaced on the entry, or recomputed in the response builder.Slice 4 (#4756):
class-card-expiry-warning.php'snewspack_email_configsregistration adds the four new fields. Single entry.Slices 1537 (#4758) and 1538 (#4759):
Merge protocol
Per dkoo's instructions in #4727, this PR merges back into
nppd-945-unified-emails-newspack-slicewithout squashing, so the per-provider commit history is preserved when slice 1 eventually merges totrunk. Descendant slices rebase onto the updatednppd-945head before reviewing the rebase-task changes listed above.Related