Add the usage of the render_email_callback to the email rendering. #59070
Conversation
📝 WalkthroughWalkthroughThis change removes the custom Blocks_Registry system and migrates block registration and rendering in the email editor to use WordPress’s native WP_Block_Type_Registry and the render_email_callback mechanism. Related JavaScript logic is deleted, and block support configuration is consolidated in PHP. Tests and documentation are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant WP as WordPress Core
participant Initializer as Initializer (PHP)
participant Block as Block Renderer (e.g., WooContent)
participant Renderer as Content_Renderer
WP->>Initializer: Apply 'block_type_metadata_settings' filter
Initializer->>WP: For allowed blocks, set supports.email & render_email_callback
WP->>Renderer: Request to render a block
Renderer->>WP: Get block type from WP_Block_Type_Registry
alt Block has render_email_callback
Renderer->>Block: Call render_email_callback
Block-->>Renderer: Return rendered content
else No callback
Renderer->>Renderer: Use fallback renderer
end
Renderer-->>WP: Return rendered content
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)packages/php/email-editor/src/Integrations/Core/class-initializer.php (1)⏰ Context from checks skipped due to timeout of 90000ms (23)
🔇 Additional comments (7)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
8f68dbf to
522f3ad
Compare
This change allows defining a rendering callback that is used for rendering in emails.
The class Blocks_Registry is not needed anymore when we use the block custom callback.
f98d61a to
c52fb84
Compare
- I didn't move placeholder const to avoid changes in the email templates.
We decided to keep rendering callbacks and block configuration at one place. This place should be PHP package.
c52fb84 to
07a4f8d
Compare
|
Size Change: -1.06 kB (-0.02%) Total Size: 5.84 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/php/email-editor/tests/integration/Integrations/Core/Renderer/Renderer_Test.php (1)
44-62: Consider simplifying the block re-registration logic.While the current approach works, the manual unregistering and re-registering of blocks in
setUpis complex and potentially fragile. Consider these alternatives:
- Mock the block registry: Use dependency injection to provide a test-specific block registry
- Separate test helper method: Extract the re-registration logic into a reusable helper method
- Integration test approach: Consider if this level of block manipulation is necessary or if the functionality could be tested differently
The current implementation modifies global state which could interfere with other tests.
+ /** + * Helper method to register blocks with email callbacks for testing + * + * @param array $blocks Array of block names to register + */ + private function registerBlocksWithEmailCallbacks(array $blocks): void { + $initializer = $this->di_container->get(Initializer::class); + + foreach ($blocks as $block) { + $block_type = \WP_Block_Type_Registry::get_instance()->get_registered($block); + $this->assertInstanceOf(\WP_Block_Type::class, $block_type); + // ... rest of the registration logic + } + }Do you want me to help refactor this test setup to be more maintainable?
packages/php/email-editor/src/Integrations/Core/class-initializer.php (1)
98-144: Consider caching renderer instances for better performance.Creating new renderer instances on every call to
update_block_settingscould be inefficient if this filter is applied frequently during block registration. Consider implementing a singleton pattern or caching these instances.Example approach using static caching:
+ /** + * Cached renderer instances. + */ + private static $renderer_cache = array(); + + /** + * Get or create a renderer instance. + */ + private static function get_renderer( string $renderer_class, ...$args ) { + $cache_key = $renderer_class . serialize( $args ); + if ( ! isset( self::$renderer_cache[ $cache_key ] ) ) { + self::$renderer_cache[ $cache_key ] = new $renderer_class( ...$args ); + } + return self::$renderer_cache[ $cache_key ]; + } public function update_block_settings( array $settings ): array { // ... existing code ... switch ( $settings['name'] ) { case 'core/heading': case 'core/paragraph': - $settings['render_email_callback'] = array( new Text(), 'render' ); + $settings['render_email_callback'] = array( self::get_renderer( Text::class ), 'render' ); break;
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
packages/js/email-editor/changelog/wooplug-4676-block-registration-api-add-usage-of-render_email_callback-to(1 hunks)packages/js/email-editor/src/blocks/core/supported-blocks.ts(0 hunks)packages/js/email-editor/src/blocks/index.ts(0 hunks)packages/php/email-editor/changelog/wooplug-4676-block-registration-api-add-usage-of-render_email_callback-to(1 hunks)packages/php/email-editor/src/Engine/Renderer/ContentRenderer/class-blocks-registry.php(0 hunks)packages/php/email-editor/src/Engine/Renderer/ContentRenderer/class-content-renderer.php(4 hunks)packages/php/email-editor/src/Engine/Renderer/class-renderer.php(0 hunks)packages/php/email-editor/src/Engine/Renderer/readme.md(1 hunks)packages/php/email-editor/src/Integrations/Core/class-initializer.php(2 hunks)packages/php/email-editor/src/class-bootstrap.php(1 hunks)packages/php/email-editor/src/class-email-editor-container.php(0 hunks)packages/php/email-editor/tests/integration/Email_Editor_Integration_Test_Case.php(0 hunks)packages/php/email-editor/tests/integration/Engine/Renderer/ContentRenderer/Blocks_Registry_Test.php(0 hunks)packages/php/email-editor/tests/integration/Engine/Renderer/ContentRenderer/Content_Renderer_Test.php(2 hunks)packages/php/email-editor/tests/integration/Integrations/Core/Renderer/Renderer_Test.php(2 hunks)plugins/woocommerce/changelog/wooplug-4676-block-registration-api-add-usage-of-render_email_callback-to(1 hunks)plugins/woocommerce/src/Internal/EmailEditor/BlockEmailRenderer.php(1 hunks)plugins/woocommerce/src/Internal/EmailEditor/Integration.php(3 hunks)plugins/woocommerce/src/Internal/EmailEditor/Renderer/Blocks/WooContent.php(1 hunks)
💤 Files with no reviewable changes (7)
- packages/php/email-editor/src/Engine/Renderer/class-renderer.php
- packages/js/email-editor/src/blocks/index.ts
- packages/php/email-editor/tests/integration/Email_Editor_Integration_Test_Case.php
- packages/js/email-editor/src/blocks/core/supported-blocks.ts
- packages/php/email-editor/src/class-email-editor-container.php
- packages/php/email-editor/tests/integration/Engine/Renderer/ContentRenderer/Blocks_Registry_Test.php
- packages/php/email-editor/src/Engine/Renderer/ContentRenderer/class-blocks-registry.php
🔇 Additional comments (18)
plugins/woocommerce/changelog/wooplug-4676-block-registration-api-add-usage-of-render_email_callback-to (1)
1-4: LGTM! Well-structured changelog entry.The changelog entry appropriately describes the migration to
render_email_callbackwith correct significance level and type classification.plugins/woocommerce/src/Internal/EmailEditor/Integration.php (3)
12-12: LGTM! Proper import added.The import for
WooContentis correctly added to support the new block registration functionality.
83-83: LGTM! Proper initialization sequence.The call to
register_blocks()is appropriately placed in the initialization sequence.
128-134: LGTM! Clean block registration implementation.The
register_blocks()method properly instantiates and registers theWooContentblock. The implementation follows good practices by delegating the registration logic to the block class itself.plugins/woocommerce/src/Internal/EmailEditor/BlockEmailRenderer.php (2)
15-15: LGTM! Constant simplified appropriately.The
WOO_EMAIL_CONTENT_PLACEHOLDERconstant has been properly simplified while maintaining its functionality.
64-64: ```shell
#!/bin/bashSearch for the register_block_renderers method definition
rg -n "function register_block_renderers" -C2
Search for any references to register_block_renderers
rg -n "register_block_renderers"
Search for the action hook usage
rg -n "woocommerce_email_blocks_renderer_initialized"
Locate the Integration class and its register_blocks method
rg -n "class Integration" -C2
rg -n "function register_blocks" -C2</details> <details> <summary>plugins/woocommerce/src/Internal/EmailEditor/Renderer/Blocks/WooContent.php (3)</summary> `12-18`: **LGTM! Clean class structure.** The class has been properly refactored to remove the inheritance from `Abstract_Block_Renderer` and includes a well-defined block name property. --- `20-36`: **LGTM! Proper WordPress block registration.** The `register()` method correctly implements the new pattern by: - Using WordPress's native `register_block_type` function - Setting appropriate block supports (inserter disabled, email enabled) - Properly configuring the `render_email_callback` to use the class's render method This aligns well with the migration from the custom Blocks_Registry system to WordPress's native block type registry. --- `43-45`: **LGTM! Simplified render method.** The render method has been correctly refactored to be public and parameterless, following the new `render_email_callback` pattern while maintaining its core functionality of returning the placeholder constant. </details> <details> <summary>packages/js/email-editor/changelog/wooplug-4676-block-registration-api-add-usage-of-render_email_callback-to (1)</summary> `1-5`: **LGTM - Clear changelog entry.** The changelog entry appropriately documents the migration of the allowed blocks constant from JavaScript to PHP, with correct significance and type classifications. </details> <details> <summary>packages/php/email-editor/changelog/wooplug-4676-block-registration-api-add-usage-of-render_email_callback-to (1)</summary> `1-5`: **LGTM - Comprehensive changelog entry.** The changelog entry accurately documents the introduction of `render_email_callback` support and removal of the `Blocks_Registry` class, representing significant architectural improvements. </details> <details> <summary>packages/php/email-editor/src/class-bootstrap.php (1)</summary> `66-71`: Let’s search the entire repo for the method and its class to confirm the signature. ```shell #!/bin/bash # 1) Find every occurrence of update_block_settings rg -n 'update_block_settings' . # 2) Locate the CoreEmailEditorIntegration class and inspect its methods rg -n 'class CoreEmailEditorIntegration' -A20 .packages/php/email-editor/src/Engine/Renderer/readme.md (1)
7-7: LGTM - Documentation correctly updated.The documentation update properly reflects the architectural change, directing developers to the new PHP location for adding allowed core blocks instead of the removed JavaScript location.
packages/php/email-editor/tests/integration/Integrations/Core/Renderer/Renderer_Test.php (1)
21-29: LGTM - Well-defined test constants.The
USED_CORE_BLOCKSconstant clearly defines which blocks are tested and provides good documentation for the test scope.packages/php/email-editor/src/Integrations/Core/class-initializer.php (2)
12-23: LGTM!The imports are properly organized and follow PSR-4 namespace conventions.
29-48: LGTM!Good approach to centralize the list of supported blocks in PHP, replacing the JavaScript-based configuration.
packages/php/email-editor/tests/integration/Engine/Renderer/ContentRenderer/Content_Renderer_Test.php (1)
12-12: LGTM!The test changes correctly reflect the architectural shift from the custom Blocks_Registry to WordPress's native block type registry with render_email_callback support.
Also applies to: 83-111
packages/php/email-editor/src/Engine/Renderer/ContentRenderer/class-content-renderer.php (1)
13-15: LGTM!The refactoring properly implements the render_email_callback pattern using WordPress's native WP_Block_Type_Registry, with appropriate fallback handling for blocks without email-specific rendering.
Also applies to: 38-43, 80-85, 99-103, 163-168
packages/php/email-editor/src/Integrations/Core/class-initializer.php
Outdated
Show resolved
Hide resolved
Testing GuidelinesHi @costasovo , 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:
|
This refactoring should prevent the creation of class instances earlier than necessary.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/php/email-editor/src/Engine/Renderer/ContentRenderer/class-content-renderer.php(4 hunks)packages/php/email-editor/src/Integrations/Core/class-initializer.php(2 hunks)packages/php/email-editor/src/class-email-editor-container.php(1 hunks)packages/php/email-editor/tests/integration/Email_Editor_Integration_Test_Case.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/php/email-editor/tests/integration/Email_Editor_Integration_Test_Case.php
- packages/php/email-editor/src/class-email-editor-container.php
- packages/php/email-editor/src/Integrations/Core/class-initializer.php
⏰ Context from checks skipped due to timeout of 90000ms (21)
- GitHub Check: Core API tests - @woocommerce/plugin-woocommerce [api]
- GitHub Check: Core e2e tests 6/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 3/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.2] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Core e2e tests 5/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 4/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Metrics - @woocommerce/plugin-woocommerce [performance]
- GitHub Check: PHP: 8.4 WP: latest [WP latest] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Core e2e tests 2/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 1/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.2] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: PHPStan: PHP 7.4 - @woocommerce/email-editor-php-config [unit]
- GitHub Check: JavaScript - @woocommerce/admin-library [unit]
- GitHub Check: JavaScript - @woocommerce/email-editor [unit]
- GitHub Check: PHP: 8.4 WP: latest [WP latest] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: PHP: 8.1 WP: latest [WP latest] - @woocommerce/email-editor-php-config [unit:php]
- GitHub Check: PHPStan: PHP 8.4 - @woocommerce/email-editor-php-config [unit]
- GitHub Check: Lint - @woocommerce/plugin-woocommerce
- GitHub Check: Validate markdown
- GitHub Check: Check Asset Sizes
- GitHub Check: build
🔇 Additional comments (3)
packages/php/email-editor/src/Engine/Renderer/ContentRenderer/class-content-renderer.php (3)
11-11: LGTM: Clean import additions for the refactor.The new imports properly support the transition from custom
Blocks_Registryto WordPress coreWP_Block_Type_Registry, along with the new logging and fallback capabilities.Also applies to: 14-14, 16-16
39-94: LGTM: Well-structured property additions.The new properties properly encapsulate the dependencies for the refactored architecture:
WP_Block_Type_Registryfor WordPress core block managementFallbackrenderer for graceful degradationEmail_Editor_Loggerfor error trackingThe documentation is clear and follows consistent patterns.
96-115: Constructor refactor looks good with proper dependency management.The constructor properly initializes the WordPress core registry singleton and creates the fallback renderer instance. The parameter change from
Blocks_RegistrytoEmail_Editor_Loggeraligns with the architectural refactor.Verify that all instantiations of this class have been updated to match the new constructor signature:
#!/bin/bash # Description: Check for instantiations of Content_Renderer to ensure they match new constructor signature # Search for Content_Renderer instantiations rg -A 10 "new Content_Renderer|Content_Renderer\(" --type php
packages/php/email-editor/src/Engine/Renderer/ContentRenderer/class-content-renderer.php
Outdated
Show resolved
Hide resolved
|
@costasovo The PR is updated. |
There was a problem hiding this comment.
Thank you for the updates! Great job!
Testing performed
- I created an email with various blocks and checked that all blocks are available in inserter. This PR refactors how they are registered to the editor.
- Checked blocks are rendered in the preview
- Sent a test email to Mailpit and verified all blocks were rendered
…oocommerce#59070) * Use the custom block property render_email_callback for rendering This change allows defining a rendering callback that is used for rendering in emails. * Remove the class Blocks_Registry and its test The class Blocks_Registry is not needed anymore when we use the block custom callback. * Replace using Blocks_Registry with register_block_type function - I didn't move placeholder const to avoid changes in the email templates. * Add changelogs * Remove the list of allowed blocks from email-editor JS package We decided to keep rendering callbacks and block configuration at one place. This place should be PHP package. * Add settings supports.email to true for allowed core blocks * Add changelog and update readme.md * Refactor setting render_email_callback to one method This refactoring should prevent the creation of class instances earlier than necessary. * Add internal property for caching renderer instances * Add try-catch statement around calling the custom callback * Fix the incorrect exit and change method to private
Submission Review Guidelines:
Changes proposed in this Pull Request:
Closes WOOPLUG-4676
Screenshots or screen recordings:
N/A
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment