From 5662da710732f6d30175e255ece832bf042f6f56 Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Tue, 12 May 2026 15:34:47 -0500 Subject: [PATCH 01/19] chore(deps): update php-loremipsum source URL The joshtronic/php-loremipsum package source moved from GitHub to git.sherver.org. Same version (2.1.0), same commit hash. The dist and support blocks referencing old GitHub URLs were dropped. Co-Authored-By: Claude Opus 4.6 --- composer.lock | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/composer.lock b/composer.lock index e0de43dadd..8c7619decc 100644 --- a/composer.lock +++ b/composer.lock @@ -608,15 +608,9 @@ "version": "2.1.0", "source": { "type": "git", - "url": "https://github.com/joshtronic/php-loremipsum.git", + "url": "https://git.sherver.org/joshtronic/php-loremipsum", "reference": "6d7f8cff6a092a53d66b5237edbe97d398b14f88" }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/joshtronic/php-loremipsum/zipball/6d7f8cff6a092a53d66b5237edbe97d398b14f88", - "reference": "6d7f8cff6a092a53d66b5237edbe97d398b14f88", - "shasum": "" - }, "require": { "php": ">=5.3" }, @@ -647,10 +641,6 @@ "ipsum", "lorem" ], - "support": { - "issues": "https://github.com/joshtronic/php-loremipsum/issues", - "source": "https://github.com/joshtronic/php-loremipsum/tree/2.1.0" - }, "time": "2022-01-23T23:27:44+00:00" }, { From a1ebea4394e5dd1e56eaf7c70a2c79e87669e8d7 Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Tue, 12 May 2026 17:18:50 -0500 Subject: [PATCH 02/19] feat(emails): unified email registry + DataViews list (NPPD-945, slice 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First slice of the unified email management UI (NPPD-945). This PR: - Adds `Emails_Section::get_email_registry()` with 23 curated entries covering Newspack and WooCommerce transactional emails, with metadata for default-view filtering and plugin-conditional surfacing. - Extends `api_get_email_settings()` to return enriched `newspack_emails` joined against the registry. - Replaces the existing WizardsActionCard list with a DataViews list following the institutions view pattern. Adds a "Show all emails" link to reveal lower-priority entries. - Preserves the existing Reset action (receipt/welcome only) and inactive- email notification copy. - Does not yet merge WooCommerce email surfacing into the unified view — that's a follow-up. The Woo block editor toggle remains a separate section. Known follow-ups (out of scope for this PR): - Re-add `login-otp-oauth` registry entry once the OAuth OTP email type is registered in `Reader_Activation_Emails::EMAIL_TYPES`. - PRD rationale for `woo-processing-order`, `woo-completed-order`, and `woo-on-hold-order` should be updated; these are customer-facing, not admin-facing. See TODO comments inline. Co-Authored-By: Claude Opus 4.6 --- .../wizards/newspack/class-emails-section.php | 272 ++++++++++++++-- .../views/settings/emails/emails.test.js | 163 ++++++++++ .../newspack/views/settings/emails/emails.tsx | 305 +++++++++++++----- tests/unit-tests/emails-section.php | 60 ++++ 4 files changed, 703 insertions(+), 97 deletions(-) create mode 100644 src/wizards/newspack/views/settings/emails/emails.test.js create mode 100644 tests/unit-tests/emails-section.php diff --git a/includes/wizards/newspack/class-emails-section.php b/includes/wizards/newspack/class-emails-section.php index 148face613..381d55986b 100644 --- a/includes/wizards/newspack/class-emails-section.php +++ b/includes/wizards/newspack/class-emails-section.php @@ -7,6 +7,9 @@ namespace Newspack\Wizards\Newspack; +use Newspack\Emails; +use Newspack\Reader_Activation; +use Newspack\Reader_Revenue_Emails; use Newspack\Wizards\Wizard_Section; use Newspack\WooCommerce_Emails; use WP_REST_Server; @@ -28,9 +31,6 @@ class Emails_Section extends Wizard_Section { * Register the endpoints needed for the wizard screens. */ public function register_rest_routes() { - if ( ! WooCommerce_Emails::is_active() ) { - return; - } register_rest_route( NEWSPACK_API_NAMESPACE, 'wizard/' . $this->wizard_slug . '/emails', @@ -40,23 +40,224 @@ public function register_rest_routes() { 'permission_callback' => [ $this, 'api_permissions_check' ], ] ); - register_rest_route( - NEWSPACK_API_NAMESPACE, - 'wizard/' . $this->wizard_slug . '/emails', - [ - 'methods' => WP_REST_Server::EDITABLE, - 'callback' => [ __CLASS__, 'api_update_email_settings' ], - 'permission_callback' => [ $this, 'api_permissions_check' ], - 'args' => [ - 'enable_woocommerce_email_editor' => [ - 'type' => 'boolean', - 'required' => true, - 'sanitize_callback' => 'rest_sanitize_boolean', + if ( WooCommerce_Emails::is_active() ) { + register_rest_route( + NEWSPACK_API_NAMESPACE, + 'wizard/' . $this->wizard_slug . '/emails', + [ + 'methods' => WP_REST_Server::EDITABLE, + 'callback' => [ __CLASS__, 'api_update_email_settings' ], + 'permission_callback' => [ $this, 'api_permissions_check' ], + 'args' => [ + 'enable_woocommerce_email_editor' => [ + 'type' => 'boolean', + 'required' => true, + 'sanitize_callback' => 'rest_sanitize_boolean', + ], ], - ], + ] + ); + } + } - ] - ); + /** + * Get the unified email registry. + * + * Returns all known email entries keyed by a stable slug. Each entry + * includes metadata used by the Settings > Emails UI. + * + * @return array Registry entries keyed by slug. + */ + public static function get_email_registry(): array { + return [ + 'verification' => [ + 'source' => 'newspack', + 'newspack_type' => 'reader-activation-verification', + 'default_shown' => true, + 'plugin_dependency' => null, + 'label' => __( 'Reader verification', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a reader needs to verify their email address.', 'newspack-plugin' ), + ], + 'login-link' => [ + 'source' => 'newspack', + 'newspack_type' => 'reader-activation-magic-link', + 'default_shown' => true, + 'plugin_dependency' => null, + 'label' => __( 'Magic login link', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a reader requests a magic login link.', 'newspack-plugin' ), + ], + 'login-otp' => [ + 'source' => 'newspack', + 'newspack_type' => 'reader-activation-otp-authentication', + 'default_shown' => true, + 'plugin_dependency' => null, + 'label' => __( 'Login one-time password', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a reader logs in with a one-time password.', 'newspack-plugin' ), + ], + 'set-new-password' => [ + 'source' => 'newspack', + 'newspack_type' => 'reader-activation-reset-password', + 'default_shown' => true, + 'plugin_dependency' => null, + 'label' => __( 'Password reset', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a reader requests a password reset.', 'newspack-plugin' ), + ], + 'receipt' => [ + 'source' => 'newspack', + 'newspack_type' => 'receipt', + 'default_shown' => true, + 'plugin_dependency' => null, + 'label' => __( 'Payment receipt', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent after a successful payment.', 'newspack-plugin' ), + ], + 'welcome' => [ + 'source' => 'newspack', + 'newspack_type' => 'welcome', + 'default_shown' => true, + 'plugin_dependency' => null, + 'label' => __( 'Welcome email', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent to new supporters after their first payment.', 'newspack-plugin' ), + ], + 'cancellation' => [ + 'source' => 'newspack', + 'newspack_type' => 'cancellation', + 'default_shown' => true, + 'plugin_dependency' => null, + 'label' => __( 'Cancellation confirmation', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a reader cancels their subscription.', 'newspack-plugin' ), + ], + 'woo-renewal-reminder' => [ + 'source' => 'woocommerce', + 'woo_email_id' => 'customer_renewal_invoice', + 'default_shown' => true, + 'plugin_dependency' => 'woocommerce-subscriptions', + 'label' => __( 'Subscription renewal invoice', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent to remind a customer that a renewal payment is due.', 'newspack-plugin' ), + ], + 'woo-payment-retry' => [ + 'source' => 'woocommerce', + 'woo_email_id' => 'customer_payment_retry', + 'default_shown' => true, + 'plugin_dependency' => 'woocommerce-subscriptions', + 'label' => __( 'Subscription payment retry', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a failed subscription payment is about to be retried.', 'newspack-plugin' ), + ], + 'woo-subscription-cancelled' => [ + 'source' => 'woocommerce', + 'woo_email_id' => 'cancelled_subscription', + 'default_shown' => true, + 'plugin_dependency' => 'woocommerce-subscriptions', + 'label' => __( 'Subscription cancelled', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a subscription is cancelled.', 'newspack-plugin' ), + ], + 'woo-expired-subscription' => [ + 'source' => 'woocommerce', + 'woo_email_id' => 'expired_subscription', + 'default_shown' => true, + 'plugin_dependency' => 'woocommerce-subscriptions', + 'label' => __( 'Subscription expired', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a subscription reaches its expiration date.', 'newspack-plugin' ), + ], + 'woo-customer-new-account' => [ + 'source' => 'woocommerce', + 'woo_email_id' => 'customer_new_account', + 'default_shown' => true, + 'plugin_dependency' => null, + 'label' => __( 'New account', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a customer creates a new account.', 'newspack-plugin' ), + ], + 'woo-password-reset' => [ + 'source' => 'woocommerce', + 'woo_email_id' => 'customer_reset_password', + 'default_shown' => true, + 'plugin_dependency' => null, + 'label' => __( 'Password reset (WooCommerce)', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a customer resets their password via WooCommerce.', 'newspack-plugin' ), + ], + 'delete-account' => [ + 'source' => 'newspack', + 'newspack_type' => 'reader-activation-delete-account', + 'default_shown' => false, + 'plugin_dependency' => null, + 'label' => __( 'Account deletion', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a reader requests to delete their account.', 'newspack-plugin' ), + ], + 'change-email-notification' => [ + 'source' => 'newspack', + 'newspack_type' => 'reader-activation-change-email-cancel', + 'default_shown' => false, + 'plugin_dependency' => null, + 'label' => __( 'Email change notification', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent to the old address when a reader changes their email.', 'newspack-plugin' ), + ], + 'change-email-confirmation' => [ + 'source' => 'newspack', + 'newspack_type' => 'reader-activation-change-email', + 'default_shown' => false, + 'plugin_dependency' => null, + 'label' => __( 'Email change confirmation', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent to the new address to confirm an email change.', 'newspack-plugin' ), + ], + 'non-reader-login-reminder' => [ + 'source' => 'newspack', + 'newspack_type' => 'reader-activation-non-reader-user', + 'default_shown' => false, + 'plugin_dependency' => null, + 'label' => __( 'Non-reader login reminder', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when a non-reader WordPress user tries to log in as a reader.', 'newspack-plugin' ), + ], + 'group-subscription-invitation' => [ + 'source' => 'newspack', + 'newspack_type' => 'group-subscription-invite', + 'default_shown' => false, + 'plugin_dependency' => null, + 'label' => __( 'Group subscription invitation', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent to invite a reader to join a group subscription.', 'newspack-plugin' ), + ], + 'woo-refund' => [ + 'source' => 'woocommerce', + 'woo_email_id' => 'customer_refunded_order', + 'default_shown' => false, + 'plugin_dependency' => null, + 'label' => __( 'Order refund', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when an order is refunded.', 'newspack-plugin' ), + ], + // TODO: Customer-facing email. PRD rationale should be "lower customization priority for subscription publishers" instead of "admin-facing". + 'woo-processing-order' => [ + 'source' => 'woocommerce', + 'woo_email_id' => 'customer_processing_order', + 'default_shown' => false, + 'plugin_dependency' => null, + 'label' => __( 'Order processing', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when an order payment is received and the order begins processing.', 'newspack-plugin' ), + ], + // TODO: Customer-facing email. PRD rationale should be "lower customization priority for subscription publishers" instead of "admin-facing". + 'woo-completed-order' => [ + 'source' => 'woocommerce', + 'woo_email_id' => 'customer_completed_order', + 'default_shown' => false, + 'plugin_dependency' => null, + 'label' => __( 'Order complete', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when an order is marked as complete.', 'newspack-plugin' ), + ], + // TODO: Customer-facing email. PRD rationale should be "lower customization priority for subscription publishers" instead of "admin-facing". + 'woo-on-hold-order' => [ + 'source' => 'woocommerce', + 'woo_email_id' => 'customer_on_hold_order', + 'default_shown' => false, + 'plugin_dependency' => null, + 'label' => __( 'Order on hold', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent when an order is placed on hold.', 'newspack-plugin' ), + ], + 'woo-new-order' => [ + 'source' => 'woocommerce', + 'woo_email_id' => 'new_order', + 'default_shown' => false, + 'plugin_dependency' => null, + 'label' => __( 'New order (admin)', 'newspack-plugin' ), + 'trigger_description' => __( 'Sent to the admin when a new order is placed.', 'newspack-plugin' ), + ], + ]; } /** @@ -70,6 +271,41 @@ public static function api_get_email_settings(): array { $settings['admin_url'] = admin_url( 'admin.php?page=wc-settings&tab=email' ); $settings['enable_woocommerce_email_editor'] = 'yes' === WooCommerce_Emails::is_enabled(); } + + // Build newspack_emails from the Emails system, enriched with registry data. + $config_names = []; + if ( ! Reader_Activation::is_enabled() ) { + $config_names = array_values( Reader_Revenue_Emails::EMAIL_TYPES ); + } + $emails = Emails::get_emails( $config_names, false ); + + // Build a lookup from newspack_type => registry entry. + $registry = self::get_email_registry(); + $registry_lookup = []; + foreach ( $registry as $slug => $entry ) { + if ( isset( $entry['newspack_type'] ) ) { + $registry_lookup[ $entry['newspack_type'] ] = array_merge( $entry, [ 'registry_slug' => $slug ] ); + } + } + + $newspack_emails = []; + foreach ( $emails as $type => $email ) { + if ( isset( $registry_lookup[ $type ] ) ) { + $match = $registry_lookup[ $type ]; + $email['default_shown'] = $match['default_shown']; + $email['trigger_description'] = $match['trigger_description']; + $email['registry_slug'] = $match['registry_slug']; + } else { + $email['default_shown'] = false; + $email['trigger_description'] = ''; + $email['registry_slug'] = ''; + } + $newspack_emails[] = $email; + } + + $settings['newspack_emails'] = $newspack_emails; + $settings['post_type'] = Emails::POST_TYPE; + return $settings; } diff --git a/src/wizards/newspack/views/settings/emails/emails.test.js b/src/wizards/newspack/views/settings/emails/emails.test.js new file mode 100644 index 0000000000..6d818ac594 --- /dev/null +++ b/src/wizards/newspack/views/settings/emails/emails.test.js @@ -0,0 +1,163 @@ +// @jest-environment jsdom + +/** + * External dependencies + */ +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; + +/** + * WordPress dependencies + */ +import apiFetch from '@wordpress/api-fetch'; + +jest.mock( '@wordpress/api-fetch', () => jest.fn() ); + +jest.mock( '@wordpress/dataviews', () => ( { + filterSortAndPaginate: data => ( { + data, + paginationInfo: { totalItems: data.length, totalPages: 1 }, + } ), +} ) ); + +jest.mock( '../../../../../../packages/components/src', () => { + const React = require( 'react' ); + return { + DataViews: ( { data, fields } ) => ( + + + { data.map( ( item, i ) => ( + + { fields.map( field => ( + + ) ) } + + ) ) } + +
{ field.render( { item } ) }
+ ), + Notice: ( { noticeText } ) =>
{ noticeText }
, + utils: { + confirmAction: jest.fn( () => true ), + }, + }; +} ); + +jest.mock( + '../../../../wizards-plugin-card', + () => + function MockPluginCard( props ) { + return
{ props.slug }
; + } +); + +const mockEmails = [ + { + label: 'Reader verification', + description: 'Verification email', + post_id: 1, + edit_link: '/edit/1', + subject: 'Verify your email', + from_name: 'Test', + from_email: 'test@example.com', + reply_to_email: 'test@example.com', + status: 'publish', + type: 'reader-activation-verification', + category: 'reader-activation', + default_shown: true, + trigger_description: 'Sent when a reader needs to verify their email address.', + registry_slug: 'verification', + }, + { + label: 'Payment receipt', + description: 'Receipt email', + post_id: 2, + edit_link: '/edit/2', + subject: 'Your receipt', + from_name: 'Test', + from_email: 'test@example.com', + reply_to_email: 'test@example.com', + status: 'publish', + type: 'receipt', + category: 'reader-revenue', + default_shown: true, + trigger_description: 'Sent after a successful payment.', + registry_slug: 'receipt', + }, + { + label: 'Account deletion', + description: 'Delete account email', + post_id: 3, + edit_link: '/edit/3', + subject: 'Account deleted', + from_name: 'Test', + from_email: 'test@example.com', + reply_to_email: 'test@example.com', + status: 'publish', + type: 'reader-activation-delete-account', + category: 'reader-activation', + default_shown: false, + trigger_description: 'Sent when a reader requests to delete their account.', + registry_slug: 'delete-account', + }, +]; + +describe( 'Emails', () => { + beforeEach( () => { + jest.clearAllMocks(); + window.newspackSettings = { + emails: { + sections: { + emails: { + dependencies: { + newspackNewsletters: true, + }, + postType: 'newspack_rr_email', + all: {}, + isEmailEnhancementsActive: false, + }, + }, + }, + }; + apiFetch.mockResolvedValue( { + newspack_emails: mockEmails, + post_type: 'newspack_rr_email', + } ); + } ); + + it( 'renders DataViews with email data', async () => { + const Emails = require( './emails' ).default; + render( ); + + await waitFor( () => { + expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + } ); + + expect( screen.getByText( 'Reader verification' ) ).toBeInTheDocument(); + expect( screen.getByText( 'Payment receipt' ) ).toBeInTheDocument(); + } ); + + it( '"Show all" toggle changes filter', async () => { + const Emails = require( './emails' ).default; + render( ); + + await waitFor( () => { + expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + } ); + + // Default: only default_shown emails. + expect( screen.getByText( 'Reader verification' ) ).toBeInTheDocument(); + expect( screen.getByText( 'Payment receipt' ) ).toBeInTheDocument(); + expect( screen.queryByText( 'Account deletion' ) ).not.toBeInTheDocument(); + + // Click "Show all emails". + fireEvent.click( screen.getByText( 'Show all emails' ) ); + + // Now all emails should be visible. + expect( screen.getByText( 'Reader verification' ) ).toBeInTheDocument(); + expect( screen.getByText( 'Payment receipt' ) ).toBeInTheDocument(); + expect( screen.getByText( 'Account deletion' ) ).toBeInTheDocument(); + + // Toggle text should change. + expect( screen.getByText( 'Show default emails' ) ).toBeInTheDocument(); + } ); +} ); diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index 144781df48..c1942dfcb1 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -6,65 +6,237 @@ * WordPress dependencies. */ import { __ } from '@wordpress/i18n'; -import { useState, Fragment } from '@wordpress/element'; +import { useState, useEffect, useCallback, useMemo, Fragment } from '@wordpress/element'; +import apiFetch from '@wordpress/api-fetch'; +import { filterSortAndPaginate } from '@wordpress/dataviews'; +import { Button, ToggleControl } from '@wordpress/components'; +import type { Action, Field, View } from '@wordpress/dataviews'; /** * Internal dependencies. */ -import { Notice, utils } from '../../../../../../packages/components/src'; -import { useWizardApiFetch } from '../../../../hooks/use-wizard-api-fetch'; -import WizardsActionCard from '../../../../wizards-action-card'; +import { DataViews, Notice, utils } from '../../../../../../packages/components/src'; import WizardsPluginCard from '../../../../wizards-plugin-card'; +interface EmailItem { + label: string; + description: string; + post_id: number; + edit_link: string; + subject: string; + from_name: string; + from_email: string; + reply_to_email: string; + status: string; + type: string; + category: string; + default_shown: boolean; + trigger_description: string; + registry_slug: string; +} + +interface EmailSettings { + newspack_emails: EmailItem[]; + post_type: string; + admin_url?: string; + enable_woocommerce_email_editor?: boolean; +} + +const DEFAULT_VIEW: View = { + type: 'table', + page: 1, + perPage: 25, + sort: { field: 'name', direction: 'asc' }, + search: '', + fields: [ 'trigger_description', 'status', 'note' ], + filters: [], + layout: {}, + titleField: 'name', +}; + +function getInactiveNote( item: EmailItem ): string { + if ( item.status === 'publish' ) { + return ''; + } + if ( item.type === 'receipt' ) { + return __( 'This email is not active. The default receipt will be used.', 'newspack-plugin' ); + } + if ( item.type === 'welcome' ) { + return __( 'This email is not active. The receipt template will be used if active.', 'newspack-plugin' ); + } + return __( 'This email is not active.', 'newspack-plugin' ); +} + const Emails = () => { const emailSections = window.newspackSettings.emails.sections; - const postType = emailSections.emails.postType; - const [ pluginsReady, setPluginsReady ] = useState( emailSections.emails.dependencies.newspackNewsletters ); - const { wizardApiFetch, isFetching, errorMessage, resetError } = useWizardApiFetch( 'newspack-settings/emails' ); + const [ data, setData ] = useState< EmailItem[] >( [] ); + const [ postType, setPostType ] = useState< string >( emailSections.emails.postType ); + const [ isLoading, setIsLoading ] = useState( true ); + const [ showAll, setShowAll ] = useState( false ); + const [ view, setView ] = useState< View >( DEFAULT_VIEW ); + const [ error, setError ] = useState< string | null >( null ); - const [ emails, setEmails ] = useState( Object.values( emailSections.emails.all ) ); + const fetchData = useCallback( () => { + setIsLoading( true ); + setError( null ); + apiFetch< EmailSettings >( { + path: '/newspack/v1/wizard/newspack-settings/emails', + } ) + .then( result => { + setData( result.newspack_emails || [] ); + if ( result.post_type ) { + setPostType( result.post_type ); + } + } ) + .catch( () => { + setError( __( 'Failed to load emails. Please refresh the page.', 'newspack-plugin' ) ); + } ) + .finally( () => setIsLoading( false ) ); + }, [] ); - const updateStatus = ( postId: number, status: string ) => { - wizardApiFetch( - { + useEffect( () => { + fetchData(); + }, [ fetchData ] ); + + const filteredData = useMemo( () => ( showAll ? data : data.filter( email => email.default_shown ) ), [ data, showAll ] ); + + const updateStatus = useCallback( + ( postId: number, status: string ) => { + setError( null ); + // Optimistic update. + setData( prev => + prev.map( email => { + if ( email.post_id === postId ) { + return { ...email, status }; + } + return email; + } ) + ); + apiFetch( { path: `/wp/v2/${ postType }/${ postId }`, method: 'POST', data: { status }, + } ).catch( () => { + // Revert on failure. + setData( prev => + prev.map( email => { + if ( email.post_id === postId ) { + return { + ...email, + status: status === 'publish' ? 'draft' : 'publish', + }; + } + return email; + } ) + ); + setError( __( 'Failed to update email status.', 'newspack-plugin' ) ); + } ); + }, + [ postType ] + ); + + const resetEmail = useCallback( + ( postId: number ) => { + setError( null ); + apiFetch( { + path: `/newspack/v1/wizard/newspack-audience-donations/emails/${ postId }`, + method: 'DELETE', + } ) + .then( () => { + fetchData(); + } ) + .catch( () => { + setError( __( 'Failed to reset email. Please try again.', 'newspack-plugin' ) ); + } ); + }, + [ fetchData ] + ); + + const fields: Field< EmailItem >[] = useMemo( + () => [ + { + id: 'name', + label: __( 'Email', 'newspack-plugin' ), + enableGlobalSearch: true, + getValue: ( { item }: { item: EmailItem } ) => item.label, + render: ( { item }: { item: EmailItem } ) => ( +
+ { item.label } + { item.description &&
{ item.description }
} +
+ ), }, { - onStart() { - resetError(); - }, - onSuccess() { - setEmails( - emails.map( email => { - if ( email.post_id === postId ) { - return { ...email, status }; - } - return email; - } ) + id: 'trigger_description', + label: __( 'Trigger', 'newspack-plugin' ), + getValue: ( { item }: { item: EmailItem } ) => item.trigger_description || '', + render: ( { item }: { item: EmailItem } ) => { item.trigger_description }, + enableSorting: false, + }, + { + id: 'status', + label: __( 'Status', 'newspack-plugin' ), + getValue: ( { item }: { item: EmailItem } ) => item.status, + render: ( { item }: { item: EmailItem } ) => { + if ( item.category === 'reader-activation' ) { + return { item.status === 'publish' ? __( 'Active', 'newspack-plugin' ) : __( 'Inactive', 'newspack-plugin' ) }; + } + return ( + updateStatus( item.post_id, checked ? 'publish' : 'draft' ) } + label={ item.status === 'publish' ? __( 'Active', 'newspack-plugin' ) : __( 'Inactive', 'newspack-plugin' ) } + /> ); }, - } - ); - }; + enableSorting: false, + }, + { + id: 'note', + label: __( 'Note', 'newspack-plugin' ), + getValue: ( { item }: { item: EmailItem } ) => getInactiveNote( item ), + render: ( { item }: { item: EmailItem } ) => { + const note = getInactiveNote( item ); + return note ? { note } : null; + }, + enableSorting: false, + }, + ], + [ updateStatus ] + ); - const resetEmail = ( postId: number ) => { - wizardApiFetch( + const actions: Action< EmailItem >[] = useMemo( + () => [ { - path: `/newspack/v1/wizard/newspack-audience-donations/emails/${ postId }`, - method: 'DELETE', + id: 'edit', + label: __( 'Edit', 'newspack-plugin' ), + isPrimary: true, + callback: ( items: EmailItem[] ) => { + window.location.href = items[ 0 ].edit_link; + }, }, { - onSuccess( result ) { - window.newspackSettings.emails.sections.emails.all = result; - setEmails( Object.values( result ) ); + id: 'reset', + label: __( 'Reset', 'newspack-plugin' ), + isDestructive: true, + isEligible: ( item: EmailItem ) => item.type === 'receipt' || item.type === 'welcome', + callback: ( items: EmailItem[] ) => { + if ( utils.confirmAction( __( 'Are you sure you want to reset the contents of this email?', 'newspack-plugin' ) ) ) { + resetEmail( items[ 0 ].post_id ); + } }, - } - ); - }; + }, + ], + [ resetEmail ] + ); + + const { data: processedData, paginationInfo } = useMemo( + () => filterSortAndPaginate( filteredData, view, fields ), + [ filteredData, view, fields ] + ); if ( false === pluginsReady ) { return ( @@ -93,50 +265,25 @@ const Emails = () => { return ( - { emails.map( email => { - const isActive = email.status === 'publish'; - const isAudience = email.category === 'reader-activation'; - let notification = __( 'This email is not active.', 'newspack-plugin' ); - if ( email.type === 'receipt' ) { - notification = __( 'This email is not active. The default receipt will be used.', 'newspack-plugin' ); - } - if ( email.type === 'welcome' ) { - notification = __( 'This email is not active. The receipt template will be used if active.', 'newspack-plugin' ); - } - return ( - { - if ( utils.confirmAction( __( 'Are you sure you want to reset the contents of this email?', 'newspack-plugin' ) ) ) { - resetEmail( email.post_id ); - } - } } - secondaryDestructive={ true } - { ...( isAudience - ? {} - : { - toggleChecked: isActive, - toggleOnChange: value => updateStatus( email.post_id, value ? 'publish' : 'draft' ), - } ) } - { ...( isActive - ? {} - : { - notification, - notificationLevel: 'info', - } ) } - > - { errorMessage && } - - ); - } ) } + { error && } + String( item.post_id ) } + search + /> +

+ +

); }; diff --git a/tests/unit-tests/emails-section.php b/tests/unit-tests/emails-section.php new file mode 100644 index 0000000000..2f643cf570 --- /dev/null +++ b/tests/unit-tests/emails-section.php @@ -0,0 +1,60 @@ +assertCount( 23, $registry ); + } + + /** + * Test the registry has 13 default-shown entries. + */ + public function test_registry_has_13_default_shown() { + $registry = Emails_Section::get_email_registry(); + $default_shown = array_filter( + $registry, + function ( $entry ) { + return true === $entry['default_shown']; + } + ); + $this->assertCount( 13, $default_shown ); + } + + /** + * Test the registry has 4 entries with woocommerce-subscriptions dependency. + */ + public function test_registry_has_4_subscriptions_dependency() { + $registry = Emails_Section::get_email_registry(); + $with_woo_sub = array_filter( + $registry, + function ( $entry ) { + return 'woocommerce-subscriptions' === $entry['plugin_dependency']; + } + ); + $this->assertCount( 4, $with_woo_sub ); + } + + /** + * Test all registry entries have non-empty labels and trigger descriptions. + */ + public function test_registry_entries_have_labels_and_triggers() { + $registry = Emails_Section::get_email_registry(); + foreach ( $registry as $slug => $entry ) { + $this->assertNotEmpty( $entry['label'], "Entry '$slug' is missing a label." ); + $this->assertNotEmpty( $entry['trigger_description'], "Entry '$slug' is missing a trigger_description." ); + } + } +} From 01fbe6dc7fde6a27c8d4d95af6073253e0797335 Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Wed, 13 May 2026 22:08:15 -0500 Subject: [PATCH 03/19] feat(emails): refine DataViews UI per PRD mockup (NPPD-945) - Drop tabs and show-all toggle; list all 24 emails in single view sorted by category - Add recipient column (Reader/Admin) - Move email actions (Edit/Activate/Deactivate/Reset) into kebab menu on every row - Add secondary status filter (Enabled/Disabled) - Restore inactive-email notification copy via Note column - Add Edit template button and page subtitle - Use trigger_description from registry as sub-text under email title - Various polish: button variants, color overrides, fixed stale-closure bugs --- .../wizards/newspack/class-emails-section.php | 39 ++++-- .../views/settings/emails/emails.test.js | 30 +++++ .../newspack/views/settings/emails/emails.tsx | 111 ++++++++++-------- tests/unit-tests/emails-section.php | 10 ++ 4 files changed, 132 insertions(+), 58 deletions(-) diff --git a/includes/wizards/newspack/class-emails-section.php b/includes/wizards/newspack/class-emails-section.php index 381d55986b..2b4fe21fc2 100644 --- a/includes/wizards/newspack/class-emails-section.php +++ b/includes/wizards/newspack/class-emails-section.php @@ -75,6 +75,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'reader-activation-verification', 'default_shown' => true, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Reader verification', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a reader needs to verify their email address.', 'newspack-plugin' ), ], @@ -83,6 +84,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'reader-activation-magic-link', 'default_shown' => true, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Magic login link', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a reader requests a magic login link.', 'newspack-plugin' ), ], @@ -91,6 +93,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'reader-activation-otp-authentication', 'default_shown' => true, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Login one-time password', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a reader logs in with a one-time password.', 'newspack-plugin' ), ], @@ -99,6 +102,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'reader-activation-reset-password', 'default_shown' => true, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Password reset', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a reader requests a password reset.', 'newspack-plugin' ), ], @@ -107,6 +111,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'receipt', 'default_shown' => true, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Payment receipt', 'newspack-plugin' ), 'trigger_description' => __( 'Sent after a successful payment.', 'newspack-plugin' ), ], @@ -115,6 +120,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'welcome', 'default_shown' => true, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Welcome email', 'newspack-plugin' ), 'trigger_description' => __( 'Sent to new supporters after their first payment.', 'newspack-plugin' ), ], @@ -123,6 +129,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'cancellation', 'default_shown' => true, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Cancellation confirmation', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a reader cancels their subscription.', 'newspack-plugin' ), ], @@ -131,6 +138,7 @@ public static function get_email_registry(): array { 'woo_email_id' => 'customer_renewal_invoice', 'default_shown' => true, 'plugin_dependency' => 'woocommerce-subscriptions', + 'recipient' => 'reader', 'label' => __( 'Subscription renewal invoice', 'newspack-plugin' ), 'trigger_description' => __( 'Sent to remind a customer that a renewal payment is due.', 'newspack-plugin' ), ], @@ -139,6 +147,7 @@ public static function get_email_registry(): array { 'woo_email_id' => 'customer_payment_retry', 'default_shown' => true, 'plugin_dependency' => 'woocommerce-subscriptions', + 'recipient' => 'reader', 'label' => __( 'Subscription payment retry', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a failed subscription payment is about to be retried.', 'newspack-plugin' ), ], @@ -147,6 +156,7 @@ public static function get_email_registry(): array { 'woo_email_id' => 'cancelled_subscription', 'default_shown' => true, 'plugin_dependency' => 'woocommerce-subscriptions', + 'recipient' => 'reader', 'label' => __( 'Subscription cancelled', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a subscription is cancelled.', 'newspack-plugin' ), ], @@ -155,6 +165,7 @@ public static function get_email_registry(): array { 'woo_email_id' => 'expired_subscription', 'default_shown' => true, 'plugin_dependency' => 'woocommerce-subscriptions', + 'recipient' => 'reader', 'label' => __( 'Subscription expired', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a subscription reaches its expiration date.', 'newspack-plugin' ), ], @@ -163,6 +174,7 @@ public static function get_email_registry(): array { 'woo_email_id' => 'customer_new_account', 'default_shown' => true, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'New account', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a customer creates a new account.', 'newspack-plugin' ), ], @@ -171,6 +183,7 @@ public static function get_email_registry(): array { 'woo_email_id' => 'customer_reset_password', 'default_shown' => true, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Password reset (WooCommerce)', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a customer resets their password via WooCommerce.', 'newspack-plugin' ), ], @@ -179,6 +192,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'reader-activation-delete-account', 'default_shown' => false, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Account deletion', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a reader requests to delete their account.', 'newspack-plugin' ), ], @@ -187,6 +201,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'reader-activation-change-email-cancel', 'default_shown' => false, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Email change notification', 'newspack-plugin' ), 'trigger_description' => __( 'Sent to the old address when a reader changes their email.', 'newspack-plugin' ), ], @@ -195,6 +210,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'reader-activation-change-email', 'default_shown' => false, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Email change confirmation', 'newspack-plugin' ), 'trigger_description' => __( 'Sent to the new address to confirm an email change.', 'newspack-plugin' ), ], @@ -203,6 +219,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'reader-activation-non-reader-user', 'default_shown' => false, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Non-reader login reminder', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when a non-reader WordPress user tries to log in as a reader.', 'newspack-plugin' ), ], @@ -211,6 +228,7 @@ public static function get_email_registry(): array { 'newspack_type' => 'group-subscription-invite', 'default_shown' => false, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Group subscription invitation', 'newspack-plugin' ), 'trigger_description' => __( 'Sent to invite a reader to join a group subscription.', 'newspack-plugin' ), ], @@ -219,6 +237,7 @@ public static function get_email_registry(): array { 'woo_email_id' => 'customer_refunded_order', 'default_shown' => false, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Order refund', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when an order is refunded.', 'newspack-plugin' ), ], @@ -228,6 +247,7 @@ public static function get_email_registry(): array { 'woo_email_id' => 'customer_processing_order', 'default_shown' => false, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Order processing', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when an order payment is received and the order begins processing.', 'newspack-plugin' ), ], @@ -237,6 +257,7 @@ public static function get_email_registry(): array { 'woo_email_id' => 'customer_completed_order', 'default_shown' => false, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Order complete', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when an order is marked as complete.', 'newspack-plugin' ), ], @@ -246,6 +267,7 @@ public static function get_email_registry(): array { 'woo_email_id' => 'customer_on_hold_order', 'default_shown' => false, 'plugin_dependency' => null, + 'recipient' => 'reader', 'label' => __( 'Order on hold', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when an order is placed on hold.', 'newspack-plugin' ), ], @@ -254,6 +276,7 @@ public static function get_email_registry(): array { 'woo_email_id' => 'new_order', 'default_shown' => false, 'plugin_dependency' => null, + 'recipient' => 'admin', 'label' => __( 'New order (admin)', 'newspack-plugin' ), 'trigger_description' => __( 'Sent to the admin when a new order is placed.', 'newspack-plugin' ), ], @@ -291,14 +314,16 @@ public static function api_get_email_settings(): array { $newspack_emails = []; foreach ( $emails as $type => $email ) { if ( isset( $registry_lookup[ $type ] ) ) { - $match = $registry_lookup[ $type ]; - $email['default_shown'] = $match['default_shown']; - $email['trigger_description'] = $match['trigger_description']; - $email['registry_slug'] = $match['registry_slug']; + $match = $registry_lookup[ $type ]; + $email['default_shown'] = $match['default_shown']; + $email['trigger_description'] = $match['trigger_description']; + $email['registry_slug'] = $match['registry_slug']; + $email['recipient'] = $match['recipient']; } else { - $email['default_shown'] = false; - $email['trigger_description'] = ''; - $email['registry_slug'] = ''; + $email['default_shown'] = false; + $email['trigger_description'] = ''; + $email['registry_slug'] = ''; + $email['recipient'] = 'reader'; } $newspack_emails[] = $email; } diff --git a/src/wizards/newspack/views/settings/emails/emails.test.js b/src/wizards/newspack/views/settings/emails/emails.test.js index 6d818ac594..846e15698b 100644 --- a/src/wizards/newspack/views/settings/emails/emails.test.js +++ b/src/wizards/newspack/views/settings/emails/emails.test.js @@ -35,6 +35,7 @@ jest.mock( '../../../../../../packages/components/src', () => { ), + Card: ( { children } ) =>
{ children }
, Notice: ( { noticeText } ) =>
{ noticeText }
, utils: { confirmAction: jest.fn( () => true ), @@ -66,6 +67,7 @@ const mockEmails = [ default_shown: true, trigger_description: 'Sent when a reader needs to verify their email address.', registry_slug: 'verification', + recipient: 'reader', }, { label: 'Payment receipt', @@ -82,6 +84,7 @@ const mockEmails = [ default_shown: true, trigger_description: 'Sent after a successful payment.', registry_slug: 'receipt', + recipient: 'reader', }, { label: 'Account deletion', @@ -98,6 +101,7 @@ const mockEmails = [ default_shown: false, trigger_description: 'Sent when a reader requests to delete their account.', registry_slug: 'delete-account', + recipient: 'reader', }, ]; @@ -136,6 +140,32 @@ describe( 'Emails', () => { expect( screen.getByText( 'Payment receipt' ) ).toBeInTheDocument(); } ); + it( 'renders Recipient column with correct values', async () => { + const Emails = require( './emails' ).default; + render( ); + + await waitFor( () => { + expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + } ); + + // All mock emails have recipient: 'reader'. + const readerCells = screen.getAllByText( 'Reader' ); + expect( readerCells.length ).toBeGreaterThanOrEqual( 2 ); + } ); + + it( 'renders status as Enabled / Disabled', async () => { + const Emails = require( './emails' ).default; + render( ); + + await waitFor( () => { + expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + } ); + + // All mock emails have status: 'publish'. + const enabledCells = screen.getAllByText( 'Enabled' ); + expect( enabledCells.length ).toBeGreaterThanOrEqual( 2 ); + } ); + it( '"Show all" toggle changes filter', async () => { const Emails = require( './emails' ).default; render( ); diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index c1942dfcb1..3cb1ca0a06 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -9,13 +9,13 @@ import { __ } from '@wordpress/i18n'; import { useState, useEffect, useCallback, useMemo, Fragment } from '@wordpress/element'; import apiFetch from '@wordpress/api-fetch'; import { filterSortAndPaginate } from '@wordpress/dataviews'; -import { Button, ToggleControl } from '@wordpress/components'; +import { Button } from '@wordpress/components'; import type { Action, Field, View } from '@wordpress/dataviews'; /** * Internal dependencies. */ -import { DataViews, Notice, utils } from '../../../../../../packages/components/src'; +import { Card, DataViews, Notice, utils } from '../../../../../../packages/components/src'; import WizardsPluginCard from '../../../../wizards-plugin-card'; interface EmailItem { @@ -33,6 +33,7 @@ interface EmailItem { default_shown: boolean; trigger_description: string; registry_slug: string; + recipient: 'reader' | 'admin'; } interface EmailSettings { @@ -42,31 +43,22 @@ interface EmailSettings { enable_woocommerce_email_editor?: boolean; } +const CATEGORY_ORDER: Record< string, number > = { + 'reader-revenue': 0, + 'reader-activation': 1, +}; + const DEFAULT_VIEW: View = { type: 'table', page: 1, perPage: 25, - sort: { field: 'name', direction: 'asc' }, search: '', - fields: [ 'trigger_description', 'status', 'note' ], + fields: [ 'recipient', 'status' ], filters: [], layout: {}, titleField: 'name', }; -function getInactiveNote( item: EmailItem ): string { - if ( item.status === 'publish' ) { - return ''; - } - if ( item.type === 'receipt' ) { - return __( 'This email is not active. The default receipt will be used.', 'newspack-plugin' ); - } - if ( item.type === 'welcome' ) { - return __( 'This email is not active. The receipt template will be used if active.', 'newspack-plugin' ); - } - return __( 'This email is not active.', 'newspack-plugin' ); -} - const Emails = () => { const emailSections = window.newspackSettings.emails.sections; const [ pluginsReady, setPluginsReady ] = useState( emailSections.emails.dependencies.newspackNewsletters ); @@ -85,7 +77,14 @@ const Emails = () => { path: '/newspack/v1/wizard/newspack-settings/emails', } ) .then( result => { - setData( result.newspack_emails || [] ); + const emails = result.newspack_emails || []; + // Sort by category: reader-revenue first, reader-activation second, everything else last. + emails.sort( ( a, b ) => { + const orderA = CATEGORY_ORDER[ a.category ] ?? 2; + const orderB = CATEGORY_ORDER[ b.category ] ?? 2; + return orderA - orderB; + } ); + setData( emails ); if ( result.post_type ) { setPostType( result.post_type ); } @@ -164,48 +163,30 @@ const Emails = () => { render: ( { item }: { item: EmailItem } ) => (
{ item.label } - { item.description &&
{ item.description }
} + { item.trigger_description && ( +
{ item.trigger_description }
+ ) }
), }, { - id: 'trigger_description', - label: __( 'Trigger', 'newspack-plugin' ), - getValue: ( { item }: { item: EmailItem } ) => item.trigger_description || '', - render: ( { item }: { item: EmailItem } ) => { item.trigger_description }, - enableSorting: false, + id: 'recipient', + label: __( 'Recipient', 'newspack-plugin' ), + getValue: ( { item }: { item: EmailItem } ) => item.recipient, + render: ( { item }: { item: EmailItem } ) => ( + { item.recipient === 'admin' ? __( 'Admin', 'newspack-plugin' ) : __( 'Reader', 'newspack-plugin' ) } + ), }, { id: 'status', label: __( 'Status', 'newspack-plugin' ), getValue: ( { item }: { item: EmailItem } ) => item.status, - render: ( { item }: { item: EmailItem } ) => { - if ( item.category === 'reader-activation' ) { - return { item.status === 'publish' ? __( 'Active', 'newspack-plugin' ) : __( 'Inactive', 'newspack-plugin' ) }; - } - return ( - updateStatus( item.post_id, checked ? 'publish' : 'draft' ) } - label={ item.status === 'publish' ? __( 'Active', 'newspack-plugin' ) : __( 'Inactive', 'newspack-plugin' ) } - /> - ); - }, - enableSorting: false, - }, - { - id: 'note', - label: __( 'Note', 'newspack-plugin' ), - getValue: ( { item }: { item: EmailItem } ) => getInactiveNote( item ), - render: ( { item }: { item: EmailItem } ) => { - const note = getInactiveNote( item ); - return note ? { note } : null; - }, - enableSorting: false, + render: ( { item }: { item: EmailItem } ) => ( + { item.status === 'publish' ? __( 'Enabled', 'newspack-plugin' ) : __( 'Disabled', 'newspack-plugin' ) } + ), }, ], - [ updateStatus ] + [] ); const actions: Action< EmailItem >[] = useMemo( @@ -213,11 +194,26 @@ const Emails = () => { { id: 'edit', label: __( 'Edit', 'newspack-plugin' ), - isPrimary: true, callback: ( items: EmailItem[] ) => { window.location.href = items[ 0 ].edit_link; }, }, + { + id: 'deactivate', + label: __( 'Deactivate', 'newspack-plugin' ), + isEligible: ( item: EmailItem ) => item.category !== 'reader-activation' && item.status === 'publish', + callback: ( items: EmailItem[] ) => { + updateStatus( items[ 0 ].post_id, 'draft' ); + }, + }, + { + id: 'activate', + label: __( 'Activate', 'newspack-plugin' ), + isEligible: ( item: EmailItem ) => item.category !== 'reader-activation' && item.status !== 'publish', + callback: ( items: EmailItem[] ) => { + updateStatus( items[ 0 ].post_id, 'publish' ); + }, + }, { id: 'reset', label: __( 'Reset', 'newspack-plugin' ), @@ -230,7 +226,7 @@ const Emails = () => { }, }, ], - [ resetEmail ] + [ resetEmail, updateStatus ] ); const { data: processedData, paginationInfo } = useMemo( @@ -265,6 +261,19 @@ const Emails = () => { return ( + +
+

+ { __( + "Manage the transactional emails your readers receive. Use 'Edit template' to customize the design that wraps every email.", + 'newspack-plugin' + ) } +

+
+ +
{ error && } assertCount( 4, $with_woo_sub ); } + /** + * Test all registry entries have a valid recipient value. + */ + public function test_registry_entries_have_recipient() { + $registry = Emails_Section::get_email_registry(); + foreach ( $registry as $slug => $entry ) { + $this->assertContains( $entry['recipient'], [ 'reader', 'admin' ], "Entry '$slug' has an invalid recipient value." ); + } + } + /** * Test all registry entries have non-empty labels and trigger descriptions. */ From 7de56e8cabf68a02d2699393c158c2efff37fc08 Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Thu, 14 May 2026 11:25:19 -0500 Subject: [PATCH 04/19] fix(emails): apply design polish, status dots, grid default, and test fixes (NPPD-945) --- .../wizards/newspack/class-emails-section.php | 67 +++++--- .../views/settings/emails/emails.scss | 67 ++++++++ .../views/settings/emails/emails.test.js | 152 +++++++++++------- .../newspack/views/settings/emails/emails.tsx | 97 ++++++----- .../newspack/views/settings/emails/index.tsx | 7 +- 5 files changed, 252 insertions(+), 138 deletions(-) create mode 100644 src/wizards/newspack/views/settings/emails/emails.scss diff --git a/includes/wizards/newspack/class-emails-section.php b/includes/wizards/newspack/class-emails-section.php index 2b4fe21fc2..a302b85005 100644 --- a/includes/wizards/newspack/class-emails-section.php +++ b/includes/wizards/newspack/class-emails-section.php @@ -73,7 +73,7 @@ public static function get_email_registry(): array { 'verification' => [ 'source' => 'newspack', 'newspack_type' => 'reader-activation-verification', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Reader verification', 'newspack-plugin' ), @@ -82,7 +82,7 @@ public static function get_email_registry(): array { 'login-link' => [ 'source' => 'newspack', 'newspack_type' => 'reader-activation-magic-link', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Magic login link', 'newspack-plugin' ), @@ -91,7 +91,7 @@ public static function get_email_registry(): array { 'login-otp' => [ 'source' => 'newspack', 'newspack_type' => 'reader-activation-otp-authentication', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Login one-time password', 'newspack-plugin' ), @@ -100,7 +100,7 @@ public static function get_email_registry(): array { 'set-new-password' => [ 'source' => 'newspack', 'newspack_type' => 'reader-activation-reset-password', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Password reset', 'newspack-plugin' ), @@ -109,7 +109,7 @@ public static function get_email_registry(): array { 'receipt' => [ 'source' => 'newspack', 'newspack_type' => 'receipt', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Payment receipt', 'newspack-plugin' ), @@ -118,7 +118,7 @@ public static function get_email_registry(): array { 'welcome' => [ 'source' => 'newspack', 'newspack_type' => 'welcome', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Welcome email', 'newspack-plugin' ), @@ -127,7 +127,7 @@ public static function get_email_registry(): array { 'cancellation' => [ 'source' => 'newspack', 'newspack_type' => 'cancellation', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Cancellation confirmation', 'newspack-plugin' ), @@ -136,7 +136,7 @@ public static function get_email_registry(): array { 'woo-renewal-reminder' => [ 'source' => 'woocommerce', 'woo_email_id' => 'customer_renewal_invoice', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => 'woocommerce-subscriptions', 'recipient' => 'reader', 'label' => __( 'Subscription renewal invoice', 'newspack-plugin' ), @@ -145,7 +145,7 @@ public static function get_email_registry(): array { 'woo-payment-retry' => [ 'source' => 'woocommerce', 'woo_email_id' => 'customer_payment_retry', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => 'woocommerce-subscriptions', 'recipient' => 'reader', 'label' => __( 'Subscription payment retry', 'newspack-plugin' ), @@ -154,7 +154,7 @@ public static function get_email_registry(): array { 'woo-subscription-cancelled' => [ 'source' => 'woocommerce', 'woo_email_id' => 'cancelled_subscription', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => 'woocommerce-subscriptions', 'recipient' => 'reader', 'label' => __( 'Subscription cancelled', 'newspack-plugin' ), @@ -163,7 +163,7 @@ public static function get_email_registry(): array { 'woo-expired-subscription' => [ 'source' => 'woocommerce', 'woo_email_id' => 'expired_subscription', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => 'woocommerce-subscriptions', 'recipient' => 'reader', 'label' => __( 'Subscription expired', 'newspack-plugin' ), @@ -172,7 +172,7 @@ public static function get_email_registry(): array { 'woo-customer-new-account' => [ 'source' => 'woocommerce', 'woo_email_id' => 'customer_new_account', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'New account', 'newspack-plugin' ), @@ -181,7 +181,7 @@ public static function get_email_registry(): array { 'woo-password-reset' => [ 'source' => 'woocommerce', 'woo_email_id' => 'customer_reset_password', - 'default_shown' => true, + 'recommended' => true, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Password reset (WooCommerce)', 'newspack-plugin' ), @@ -190,7 +190,7 @@ public static function get_email_registry(): array { 'delete-account' => [ 'source' => 'newspack', 'newspack_type' => 'reader-activation-delete-account', - 'default_shown' => false, + 'recommended' => false, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Account deletion', 'newspack-plugin' ), @@ -199,7 +199,7 @@ public static function get_email_registry(): array { 'change-email-notification' => [ 'source' => 'newspack', 'newspack_type' => 'reader-activation-change-email-cancel', - 'default_shown' => false, + 'recommended' => false, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Email change notification', 'newspack-plugin' ), @@ -208,7 +208,7 @@ public static function get_email_registry(): array { 'change-email-confirmation' => [ 'source' => 'newspack', 'newspack_type' => 'reader-activation-change-email', - 'default_shown' => false, + 'recommended' => false, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Email change confirmation', 'newspack-plugin' ), @@ -217,7 +217,7 @@ public static function get_email_registry(): array { 'non-reader-login-reminder' => [ 'source' => 'newspack', 'newspack_type' => 'reader-activation-non-reader-user', - 'default_shown' => false, + 'recommended' => false, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Non-reader login reminder', 'newspack-plugin' ), @@ -226,7 +226,7 @@ public static function get_email_registry(): array { 'group-subscription-invitation' => [ 'source' => 'newspack', 'newspack_type' => 'group-subscription-invite', - 'default_shown' => false, + 'recommended' => false, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Group subscription invitation', 'newspack-plugin' ), @@ -235,7 +235,7 @@ public static function get_email_registry(): array { 'woo-refund' => [ 'source' => 'woocommerce', 'woo_email_id' => 'customer_refunded_order', - 'default_shown' => false, + 'recommended' => false, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Order refund', 'newspack-plugin' ), @@ -245,7 +245,7 @@ public static function get_email_registry(): array { 'woo-processing-order' => [ 'source' => 'woocommerce', 'woo_email_id' => 'customer_processing_order', - 'default_shown' => false, + 'recommended' => false, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Order processing', 'newspack-plugin' ), @@ -255,7 +255,7 @@ public static function get_email_registry(): array { 'woo-completed-order' => [ 'source' => 'woocommerce', 'woo_email_id' => 'customer_completed_order', - 'default_shown' => false, + 'recommended' => false, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Order complete', 'newspack-plugin' ), @@ -265,7 +265,7 @@ public static function get_email_registry(): array { 'woo-on-hold-order' => [ 'source' => 'woocommerce', 'woo_email_id' => 'customer_on_hold_order', - 'default_shown' => false, + 'recommended' => false, 'plugin_dependency' => null, 'recipient' => 'reader', 'label' => __( 'Order on hold', 'newspack-plugin' ), @@ -274,7 +274,7 @@ public static function get_email_registry(): array { 'woo-new-order' => [ 'source' => 'woocommerce', 'woo_email_id' => 'new_order', - 'default_shown' => false, + 'recommended' => false, 'plugin_dependency' => null, 'recipient' => 'admin', 'label' => __( 'New order (admin)', 'newspack-plugin' ), @@ -315,12 +315,14 @@ public static function api_get_email_settings(): array { foreach ( $emails as $type => $email ) { if ( isset( $registry_lookup[ $type ] ) ) { $match = $registry_lookup[ $type ]; - $email['default_shown'] = $match['default_shown']; + $email['recommended'] = $match['recommended']; + $email['view_category'] = $match['recommended'] ? 'essentials' : 'all-enabled'; $email['trigger_description'] = $match['trigger_description']; $email['registry_slug'] = $match['registry_slug']; $email['recipient'] = $match['recipient']; } else { - $email['default_shown'] = false; + $email['recommended'] = false; + $email['view_category'] = 'available'; $email['trigger_description'] = ''; $email['registry_slug'] = ''; $email['recipient'] = 'reader'; @@ -328,6 +330,21 @@ public static function api_get_email_settings(): array { $newspack_emails[] = $email; } + // Sort: reader-revenue first, reader-activation second, woocommerce last. + // Within each group, preserve registry insertion order. + $category_order = [ + 'reader-revenue' => 0, + 'reader-activation' => 1, + ]; + usort( + $newspack_emails, + function ( $a, $b ) use ( $category_order ) { + $order_a = $category_order[ $a['category'] ?? '' ] ?? 2; + $order_b = $category_order[ $b['category'] ?? '' ] ?? 2; + return $order_a - $order_b; + } + ); + $settings['newspack_emails'] = $newspack_emails; $settings['post_type'] = Emails::POST_TYPE; diff --git a/src/wizards/newspack/views/settings/emails/emails.scss b/src/wizards/newspack/views/settings/emails/emails.scss new file mode 100644 index 0000000000..6505bffbef --- /dev/null +++ b/src/wizards/newspack/views/settings/emails/emails.scss @@ -0,0 +1,67 @@ +@use "~@wordpress/base-styles/colors" as wp-colors; + +// Full-width override for the emails tab. +// The parent `.newspack-wizard__sections` constrains width via max-width. +// Break out of that for the DataViews table. +.newspack-emails-tab.newspack-wizard__sections { + max-width: 100%; +} + +// DataViews search/filter/icon alignment. +.newspack-emails-tab { + .dataviews__view-actions { + display: flex; + align-items: center; + margin-bottom: 16px; + } + + .dataviews__search { + display: flex; + align-items: flex-start; + gap: 8px; + } +} + +// Preview placeholder (grid view media field). +.newspack-emails__preview-placeholder { + display: flex; + align-items: center; + justify-content: center; + aspect-ratio: 1; + width: 100%; + background: var(--wp-admin-theme-color-darker-10, #ddd); + border-radius: 4px; + color: var(--wp-admin-theme-color, #3858e9); + opacity: 0.4; +} + +// Trigger description sub-text under the title. +.newspack-emails__trigger-description { + color: wp-colors.$gray-900; + font-size: 12px; + margin-top: 4px; +} + +// Status indicator dot + text. +.newspack-emails__status { + display: inline-flex; + align-items: center; + gap: 6px; +} + +.newspack-emails__status-dot { + display: inline-block; + width: 8px; + height: 8px; + border-radius: 50%; + flex-shrink: 0; + + &--enabled { + background: #1a7e5a; + } + + &--disabled { + background: transparent; + border: 1.5px solid #949494; + } +} diff --git a/src/wizards/newspack/views/settings/emails/emails.test.js b/src/wizards/newspack/views/settings/emails/emails.test.js index 846e15698b..5e054bf18b 100644 --- a/src/wizards/newspack/views/settings/emails/emails.test.js +++ b/src/wizards/newspack/views/settings/emails/emails.test.js @@ -3,14 +3,23 @@ /** * External dependencies */ -import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; /** * WordPress dependencies */ import apiFetch from '@wordpress/api-fetch'; -jest.mock( '@wordpress/api-fetch', () => jest.fn() ); +jest.mock( './emails.scss', () => ( {} ) ); +jest.mock( '@wordpress/api-fetch', () => ( { + __esModule: true, + default: jest.fn(), +} ) ); + +jest.mock( '@wordpress/icons', () => ( { + Icon: ( { icon } ) => { icon }, + envelope: 'envelope', +} ) ); jest.mock( '@wordpress/dataviews', () => ( { filterSortAndPaginate: data => ( { @@ -20,7 +29,15 @@ jest.mock( '@wordpress/dataviews', () => ( { } ) ); jest.mock( '../../../../../../packages/components/src', () => { - const React = require( 'react' ); + function renderField( field, item ) { + if ( field.render ) { + return field.render( { item } ); + } + if ( field.getValue ) { + return field.getValue( { item } ); + } + return null; + } return { DataViews: ( { data, fields } ) => ( @@ -28,7 +45,7 @@ jest.mock( '../../../../../../packages/components/src', () => { { data.map( ( item, i ) => ( { fields.map( field => ( - + ) ) } ) ) } @@ -53,56 +70,90 @@ jest.mock( const mockEmails = [ { - label: 'Reader verification', - description: 'Verification email', + label: 'Payment receipt', + description: 'Receipt email', post_id: 1, edit_link: '/edit/1', - subject: 'Verify your email', + subject: 'Your receipt', from_name: 'Test', from_email: 'test@example.com', reply_to_email: 'test@example.com', status: 'publish', - type: 'reader-activation-verification', - category: 'reader-activation', - default_shown: true, - trigger_description: 'Sent when a reader needs to verify their email address.', - registry_slug: 'verification', + type: 'receipt', + category: 'reader-revenue', + recommended: true, + trigger_description: 'Sent after a successful payment.', + registry_slug: 'receipt', recipient: 'reader', }, { - label: 'Payment receipt', - description: 'Receipt email', + label: 'Cancellation confirmation', + description: 'Cancellation email', post_id: 2, edit_link: '/edit/2', - subject: 'Your receipt', + subject: 'Subscription cancelled', from_name: 'Test', from_email: 'test@example.com', reply_to_email: 'test@example.com', status: 'publish', - type: 'receipt', + type: 'cancellation', category: 'reader-revenue', - default_shown: true, - trigger_description: 'Sent after a successful payment.', - registry_slug: 'receipt', + recommended: true, + trigger_description: 'Sent when a reader cancels their subscription.', + registry_slug: 'cancellation', recipient: 'reader', }, { - label: 'Account deletion', - description: 'Delete account email', + label: 'Reader verification', + description: 'Verification email', post_id: 3, edit_link: '/edit/3', - subject: 'Account deleted', + subject: 'Verify your email', from_name: 'Test', from_email: 'test@example.com', reply_to_email: 'test@example.com', status: 'publish', + type: 'reader-activation-verification', + category: 'reader-activation', + recommended: true, + trigger_description: 'Sent when a reader needs to verify their email address.', + registry_slug: 'verification', + recipient: 'reader', + }, + { + label: 'Account deletion', + description: 'Delete account email', + post_id: 4, + edit_link: '/edit/4', + subject: 'Account deleted', + from_name: 'Test', + from_email: 'test@example.com', + reply_to_email: 'test@example.com', + status: 'draft', type: 'reader-activation-delete-account', category: 'reader-activation', - default_shown: false, + recommended: false, trigger_description: 'Sent when a reader requests to delete their account.', registry_slug: 'delete-account', recipient: 'reader', }, + { + label: 'New order (admin)', + description: 'New order admin notification', + post_id: 5, + edit_link: '/edit/5', + subject: 'New order', + from_name: 'Test', + from_email: 'test@example.com', + reply_to_email: 'test@example.com', + status: 'publish', + type: 'new_order', + category: 'woocommerce', + recommended: false, + trigger_description: 'Sent to the admin when a new order is placed.', + registry_slug: 'woo-new-order', + recipient: 'admin', + }, ]; describe( 'Emails', () => { @@ -128,19 +179,21 @@ describe( 'Emails', () => { } ); } ); - it( 'renders DataViews with email data', async () => { + it( 'renders all emails in a single view', async () => { const Emails = require( './emails' ).default; render( ); await waitFor( () => { - expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + // Emails from across the spectrum are all visible. + expect( screen.getByText( 'Payment receipt' ) ).toBeInTheDocument(); + expect( screen.getByText( 'Cancellation confirmation' ) ).toBeInTheDocument(); + expect( screen.getByText( 'Reader verification' ) ).toBeInTheDocument(); + expect( screen.getByText( 'Account deletion' ) ).toBeInTheDocument(); + expect( screen.getByText( 'New order (admin)' ) ).toBeInTheDocument(); } ); - - expect( screen.getByText( 'Reader verification' ) ).toBeInTheDocument(); - expect( screen.getByText( 'Payment receipt' ) ).toBeInTheDocument(); } ); - it( 'renders Recipient column with correct values', async () => { + it( 'does not render tabs, show-all toggle, or subtitle', async () => { const Emails = require( './emails' ).default; render( ); @@ -148,46 +201,33 @@ describe( 'Emails', () => { expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); } ); - // All mock emails have recipient: 'reader'. - const readerCells = screen.getAllByText( 'Reader' ); - expect( readerCells.length ).toBeGreaterThanOrEqual( 2 ); + expect( screen.queryByText( 'Essentials' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'All enabled' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Show all emails' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Manage the transactional emails your readers receive.' ) ).not.toBeInTheDocument(); } ); - it( 'renders status as Enabled / Disabled', async () => { + it( 'renders Recipient column with correct values', async () => { const Emails = require( './emails' ).default; render( ); await waitFor( () => { - expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + const readerCells = screen.getAllByText( 'Reader' ); + expect( readerCells.length ).toBeGreaterThanOrEqual( 4 ); + // Admin recipient. + expect( screen.getByText( 'Admin' ) ).toBeInTheDocument(); } ); - - // All mock emails have status: 'publish'. - const enabledCells = screen.getAllByText( 'Enabled' ); - expect( enabledCells.length ).toBeGreaterThanOrEqual( 2 ); } ); - it( '"Show all" toggle changes filter', async () => { + it( 'renders status as Enabled / Disabled', async () => { const Emails = require( './emails' ).default; render( ); await waitFor( () => { - expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + const enabledCells = screen.getAllByText( 'Enabled' ); + expect( enabledCells.length ).toBeGreaterThanOrEqual( 4 ); + // Account deletion is draft. + expect( screen.getByText( 'Disabled' ) ).toBeInTheDocument(); } ); - - // Default: only default_shown emails. - expect( screen.getByText( 'Reader verification' ) ).toBeInTheDocument(); - expect( screen.getByText( 'Payment receipt' ) ).toBeInTheDocument(); - expect( screen.queryByText( 'Account deletion' ) ).not.toBeInTheDocument(); - - // Click "Show all emails". - fireEvent.click( screen.getByText( 'Show all emails' ) ); - - // Now all emails should be visible. - expect( screen.getByText( 'Reader verification' ) ).toBeInTheDocument(); - expect( screen.getByText( 'Payment receipt' ) ).toBeInTheDocument(); - expect( screen.getByText( 'Account deletion' ) ).toBeInTheDocument(); - - // Toggle text should change. - expect( screen.getByText( 'Show default emails' ) ).toBeInTheDocument(); } ); } ); diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index 3cb1ca0a06..8922912c0f 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -9,14 +9,15 @@ import { __ } from '@wordpress/i18n'; import { useState, useEffect, useCallback, useMemo, Fragment } from '@wordpress/element'; import apiFetch from '@wordpress/api-fetch'; import { filterSortAndPaginate } from '@wordpress/dataviews'; -import { Button } from '@wordpress/components'; import type { Action, Field, View } from '@wordpress/dataviews'; +import { Icon, envelope } from '@wordpress/icons'; /** * Internal dependencies. */ -import { Card, DataViews, Notice, utils } from '../../../../../../packages/components/src'; +import { DataViews, Notice, utils } from '../../../../../../packages/components/src'; import WizardsPluginCard from '../../../../wizards-plugin-card'; +import './emails.scss'; interface EmailItem { label: string; @@ -30,7 +31,7 @@ interface EmailItem { status: string; type: string; category: string; - default_shown: boolean; + recommended: boolean; trigger_description: string; registry_slug: string; recipient: 'reader' | 'admin'; @@ -43,20 +44,17 @@ interface EmailSettings { enable_woocommerce_email_editor?: boolean; } -const CATEGORY_ORDER: Record< string, number > = { - 'reader-revenue': 0, - 'reader-activation': 1, -}; - const DEFAULT_VIEW: View = { - type: 'table', + type: 'grid', page: 1, - perPage: 25, + perPage: 50, search: '', fields: [ 'recipient', 'status' ], filters: [], layout: {}, titleField: 'name', + descriptionField: 'trigger_description', + mediaField: 'preview', }; const Emails = () => { @@ -66,7 +64,6 @@ const Emails = () => { const [ data, setData ] = useState< EmailItem[] >( [] ); const [ postType, setPostType ] = useState< string >( emailSections.emails.postType ); const [ isLoading, setIsLoading ] = useState( true ); - const [ showAll, setShowAll ] = useState( false ); const [ view, setView ] = useState< View >( DEFAULT_VIEW ); const [ error, setError ] = useState< string | null >( null ); @@ -77,14 +74,7 @@ const Emails = () => { path: '/newspack/v1/wizard/newspack-settings/emails', } ) .then( result => { - const emails = result.newspack_emails || []; - // Sort by category: reader-revenue first, reader-activation second, everything else last. - emails.sort( ( a, b ) => { - const orderA = CATEGORY_ORDER[ a.category ] ?? 2; - const orderB = CATEGORY_ORDER[ b.category ] ?? 2; - return orderA - orderB; - } ); - setData( emails ); + setData( result.newspack_emails || [] ); if ( result.post_type ) { setPostType( result.post_type ); } @@ -99,8 +89,6 @@ const Emails = () => { fetchData(); }, [ fetchData ] ); - const filteredData = useMemo( () => ( showAll ? data : data.filter( email => email.default_shown ) ), [ data, showAll ] ); - const updateStatus = useCallback( ( postId: number, status: string ) => { setError( null ); @@ -155,19 +143,35 @@ const Emails = () => { const fields: Field< EmailItem >[] = useMemo( () => [ + { + id: 'preview', + label: __( 'Preview', 'newspack-plugin' ), + type: 'media', + enableSorting: false, + enableHiding: true, + // TODO: Replace with component when built. + render: () => ( +
+ +
+ ), + }, { id: 'name', label: __( 'Email', 'newspack-plugin' ), enableGlobalSearch: true, getValue: ( { item }: { item: EmailItem } ) => item.label, + render: ( { item }: { item: EmailItem } ) => { item.label }, + }, + { + id: 'trigger_description', + label: __( 'Description', 'newspack-plugin' ), + getValue: ( { item }: { item: EmailItem } ) => item.trigger_description, render: ( { item }: { item: EmailItem } ) => ( -
- { item.label } - { item.trigger_description && ( -
{ item.trigger_description }
- ) } -
+ { item.trigger_description } ), + enableHiding: false, + enableSorting: false, }, { id: 'recipient', @@ -182,8 +186,20 @@ const Emails = () => { label: __( 'Status', 'newspack-plugin' ), getValue: ( { item }: { item: EmailItem } ) => item.status, render: ( { item }: { item: EmailItem } ) => ( - { item.status === 'publish' ? __( 'Enabled', 'newspack-plugin' ) : __( 'Disabled', 'newspack-plugin' ) } + + + { item.status === 'publish' ? __( 'Enabled', 'newspack-plugin' ) : __( 'Disabled', 'newspack-plugin' ) } + ), + elements: [ + { value: 'publish', label: __( 'Enabled', 'newspack-plugin' ) }, + { value: 'draft', label: __( 'Disabled', 'newspack-plugin' ) }, + ], + filterBy: { isPrimary: false, operators: [ 'is' ] }, }, ], [] @@ -229,10 +245,7 @@ const Emails = () => { [ resetEmail, updateStatus ] ); - const { data: processedData, paginationInfo } = useMemo( - () => filterSortAndPaginate( filteredData, view, fields ), - [ filteredData, view, fields ] - ); + const { data: processedData, paginationInfo } = useMemo( () => filterSortAndPaginate( data, view, fields ), [ data, view, fields ] ); if ( false === pluginsReady ) { return ( @@ -261,19 +274,6 @@ const Emails = () => { return ( - -
-

- { __( - "Manage the transactional emails your readers receive. Use 'Edit template' to customize the design that wraps every email.", - 'newspack-plugin' - ) } -

-
- -
{ error && } { onChangeView={ setView } actions={ actions } paginationInfo={ paginationInfo } - defaultLayouts={ { table: {} } } + defaultLayouts={ { table: {}, grid: {} } } isLoading={ isLoading } getItemId={ ( item: EmailItem ) => String( item.post_id ) } search /> -

- -

); }; diff --git a/src/wizards/newspack/views/settings/emails/index.tsx b/src/wizards/newspack/views/settings/emails/index.tsx index aea7f446a0..6f6d6cdaec 100644 --- a/src/wizards/newspack/views/settings/emails/index.tsx +++ b/src/wizards/newspack/views/settings/emails/index.tsx @@ -2,11 +2,6 @@ * Newspack > Settings > Emails */ -/** - * WordPress dependencies. - */ -import { __ } from '@wordpress/i18n'; - /** * Internal dependencies. */ @@ -19,7 +14,7 @@ const { emails } = window.newspackSettings; function Emails() { return ( - + From 1bae2ad7fe05c5c21a5f98cc8593bc58435ee8ca Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Thu, 14 May 2026 12:07:23 -0500 Subject: [PATCH 05/19] test(emails): update PHPUnit test to use renamed 'recommended' field (NPPD-945) --- tests/unit-tests/emails-section.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit-tests/emails-section.php b/tests/unit-tests/emails-section.php index c02d870de5..3f6ce35653 100644 --- a/tests/unit-tests/emails-section.php +++ b/tests/unit-tests/emails-section.php @@ -22,15 +22,15 @@ public function test_registry_has_23_entries() { /** * Test the registry has 13 default-shown entries. */ - public function test_registry_has_13_default_shown() { + public function test_registry_has_13_recommended() { $registry = Emails_Section::get_email_registry(); - $default_shown = array_filter( + $recommended = array_filter( $registry, function ( $entry ) { - return true === $entry['default_shown']; + return true === $entry['recommended']; } ); - $this->assertCount( 13, $default_shown ); + $this->assertCount( 13, $recommended ); } /** From e16b00a5b98953ed06ec1955f98ccec43ab2a6de Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Thu, 14 May 2026 13:58:37 -0500 Subject: [PATCH 06/19] fix(emails): stabilize sort with registry-index tiebreaker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Copilot review comment on PR #4727. The previous usort comparator returned 0 for items in the same category, but PHP's usort is not stable, so within-category ordering was undefined despite the comment claiming registry order was preserved. Add a slug-to-index map from the registry and use it as a secondary sort key. Unregistered emails sort to the end via PHP_INT_MAX fallback. No API schema change — sort is internal to the comparator. --- includes/wizards/newspack/class-emails-section.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/includes/wizards/newspack/class-emails-section.php b/includes/wizards/newspack/class-emails-section.php index a302b85005..21bf81f576 100644 --- a/includes/wizards/newspack/class-emails-section.php +++ b/includes/wizards/newspack/class-emails-section.php @@ -331,17 +331,23 @@ public static function api_get_email_settings(): array { } // Sort: reader-revenue first, reader-activation second, woocommerce last. - // Within each group, preserve registry insertion order. + // Within each group, preserve registry insertion order via a stable tiebreaker. $category_order = [ 'reader-revenue' => 0, 'reader-activation' => 1, ]; + $slug_order = array_flip( array_keys( $registry ) ); usort( $newspack_emails, - function ( $a, $b ) use ( $category_order ) { + function ( $a, $b ) use ( $category_order, $slug_order ) { $order_a = $category_order[ $a['category'] ?? '' ] ?? 2; $order_b = $category_order[ $b['category'] ?? '' ] ?? 2; - return $order_a - $order_b; + if ( $order_a !== $order_b ) { + return $order_a - $order_b; + } + $idx_a = $slug_order[ $a['registry_slug'] ?? '' ] ?? PHP_INT_MAX; + $idx_b = $slug_order[ $b['registry_slug'] ?? '' ] ?? PHP_INT_MAX; + return $idx_a - $idx_b; } ); From f8f0ee75ae9f0d6e511d0311d67d7f7c2b38b507 Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Thu, 14 May 2026 16:12:58 -0500 Subject: [PATCH 07/19] fix(emails): add visually-hidden h1 for a11y (NPPD-945) Address Copilot review comment on PR #4727. Dropping the WizardsTab title prop in slice 1 removed the page h1, which screen readers rely on as a landmark. Adds a visually-hidden h1 directly in the emails view using the WordPress core .screen-reader-text utility class. Visual layout unchanged. Co-Authored-By: Claude Opus 4.6 --- src/wizards/newspack/views/settings/emails/emails.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index 8922912c0f..90f993751a 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -274,6 +274,9 @@ const Emails = () => { return ( +

+ { __( 'Emails', 'newspack-plugin' ) } +

{ error && } Date: Thu, 14 May 2026 16:13:44 -0500 Subject: [PATCH 08/19] test(emails): cover api_get_email_settings response shape (NPPD-945) Address Copilot review comment on PR #4727. Existing tests cover the registry structure but not the api_get_email_settings() enrichment logic that the frontend depends on. Adds a test asserting the response includes the expected top-level keys and that each Newspack email is enriched with recommended, view_category, trigger_description, registry_slug, and recipient fields. Co-Authored-By: Claude Opus 4.6 --- tests/unit-tests/emails-section.php | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/unit-tests/emails-section.php b/tests/unit-tests/emails-section.php index 3f6ce35653..a47ad0f2da 100644 --- a/tests/unit-tests/emails-section.php +++ b/tests/unit-tests/emails-section.php @@ -67,4 +67,32 @@ public function test_registry_entries_have_labels_and_triggers() { $this->assertNotEmpty( $entry['trigger_description'], "Entry '$slug' is missing a trigger_description." ); } } + + /** + * Test api_get_email_settings returns the expected response shape. + */ + public function test_api_get_email_settings_response_shape() { + $result = Emails_Section::api_get_email_settings(); + + $this->assertIsArray( $result ); + $this->assertArrayHasKey( 'newspack_emails', $result ); + $this->assertArrayHasKey( 'post_type', $result ); + $this->assertIsArray( $result['newspack_emails'] ); + + if ( class_exists( 'WooCommerce' ) ) { + $this->assertArrayHasKey( 'admin_url', $result ); + $this->assertArrayHasKey( 'enable_woocommerce_email_editor', $result ); + } + + // Verify enriched fields on each Newspack email that has a registry_slug. + $enriched_keys = [ 'recommended', 'view_category', 'trigger_description', 'registry_slug', 'recipient' ]; + foreach ( $result['newspack_emails'] as $email ) { + if ( empty( $email['registry_slug'] ) ) { + continue; + } + foreach ( $enriched_keys as $key ) { + $this->assertArrayHasKey( $key, $email, "Email '{$email['label']}' is missing enriched field '$key'." ); + } + } + } } From 9693f85f253581dcb92b26990c724d2a26aca397 Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Thu, 14 May 2026 16:15:14 -0500 Subject: [PATCH 09/19] test(emails): cover DataViews action callbacks (NPPD-945) Address Copilot review comments on PR #4727. Existing tests render the DataViews grid but don't exercise the activate, deactivate, or reset action callbacks. Adds happy-path coverage that asserts apiFetch is called with the expected path and method for each action. Co-Authored-By: Claude Opus 4.6 --- .../views/settings/emails/emails.test.js | 95 ++++++++++++++++--- 1 file changed, 82 insertions(+), 13 deletions(-) diff --git a/src/wizards/newspack/views/settings/emails/emails.test.js b/src/wizards/newspack/views/settings/emails/emails.test.js index 5e054bf18b..a6c779b36b 100644 --- a/src/wizards/newspack/views/settings/emails/emails.test.js +++ b/src/wizards/newspack/views/settings/emails/emails.test.js @@ -28,6 +28,8 @@ jest.mock( '@wordpress/dataviews', () => ( { } ), } ) ); +let capturedActions = []; + jest.mock( '../../../../../../packages/components/src', () => { function renderField( field, item ) { if ( field.render ) { @@ -39,19 +41,22 @@ jest.mock( '../../../../../../packages/components/src', () => { return null; } return { - DataViews: ( { data, fields } ) => ( -
{ field.render( { item } ) }{ renderField( field, item ) }
- - { data.map( ( item, i ) => ( - - { fields.map( field => ( - - ) ) } - - ) ) } - -
{ renderField( field, item ) }
- ), + DataViews: ( { data, fields, actions } ) => { + capturedActions = actions || []; + return ( + + + { data.map( ( item, i ) => ( + + { fields.map( field => ( + + ) ) } + + ) ) } + +
{ renderField( field, item ) }
+ ); + }, Card: ( { children } ) =>
{ children }
, Notice: ( { noticeText } ) =>
{ noticeText }
, utils: { @@ -230,4 +235,68 @@ describe( 'Emails', () => { expect( screen.getByText( 'Disabled' ) ).toBeInTheDocument(); } ); } ); + + it( 'deactivate action calls apiFetch with draft status', async () => { + const Emails = require( './emails' ).default; + render( ); + + await waitFor( () => { + expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + } ); + + const deactivate = capturedActions.find( a => a.id === 'deactivate' ); + deactivate.callback( [ mockEmails[ 0 ] ] ); + + await waitFor( () => { + expect( apiFetch ).toHaveBeenCalledWith( { + path: '/wp/v2/newspack_rr_email/1', + method: 'POST', + data: { status: 'draft' }, + } ); + } ); + } ); + + it( 'activate action calls apiFetch with publish status', async () => { + const Emails = require( './emails' ).default; + render( ); + + await waitFor( () => { + expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + } ); + + const activate = capturedActions.find( a => a.id === 'activate' ); + // mockEmails[3] (Account deletion) is draft, so eligible for activate. + activate.callback( [ mockEmails[ 3 ] ] ); + + await waitFor( () => { + expect( apiFetch ).toHaveBeenCalledWith( { + path: '/wp/v2/newspack_rr_email/4', + method: 'POST', + data: { status: 'publish' }, + } ); + } ); + } ); + + it( 'reset action calls apiFetch with DELETE after confirmation', async () => { + const { utils } = require( '../../../../../../packages/components/src' ); + const Emails = require( './emails' ).default; + render( ); + + await waitFor( () => { + expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + } ); + + const reset = capturedActions.find( a => a.id === 'reset' ); + // mockEmails[0] (Payment receipt) has type 'receipt', so eligible for reset. + reset.callback( [ mockEmails[ 0 ] ] ); + + expect( utils.confirmAction ).toHaveBeenCalled(); + + await waitFor( () => { + expect( apiFetch ).toHaveBeenCalledWith( { + path: '/newspack/v1/wizard/newspack-audience-donations/emails/1', + method: 'DELETE', + } ); + } ); + } ); } ); From ff6ae9e4b8539da34a2129bdcadb5e57062e8148 Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Thu, 14 May 2026 16:24:35 -0500 Subject: [PATCH 10/19] lint(emails): collapse visually-hidden h1 to single line (NPPD-945) Fix Prettier formatting error from the previous a11y h1 commit. Short JSX content should be inline, not on its own line. Co-Authored-By: Claude Opus 4.6 --- src/wizards/newspack/views/settings/emails/emails.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index 90f993751a..f41dabbfb3 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -274,9 +274,7 @@ const Emails = () => { return ( -

- { __( 'Emails', 'newspack-plugin' ) } -

+

{ __( 'Emails', 'newspack-plugin' ) }

{ error && } Date: Fri, 15 May 2026 09:23:01 -0500 Subject: [PATCH 11/19] fix(emails): address PR review feedback (NPPD-945) - Replace custom status dots with component (thomasguillot) - Add visually-hidden h1 to pluginsReady early return (Copilot) - Apply registry label in enrichment loop so curated names display (Copilot) - Fix activate test to use eligible non-reader-activation email (Copilot) - Rename capturedActions to mockCapturedActions for Jest hoisting (Copilot) - Assert at least one enriched email exists in PHPUnit test (Copilot) - Remove unused status dot SCSS rules Co-Authored-By: Claude Opus 4.6 --- .../wizards/newspack/class-emails-section.php | 1 + .../views/settings/emails/emails.scss | 23 ------------------- .../views/settings/emails/emails.test.js | 22 +++++++++++------- .../newspack/views/settings/emails/emails.tsx | 20 ++++++++-------- tests/unit-tests/emails-section.php | 6 ++++- 5 files changed, 30 insertions(+), 42 deletions(-) diff --git a/includes/wizards/newspack/class-emails-section.php b/includes/wizards/newspack/class-emails-section.php index 21bf81f576..795c4d96df 100644 --- a/includes/wizards/newspack/class-emails-section.php +++ b/includes/wizards/newspack/class-emails-section.php @@ -315,6 +315,7 @@ public static function api_get_email_settings(): array { foreach ( $emails as $type => $email ) { if ( isset( $registry_lookup[ $type ] ) ) { $match = $registry_lookup[ $type ]; + $email['label'] = $match['label']; $email['recommended'] = $match['recommended']; $email['view_category'] = $match['recommended'] ? 'essentials' : 'all-enabled'; $email['trigger_description'] = $match['trigger_description']; diff --git a/src/wizards/newspack/views/settings/emails/emails.scss b/src/wizards/newspack/views/settings/emails/emails.scss index 6505bffbef..4fa8943fc1 100644 --- a/src/wizards/newspack/views/settings/emails/emails.scss +++ b/src/wizards/newspack/views/settings/emails/emails.scss @@ -42,26 +42,3 @@ margin-top: 4px; } -// Status indicator dot + text. -.newspack-emails__status { - display: inline-flex; - align-items: center; - gap: 6px; -} - -.newspack-emails__status-dot { - display: inline-block; - width: 8px; - height: 8px; - border-radius: 50%; - flex-shrink: 0; - - &--enabled { - background: #1a7e5a; - } - - &--disabled { - background: transparent; - border: 1.5px solid #949494; - } -} diff --git a/src/wizards/newspack/views/settings/emails/emails.test.js b/src/wizards/newspack/views/settings/emails/emails.test.js index a6c779b36b..3b7f508498 100644 --- a/src/wizards/newspack/views/settings/emails/emails.test.js +++ b/src/wizards/newspack/views/settings/emails/emails.test.js @@ -28,7 +28,8 @@ jest.mock( '@wordpress/dataviews', () => ( { } ), } ) ); -let capturedActions = []; +// Use mock-prefixed name so Jest's hoisted jest.mock can close over it. +let mockCapturedActions = []; jest.mock( '../../../../../../packages/components/src', () => { function renderField( field, item ) { @@ -41,8 +42,9 @@ jest.mock( '../../../../../../packages/components/src', () => { return null; } return { + Badge: ( { text } ) => { text }, DataViews: ( { data, fields, actions } ) => { - capturedActions = actions || []; + mockCapturedActions = actions || []; return ( @@ -244,7 +246,7 @@ describe( 'Emails', () => { expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); } ); - const deactivate = capturedActions.find( a => a.id === 'deactivate' ); + const deactivate = mockCapturedActions.find( a => a.id === 'deactivate' ); deactivate.callback( [ mockEmails[ 0 ] ] ); await waitFor( () => { @@ -264,13 +266,17 @@ describe( 'Emails', () => { expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); } ); - const activate = capturedActions.find( a => a.id === 'activate' ); - // mockEmails[3] (Account deletion) is draft, so eligible for activate. - activate.callback( [ mockEmails[ 3 ] ] ); + const activate = mockCapturedActions.find( a => a.id === 'activate' ); + // mockEmails[4] (New order) is woocommerce + publish, so use a draft + // woocommerce email to test activate eligibility correctly. + // Account deletion (mockEmails[3]) is reader-activation and excluded by isEligible. + // Instead, create an inline eligible item: woocommerce + draft. + const eligibleItem = { ...mockEmails[ 4 ], status: 'draft' }; + activate.callback( [ eligibleItem ] ); await waitFor( () => { expect( apiFetch ).toHaveBeenCalledWith( { - path: '/wp/v2/newspack_rr_email/4', + path: '/wp/v2/newspack_rr_email/5', method: 'POST', data: { status: 'publish' }, } ); @@ -286,7 +292,7 @@ describe( 'Emails', () => { expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); } ); - const reset = capturedActions.find( a => a.id === 'reset' ); + const reset = mockCapturedActions.find( a => a.id === 'reset' ); // mockEmails[0] (Payment receipt) has type 'receipt', so eligible for reset. reset.callback( [ mockEmails[ 0 ] ] ); diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index f41dabbfb3..a63eb56eb3 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -15,7 +15,7 @@ import { Icon, envelope } from '@wordpress/icons'; /** * Internal dependencies. */ -import { DataViews, Notice, utils } from '../../../../../../packages/components/src'; +import { Badge, DataViews, Notice, utils } from '../../../../../../packages/components/src'; import WizardsPluginCard from '../../../../wizards-plugin-card'; import './emails.scss'; @@ -185,16 +185,15 @@ const Emails = () => { id: 'status', label: __( 'Status', 'newspack-plugin' ), getValue: ( { item }: { item: EmailItem } ) => item.status, - render: ( { item }: { item: EmailItem } ) => ( - - { + const isEnabled = item.status === 'publish'; + return ( + - { item.status === 'publish' ? __( 'Enabled', 'newspack-plugin' ) : __( 'Disabled', 'newspack-plugin' ) } - - ), + ); + }, elements: [ { value: 'publish', label: __( 'Enabled', 'newspack-plugin' ) }, { value: 'draft', label: __( 'Disabled', 'newspack-plugin' ) }, @@ -250,6 +249,7 @@ const Emails = () => { if ( false === pluginsReady ) { return ( +

{ __( 'Emails', 'newspack-plugin' ) }

{ __( 'Newspack uses Newspack Newsletters to handle editing email-type content. Please activate this plugin to proceed.', diff --git a/tests/unit-tests/emails-section.php b/tests/unit-tests/emails-section.php index a47ad0f2da..0b46e785ca 100644 --- a/tests/unit-tests/emails-section.php +++ b/tests/unit-tests/emails-section.php @@ -85,14 +85,18 @@ public function test_api_get_email_settings_response_shape() { } // Verify enriched fields on each Newspack email that has a registry_slug. - $enriched_keys = [ 'recommended', 'view_category', 'trigger_description', 'registry_slug', 'recipient' ]; + // Guard: at least one enriched email must exist, otherwise the loop is vacuous. + $enriched_keys = [ 'label', 'recommended', 'view_category', 'trigger_description', 'registry_slug', 'recipient' ]; + $enriched_count = 0; foreach ( $result['newspack_emails'] as $email ) { if ( empty( $email['registry_slug'] ) ) { continue; } + ++$enriched_count; foreach ( $enriched_keys as $key ) { $this->assertArrayHasKey( $key, $email, "Email '{$email['label']}' is missing enriched field '$key'." ); } } + $this->assertGreaterThan( 0, $enriched_count, 'Expected at least one enriched email in the response, but found none.' ); } } From 90b09bfefbee2b08bbcc1fe710b67f06c751e88a Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Fri, 15 May 2026 09:52:31 -0500 Subject: [PATCH 12/19] Restore Reset action on all Newspack-managed emails - NPPD-1528 Co-Authored-By: Claude Opus 4.6 --- src/wizards/newspack/views/settings/emails/emails.test.js | 2 +- src/wizards/newspack/views/settings/emails/emails.tsx | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/wizards/newspack/views/settings/emails/emails.test.js b/src/wizards/newspack/views/settings/emails/emails.test.js index 3b7f508498..59d9520810 100644 --- a/src/wizards/newspack/views/settings/emails/emails.test.js +++ b/src/wizards/newspack/views/settings/emails/emails.test.js @@ -293,7 +293,7 @@ describe( 'Emails', () => { } ); const reset = mockCapturedActions.find( a => a.id === 'reset' ); - // mockEmails[0] (Payment receipt) has type 'receipt', so eligible for reset. + // Reset is available on all emails (no isEligible filter). reset.callback( [ mockEmails[ 0 ] ] ); expect( utils.confirmAction ).toHaveBeenCalled(); diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index a63eb56eb3..6000501c96 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -233,7 +233,6 @@ const Emails = () => { id: 'reset', label: __( 'Reset', 'newspack-plugin' ), isDestructive: true, - isEligible: ( item: EmailItem ) => item.type === 'receipt' || item.type === 'welcome', callback: ( items: EmailItem[] ) => { if ( utils.confirmAction( __( 'Are you sure you want to reset the contents of this email?', 'newspack-plugin' ) ) ) { resetEmail( items[ 0 ].post_id ); From ebb58fbed82bb13b1ddd5ff5ccd1041c3e5aa7ed Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Fri, 15 May 2026 10:04:37 -0500 Subject: [PATCH 13/19] fix(emails): use inline-block for Badge so it shrink-wraps in grid view Co-Authored-By: Claude Opus 4.6 --- packages/components/src/badge/style.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/badge/style.scss b/packages/components/src/badge/style.scss index 72615a38c3..241d4faad1 100644 --- a/packages/components/src/badge/style.scss +++ b/packages/components/src/badge/style.scss @@ -16,7 +16,7 @@ $badge-colors: ( background-color: color-mix(in srgb, wp-colors.$white 90%, var(--base-color)); border-radius: 2px; color: color-mix(in srgb, wp-colors.$black 50%, var(--base-color)); - display: block; + display: inline-block; flex: 0 0 auto; font-size: 12px; line-height: 1.5; From db4113dcb44155a46751617582b15fb1002c3bf7 Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Fri, 15 May 2026 10:20:33 -0500 Subject: [PATCH 14/19] fix(emails): address thomasguillot review findings on slice 1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add isEligible guard on Reset: excluded for woocommerce-source emails and emails without a registry_slug (finding #1) - Capture previous status before optimistic update so rollback doesn't assume binary publish/draft (finding #2) - Drop view_category from enrichment response — dead data not consumed by the frontend (finding #3) - Add @todo NPPD-1532 comment on reset endpoint coupling (finding #4) - Add NPPD-1531 comment on optimistic update / store consolidation (#5) - Replace brittle PHPUnit count assertions with structural invariants: required keys, valid source/recipient, exclusive type keys, no duplicates, sort order (findings #6, #7, #8) - Add optimistic-rollback failure test and reset eligibility test (#9, #11) - Add 6th mockEmail fixture (WC draft) for cleaner activate test (#10) - Coerce pluginsReady with Boolean() to handle undefined (#12) - Use noticeText prop consistently on Notice (#13) - Fix preview placeholder to use neutral gray tokens (#14) - Trim EmailItem interface to consumed fields, add source (#15) - Add source-of-truth comment on category_order strings (#16) - Consolidate three duplicate TODO comments into one (#17) - Add NPPD-1525 ticket reference on EmailPreview TODO (#18) - Extract duplicated screen-reader h1 into PageHeading component (#19) - Add text-overflow ellipsis on trigger description column (#20) - Return localized string from recipient getValue for search (#21) Co-Authored-By: Claude Opus 4.6 --- .../wizards/newspack/class-emails-section.php | 12 +- .../views/settings/emails/emails.scss | 11 +- .../views/settings/emails/emails.test.js | 105 ++++++++------ .../newspack/views/settings/emails/emails.tsx | 54 ++++---- tests/unit-tests/emails-section.php | 131 ++++++++++++++---- 5 files changed, 203 insertions(+), 110 deletions(-) diff --git a/includes/wizards/newspack/class-emails-section.php b/includes/wizards/newspack/class-emails-section.php index 795c4d96df..65057b4145 100644 --- a/includes/wizards/newspack/class-emails-section.php +++ b/includes/wizards/newspack/class-emails-section.php @@ -241,7 +241,9 @@ public static function get_email_registry(): array { 'label' => __( 'Order refund', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when an order is refunded.', 'newspack-plugin' ), ], - // TODO: Customer-facing email. PRD rationale should be "lower customization priority for subscription publishers" instead of "admin-facing". + // The following three WooCommerce emails are customer-facing but marked + // recommended=false because they are lower customization priority for + // subscription-focused publishers. 'woo-processing-order' => [ 'source' => 'woocommerce', 'woo_email_id' => 'customer_processing_order', @@ -251,7 +253,6 @@ public static function get_email_registry(): array { 'label' => __( 'Order processing', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when an order payment is received and the order begins processing.', 'newspack-plugin' ), ], - // TODO: Customer-facing email. PRD rationale should be "lower customization priority for subscription publishers" instead of "admin-facing". 'woo-completed-order' => [ 'source' => 'woocommerce', 'woo_email_id' => 'customer_completed_order', @@ -261,7 +262,6 @@ public static function get_email_registry(): array { 'label' => __( 'Order complete', 'newspack-plugin' ), 'trigger_description' => __( 'Sent when an order is marked as complete.', 'newspack-plugin' ), ], - // TODO: Customer-facing email. PRD rationale should be "lower customization priority for subscription publishers" instead of "admin-facing". 'woo-on-hold-order' => [ 'source' => 'woocommerce', 'woo_email_id' => 'customer_on_hold_order', @@ -317,22 +317,24 @@ public static function api_get_email_settings(): array { $match = $registry_lookup[ $type ]; $email['label'] = $match['label']; $email['recommended'] = $match['recommended']; - $email['view_category'] = $match['recommended'] ? 'essentials' : 'all-enabled'; $email['trigger_description'] = $match['trigger_description']; $email['registry_slug'] = $match['registry_slug']; $email['recipient'] = $match['recipient']; + $email['source'] = $match['source']; } else { $email['recommended'] = false; - $email['view_category'] = 'available'; $email['trigger_description'] = ''; $email['registry_slug'] = ''; $email['recipient'] = 'reader'; + $email['source'] = 'newspack'; } $newspack_emails[] = $email; } // Sort: reader-revenue first, reader-activation second, woocommerce last. // Within each group, preserve registry insertion order via a stable tiebreaker. + // Category strings originate from Reader_Revenue_Emails::add_email_configs(), + // Reader_Activation_Emails::add_email_configs(), and WooCommerce_Emails. $category_order = [ 'reader-revenue' => 0, 'reader-activation' => 1, diff --git a/src/wizards/newspack/views/settings/emails/emails.scss b/src/wizards/newspack/views/settings/emails/emails.scss index 4fa8943fc1..11ded35644 100644 --- a/src/wizards/newspack/views/settings/emails/emails.scss +++ b/src/wizards/newspack/views/settings/emails/emails.scss @@ -29,10 +29,9 @@ justify-content: center; aspect-ratio: 1; width: 100%; - background: var(--wp-admin-theme-color-darker-10, #ddd); + background: wp-colors.$gray-100; border-radius: 4px; - color: var(--wp-admin-theme-color, #3858e9); - opacity: 0.4; + color: wp-colors.$gray-700; } // Trigger description sub-text under the title. @@ -40,5 +39,9 @@ color: wp-colors.$gray-900; font-size: 12px; margin-top: 4px; + display: block; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + max-width: 360px; } - diff --git a/src/wizards/newspack/views/settings/emails/emails.test.js b/src/wizards/newspack/views/settings/emails/emails.test.js index 59d9520810..f63b3617d2 100644 --- a/src/wizards/newspack/views/settings/emails/emails.test.js +++ b/src/wizards/newspack/views/settings/emails/emails.test.js @@ -78,88 +78,75 @@ jest.mock( const mockEmails = [ { label: 'Payment receipt', - description: 'Receipt email', post_id: 1, edit_link: '/edit/1', - subject: 'Your receipt', - from_name: 'Test', - from_email: 'test@example.com', - reply_to_email: 'test@example.com', status: 'publish', type: 'receipt', category: 'reader-revenue', - recommended: true, trigger_description: 'Sent after a successful payment.', registry_slug: 'receipt', recipient: 'reader', + source: 'newspack', }, { label: 'Cancellation confirmation', - description: 'Cancellation email', post_id: 2, edit_link: '/edit/2', - subject: 'Subscription cancelled', - from_name: 'Test', - from_email: 'test@example.com', - reply_to_email: 'test@example.com', status: 'publish', type: 'cancellation', category: 'reader-revenue', - recommended: true, trigger_description: 'Sent when a reader cancels their subscription.', registry_slug: 'cancellation', recipient: 'reader', + source: 'newspack', }, { label: 'Reader verification', - description: 'Verification email', post_id: 3, edit_link: '/edit/3', - subject: 'Verify your email', - from_name: 'Test', - from_email: 'test@example.com', - reply_to_email: 'test@example.com', status: 'publish', type: 'reader-activation-verification', category: 'reader-activation', - recommended: true, trigger_description: 'Sent when a reader needs to verify their email address.', registry_slug: 'verification', recipient: 'reader', + source: 'newspack', }, { label: 'Account deletion', - description: 'Delete account email', post_id: 4, edit_link: '/edit/4', - subject: 'Account deleted', - from_name: 'Test', - from_email: 'test@example.com', - reply_to_email: 'test@example.com', status: 'draft', type: 'reader-activation-delete-account', category: 'reader-activation', - recommended: false, trigger_description: 'Sent when a reader requests to delete their account.', registry_slug: 'delete-account', recipient: 'reader', + source: 'newspack', }, { label: 'New order (admin)', - description: 'New order admin notification', post_id: 5, edit_link: '/edit/5', - subject: 'New order', - from_name: 'Test', - from_email: 'test@example.com', - reply_to_email: 'test@example.com', status: 'publish', type: 'new_order', category: 'woocommerce', - recommended: false, trigger_description: 'Sent to the admin when a new order is placed.', registry_slug: 'woo-new-order', recipient: 'admin', + source: 'woocommerce', + }, + { + label: 'Order on hold', + post_id: 6, + edit_link: '/edit/6', + status: 'draft', + type: 'customer_on_hold_order', + category: 'woocommerce', + trigger_description: 'Sent when an order is placed on hold.', + registry_slug: 'woo-on-hold-order', + recipient: 'reader', + source: 'woocommerce', }, ]; @@ -191,12 +178,12 @@ describe( 'Emails', () => { render( ); await waitFor( () => { - // Emails from across the spectrum are all visible. expect( screen.getByText( 'Payment receipt' ) ).toBeInTheDocument(); expect( screen.getByText( 'Cancellation confirmation' ) ).toBeInTheDocument(); expect( screen.getByText( 'Reader verification' ) ).toBeInTheDocument(); expect( screen.getByText( 'Account deletion' ) ).toBeInTheDocument(); expect( screen.getByText( 'New order (admin)' ) ).toBeInTheDocument(); + expect( screen.getByText( 'Order on hold' ) ).toBeInTheDocument(); } ); } ); @@ -221,7 +208,6 @@ describe( 'Emails', () => { await waitFor( () => { const readerCells = screen.getAllByText( 'Reader' ); expect( readerCells.length ).toBeGreaterThanOrEqual( 4 ); - // Admin recipient. expect( screen.getByText( 'Admin' ) ).toBeInTheDocument(); } ); } ); @@ -232,9 +218,9 @@ describe( 'Emails', () => { await waitFor( () => { const enabledCells = screen.getAllByText( 'Enabled' ); - expect( enabledCells.length ).toBeGreaterThanOrEqual( 4 ); - // Account deletion is draft. - expect( screen.getByText( 'Disabled' ) ).toBeInTheDocument(); + expect( enabledCells.length ).toBeGreaterThanOrEqual( 3 ); + const disabledCells = screen.getAllByText( 'Disabled' ); + expect( disabledCells.length ).toBeGreaterThanOrEqual( 2 ); } ); } ); @@ -258,6 +244,27 @@ describe( 'Emails', () => { } ); } ); + it( 'deactivate optimistically updates status and reverts on failure', async () => { + apiFetch + .mockResolvedValueOnce( { newspack_emails: mockEmails, post_type: 'newspack_rr_email' } ) + .mockRejectedValueOnce( new Error( 'fail' ) ); + + const Emails = require( './emails' ).default; + render( ); + + await waitFor( () => { + expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + } ); + + const deactivate = mockCapturedActions.find( a => a.id === 'deactivate' ); + deactivate.callback( [ mockEmails[ 0 ] ] ); + + // After rejection, error notice should appear. + await waitFor( () => { + expect( screen.getByTestId( 'notice' ) ).toBeInTheDocument(); + } ); + } ); + it( 'activate action calls apiFetch with publish status', async () => { const Emails = require( './emails' ).default; render( ); @@ -267,16 +274,12 @@ describe( 'Emails', () => { } ); const activate = mockCapturedActions.find( a => a.id === 'activate' ); - // mockEmails[4] (New order) is woocommerce + publish, so use a draft - // woocommerce email to test activate eligibility correctly. - // Account deletion (mockEmails[3]) is reader-activation and excluded by isEligible. - // Instead, create an inline eligible item: woocommerce + draft. - const eligibleItem = { ...mockEmails[ 4 ], status: 'draft' }; - activate.callback( [ eligibleItem ] ); + // mockEmails[5] (Order on hold) is woocommerce + draft — eligible for activate. + activate.callback( [ mockEmails[ 5 ] ] ); await waitFor( () => { expect( apiFetch ).toHaveBeenCalledWith( { - path: '/wp/v2/newspack_rr_email/5', + path: '/wp/v2/newspack_rr_email/6', method: 'POST', data: { status: 'publish' }, } ); @@ -293,7 +296,6 @@ describe( 'Emails', () => { } ); const reset = mockCapturedActions.find( a => a.id === 'reset' ); - // Reset is available on all emails (no isEligible filter). reset.callback( [ mockEmails[ 0 ] ] ); expect( utils.confirmAction ).toHaveBeenCalled(); @@ -305,4 +307,21 @@ describe( 'Emails', () => { } ); } ); } ); + + it( 'reset is eligible for newspack-source emails with a registry_slug', async () => { + const Emails = require( './emails' ).default; + render( ); + + await waitFor( () => { + expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + } ); + + const reset = mockCapturedActions.find( a => a.id === 'reset' ); + // Newspack email with registry_slug — eligible. + expect( reset.isEligible( mockEmails[ 0 ] ) ).toBe( true ); + // WooCommerce email — not eligible. + expect( reset.isEligible( mockEmails[ 4 ] ) ).toBe( false ); + // Email without registry_slug — not eligible. + expect( reset.isEligible( { ...mockEmails[ 0 ], registry_slug: '' } ) ).toBe( false ); + } ); } ); diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index 6000501c96..a0d47fc722 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -21,20 +21,15 @@ import './emails.scss'; interface EmailItem { label: string; - description: string; post_id: number; edit_link: string; - subject: string; - from_name: string; - from_email: string; - reply_to_email: string; status: string; type: string; category: string; - recommended: boolean; trigger_description: string; registry_slug: string; recipient: 'reader' | 'admin'; + source: 'newspack' | 'woocommerce'; } interface EmailSettings { @@ -57,9 +52,11 @@ const DEFAULT_VIEW: View = { mediaField: 'preview', }; +const PageHeading = () =>

{ __( 'Emails', 'newspack-plugin' ) }

; + const Emails = () => { const emailSections = window.newspackSettings.emails.sections; - const [ pluginsReady, setPluginsReady ] = useState( emailSections.emails.dependencies.newspackNewsletters ); + const [ pluginsReady, setPluginsReady ] = useState( Boolean( emailSections.emails.dependencies.newspackNewsletters ) ); const [ data, setData ] = useState< EmailItem[] >( [] ); const [ postType, setPostType ] = useState< string >( emailSections.emails.postType ); @@ -90,13 +87,15 @@ const Emails = () => { }, [ fetchData ] ); const updateStatus = useCallback( - ( postId: number, status: string ) => { + ( postId: number, nextStatus: string ) => { setError( null ); - // Optimistic update. + let previousStatus: string | undefined; + // Optimistic update — see NPPD-1531 for consolidating with the wizard data store. setData( prev => prev.map( email => { if ( email.post_id === postId ) { - return { ...email, status }; + previousStatus = email.status; + return { ...email, status: nextStatus }; } return email; } ) @@ -104,16 +103,13 @@ const Emails = () => { apiFetch( { path: `/wp/v2/${ postType }/${ postId }`, method: 'POST', - data: { status }, + data: { status: nextStatus }, } ).catch( () => { // Revert on failure. setData( prev => prev.map( email => { - if ( email.post_id === postId ) { - return { - ...email, - status: status === 'publish' ? 'draft' : 'publish', - }; + if ( email.post_id === postId && previousStatus !== undefined ) { + return { ...email, status: previousStatus }; } return email; } ) @@ -127,6 +123,9 @@ const Emails = () => { const resetEmail = useCallback( ( postId: number ) => { setError( null ); + // @todo NPPD-1532 Move reset handler to class-emails-section.php so it + // lives under wizard/newspack-settings/emails/{id} instead of reaching + // into the donations wizard namespace. apiFetch( { path: `/newspack/v1/wizard/newspack-audience-donations/emails/${ postId }`, method: 'DELETE', @@ -149,7 +148,7 @@ const Emails = () => { type: 'media', enableSorting: false, enableHiding: true, - // TODO: Replace with component when built. + // @todo NPPD-1525 Replace with component. render: () => (
@@ -176,7 +175,8 @@ const Emails = () => { { id: 'recipient', label: __( 'Recipient', 'newspack-plugin' ), - getValue: ( { item }: { item: EmailItem } ) => item.recipient, + getValue: ( { item }: { item: EmailItem } ) => + item.recipient === 'admin' ? __( 'Admin', 'newspack-plugin' ) : __( 'Reader', 'newspack-plugin' ), render: ( { item }: { item: EmailItem } ) => ( { item.recipient === 'admin' ? __( 'Admin', 'newspack-plugin' ) : __( 'Reader', 'newspack-plugin' ) } ), @@ -233,6 +233,7 @@ const Emails = () => { id: 'reset', label: __( 'Reset', 'newspack-plugin' ), isDestructive: true, + isEligible: ( item: EmailItem ) => item.source !== 'woocommerce' && Boolean( item.registry_slug ), callback: ( items: EmailItem[] ) => { if ( utils.confirmAction( __( 'Are you sure you want to reset the contents of this email?', 'newspack-plugin' ) ) ) { resetEmail( items[ 0 ].post_id ); @@ -248,15 +249,12 @@ const Emails = () => { if ( false === pluginsReady ) { return ( -

{ __( 'Emails', 'newspack-plugin' ) }

- - { __( - 'Newspack uses Newspack Newsletters to handle editing email-type content. Please activate this plugin to proceed.', - 'newspack-plugin' - ) } -
- { __( 'Until this feature is configured, default receipts will be used.', 'newspack-plugin' ) } -
+ + { return ( -

{ __( 'Emails', 'newspack-plugin' ) }

+ { error && } assertCount( 23, $registry ); - } + public function test_registry_entries_have_required_keys() { + $registry = Emails_Section::get_email_registry(); + $required_keys = [ 'source', 'recommended', 'plugin_dependency', 'recipient', 'label', 'trigger_description' ]; - /** - * Test the registry has 13 default-shown entries. - */ - public function test_registry_has_13_recommended() { - $registry = Emails_Section::get_email_registry(); - $recommended = array_filter( - $registry, - function ( $entry ) { - return true === $entry['recommended']; + foreach ( $registry as $slug => $entry ) { + foreach ( $required_keys as $key ) { + $this->assertArrayHasKey( $key, $entry, "Entry '$slug' is missing required key '$key'." ); } - ); - $this->assertCount( 13, $recommended ); + } } /** - * Test the registry has 4 entries with woocommerce-subscriptions dependency. + * Test all registry entries have a valid source value. */ - public function test_registry_has_4_subscriptions_dependency() { - $registry = Emails_Section::get_email_registry(); - $with_woo_sub = array_filter( - $registry, - function ( $entry ) { - return 'woocommerce-subscriptions' === $entry['plugin_dependency']; - } - ); - $this->assertCount( 4, $with_woo_sub ); + public function test_registry_entries_have_valid_source() { + $registry = Emails_Section::get_email_registry(); + foreach ( $registry as $slug => $entry ) { + $this->assertContains( $entry['source'], [ 'newspack', 'woocommerce' ], "Entry '$slug' has an invalid source value." ); + } } /** * Test all registry entries have a valid recipient value. */ - public function test_registry_entries_have_recipient() { + public function test_registry_entries_have_valid_recipient() { $registry = Emails_Section::get_email_registry(); foreach ( $registry as $slug => $entry ) { $this->assertContains( $entry['recipient'], [ 'reader', 'admin' ], "Entry '$slug' has an invalid recipient value." ); @@ -68,6 +56,66 @@ public function test_registry_entries_have_labels_and_triggers() { } } + /** + * Test recommended is always a boolean. + */ + public function test_registry_recommended_is_boolean() { + $registry = Emails_Section::get_email_registry(); + foreach ( $registry as $slug => $entry ) { + $this->assertIsBool( $entry['recommended'], "Entry '$slug' has a non-boolean recommended value." ); + } + } + + /** + * Test each entry has either newspack_type or woo_email_id, never both. + */ + public function test_registry_entries_have_exclusive_type_keys() { + $registry = Emails_Section::get_email_registry(); + foreach ( $registry as $slug => $entry ) { + $has_newspack = isset( $entry['newspack_type'] ); + $has_woo = isset( $entry['woo_email_id'] ); + $this->assertTrue( $has_newspack || $has_woo, "Entry '$slug' has neither newspack_type nor woo_email_id." ); + $this->assertFalse( $has_newspack && $has_woo, "Entry '$slug' has both newspack_type and woo_email_id." ); + } + } + + /** + * Test newspack-source entries have newspack_type and woocommerce-source entries have woo_email_id. + */ + public function test_registry_source_matches_type_key() { + $registry = Emails_Section::get_email_registry(); + foreach ( $registry as $slug => $entry ) { + if ( 'newspack' === $entry['source'] ) { + $this->assertArrayHasKey( 'newspack_type', $entry, "Newspack-source entry '$slug' is missing newspack_type." ); + $this->assertNotEmpty( $entry['newspack_type'], "Newspack-source entry '$slug' has empty newspack_type." ); + } + if ( 'woocommerce' === $entry['source'] ) { + $this->assertArrayHasKey( 'woo_email_id', $entry, "WooCommerce-source entry '$slug' is missing woo_email_id." ); + $this->assertNotEmpty( $entry['woo_email_id'], "WooCommerce-source entry '$slug' has empty woo_email_id." ); + } + } + } + + /** + * Test no duplicate newspack_type or woo_email_id values across entries. + */ + public function test_registry_no_duplicate_type_values() { + $registry = Emails_Section::get_email_registry(); + $newspack_types = []; + $woo_ids = []; + + foreach ( $registry as $slug => $entry ) { + if ( isset( $entry['newspack_type'] ) ) { + $this->assertNotContains( $entry['newspack_type'], $newspack_types, "Duplicate newspack_type '{$entry['newspack_type']}' in entry '$slug'." ); + $newspack_types[] = $entry['newspack_type']; + } + if ( isset( $entry['woo_email_id'] ) ) { + $this->assertNotContains( $entry['woo_email_id'], $woo_ids, "Duplicate woo_email_id '{$entry['woo_email_id']}' in entry '$slug'." ); + $woo_ids[] = $entry['woo_email_id']; + } + } + } + /** * Test api_get_email_settings returns the expected response shape. */ @@ -85,11 +133,14 @@ public function test_api_get_email_settings_response_shape() { } // Verify enriched fields on each Newspack email that has a registry_slug. - // Guard: at least one enriched email must exist, otherwise the loop is vacuous. - $enriched_keys = [ 'label', 'recommended', 'view_category', 'trigger_description', 'registry_slug', 'recipient' ]; - $enriched_count = 0; + $enriched_keys = [ 'label', 'recommended', 'trigger_description', 'registry_slug', 'recipient', 'source' ]; + $enriched_count = 0; foreach ( $result['newspack_emails'] as $email ) { if ( empty( $email['registry_slug'] ) ) { + // Fallback branch: verify defaults are set. + $this->assertArrayHasKey( 'recommended', $email, 'Fallback email is missing recommended.' ); + $this->assertFalse( $email['recommended'], 'Fallback email should have recommended=false.' ); + $this->assertArrayHasKey( 'source', $email, 'Fallback email is missing source.' ); continue; } ++$enriched_count; @@ -99,4 +150,24 @@ public function test_api_get_email_settings_response_shape() { } $this->assertGreaterThan( 0, $enriched_count, 'Expected at least one enriched email in the response, but found none.' ); } + + /** + * Test sort order: reader-revenue first, reader-activation second, other categories last. + */ + public function test_api_get_email_settings_sort_order() { + $result = Emails_Section::api_get_email_settings(); + $categories = array_column( $result['newspack_emails'], 'category' ); + + // Build the expected group order: reader-revenue → reader-activation → everything else. + $last_group = -1; + $group_map = [ + 'reader-revenue' => 0, + 'reader-activation' => 1, + ]; + foreach ( $categories as $i => $cat ) { + $group = $group_map[ $cat ] ?? 2; + $this->assertGreaterThanOrEqual( $last_group, $group, "Email at index $i (category '$cat') is out of sort order." ); + $last_group = $group; + } + } } From d66fa5fe0f35ae5691f511db5e5e25a2a9b6e26e Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Fri, 15 May 2026 10:32:40 -0500 Subject: [PATCH 15/19] fix(emails): prettier formatting on Notice component Co-Authored-By: Claude Opus 4.6 --- .../newspack/views/settings/emails/emails.tsx | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index a0d47fc722..973d144773 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -250,11 +250,17 @@ const Emails = () => { return ( - + Date: Fri, 15 May 2026 11:43:32 -0500 Subject: [PATCH 16/19] fix(emails): address round-2 review feedback (NPPD-945) - Tighten activate/deactivate isEligible to exclude WooCommerce and reader-activation emails from status toggling. - Add enableGlobalSearch to trigger_description field so descriptions are searchable. - Switch trigger-description CSS from single-line ellipsis to 2-line clamp for better grid-view legibility. - Strengthen rollback test to assert status reverts after failure. - Add eligibility test for activate/deactivate/woocommerce gating. - Fix activate test to use a Newspack email (not WooCommerce). - Add PHPUnit tests for admin-recipient classification and within-group registry insertion order. - Add explanatory comment on fallback `source = 'newspack'` default. Co-Authored-By: Claude Opus 4.6 --- .../wizards/newspack/class-emails-section.php | 2 +- .../views/settings/emails/emails.scss | 8 +-- .../views/settings/emails/emails.test.js | 35 ++++++++++-- .../newspack/views/settings/emails/emails.tsx | 7 ++- tests/unit-tests/emails-section.php | 53 +++++++++++++++++++ 5 files changed, 94 insertions(+), 11 deletions(-) diff --git a/includes/wizards/newspack/class-emails-section.php b/includes/wizards/newspack/class-emails-section.php index 65057b4145..0b297efd9e 100644 --- a/includes/wizards/newspack/class-emails-section.php +++ b/includes/wizards/newspack/class-emails-section.php @@ -326,7 +326,7 @@ public static function api_get_email_settings(): array { $email['trigger_description'] = ''; $email['registry_slug'] = ''; $email['recipient'] = 'reader'; - $email['source'] = 'newspack'; + $email['source'] = 'newspack'; // Default; WooCommerce emails always match above. } $newspack_emails[] = $email; } diff --git a/src/wizards/newspack/views/settings/emails/emails.scss b/src/wizards/newspack/views/settings/emails/emails.scss index 11ded35644..36d7448bfb 100644 --- a/src/wizards/newspack/views/settings/emails/emails.scss +++ b/src/wizards/newspack/views/settings/emails/emails.scss @@ -35,13 +35,13 @@ } // Trigger description sub-text under the title. +// Two-line clamp keeps grid cards uniform while still showing enough context. .newspack-emails__trigger-description { color: wp-colors.$gray-900; font-size: 12px; margin-top: 4px; - display: block; + display: -webkit-box; + -webkit-line-clamp: 2; + -webkit-box-orient: vertical; overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - max-width: 360px; } diff --git a/src/wizards/newspack/views/settings/emails/emails.test.js b/src/wizards/newspack/views/settings/emails/emails.test.js index f63b3617d2..d1cfb740e1 100644 --- a/src/wizards/newspack/views/settings/emails/emails.test.js +++ b/src/wizards/newspack/views/settings/emails/emails.test.js @@ -256,12 +256,16 @@ describe( 'Emails', () => { expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); } ); + // Before deactivation, count Enabled badges. + const enabledBefore = screen.getAllByText( 'Enabled' ).length; + const deactivate = mockCapturedActions.find( a => a.id === 'deactivate' ); deactivate.callback( [ mockEmails[ 0 ] ] ); - // After rejection, error notice should appear. + // After rejection, error notice should appear and status should revert. await waitFor( () => { expect( screen.getByTestId( 'notice' ) ).toBeInTheDocument(); + expect( screen.getAllByText( 'Enabled' ).length ).toBe( enabledBefore ); } ); } ); @@ -274,18 +278,41 @@ describe( 'Emails', () => { } ); const activate = mockCapturedActions.find( a => a.id === 'activate' ); - // mockEmails[5] (Order on hold) is woocommerce + draft — eligible for activate. - activate.callback( [ mockEmails[ 5 ] ] ); + // mockEmails[3] (Account deletion) is newspack + draft — eligible for activate. + activate.callback( [ mockEmails[ 3 ] ] ); await waitFor( () => { expect( apiFetch ).toHaveBeenCalledWith( { - path: '/wp/v2/newspack_rr_email/6', + path: '/wp/v2/newspack_rr_email/4', method: 'POST', data: { status: 'publish' }, } ); } ); } ); + it( 'deactivate/activate are not eligible for reader-activation or woocommerce emails', async () => { + const Emails = require( './emails' ).default; + render( ); + + await waitFor( () => { + expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); + } ); + + const deactivate = mockCapturedActions.find( a => a.id === 'deactivate' ); + const activate = mockCapturedActions.find( a => a.id === 'activate' ); + + // Reader-activation emails cannot be toggled. + expect( deactivate.isEligible( mockEmails[ 2 ] ) ).toBe( false ); + // WooCommerce emails cannot be toggled. + expect( deactivate.isEligible( mockEmails[ 4 ] ) ).toBe( false ); + // Newspack reader-revenue email can be deactivated. + expect( deactivate.isEligible( mockEmails[ 0 ] ) ).toBe( true ); + // Draft reader-activation email cannot be activated. + expect( activate.isEligible( mockEmails[ 3 ] ) ).toBe( false ); + // Draft WooCommerce email cannot be activated. + expect( activate.isEligible( mockEmails[ 5 ] ) ).toBe( false ); + } ); + it( 'reset action calls apiFetch with DELETE after confirmation', async () => { const { utils } = require( '../../../../../../packages/components/src' ); const Emails = require( './emails' ).default; diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index 973d144773..3692ea7cef 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -165,6 +165,7 @@ const Emails = () => { { id: 'trigger_description', label: __( 'Description', 'newspack-plugin' ), + enableGlobalSearch: true, getValue: ( { item }: { item: EmailItem } ) => item.trigger_description, render: ( { item }: { item: EmailItem } ) => ( { item.trigger_description } @@ -216,7 +217,8 @@ const Emails = () => { { id: 'deactivate', label: __( 'Deactivate', 'newspack-plugin' ), - isEligible: ( item: EmailItem ) => item.category !== 'reader-activation' && item.status === 'publish', + isEligible: ( item: EmailItem ) => + item.source !== 'woocommerce' && item.category !== 'reader-activation' && item.status === 'publish', callback: ( items: EmailItem[] ) => { updateStatus( items[ 0 ].post_id, 'draft' ); }, @@ -224,7 +226,8 @@ const Emails = () => { { id: 'activate', label: __( 'Activate', 'newspack-plugin' ), - isEligible: ( item: EmailItem ) => item.category !== 'reader-activation' && item.status !== 'publish', + isEligible: ( item: EmailItem ) => + item.source !== 'woocommerce' && item.category !== 'reader-activation' && item.status !== 'publish', callback: ( items: EmailItem[] ) => { updateStatus( items[ 0 ].post_id, 'publish' ); }, diff --git a/tests/unit-tests/emails-section.php b/tests/unit-tests/emails-section.php index e9873e164d..6aee3387e0 100644 --- a/tests/unit-tests/emails-section.php +++ b/tests/unit-tests/emails-section.php @@ -170,4 +170,57 @@ public function test_api_get_email_settings_sort_order() { $last_group = $group; } } + + /** + * Test admin-recipient emails are correctly classified. + */ + public function test_admin_recipient_emails() { + $registry = Emails_Section::get_email_registry(); + $admin_slugs = array_keys( + array_filter( + $registry, + function ( $entry ) { + return 'admin' === $entry['recipient']; + } + ) + ); + $this->assertContains( 'woo-new-order', $admin_slugs, 'new_order is an admin email.' ); + // woo-subscription-cancelled is reader-facing (Newspack notifies the reader); + // the separate WC admin notification is handled by WooCommerce core. + $this->assertNotContains( 'woo-subscription-cancelled', $admin_slugs ); + } + + /** + * Test registry insertion order within source groups. + * + * The UI relies on registry order to determine display order within + * each category group (reader-revenue, reader-activation, woocommerce). + */ + public function test_registry_order_within_groups() { + $slugs = array_keys( Emails_Section::get_email_registry() ); + + // Reader-revenue group: receipt → welcome → cancellation. + $this->assertLessThan( + array_search( 'welcome', $slugs, true ), + array_search( 'receipt', $slugs, true ), + 'receipt should appear before welcome.' + ); + $this->assertLessThan( + array_search( 'cancellation', $slugs, true ), + array_search( 'welcome', $slugs, true ), + 'welcome should appear before cancellation.' + ); + + // Reader-activation group: verification → login-link → set-new-password. + $this->assertLessThan( + array_search( 'login-link', $slugs, true ), + array_search( 'verification', $slugs, true ), + 'verification should appear before login-link.' + ); + $this->assertLessThan( + array_search( 'set-new-password', $slugs, true ), + array_search( 'login-link', $slugs, true ), + 'login-link should appear before set-new-password.' + ); + } } From c755a1dd674e46d41bc198e80b805470e9074d79 Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Fri, 15 May 2026 11:47:49 -0500 Subject: [PATCH 17/19] Make get_email_registry() filterable for external email integrations - NPPD-1533 Co-Authored-By: Claude Opus 4.6 --- .../wizards/newspack/class-emails-section.php | 12 ++++++++- tests/unit-tests/emails-section.php | 27 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/includes/wizards/newspack/class-emails-section.php b/includes/wizards/newspack/class-emails-section.php index 0b297efd9e..fd5bd302c0 100644 --- a/includes/wizards/newspack/class-emails-section.php +++ b/includes/wizards/newspack/class-emails-section.php @@ -69,7 +69,7 @@ public function register_rest_routes() { * @return array Registry entries keyed by slug. */ public static function get_email_registry(): array { - return [ + $registry = [ 'verification' => [ 'source' => 'newspack', 'newspack_type' => 'reader-activation-verification', @@ -281,6 +281,16 @@ public static function get_email_registry(): array { 'trigger_description' => __( 'Sent to the admin when a new order is placed.', 'newspack-plugin' ), ], ]; + + /** + * Filters the unified email registry. + * + * Allows external integration plugins to register additional email + * entries that appear in the Settings > Emails UI. + * + * @param array $registry Registry entries keyed by slug. + */ + return apply_filters( 'newspack_emails_registry', $registry ); } /** diff --git a/tests/unit-tests/emails-section.php b/tests/unit-tests/emails-section.php index 6aee3387e0..256219a44a 100644 --- a/tests/unit-tests/emails-section.php +++ b/tests/unit-tests/emails-section.php @@ -190,6 +190,33 @@ function ( $entry ) { $this->assertNotContains( 'woo-subscription-cancelled', $admin_slugs ); } + /** + * Test that the newspack_emails_registry filter can add entries. + */ + public function test_emails_registry_filter() { + $fake_entry = [ + 'source' => 'newspack', + 'newspack_type' => 'test-filter-email', + 'recommended' => false, + 'plugin_dependency' => null, + 'recipient' => 'reader', + 'label' => 'Test filter email', + 'trigger_description' => 'Added via filter.', + ]; + + $callback = function ( $registry ) use ( $fake_entry ) { + $registry['test-filter-email'] = $fake_entry; + return $registry; + }; + + add_filter( 'newspack_emails_registry', $callback ); + $registry = Emails_Section::get_email_registry(); + remove_filter( 'newspack_emails_registry', $callback ); + + $this->assertArrayHasKey( 'test-filter-email', $registry, 'Filter-added entry should be present in the registry.' ); + $this->assertSame( $fake_entry, $registry['test-filter-email'] ); + } + /** * Test registry insertion order within source groups. * From 31bd102c8e3902734ae07dc1fe67424a466a135f Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Tue, 26 May 2026 12:30:55 -0500 Subject: [PATCH 18/19] fix(emails): address review feedback (NPPD-1527) - Remove dead test asserting absence of never-present strings - Drop optimistic status update in favor of server refetch - Make preview thumbnail and email name clickable edit links - Remove redundant `all` key from localized settings data - Add enableGlobalSearch to recipient field for search filtering Co-Authored-By: Claude Opus 4.6 --- .../newspack/class-newspack-settings.php | 1 - .../views/settings/emails/emails.scss | 11 +++++ .../views/settings/emails/emails.test.js | 22 +--------- .../newspack/views/settings/emails/emails.tsx | 43 +++++++------------ 4 files changed, 27 insertions(+), 50 deletions(-) diff --git a/includes/wizards/newspack/class-newspack-settings.php b/includes/wizards/newspack/class-newspack-settings.php index b8886a3d24..4e7b6e1180 100644 --- a/includes/wizards/newspack/class-newspack-settings.php +++ b/includes/wizards/newspack/class-newspack-settings.php @@ -87,7 +87,6 @@ public function get_local_data() { 'dependencies' => [ 'newspackNewsletters' => is_plugin_active( 'newspack-newsletters/newspack-newsletters.php' ), ], - 'all' => Emails::get_emails( Reader_Activation::is_enabled() ? [] : array_values( Reader_Revenue_Emails::EMAIL_TYPES ), false ), 'postType' => Emails::POST_TYPE, 'isEmailEnhancementsActive' => WooCommerce_Emails::is_active(), ], diff --git a/src/wizards/newspack/views/settings/emails/emails.scss b/src/wizards/newspack/views/settings/emails/emails.scss index 36d7448bfb..6719a3d3e7 100644 --- a/src/wizards/newspack/views/settings/emails/emails.scss +++ b/src/wizards/newspack/views/settings/emails/emails.scss @@ -34,6 +34,17 @@ color: wp-colors.$gray-700; } +// Clickable name link in grid/table view. +.newspack-emails__name-link { + color: inherit; + text-decoration: none; + + &:hover, + &:focus { + text-decoration: underline; + } +} + // Trigger description sub-text under the title. // Two-line clamp keeps grid cards uniform while still showing enough context. .newspack-emails__trigger-description { diff --git a/src/wizards/newspack/views/settings/emails/emails.test.js b/src/wizards/newspack/views/settings/emails/emails.test.js index d1cfb740e1..974f416c2d 100644 --- a/src/wizards/newspack/views/settings/emails/emails.test.js +++ b/src/wizards/newspack/views/settings/emails/emails.test.js @@ -161,7 +161,6 @@ describe( 'Emails', () => { newspackNewsletters: true, }, postType: 'newspack_rr_email', - all: {}, isEmailEnhancementsActive: false, }, }, @@ -187,20 +186,6 @@ describe( 'Emails', () => { } ); } ); - it( 'does not render tabs, show-all toggle, or subtitle', async () => { - const Emails = require( './emails' ).default; - render( ); - - await waitFor( () => { - expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); - } ); - - expect( screen.queryByText( 'Essentials' ) ).not.toBeInTheDocument(); - expect( screen.queryByText( 'All enabled' ) ).not.toBeInTheDocument(); - expect( screen.queryByText( 'Show all emails' ) ).not.toBeInTheDocument(); - expect( screen.queryByText( 'Manage the transactional emails your readers receive.' ) ).not.toBeInTheDocument(); - } ); - it( 'renders Recipient column with correct values', async () => { const Emails = require( './emails' ).default; render( ); @@ -244,7 +229,7 @@ describe( 'Emails', () => { } ); } ); - it( 'deactivate optimistically updates status and reverts on failure', async () => { + it( 'deactivate shows error notice on failure', async () => { apiFetch .mockResolvedValueOnce( { newspack_emails: mockEmails, post_type: 'newspack_rr_email' } ) .mockRejectedValueOnce( new Error( 'fail' ) ); @@ -256,16 +241,11 @@ describe( 'Emails', () => { expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); } ); - // Before deactivation, count Enabled badges. - const enabledBefore = screen.getAllByText( 'Enabled' ).length; - const deactivate = mockCapturedActions.find( a => a.id === 'deactivate' ); deactivate.callback( [ mockEmails[ 0 ] ] ); - // After rejection, error notice should appear and status should revert. await waitFor( () => { expect( screen.getByTestId( 'notice' ) ).toBeInTheDocument(); - expect( screen.getAllByText( 'Enabled' ).length ).toBe( enabledBefore ); } ); } ); diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index 3692ea7cef..9843b84493 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -89,35 +89,17 @@ const Emails = () => { const updateStatus = useCallback( ( postId: number, nextStatus: string ) => { setError( null ); - let previousStatus: string | undefined; - // Optimistic update — see NPPD-1531 for consolidating with the wizard data store. - setData( prev => - prev.map( email => { - if ( email.post_id === postId ) { - previousStatus = email.status; - return { ...email, status: nextStatus }; - } - return email; - } ) - ); apiFetch( { path: `/wp/v2/${ postType }/${ postId }`, method: 'POST', data: { status: nextStatus }, - } ).catch( () => { - // Revert on failure. - setData( prev => - prev.map( email => { - if ( email.post_id === postId && previousStatus !== undefined ) { - return { ...email, status: previousStatus }; - } - return email; - } ) - ); - setError( __( 'Failed to update email status.', 'newspack-plugin' ) ); - } ); + } ) + .then( () => fetchData() ) + .catch( () => { + setError( __( 'Failed to update email status.', 'newspack-plugin' ) ); + } ); }, - [ postType ] + [ postType, fetchData ] ); const resetEmail = useCallback( @@ -149,10 +131,10 @@ const Emails = () => { enableSorting: false, enableHiding: true, // @todo NPPD-1525 Replace with component. - render: () => ( -
+ render: ( { item }: { item: EmailItem } ) => ( + -
+ ), }, { @@ -160,7 +142,11 @@ const Emails = () => { label: __( 'Email', 'newspack-plugin' ), enableGlobalSearch: true, getValue: ( { item }: { item: EmailItem } ) => item.label, - render: ( { item }: { item: EmailItem } ) => { item.label }, + render: ( { item }: { item: EmailItem } ) => ( + + { item.label } + + ), }, { id: 'trigger_description', @@ -176,6 +162,7 @@ const Emails = () => { { id: 'recipient', label: __( 'Recipient', 'newspack-plugin' ), + enableGlobalSearch: true, getValue: ( { item }: { item: EmailItem } ) => item.recipient === 'admin' ? __( 'Admin', 'newspack-plugin' ) : __( 'Reader', 'newspack-plugin' ), render: ( { item }: { item: EmailItem } ) => ( From 434a617acd32cbf2e107ef5e07168feb87b1ab68 Mon Sep 17 00:00:00 2001 From: Katie Wilkerson Rethman Date: Tue, 26 May 2026 21:05:39 -0500 Subject: [PATCH 19/19] refactor(emails): use useWizardApiFetch for loading states Replace direct apiFetch calls with useWizardApiFetch hook so that toggle actions (activate/deactivate) and the initial data fetch surface a unified loading state through the hook's isFetching flag. Co-Authored-By: Claude Opus 4.6 --- .../views/settings/emails/emails.test.js | 86 +++++---- .../newspack/views/settings/emails/emails.tsx | 171 ++++++++---------- 2 files changed, 128 insertions(+), 129 deletions(-) diff --git a/src/wizards/newspack/views/settings/emails/emails.test.js b/src/wizards/newspack/views/settings/emails/emails.test.js index 974f416c2d..d9cf87108b 100644 --- a/src/wizards/newspack/views/settings/emails/emails.test.js +++ b/src/wizards/newspack/views/settings/emails/emails.test.js @@ -5,15 +5,20 @@ */ import { render, screen, waitFor } from '@testing-library/react'; -/** - * WordPress dependencies - */ -import apiFetch from '@wordpress/api-fetch'; - jest.mock( './emails.scss', () => ( {} ) ); -jest.mock( '@wordpress/api-fetch', () => ( { - __esModule: true, - default: jest.fn(), + +// Use mock-prefixed names so Jest's hoisted jest.mock can close over them. +const mockWizardApiFetch = jest.fn(); +const mockResetError = jest.fn(); +let mockErrorMessage = null; + +jest.mock( '../../../../hooks/use-wizard-api-fetch', () => ( { + useWizardApiFetch: () => ( { + wizardApiFetch: ( ...args ) => mockWizardApiFetch( ...args ), + isFetching: false, + errorMessage: mockErrorMessage, + resetError: ( ...args ) => mockResetError( ...args ), + } ), } ) ); jest.mock( '@wordpress/icons', () => ( { @@ -153,6 +158,7 @@ const mockEmails = [ describe( 'Emails', () => { beforeEach( () => { jest.clearAllMocks(); + mockErrorMessage = null; window.newspackSettings = { emails: { sections: { @@ -166,9 +172,14 @@ describe( 'Emails', () => { }, }, }; - apiFetch.mockResolvedValue( { - newspack_emails: mockEmails, - post_type: 'newspack_rr_email', + mockWizardApiFetch.mockImplementation( ( opts, callbacks ) => { + if ( opts.path === '/newspack/v1/wizard/newspack-settings/emails' ) { + callbacks?.onSuccess?.( { + newspack_emails: mockEmails, + post_type: 'newspack_rr_email', + } ); + } + return Promise.resolve(); } ); } ); @@ -209,7 +220,7 @@ describe( 'Emails', () => { } ); } ); - it( 'deactivate action calls apiFetch with draft status', async () => { + it( 'deactivate action calls wizardApiFetch with draft status', async () => { const Emails = require( './emails' ).default; render( ); @@ -220,36 +231,31 @@ describe( 'Emails', () => { const deactivate = mockCapturedActions.find( a => a.id === 'deactivate' ); deactivate.callback( [ mockEmails[ 0 ] ] ); - await waitFor( () => { - expect( apiFetch ).toHaveBeenCalledWith( { + expect( mockWizardApiFetch ).toHaveBeenCalledWith( + expect.objectContaining( { path: '/wp/v2/newspack_rr_email/1', method: 'POST', data: { status: 'draft' }, - } ); - } ); + } ), + expect.objectContaining( { + onSuccess: expect.any( Function ), + } ) + ); } ); - it( 'deactivate shows error notice on failure', async () => { - apiFetch - .mockResolvedValueOnce( { newspack_emails: mockEmails, post_type: 'newspack_rr_email' } ) - .mockRejectedValueOnce( new Error( 'fail' ) ); + it( 'displays error notice when hook reports an error', async () => { + mockErrorMessage = 'Something went wrong'; const Emails = require( './emails' ).default; render( ); - await waitFor( () => { - expect( screen.getByTestId( 'dataviews' ) ).toBeInTheDocument(); - } ); - - const deactivate = mockCapturedActions.find( a => a.id === 'deactivate' ); - deactivate.callback( [ mockEmails[ 0 ] ] ); - await waitFor( () => { expect( screen.getByTestId( 'notice' ) ).toBeInTheDocument(); + expect( screen.getByTestId( 'notice' ) ).toHaveTextContent( 'Something went wrong' ); } ); } ); - it( 'activate action calls apiFetch with publish status', async () => { + it( 'activate action calls wizardApiFetch with publish status', async () => { const Emails = require( './emails' ).default; render( ); @@ -261,13 +267,16 @@ describe( 'Emails', () => { // mockEmails[3] (Account deletion) is newspack + draft — eligible for activate. activate.callback( [ mockEmails[ 3 ] ] ); - await waitFor( () => { - expect( apiFetch ).toHaveBeenCalledWith( { + expect( mockWizardApiFetch ).toHaveBeenCalledWith( + expect.objectContaining( { path: '/wp/v2/newspack_rr_email/4', method: 'POST', data: { status: 'publish' }, - } ); - } ); + } ), + expect.objectContaining( { + onSuccess: expect.any( Function ), + } ) + ); } ); it( 'deactivate/activate are not eligible for reader-activation or woocommerce emails', async () => { @@ -293,7 +302,7 @@ describe( 'Emails', () => { expect( activate.isEligible( mockEmails[ 5 ] ) ).toBe( false ); } ); - it( 'reset action calls apiFetch with DELETE after confirmation', async () => { + it( 'reset action calls wizardApiFetch with DELETE after confirmation', async () => { const { utils } = require( '../../../../../../packages/components/src' ); const Emails = require( './emails' ).default; render( ); @@ -307,12 +316,15 @@ describe( 'Emails', () => { expect( utils.confirmAction ).toHaveBeenCalled(); - await waitFor( () => { - expect( apiFetch ).toHaveBeenCalledWith( { + expect( mockWizardApiFetch ).toHaveBeenCalledWith( + expect.objectContaining( { path: '/newspack/v1/wizard/newspack-audience-donations/emails/1', method: 'DELETE', - } ); - } ); + } ), + expect.objectContaining( { + onSuccess: expect.any( Function ), + } ) + ); } ); it( 'reset is eligible for newspack-source emails with a registry_slug', async () => { diff --git a/src/wizards/newspack/views/settings/emails/emails.tsx b/src/wizards/newspack/views/settings/emails/emails.tsx index 9843b84493..4ec8aaaded 100644 --- a/src/wizards/newspack/views/settings/emails/emails.tsx +++ b/src/wizards/newspack/views/settings/emails/emails.tsx @@ -6,8 +6,7 @@ * WordPress dependencies. */ import { __ } from '@wordpress/i18n'; -import { useState, useEffect, useCallback, useMemo, Fragment } from '@wordpress/element'; -import apiFetch from '@wordpress/api-fetch'; +import { useState, useEffect, useMemo, Fragment } from '@wordpress/element'; import { filterSortAndPaginate } from '@wordpress/dataviews'; import type { Action, Field, View } from '@wordpress/dataviews'; import { Icon, envelope } from '@wordpress/icons'; @@ -17,6 +16,7 @@ import { Icon, envelope } from '@wordpress/icons'; */ import { Badge, DataViews, Notice, utils } from '../../../../../../packages/components/src'; import WizardsPluginCard from '../../../../wizards-plugin-card'; +import { useWizardApiFetch } from '../../../../hooks/use-wizard-api-fetch'; import './emails.scss'; interface EmailItem { @@ -60,67 +60,59 @@ const Emails = () => { const [ data, setData ] = useState< EmailItem[] >( [] ); const [ postType, setPostType ] = useState< string >( emailSections.emails.postType ); - const [ isLoading, setIsLoading ] = useState( true ); const [ view, setView ] = useState< View >( DEFAULT_VIEW ); - const [ error, setError ] = useState< string | null >( null ); - const fetchData = useCallback( () => { - setIsLoading( true ); - setError( null ); - apiFetch< EmailSettings >( { - path: '/newspack/v1/wizard/newspack-settings/emails', - } ) - .then( result => { - setData( result.newspack_emails || [] ); - if ( result.post_type ) { - setPostType( result.post_type ); - } - } ) - .catch( () => { - setError( __( 'Failed to load emails. Please refresh the page.', 'newspack-plugin' ) ); - } ) - .finally( () => setIsLoading( false ) ); - }, [] ); + const { wizardApiFetch, isFetching, errorMessage, resetError } = useWizardApiFetch( 'newspack-settings/emails' ); + + const fetchData = () => { + resetError(); + wizardApiFetch< EmailSettings >( + { + path: '/newspack/v1/wizard/newspack-settings/emails', + isCached: false, + }, + { + onSuccess( result: EmailSettings ) { + setData( result.newspack_emails || [] ); + if ( result.post_type ) { + setPostType( result.post_type ); + } + }, + } + ); + }; - useEffect( () => { - fetchData(); - }, [ fetchData ] ); + useEffect( fetchData, [] ); - const updateStatus = useCallback( - ( postId: number, nextStatus: string ) => { - setError( null ); - apiFetch( { + const updateStatus = ( postId: number, nextStatus: string ) => { + resetError(); + wizardApiFetch( + { path: `/wp/v2/${ postType }/${ postId }`, method: 'POST', data: { status: nextStatus }, - } ) - .then( () => fetchData() ) - .catch( () => { - setError( __( 'Failed to update email status.', 'newspack-plugin' ) ); - } ); - }, - [ postType, fetchData ] - ); + }, + { + onSuccess: () => fetchData(), + } + ); + }; - const resetEmail = useCallback( - ( postId: number ) => { - setError( null ); - // @todo NPPD-1532 Move reset handler to class-emails-section.php so it - // lives under wizard/newspack-settings/emails/{id} instead of reaching - // into the donations wizard namespace. - apiFetch( { + const resetEmail = ( postId: number ) => { + resetError(); + // @todo NPPD-1532 Move reset handler to class-emails-section.php so it + // lives under wizard/newspack-settings/emails/{id} instead of reaching + // into the donations wizard namespace. + wizardApiFetch( + { path: `/newspack/v1/wizard/newspack-audience-donations/emails/${ postId }`, method: 'DELETE', - } ) - .then( () => { - fetchData(); - } ) - .catch( () => { - setError( __( 'Failed to reset email. Please try again.', 'newspack-plugin' ) ); - } ); - }, - [ fetchData ] - ); + }, + { + onSuccess: () => fetchData(), + } + ); + }; const fields: Field< EmailItem >[] = useMemo( () => [ @@ -192,47 +184,42 @@ const Emails = () => { [] ); - const actions: Action< EmailItem >[] = useMemo( - () => [ - { - id: 'edit', - label: __( 'Edit', 'newspack-plugin' ), - callback: ( items: EmailItem[] ) => { - window.location.href = items[ 0 ].edit_link; - }, + const actions: Action< EmailItem >[] = [ + { + id: 'edit', + label: __( 'Edit', 'newspack-plugin' ), + callback: ( items: EmailItem[] ) => { + window.location.href = items[ 0 ].edit_link; }, - { - id: 'deactivate', - label: __( 'Deactivate', 'newspack-plugin' ), - isEligible: ( item: EmailItem ) => - item.source !== 'woocommerce' && item.category !== 'reader-activation' && item.status === 'publish', - callback: ( items: EmailItem[] ) => { - updateStatus( items[ 0 ].post_id, 'draft' ); - }, + }, + { + id: 'deactivate', + label: __( 'Deactivate', 'newspack-plugin' ), + isEligible: ( item: EmailItem ) => item.source !== 'woocommerce' && item.category !== 'reader-activation' && item.status === 'publish', + callback: ( items: EmailItem[] ) => { + updateStatus( items[ 0 ].post_id, 'draft' ); }, - { - id: 'activate', - label: __( 'Activate', 'newspack-plugin' ), - isEligible: ( item: EmailItem ) => - item.source !== 'woocommerce' && item.category !== 'reader-activation' && item.status !== 'publish', - callback: ( items: EmailItem[] ) => { - updateStatus( items[ 0 ].post_id, 'publish' ); - }, + }, + { + id: 'activate', + label: __( 'Activate', 'newspack-plugin' ), + isEligible: ( item: EmailItem ) => item.source !== 'woocommerce' && item.category !== 'reader-activation' && item.status !== 'publish', + callback: ( items: EmailItem[] ) => { + updateStatus( items[ 0 ].post_id, 'publish' ); }, - { - id: 'reset', - label: __( 'Reset', 'newspack-plugin' ), - isDestructive: true, - isEligible: ( item: EmailItem ) => item.source !== 'woocommerce' && Boolean( item.registry_slug ), - callback: ( items: EmailItem[] ) => { - if ( utils.confirmAction( __( 'Are you sure you want to reset the contents of this email?', 'newspack-plugin' ) ) ) { - resetEmail( items[ 0 ].post_id ); - } - }, + }, + { + id: 'reset', + label: __( 'Reset', 'newspack-plugin' ), + isDestructive: true, + isEligible: ( item: EmailItem ) => item.source !== 'woocommerce' && Boolean( item.registry_slug ), + callback: ( items: EmailItem[] ) => { + if ( utils.confirmAction( __( 'Are you sure you want to reset the contents of this email?', 'newspack-plugin' ) ) ) { + resetEmail( items[ 0 ].post_id ); + } }, - ], - [ resetEmail, updateStatus ] - ); + }, + ]; const { data: processedData, paginationInfo } = useMemo( () => filterSortAndPaginate( data, view, fields ), [ data, view, fields ] ); @@ -268,7 +255,7 @@ const Emails = () => { return ( - { error && } + { errorMessage && } { actions={ actions } paginationInfo={ paginationInfo } defaultLayouts={ { table: {}, grid: {} } } - isLoading={ isLoading } + isLoading={ isFetching } getItemId={ ( item: EmailItem ) => String( item.post_id ) } search />