Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the rendering filter for the Course Completed Actions Block by changing from the generic render_block filter to the block-specific render_block_core/button filter. This improves performance by only executing the callback for button blocks instead of all block types.
- Changed filter hook from
render_blocktorender_block_core/buttonfor better performance - Maintains the same functionality while reducing unnecessary function calls
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| public function __construct() { | ||
| add_filter( 'render_block', [ $this, 'update_more_courses_button_url' ], 10, 2 ); | ||
| add_filter( 'render_block_core/button', [ $this, 'update_more_courses_button_url' ], 10, 2 ); |
There was a problem hiding this comment.
The render_block_core/button filter passes 3 parameters (block content, block array, and WP_Block instance), but only 2 are specified here. While the current implementation only needs 2 parameters, this is inconsistent with the pattern used elsewhere in the codebase (see class-sensei-page-blocks.php line 28 which uses 3 parameters). Consider updating to accept 3 parameters for consistency and future extensibility: add_filter( 'render_block_core/button', [ $this, 'update_more_courses_button_url' ], 10, 3 );
There was a problem hiding this comment.
It's fine, Copilot. Don't worry about it.
|
@m1r0 - can you help with this? This is my first time working on Sensei, so I'm not sure of the process. Just wanted to help out because I saw 100+ duplicate queries on ( internal url ). |
|
All makes sense, I just updated the changelog to make it more user-facing! Thanks for the help, @mreishus! 🙇 |
Proposed Changes
Sensei_Course_Completed_Actions_Blockruns a filter on all block renders, but it calls intoupdate_button_block_urlwhich is only concerned with one block type, enforced by a guard clause at the top:Because it's only concerned with core/button, and because WordPress gives us a more specific filter: (https://github.com/WordPress/wordpress-develop/blob/26230c33261cf559907a90547b4827ea66ff9f30/src/wp-includes/class-wp-block.php#L666)
Let's reduce the scope of this filter so it only runs on the button blocks we are concerned with.
Note that this has the side effect of reducing 100+ duplicate SQL queries on (internal url). This seems to be a separate problem, but reducing of the scope of this filter is a pretty big mitigation, since it stops a query from running on every block render.
Testing Instructions
New/Updated Hooks
Deprecated Code
Pre-Merge Checklist