Consolidate custom post type and meta registrations into Post_Types class #2215
Conversation
github-actions
bot
added
the
[Focus] Editor
obenland
added
the
Skip Changelog
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates the registration of custom post types and post meta into a dedicated Post_Types class to improve code organization and maintainability. The changes move scattered registration logic from multiple locations into semantic methods within a single class.
- Created new
Post_Typesclass with dedicated methods for each post type and meta registration - Moved post type registrations from
Activitypub::register_post_types()and post meta fromBlocks::register_postmeta() - Updated test to use the new class method instead of the old
Activitypubmethod
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| includes/class-post-types.php | New class containing all custom post type and meta registrations with dedicated methods |
| includes/class-activitypub.php | Removed register_post_types() method and unused imports, minor grammar fixes to user meta descriptions |
| includes/class-blocks.php | Removed register_postmeta() method and its hook registration |
| tests/includes/collection/class-test-inbox.php | Updated test to use new Post_Types::register_inbox_post_type() method |
| activitypub.php | Added initialization of new Post_Types class |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…lass - Created new Post_Types class to manage all custom post type and meta registrations - Moved post type registrations from Activitypub::register_post_types() to dedicated methods - Consolidated post meta registration with each post type for better organization - Removed duplicate post meta registrations from Blocks class - Updated tests to use new Post_Types class methods - Maintained user meta registrations in original Activitypub class location - All 895 tests pass, ensuring backward compatibility
obenland
force-pushed
the
consolidate-post-type-registrations
branch
from
5b5c632 to
72167b4
Compare
|
@pfefferle Should |
Deleted an unnecessary blank line for improved code cleanliness in the Activitypub class.
The regex pattern was failing to match @Covers warnings with escaped backslashes in class names. Changed from .* to [^"]* to properly capture content within quotes.
- Changed from trying to match specific warning count format to just looking for WARNINGS! - Updated sed pattern to handle both singular and plural warning formats - This is more robust and handles any number of warnings
|
If we also move every meta registration I am not sure if Post_Type is still the right name!? |
I guess I defined it generously in my head, anything post types related, including built-in post types. |
- Updated @Covers annotation in test_activity_meta_sanitization to reference Post_Types::register_outbox_post_type - Updated @Covers annotation in test_register_post_meta to reference Post_Types::register_activitypub_post_meta - These methods were moved from Activitypub::register_post_types and Blocks::register_postmeta to the new Post_Types class
- Added tracking of processed file:line combinations to avoid duplicate annotations - Tests with dataProvider create multiple warnings for the same @Covers annotation - Now only creates one checkstyle annotation per unique file/line location - Reduces noise in PR annotations while still catching all unique issues
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
When we talk about these, we would also have to move:
But then, the class would be very bloated again. Should we only have the Post_Types defined there and introduce two other classes for |
|
I think I would move all the post_metas to the |
What's the connection between |
There is none, except what I wrote above: We might want to also have a dedicated space for user metas. That said, I had a look at the code and the metas and options are all over the place, because it is not always the best solution to have them in one place (options for example). So I think we are fine with your PR for now. |
Refactored prevent_empty_post_meta and default_post_metadata methods from Activitypub to Post_Types for better separation of concerns. Updated hooks in Activitypub to use the new Post_Types methods. Added unit tests for these methods in a new Test_Post_Types class, and removed related tests from Test_Activitypub.
Extract duplicate post meta args into reusable private helper methods: - get_object_id_meta_args() for _activitypub_object_id - get_activity_actor_meta_args() for _activitypub_activity_actor - get_activity_type_meta_args() for _activitypub_activity_type - get_content_visibility_meta_args() for activitypub_content_visibility This eliminates code duplication between inbox/outbox registrations and ensures consistency across all post meta configurations.
Replaces custom and esc_sql sanitize_callback functions with WordPress's built-in sanitize_text_field for better input sanitization and consistency.
Remove duplicate annotation tracking logic that was added in this branch. Will be fixed in a separate PR focused on CI improvements.
This reverts commit 1cd303f.
…com/Automattic/wordpress-activitypub into consolidate-post-type-registrations
Relocates the registration of post meta-related filters from Activitypub to the Post_Types class for better organization and separation of concerns. Props @pfefferle.
The register_ap_actor_rest_field method and its related rest_api_init hook have been moved from the Activitypub class to the Post_Types class. This change centralizes REST field registration logic related to post types, improving code organization.
Relocates the registration of ActivityPub support for custom post types and the actor extra fields filter from class-activitypub.php to class-post-types.php. This change improves code organization by grouping post type related logic within the Post_Types class.
This has been on my mental list for a while. Mainly because we register post meta in two places. After that, because the Activitypub class is a junk drawer class and this is something that we can pull out and it makes sense on its own.
Proposed changes:
This PR consolidates all custom post type and post meta registrations into a single, well-organized
Post_Typesclass to improve code maintainability and organization.Post_Typesclass with dedicated methods for each post type registrationActivitypub::register_post_types()andBlocks::register_postmeta()) into semantic functionsOther information:
Testing instructions:
npm run env-testto verify all 895 tests passap_actor,ap_inbox,ap_outbox, extra fields) are properly registered