feat(emails): move sender/contact settings into Emails screen via Settings modal (NPPD-1566)#162
Open
kmwilkerson wants to merge 7 commits into
Conversation
…PPD-1566) GET + POST /wizard/newspack-settings/emails/settings carry the three transactional-email settings (sender_name, sender_email_address, contact_email_address) under the emails namespace, where they belong architecturally — independent of the legacy shared audience-management endpoint. Reads + writes preserve the existing newspack_reader_activation_* wp_options keys. GET returns saved values at the top-level keys (empty string when no override exists) plus a `defaults` sub-array with the derived fallbacks (blog title, no-reply@yourdomain, admin email). Returning value + defaults separately — rather than collapsing through `Reader_Activation::get_setting()` — lets the frontend render saved overrides as the input's `value` and derived fallbacks as `placeholder`, which keeps publishers from accidentally locking in dynamic defaults as static option rows on first save (a real launch-safety concern: once written, the static row no longer tracks later site-title or admin-email changes). Default-source computation is inlined rather than abstracted into Emails::get_from_*() — those helpers resolve to the *saved value when one exists*, so calling them for the "default" slot would just return the saved value when present, defeating the purpose of returning both separately. The sender-email default mirrors Emails::get_from_email()'s fallback logic (network_home_url host with `www.` prefix stripped). POST treats empty values as "revert to derived default" — the corresponding option row is deleted (not written empty), letting the dynamic-default behavior re-engage. Format validation on email fields runs only when value is non-empty; empty sender_name is accepted. Validation: the legacy audience-management endpoint accepted these fields without sanitization or format validation. This commit adds server-side sanitization (sanitize_email + sanitize_text_field) plus re-validation via is_email() in the handler (sanitize_email leaves partials like "user@" intact). Error codes newspack_invalid_sender_email and newspack_invalid_contact_email each return 400. No newspack_invalid_sender_name code — empty is the intentional revert path. POST response mirrors the GET shape via api_get_settings() so the frontend can confirm the saved state without a second round-trip. REST_BASE: introduces the same REST_BASE constant pattern NPPD-1535 establishes on its parallel branch. Existing routes left on their string form here to minimize rebase surface — NPPD-1535's diff refactors those. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…settings (NPPD-1566) Wraps the chip-bar row with an HStack (justify="space-between") and adds a "Settings" button on the right. Clicking the button opens a new SettingsModal that hosts the three transactional-email settings (sender_name, sender_email_address, contact_email_address) previously surfaced inside the Reader Activation "Transactional Emails" prerequisite card. Modal pattern mirrors src/wizards/newsletters/views/premium-newsletters/ advanced-settings.tsx: - Newspack Modal wrapper from packages/components/src - VStack body, HStack footer (Cancel tertiary + Save primary) - Dirty-detection via JSON.stringify(current) !== JSON.stringify(initial) - addNotice( 'Saved.', success ) from the newspack/wizards store - Save disabled when not loaded, fetching, not dirty, or client-side-invalid Renders saved value as the input's `value` and the derived default as its `placeholder`. The mount-time GET response carries both at top-level (saved) and under `defaults` (derived); the frontend keeps them separated so an empty saved value visually shows the dynamic-default fallback in lighter placeholder text rather than populating the input with what would become a static override on save. Helper text reads "Leave blank to use [the default]" to make the revert affordance discoverable. No `required` HTML5 attribute on any field — empty is the intentional revert path, not a validation error. Client-side validation gates Save: empty values pass for every field, but email fields with non-empty malformed input keep Save disabled. Mirrors the server's contract — prevents firing a POST that would 400, along with the jarring submit/reject/Notice-flash cycle that would follow. Discard-confirm via useConfirmDialog({ when: isDirty }) — Cancel, X, Escape, and click-outside all route through the same handleClose that pops the dialog only when there are unsaved changes. Promise rejection handling: useWizardApiFetch's catchCallback populates errorMessage state THEN re-throws to preserve async-API semantics. Without an onError stub + trailing .catch(), the rejection surfaces as "Uncaught (in promise) WizardApiError" in the console. Empty onError handlers + .catch() chained on both GET (mount-time fetch) and POST (save) suppress the unhandled-rejection noise; the error UX (inline Notice rendering errorMessage from the hook) is unchanged. SCSS: the 24px horizontal padding that aligned the chip-bar's left edge with DataViews' search row moves from .newspack-emails__chips to the new .newspack-emails__chip-bar HStack wrapper, so the right-side Settings button gets the same 24px inset from the right. emails.test.js: stubs the SettingsModal as a no-op render. The grid tests don't exercise modal behavior, and the modal pulls in @wordpress/data's useDispatch against the wizards store, which isn't registered in the test env. Modal coverage lives in its own test file (added in the test commit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The three transactional-email settings (sender_name, sender_email_address, contact_email_address) now live in a dedicated Settings modal on the Newspack > Settings > Emails screen — opened via the new "Settings" button on the chip-bar row added in the previous commit. (NPPD-1538 will later move the Emails screen itself to Audience > Configuration.) The underlying wp_options keys are unchanged: - newspack_reader_activation_sender_name - newspack_reader_activation_sender_email_address - newspack_reader_activation_contact_email_address Reads and writes still flow through Reader_Activation::get_setting() and update_setting() — those continue to work because the three fields are independently defined in get_settings_config() (lines 382-384), not derived from the prerequisite entry being removed here. Removal verified safe via cascade check: - All callers of get_prerequisites_status() iterate generically (no hardcoded 'emails' lookup) - is_ras_ready_to_configure() (line 854) calls is_transactional_email_configured() inline, not through the prereq entry — keep working - Frontend (src/wizards/audience/) iterates prerequisites via Object.keys().map / .every() — no key-specific code - Zero tests reference the 'emails' prereq Orphaned option: newspack_reader_activation_emails_skipped (written by the generic skip mechanism on the now-removed prereq) becomes unreferenced. Harmless — no consumer reads it. Optional cleanup follow-up if anyone cares to delete the row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tings (NPPD-1566) PHP — Bucket F (Settings endpoint) in tests/unit-tests/emails-section.php: 1. test_get_settings_returns_three_fields — GET returns saved values + a `defaults` sub-array of derived fallbacks 2. test_get_settings_returns_empty_values_when_no_override_saved — fresh-install: empty top-level values, populated defaults. Locks in the launch-safety story (publishers who've never explicitly saved should see empty fields with placeholder hints, not auto-derived values that could be locked in on first save). 3. test_post_settings_persists_three_fields — POST writes the three newspack_reader_activation_* options and returns refreshed shape 4. test_post_settings_rejects_invalid_email_in_sender — 400 + newspack_invalid_sender_email 5. test_post_settings_rejects_invalid_email_in_contact — 400 + newspack_invalid_contact_email 6. test_post_settings_empty_value_deletes_option — empty value on any field deletes the option row, reverting the field to its derived default 7. test_settings_endpoint_permission_check — subscriber hits Wizard_Section::api_permissions_check() → WP_Error 403. Both GET and POST share the same permission callback. JS — src/wizards/newspack/views/settings/emails/settings-modal.test.js (matching the sibling .test.js convention used by emails.test.js and email-preview.test.js — not .test.tsx, which would trigger strict tsc type-checking that other .test.js files in this repo escape): 1. test_modal_opens_on_settings_button_click — renders <Emails />, clicks Settings, asserts dialog appears 2. saves: POSTs payload, dispatches success notice, closes modal — verifies the path, payload, addNotice call shape, and close 3. cancel: prompts for confirmation when fields are dirty — useConfirmDialog called with when=true, closeModal NOT invoked 4. cancel: closes directly when fields are clean — useConfirmDialog called with when=false, closeModal invoked immediately Mock surface for the JS file is more elaborate than emails.test.js because the modal touches WIZARD_STORE_NAMESPACE (mocked away to avoid evaluating createReduxStore against the @wordpress/data stub) and TextControl/HStack/VStack from @wordpress/components (mocked as passthroughs since the real barrel fails to evaluate in this jsdom env when reached through the modal's import chain). useConfirmDialog is mocked to capture the `when` value and conditionally invoke the callback — that lets the cancel tests assert the dirty-vs-clean branch deterministically. SAMPLE_INITIAL carries a `defaults` block mirroring the server's response shape; the fetch mock returns Promise.resolve() so the production-side `.catch()` chaining works in tests. Verification: - PHPCS exit 0, ESLint exit 0 - PHP suite: 1336 / 4282 / 5 skipped - JS suite: 217 tests / 22 suites Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kmwilkerson
force-pushed
the
nppd-1566-move-transactional-email-settings-to-emails
branch
from
838050e to
402217f
Compare
CORRECTNESS
1. **sanitize_email args callback silently destroyed validation**
(class-emails-section.php, post-args sanitize chain). The
`sender_email_address` / `contact_email_address` args used
`sanitize_email`, which COLLAPSES partial input like 'user@' or
'newaddress' to '' BEFORE the handler runs. The handler then took
the '' === $value branch as an "intentional revert" and
delete_option()'d the publisher's saved override. The
newspack_invalid_sender_email / _contact_email error codes were
unreachable for the typo cases they were meant to catch. Switched
both fields to `sanitize_text_field` — preserves typo input so the
handler's is_email() guard rejects with a proper 400.
2. **is_ras_ready_to_configure() and the removed prereq UI
disagreed** (class-reader-activation.php:854). The function still
AND'd in is_transactional_email_configured(), which requires
explicit saves of all three options. With the emails prereq UI
removed, fresh-install publishers could satisfy every visible
prereq and find RAS stuck-disabled with no in-flow hint pointing
at the (no-longer-prereq) Emails settings. Dropped the AND clause:
the three options have valid derived defaults by construction, so
requiring explicit saves is no longer the right gate.
3. **'no-reply@' surfaced as a publisher-facing default**
(class-emails-section.php api_get_settings). When wp_parse_url
can't extract a host from network_home_url (edge multisite, mal-
formed siteurl), the cast yields '' and the response carried
`sender_email_address: 'no-reply@'` — visibly broken. Guard the
empty-host case: emit '' instead of the bare 'no-reply@'.
4. **update_setting silent failure** (class-emails-section.php
api_update_settings). The write loop ignored the bool return of
Reader_Activation::update_setting(), then returned api_get_settings()
regardless. If the key were ever filtered out of get_settings_config()
via `newspack_reader_activation_settings_config`, the POST would
return 200 with the stale value — silent data loss with a success
notice. Now: convert false return into a visible 500 +
`newspack_settings_write_failed` error code.
UX / CLEANUP
5. **Modal stale state on reopen** (settings-modal.tsx useEffect).
Reset settings/initial/defaults to EMPTY_SETTINGS at the start of
each open so the first paint after reopen renders an empty form
instead of briefly flashing the previous open's values.
6. **`onClose` prop dead code on Modal** (settings-modal.tsx).
@wordpress/components Modal accepts onRequestClose, not onClose.
Removed the onClose pass — code now matches the actual contract.
7. **isClientSideValid useMemo was redundant** (settings-modal.tsx).
The dep `settings` changes ref on every keystroke; the memo never
hit. Replaced with a plain expression — same cost, less misleading.
8. **delete_option path bypassed update_setting action hook**
(class-emails-section.php api_update_settings). When clearing a
field, the action `newspack_reader_activation_update_setting` no
longer fired, so external subscribers (audit logs, ESP sync)
couldn't observe revert events. Now fire the action manually with
empty value alongside the delete_option.
DESIGN
9. **Inner chips div → HStack** (emails.tsx + emails.scss). Replaced
`<div className="newspack-emails__chips">` with custom `display:
flex; gap: 8px` CSS with `<HStack className="newspack-emails__chips"
spacing={2}>`. WP grid unit 2 = 8px = previous gap. Matches the
design system's "prefer WP layout components over custom flex CSS
in admin/wizard context" rule.
10. **Unclassed wrapper div** (settings-modal.tsx). The escape-hatch
div for the ReactNode/ReactElement TS mismatch now has a BEM
class `.newspack-emails__settings-modal-wrap` — future CSS can
target it intentionally rather than accidentally catch it via
unscoped sibling selectors.
NEW TESTS
- test_post_settings_args_use_text_field_sanitizer_for_emails:
regression guard that introspects the registered POST route's
args and asserts sanitize_callback is NOT `sanitize_email`.
- test_get_settings_default_sender_email_has_non_empty_host: when
the default is emitted, it must look like a real email (no bare
`no-reply@`).
- test_post_settings_returns_error_when_update_setting_fails: 500 +
newspack_settings_write_failed when update_setting returns false.
- test_post_settings_empty_value_fires_update_setting_action: the
revert path fires the action hook for subscriber visibility.
Verification: PHPCS exit 0, ESLint exit 0. PHP 1337 → 1341 (+4
tests / +14 assertions, 0 skipped change). JS 217/217 unchanged.
KNOWN FOLLOW-UPS (not addressed in this commit, separate scope):
- Legacy /wizard/newspack-audience/audience-management POST still
accepts these three keys with no validation. Defense-in-depth gap;
add validation at Reader_Activation::update_setting() or at the
legacy handler in a follow-up PR.
- Default-fallback computation duplicates Emails::get_from_email().
Extracting Emails::get_default_from_email() is a refactor scope;
drift-risk only if either site changes the fallback logic.
- @wordpress/components / @wordpress/data hand-rolled mocks
duplicate across emails.test.js and settings-modal.test.js. A
shared jest setup file is a tooling investment, out of this PR.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Smoketest regression caught by Katie. Item 4 of the previous review- fix commit (the update_setting() bool-return check) was treating WordPress's `update_option()` no-op return as a write failure. REPRO: type a value, save → 200 OK. Re-open modal, click Save without changing anything → 500 + newspack_settings_write_failed. ROOT CAUSE: `update_option()` returns false in two distinct cases: (a) genuine write failure (DB error, etc.) and (b) no-op (new value === existing value). The handler's `! Reader_Activation::update_setting(...)` check conflated the two. The forced-failure test (test_post_settings_returns_error_when_update_setting_fails) only exercised case (a) via a filter that drops the key from get_settings_config, missing case (b). FIX: pre-check current value via `get_option()` and `continue` past unchanged fields before the update_setting() call. Same pattern on the empty-revert path — if the option row doesn't exist, skip the delete_option + action hook (which would otherwise fire a phantom "this changed" event with nothing actually changing). Bonus correctness: the `newspack_reader_activation_update_setting` action hook is now semantically honest — "this changed" means it actually changed, not "this was submitted". Subscribers no longer get noisy re-save events that don't reflect state changes. NEW TESTS - test_post_settings_idempotent_when_value_unchanged: POST twice with identical values, assert second POST returns 200 (not 500) AND the action hook does NOT fire on the no-op re-save. - test_post_settings_empty_value_idempotent_when_already_empty: POST '' for a field with no existing row, assert no 500 AND the action hook does NOT fire AND no spurious option row is created. Verification: PHPCS exit 0. PHP 1341 → 1343 (+2 tests / +9 assertions, no regressions). JS unchanged. LESSON FILED FORWARD: when adding error-handling around third-party return values, test boundary cases (no-op, already-in-target-state) alongside happy path and forced-failure. update_option() and delete_option() both return false on no-op — those tests would have caught this before smoketest if they'd been written with the review-fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Moves transactional email sender/contact settings into the unified Newspack > Settings > Emails screen via a new “Settings” modal, backed by a dedicated REST endpoint that preserves the existing “dynamic defaults unless explicitly overridden” behavior.
Changes:
- Add
GET/POST /newspack/v1/wizard/newspack-settings/emails/settingsto read/write sender/contact settings while preserving unset-as-dynamic-default semantics. - Add a Settings button + modal UI on the Emails screen (value vs placeholder rendering, dirty-confirm flow, client/server validation).
- Remove the Reader Activation “Transactional Emails” prerequisite card and drop transactional-email configuration as an RA readiness prerequisite; add PHP/Jest coverage for the new flow.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/newspack-plugin/includes/wizards/newspack/class-emails-section.php | Adds REST base constant and registers/implements the new settings endpoint (GET defaults + POST persist/revert semantics). |
| plugins/newspack-plugin/includes/reader-activation/class-reader-activation.php | Removes transactional-email configured gating from RA readiness + removes the prereq card definition. |
| plugins/newspack-plugin/src/wizards/newspack/views/settings/emails/emails.tsx | Adds chip-bar layout updates, Settings button, and mounts the new Settings modal. |
| plugins/newspack-plugin/src/wizards/newspack/views/settings/emails/emails.scss | Styles updated chip bar wrapper to accommodate the Settings button alignment. |
| plugins/newspack-plugin/src/wizards/newspack/views/settings/emails/emails.test.js | Stubs the modal so existing grid tests remain focused and stable. |
| plugins/newspack-plugin/src/wizards/newspack/views/settings/emails/settings-modal.tsx | New modal component: fetches settings, placeholder defaults, validation, dirty confirmation, save UX. |
| plugins/newspack-plugin/src/wizards/newspack/views/settings/emails/settings-modal.test.js | New Jest coverage for modal open/save/cancel-confirm behavior and endpoint wiring. |
| plugins/newspack-plugin/tests/unit-tests/emails-section.php | Adds PHPUnit coverage for GET/POST settings endpoint semantics, validation, permissions, and idempotency. |
…mp (NPPD-1566) Three Copilot findings on PR #162 + one user-reported UX issue from smoketest. All small polish; bundled into one commit. COPILOT #1 — api_get_settings default diverges from Emails::get_from_email() A previous review-fix commit guarded the empty-host edge in api_get_settings: when wp_parse_url(network_home_url()) returns no host, return '' for sender_email_address default instead of the visibly-broken 'no-reply@'. The intent was avoiding a publisher- facing broken placeholder. Copilot caught the divergence: `Emails::get_from_email()` (the actual send path) does NOT guard the same case — it still returns the literal 'no-reply@'. Result: on a misconfigured site, the modal placeholder would show '' while outbound mail would send from 'no-reply@'. Silent UX gap: publishers see one default and get another. Aligned the response to match the send path's literal 'no-reply@'. The broken-looking placeholder is honest signal that the site's home URL doesn't resolve; hiding it didn't make the underlying issue go away. Fixing the misconfigured-host fallback at the send path is a separate follow-up. Updated the test: `test_get_settings_default_sender_email_has_non_empty_host` (which asserted no bare `no-reply@`) renamed to `test_get_settings_default_sender_email_matches_send_path`, now asserts the contract directly: `Emails::get_from_email()` === defaults.sender_email_address. Added `test_get_settings_defaults_match_send_path_helpers` for the parallel contract on sender_name and contact_email_address. COPILOT #2 — awkward help text "no-reply@ your site domain" Reads like a formatting mistake. Rewrote to natural prose: "Leave blank to use a no-reply address at your site's domain." Removes the literal `@ ` break that read as broken. COPILOT #3 — stale GET-route inline comment Said the handler "Reads through `Reader_Activation::get_setting()`", but the Option-A handler intentionally does NOT call get_setting() (that would collapse value+default into one resolved string, which is the conflation the modal's placeholder UX exists to avoid). Rewrote the comment to describe the actual behavior + explicitly forbid the regression: future refactors must preserve the value-vs-placeholder split. USER — description clamp truncating new emails at 2 lines PR #155's "Card expiry warning" and slice 2a's "Renewal reminder" have single-sentence descriptions that exceed the 2-line clamp at 12px in the narrow grid card width. The clamp was set in slice 1 when descriptions were shorter; new descriptions outgrew the budget. Bumped 2 → 3 lines: fits sentence-length descriptions cleanly while still bounding card height enough to preserve grid uniformity. Verification: PHPCS exit 0, ESLint exit 0. PHP 1343 → 1344 (+1 test / +2 assertions; one test renamed/rescoped + one new test). JS 217/217 unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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
Moves the three transactional-email settings (sender name, sender email, contact email) out of their existing home in Audience > Configuration > Transactional Emails prerequisite card and into the unified Emails screen via a "Settings" button + modal in the chip bar header.
Resolves NPPD-1566.
Stacking
main. Review order: #137 → #144 → #146 → this.When NPPD-1538 eventually moves the Emails screen from Newspack > Settings to Audience > Configuration, this PR's frontend changes get relocated naturally as part of that move. No coordination needed between this PR and NPPD-1538's work.
Headline design decision: preserve dynamic-default behavior
The most important thing in this PR isn't moving the fields — it's preserving the semantic contract of how those fields work today.
Today, when the three options are unset in
wp_options, the system computes defaults dynamically every time they're read:sender_name→get_bloginfo('name')(the current site title)sender_email_address→no-reply@{current domain}contact_email_address→get_option('admin_email')(the current admin email)This means a publisher who changes their site title, domain, or admin email gets transactional emails updated automatically. The naive "move the fields to a new UI" approach would silently destroy this behavior — the first time a publisher opens the new modal and clicks Save (even with no edits), the dynamically-derived values would be written to
wp_optionsas static rows. Subsequent site-title or admin-email changes would no longer propagate.This was caught during smoke testing rather than at design time. The fix is the architectural pattern this PR ships:
GET endpoint returns saved values and derived defaults separately:
{ "sender_name": "", // empty if unset "sender_email_address": "", "contact_email_address": "", "defaults": { "sender_name": "My Newsroom", // computed each request "sender_email_address": "no-reply@example.com", "contact_email_address": "admin@example.com" } }Frontend renders saved values as
value=and derived defaults asplaceholder=on eachTextControl, so publishers visually distinguish "what I've explicitly set" from "what the system is computing."POST endpoint accepts empty values as "revert to derived default" — empty strings trigger
delete_option()on the underlying option row, restoring the dynamic-default behavior. Email-format validation only fires when the value is non-empty.Result: publishers can hit Save with no edits and nothing breaks. The dynamic-default behavior is preserved by construction.
Helper text on each field reinforces the affordance: "Leave blank to use your site title" / "Leave blank to use no-reply@..." / "Leave blank to use your site's admin email."
Architectural notes
New dedicated REST endpoint rather than reusing the existing
audience-managementPOST. Routes registered onEmails_Sectionunder the sameREST_BASEconstant that NPPD-1535 (#158) introduces:GET /newspack/v1/wizard/newspack-settings/emails/settings— reads viaReader_Activation::get_setting()(defaults flow through site title, no-reply address, admin email helpers)POST /newspack/v1/wizard/newspack-settings/emails/settings— validates, sanitizes, delegates writes toReader_Activation::update_setting()(option storage location unchanged atnewspack_reader_activation_*)The dedicated endpoint aligns with the discipline NPPD-1535 established: email-related REST routes belong under
newspack-settings/emails/*, not in the donations wizard or theaudience-managementcatchall. Reusing the shared POST would have been the smallest-change choice but would have undone that alignment.Both this PR and #158 declare
REST_BASEindependently (slice 2b doesn't have it yet). Whichever PR merges first, the other'sREST_BASEdeclaration collapses cleanly on rebase — the declarations are byte-identical with the same docblock.Validation
Recon found that the three fields currently have no validation at all — no schema-level sanitize, no
sanitize_email(), no format checks. Today the system happily persists "not-an-email" as a sender email address, breaking deliverability silently.This PR adds light validation as part of the move:
sender_email_address,contact_email_address):type="email"HTML5 attribute on the frontend + server-sidesanitize_email()+is_email()validation. Returns 400 withnewspack_invalid_sender_email/newspack_invalid_contact_emailon failure. Validation fires only when the value is non-empty (since empty = revert).user@.sanitize_text_field(). Empty is valid (triggers revert to derived default); norequiredHTML5 attribute.The current absence of validation looks like an oversight given these are email fields; the move is the natural moment to add it.
UI changes
Chip bar layout: wrapped in
<HStack justify="space-between">. Chips on the left (existing), Settings button on the right (new). Button is text-only ("Settings"), no icon, variant=secondary.Settings modal: opened on Settings button click. Matches the pattern established by
advanced-settings.tsxin the newsletters/premium-newsletters wizard:TextControlfields withplaceholder=showing the derived defaultsJSON.stringifycomparison against initial stateuseConfirmDialogfrompackages/components/src/hooks/use-confirm-dialog.tsxasks confirmation before discardingaddNoticefrom thenewspack/wizardsstore with type=success → modal closesThe toast uses
addNoticefrom the Newspack wizards store (which has itsWizardSnackbaralready mounted at the Wizard level), notcreateNoticefrom@wordpress/notices. Matches the existing convention.Audience-side cleanup
Removes the Transactional Emails prerequisite card from
Reader_Activation::get_prerequisites_status()(lines 946-965 deleted). The three fields' settings keys are independently defined inget_settings_config()and survive — only the prereq card's UI definition goes away.Dual-surface state (leaving the card in place, both UIs editing the same options) was the alternative and would have been confusing: publishers would have two locations editing the same data with no clear winner.
The
newspack_reader_activation_emails_skippedoption becomes an orphan after this removal. Harmless — no consumer — but could be cleaned up in a follow-up if anyone cares.Testing
7 new PHP tests (
tests/unit-tests/emails-section.php, Bucket F):test_get_settings_returns_three_fields(happy path with saved values)test_get_settings_returns_empty_values_when_no_override_saved(launch-safety lock-in: fresh-install case returns empty values + populated defaults)test_post_settings_persists_three_fields(happy path: saves all three)test_post_settings_empty_value_deletes_option(revert semantics: empty triggersdelete_option)test_post_settings_rejects_invalid_email_in_sender(400 +newspack_invalid_sender_email)test_post_settings_rejects_invalid_email_in_contact(400 +newspack_invalid_contact_email)test_settings_endpoint_permission_check(subscriber-level user blocked, both GET and POST)4 new JS tests (
settings-modal.test.js):test_modal_opens_on_settings_button_clicktest_modal_save_calls_post_endpoint_and_closes_with_noticetest_modal_cancel_prompts_when_dirtytest_modal_cancel_no_prompt_when_cleanPHP suite: 1330 → 1337 (+7 tests / +39 assertions, no other counts shifted). JS suite: 213 → 217 (+4 tests / +1 suite, no regressions).
Known pre-existing tsc noise
emails.tsxhas 4 pre-existing tsc errors on lines 350, 441, 444, 448 — DataViews v14 type narrowness on the reset action'sisDestructiveand DataViews component prop type widening. Verified pre-existing baseline (errors present before this PR's changes; this PR adds zero new tsc errors). Worth a small cleanup follow-up; out of scope here.