Conversation
Registers a "Reset content to default" entity action for woo_email posts
in the email editor. When triggered, it fetches the original plugin-
distributed block template content via a new REST endpoint and restores
the post content to that default state.
- Add GET /woocommerce-email-editor/v1/emails/{id}/default-content endpoint
- Export registerEntityAction and PostWithPermissions from email editor package
- Register reset action for woo_email post type from email-editor-integration
…for WordPress 6.7 (Woo has discontinued it).
Testing GuidelinesHi @lysyjan , Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
|
Size Change: +3.17 kB (+0.05%) Total Size: 5.98 MB
|
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a reset-notification-email-content entity action (frontend UI), a backend REST endpoint to fetch plugin-default email content, and new exports from the email-editor package: Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Modal as Reset Modal UI
participant Editor as Email Editor Store
participant API as REST API
participant Generator as WCTransactionalEmailPostsGenerator
User->>Modal: Trigger reset action
Modal->>Modal: Show confirmation dialog
User->>Modal: Confirm reset
Modal->>API: GET /woocommerce-email-editor/v1/emails/{id}/default-content
API->>Generator: Request generated default content for ID
Generator-->>API: Return default blocks/content
API-->>Modal: Return default content
Modal->>Editor: Parse blocks and update editor state
Modal->>Editor: Save updated entity
Editor-->>Modal: Save complete
Modal->>User: Show success notice and close
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
plugins/woocommerce/src/Internal/EmailEditor/EmailApiController.php (2)
306-309: Use DI forWCTransactionalEmailPostsGeneratorinstead of direct instantiation.Creating the generator with
newhere bypasses container wiring and makes this path harder to mock and standardize with the rest of Internal services.♻️ Proposed refactor
- $generator = new WCTransactionalEmailPostsGenerator(); + $generator = wc_get_container()->get( WCTransactionalEmailPostsGenerator::class );As per coding guidelines "Dependencies in PHP classes should be injected via the DI container using the
init()method pattern."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/src/Internal/EmailEditor/EmailApiController.php` around lines 306 - 309, Replace the direct instantiation of WCTransactionalEmailPostsGenerator with the DI/init() pattern used by the codebase: obtain the generator via the controller's dependency initializer (e.g. $this->init(WCTransactionalEmailPostsGenerator::class) or the equivalent container accessor used elsewhere in EmailApiController) and use that instance in place of new WCTransactionalEmailPostsGenerator(); update the method that returns WP_REST_Response to call get_email_template on the injected instance so the generator can be mocked and container-wired consistently.
257-277: Add REST API tests for the newdefault-contentroute.Please add coverage for permission denied, unknown post ID (404), and successful payload shape (
content).Based on learnings: "For REST API controller tests in plugins/woocommerce/tests/**/*.php, test classes should extend WC_REST_Unit_Test_Case because REST routes need to be registered before tests run."
Also applies to: 285-311
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/src/Internal/EmailEditor/EmailApiController.php` around lines 257 - 277, Add REST API tests for the new route registered by register_routes (route: 'woocommerce-email-editor/v1' path '/emails/(?P<id>\d+)/default-content', handler get_default_content_response'): create a test class that extends WC_REST_Unit_Test_Case, ensure routes are registered in setUp, then add three tests: (1) permission_denied: set current user without manage_woocommerce capability, dispatch a GET WP_REST_Request to the endpoint for an existing or arbitrary ID and assert a 403 response; (2) unknown_post_id_returns_404: ensure no post exists for an ID (or use an obviously invalid ID), dispatch GET and assert 404; (3) successful_payload_shape: create a woo_email post (the post type used by the controller), set a user with manage_woocommerce, dispatch GET for that post ID and assert 200 and that the response data contains a 'content' field with the expected structure/value; use rest_do_request / WP_REST_Request to call the route and standard PHPUnit assertions for status and response data.plugins/woocommerce/client/admin/client/wp-admin-scripts/email-editor-integration/reset-notification-email-content.tsx (1)
38-183: Add tests for reset success/error behavior.This new action path now includes API fetch, record mutation, persistence, and snackbar outcomes; it should have focused unit/E2E coverage for confirm-only fetch, success notice, and error notice paths.
As per coding guidelines "Add unit or E2E tests where applicable."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/client/admin/client/wp-admin-scripts/email-editor-integration/reset-notification-email-content.tsx` around lines 38 - 183, Add unit/E2E tests for getResetNotificationEmailContentAction covering the three paths: successful reset, API error, and confirm-only fetch. Specifically, write tests that stub apiFetch to return default content (assert parse/serialize flow, that editEntityRecord and saveEditedEntityRecord are called with expected args and createSuccessNotice is invoked with the snackbar id 'reset-notification-email-content-action'), stub apiFetch to throw (assert createErrorNotice is called with the error message), and verify the confirm-only UI path (modal opens, Cancel closes without calling apiFetch). Use the action's exported factory getResetNotificationEmailContentAction and mock/dispatch editEntityRecord, saveEditedEntityRecord, createSuccessNotice, createErrorNotice, and apiFetch to assert behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugins/woocommerce/client/admin/client/wp-admin-scripts/email-editor-integration/reset-notification-email-content.tsx`:
- Around line 76-84: The code dereferences items[0] when building modalTitle
(using sprintf and getItemTitle) which will throw if items is empty; update the
reset-notification-email-content logic to guard against empty or falsy items
before accessing items[0]—for example, check items && items.length > 0 and
provide a safe fallback title (or bail out early) so modalTitle is only built
when getItemTitle receives a valid item; ensure the guard is applied where
modalTitle is computed and any subsequent uses of item are protected.
---
Nitpick comments:
In
`@plugins/woocommerce/client/admin/client/wp-admin-scripts/email-editor-integration/reset-notification-email-content.tsx`:
- Around line 38-183: Add unit/E2E tests for
getResetNotificationEmailContentAction covering the three paths: successful
reset, API error, and confirm-only fetch. Specifically, write tests that stub
apiFetch to return default content (assert parse/serialize flow, that
editEntityRecord and saveEditedEntityRecord are called with expected args and
createSuccessNotice is invoked with the snackbar id
'reset-notification-email-content-action'), stub apiFetch to throw (assert
createErrorNotice is called with the error message), and verify the confirm-only
UI path (modal opens, Cancel closes without calling apiFetch). Use the action's
exported factory getResetNotificationEmailContentAction and mock/dispatch
editEntityRecord, saveEditedEntityRecord, createSuccessNotice,
createErrorNotice, and apiFetch to assert behavior.
In `@plugins/woocommerce/src/Internal/EmailEditor/EmailApiController.php`:
- Around line 306-309: Replace the direct instantiation of
WCTransactionalEmailPostsGenerator with the DI/init() pattern used by the
codebase: obtain the generator via the controller's dependency initializer (e.g.
$this->init(WCTransactionalEmailPostsGenerator::class) or the equivalent
container accessor used elsewhere in EmailApiController) and use that instance
in place of new WCTransactionalEmailPostsGenerator(); update the method that
returns WP_REST_Response to call get_email_template on the injected instance so
the generator can be mocked and container-wired consistently.
- Around line 257-277: Add REST API tests for the new route registered by
register_routes (route: 'woocommerce-email-editor/v1' path
'/emails/(?P<id>\d+)/default-content', handler get_default_content_response'):
create a test class that extends WC_REST_Unit_Test_Case, ensure routes are
registered in setUp, then add three tests: (1) permission_denied: set current
user without manage_woocommerce capability, dispatch a GET WP_REST_Request to
the endpoint for an existing or arbitrary ID and assert a 403 response; (2)
unknown_post_id_returns_404: ensure no post exists for an ID (or use an
obviously invalid ID), dispatch GET and assert 404; (3)
successful_payload_shape: create a woo_email post (the post type used by the
controller), set a user with manage_woocommerce, dispatch GET for that post ID
and assert 200 and that the response data contains a 'content' field with the
expected structure/value; use rest_do_request / WP_REST_Request to call the
route and standard PHPUnit assertions for status and response data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: c15ae8f1-a60b-4400-921f-6d2b305604c4
📒 Files selected for processing (7)
packages/js/email-editor/changelog/wooprd-1019-export-register-entity-actionpackages/js/email-editor/src/index.tsplugins/woocommerce/changelog/wooprd-1019-add-reset-notification-email-contentplugins/woocommerce/client/admin/client/wp-admin-scripts/email-editor-integration/index.tsplugins/woocommerce/client/admin/client/wp-admin-scripts/email-editor-integration/reset-notification-email-content.tsxplugins/woocommerce/src/Internal/EmailEditor/EmailApiController.phpplugins/woocommerce/src/Internal/EmailEditor/Integration.php
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/woocommerce/src/Internal/EmailEditor/EmailApiController.php (1)
307-311: Consider injectingWCTransactionalEmailPostsGeneratorvia the DI container.Directly instantiating
WCTransactionalEmailPostsGeneratorworks but makes unit testing harder. You could inject it through theinit()method like$this->post_managerfor consistency with the DI pattern used elsewhere in this class.♻️ Suggested refactor
Add a property and inject via
init():private ?WCTransactionalEmailPostsManager $post_manager; +private ?WCTransactionalEmailPostsGenerator $posts_generator; final public function init(): void { $this->post_manager = WCTransactionalEmailPostsManager::get_instance(); + $this->posts_generator = new WCTransactionalEmailPostsGenerator(); }Then use the injected instance:
-$generator = new WCTransactionalEmailPostsGenerator(); -return new WP_REST_Response( - array( 'content' => $generator->get_email_template( $email ) ), - 200 -); +return new WP_REST_Response( + array( 'content' => $this->posts_generator->get_email_template( $email ) ), + 200 +);As per coding guidelines: "Dependencies in PHP classes should be injected via the DI container using the
init()method pattern."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/src/Internal/EmailEditor/EmailApiController.php` around lines 307 - 311, The code directly instantiates WCTransactionalEmailPostsGenerator in EmailApiController; instead add a private property (e.g. $email_posts_generator) to the class, accept and assign an instance in the controller's init() method (matching how $this->post_manager is injected), and replace new WCTransactionalEmailPostsGenerator() with the injected $this->email_posts_generator->get_email_template($email) call so get_email_template is used from the DI-provided instance for easier testing and consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plugins/woocommerce/src/Internal/EmailEditor/EmailApiController.php`:
- Around line 307-311: The code directly instantiates
WCTransactionalEmailPostsGenerator in EmailApiController; instead add a private
property (e.g. $email_posts_generator) to the class, accept and assign an
instance in the controller's init() method (matching how $this->post_manager is
injected), and replace new WCTransactionalEmailPostsGenerator() with the
injected $this->email_posts_generator->get_email_template($email) call so
get_email_template is used from the DI-provided instance for easier testing and
consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 8019c067-1fd8-4fb0-8618-9376557be4c9
📒 Files selected for processing (1)
plugins/woocommerce/src/Internal/EmailEditor/EmailApiController.php
…nt schema - Introduced WCTransactionalEmailPostsGenerator for managing email templates. - Added get_default_content_schema method to define the schema for the default content endpoint response. - Updated initialization logic to ensure both post manager and posts generator are set before generating email content.
* Email Editor: Add reset notification email content action
Registers a "Reset content to default" entity action for woo_email posts
in the email editor. When triggered, it fetches the original plugin-
distributed block template content via a new REST endpoint and restores
the post content to that default state.
- Add GET /woocommerce-email-editor/v1/emails/{id}/default-content endpoint
- Export registerEntityAction and PostWithPermissions from email editor package
- Register reset action for woo_email post type from email-editor-integration
* More route registration to the EmailApiController and remove support for WordPress 6.7 (Woo has discontinued it).
* Fix lint errors
* Fix lint error
* Enhance EmailApiController with new posts generator and default content schema
- Introduced WCTransactionalEmailPostsGenerator for managing email templates.
- Added get_default_content_schema method to define the schema for the default content endpoint response.
- Updated initialization logic to ensure both post manager and posts generator are set before generating email content.
* Email Editor: Add reset notification email content action
Registers a "Reset content to default" entity action for woo_email posts
in the email editor. When triggered, it fetches the original plugin-
distributed block template content via a new REST endpoint and restores
the post content to that default state.
- Add GET /woocommerce-email-editor/v1/emails/{id}/default-content endpoint
- Export registerEntityAction and PostWithPermissions from email editor package
- Register reset action for woo_email post type from email-editor-integration
* More route registration to the EmailApiController and remove support for WordPress 6.7 (Woo has discontinued it).
* Fix lint errors
* Fix lint error
* Enhance EmailApiController with new posts generator and default content schema
- Introduced WCTransactionalEmailPostsGenerator for managing email templates.
- Added get_default_content_schema method to define the schema for the default content endpoint response.
- Updated initialization logic to ensure both post manager and posts generator are set before generating email content.

Submission Review Guidelines:
Changes proposed in this Pull Request:
Closes #WOOPRD-1019.
Adds a "Reset content to default" action to the WooCommerce email editor, allowing users to reset a notification email's block content to the original plugin-distributed state.
What changed:
reset-notification-email-content.tsx): Registers awoo_email-specific action viaregisterEntityAction. Shows a confirmation modal, fetches the original template content from the API, then callseditEntityRecord+saveEditedEntityRecordto reset and persist the content.EmailApiController::register_routes()):GET/woocommerce-email-editor/v1/emails/{id}/default-content— returns the plugin-distributed block template for a given woo_email post. Registered on-demand (not as a REST field) to avoid an expensive filesystem read on every post fetch.packages/js/email-editor/src/index.ts): ExportsregisterEntityActionandPostWithPermissionsfrom the package's public API so WooCommerce (and other integrators) can consume them without accessing private APIs directly.Note: We used the
woocommerce-email-editor/v1namespace because it is always available (registered unconditionally by the email editor PHP package), unlikewc/v4, which is feature-flagged.Screenshots or screen recordings:
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
GET/wp-json/woocommerce-email-editor/v1/emails/{id}/default-contentrequest fired only when confirming the reset (not on page load).manage_woocommercecapability — the action should not appear.Testing that has already taken place:
Milestone
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment