Loading enhancement for blocks and slotfills #90
Conversation
WalkthroughThe changes primarily focus on enhancing the code quality and maintainability of the PHP files in the project. The modifications include the introduction of strict types, the addition of a new Changes
TipsChat with CodeRabbit Bot (
|
| namespace Alley\WP\WP_Curate; | ||
|
|
||
| /** | ||
| * Registers all slotfill assets so that they can be enqueued through Gutenberg in | ||
| * the corresponding context. | ||
| */ | ||
| function register_slotfills_scripts(): void { | ||
| $supported_post_types = new Supported_Post_Types(); | ||
|
|
There was a problem hiding this comment.
Does it make sense to use a singleton for this?
Co-authored-by: Greg Marshall <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (6)
- .phpcs.xml (1 hunks)
- blocks/post/index.php (1 hunks)
- blocks/query/index.php (2 hunks)
- entries/slotfills/index.php (1 hunks)
- src/class-supported-post-types.php (4 hunks)
- tests/feature/class-supported-post-types-test.php (1 hunks)
Files skipped from review due to trivial changes (1)
- .phpcs.xml
Additional comments: 11
blocks/query/index.php (3)
8-8: Adding
declare(strict_types=1);is a good practice for enforcing strict typing in PHP. This will help catch type-related errors at compile time.16-20: The
load()method is used to load the supported post types. If it fails, the function returns early. This is a good practice for error handling.35-42: The addition of the
wp_curate_allowed_post_typesandwp_curate_allowed_taxonomiesfilters is a good practice. It allows developers to modify the allowed post types and taxonomies for the Query block.blocks/post/index.php (2)
8-8: Adding
declare(strict_types=1);is a good practice for enforcing strict typing in PHP. This can help prevent type-related bugs in your code.16-20: The change from checking if the current post type is in the supported post types array to using the
load()method of the$supported_post_typesobject is a good abstraction. It encapsulates the logic of checking supported post types within theSupported_Post_Typesclass, making the code more maintainable and easier to understand.However, ensure that the
load()method of theSupported_Post_Typesclass is implemented correctly and handles all edge cases. Also, verify that theload()method does not have any side effects that could affect the rest of the code.entries/slotfills/index.php (4)
10-10: The use of
declare(strict_types=1);is a good practice for enforcing strict typing in PHP. This can help prevent bugs related to type coercion.19-19: The
Supported_Post_Typesclass is instantiated without any arguments. Ensure that the class constructor does not require any mandatory arguments.26-26: The
apply_filtersfunction allows plugins to modify the list of post types that will show the "Enable Deduplication" slotfill. This is a good use of WordPress's hooks system to allow for extensibility.28-30: The
loadmethod is used to check if the current post type is in the list of allowed post types. If it's not, the function returns early. This is a good use of the early return pattern to avoid unnecessary computation.src/class-supported-post-types.php (2)
8-8: The use of
declare(strict_types=1);is a good practice for enforcing strict typing in PHP. This can help prevent type-related bugs in the code.55-81: The
load()method is a good addition to the class. It provides a way to load a block or slotfill using either the supported post types or custom ones. The use of an optional parameter$post_typeswith a default value of an empty array is a good practice. It makes the method more flexible and easier to use.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/class-supported-post-types.php (4 hunks)
Additional comments: 5
src/class-supported-post-types.php (5)
8-8: The use of
declare(strict_types=1);is a good practice for enforcing strict typing in PHP. This can help prevent type-related bugs in the code.52-52: The use of
apply_filtersallows for the modification of the supported post types by other parts of the application or plugins. This is a good practice for extensibility.63-81: The
loadmethod checks if the current post type is in the list of post types passed to it (or the supported post types if none are passed) and returns false if it is not. This is a good way to prevent blocks or slotfills from being loaded in unsupported post types. However, it's important to ensure that this method is called in the correct context (i.e., when a block or slotfill is being loaded) and that the return value is properly handled.97-107: The use of
$_GETsuperglobal is a security risk as it can be manipulated by the user. It's recommended to use WordPress'sfilter_input()function to access these superglobals. This was already pointed out in a previous review comment and the recommendation still stands.- if ( 'post.php' === $pagenow && ! empty( $_GET['post'] ) ) { + $post_id = filter_input(INPUT_GET, 'post', FILTER_SANITIZE_NUMBER_INT); + if ( 'post.php' === $pagenow && ! empty( $post_id ) ) {- if ( ! empty( $_GET['post_type'] ) ) { + $post_type = filter_input(INPUT_GET, 'post_type', FILTER_SANITIZE_STRING); + if ( ! empty( $post_type ) ) {
- 116-116: The
get_current_post_typemethod now always returns a string, which can make it easier to use in the rest of the code. However, it's important to ensure that all uses of this method in the codebase have been updated to handle this change.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- tests/feature/class-supported-post-types-test.php (1 hunks)
Files skipped from review due to trivial changes (1)
- tests/feature/class-supported-post-types-test.php
Summary
declare(strict_types=1);;wp_curate_duduplication_slotfill_post_typesfallbacks to the default supported post types;The previous approach didn't work very well with headless sites, where we need to access the context of the block. Mainly because
get_current_post_typereturns empty in those instances and there was no way of hooking into that.This approach keeps the same logic but allows devs to load the block in headless context, if necessary, by using
wp_curate_load.Summary by CodeRabbit
Refactor:
wp-curatepackage, now using theload()method of theSupported_Post_Typesclass.New Features:
wp_curate_allowed_post_typesandwp_curate_allowed_taxonomies, providing more customization options.Tests:
Supported_Post_Types_Testto ensure the functionality of theSupported_Post_Typesclass.Chores: