Consolidate custom post type and meta registrations into Post_Types class #2215

obenland · 2025-09-19T15:07:39Z

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_Types class to improve code maintainability and organization.

Created new Post_Types class with dedicated methods for each post type registration
Each method handles both post type registration and its related meta fields together
Moved registrations from scattered locations (Activitypub::register_post_types() and Blocks::register_postmeta()) into semantic functions
Maintained user meta registrations in original location as requested
Updated test to use new class methods
Removed unused imports after consolidation
Updated code coverage annotation job to account for multiple warnings

Other information:

Have you written new tests for your changes, if applicable? (Updated existing test)

Testing instructions:

Ensure all existing functionality works as expected
Run npm run env-test to verify all 895 tests pass
Check that custom post types (ap_actor, ap_inbox, ap_outbox, extra fields) are properly registered
Verify post meta fields are correctly associated with their respective post types
Test ActivityPub functionality to ensure no regressions

Copilot

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_Types class with dedicated methods for each post type and meta registration
Moved post type registrations from Activitypub::register_post_types() and post meta from Blocks::register_postmeta()
Updated test to use the new class method instead of the old Activitypub method

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.}

Copilot · 2025-09-19T15:08:17Z

includes/class-post-types.php

+			array(
+				'type'              => 'string',
+				'single'            => false,
+				'sanitize_callback' => 'esc_sql',


Using esc_sql as a sanitize callback is incorrect. The esc_sql function is meant for preparing data for SQL queries, not for sanitizing meta values. Use sanitize_text_field instead for proper sanitization of string meta values.

Suggested change

'sanitize_callback' => 'esc_sql',

'sanitize_callback' => 'sanitize_text_field',

@pfefferle Should this be absint?

includes/class-activitypub.php

includes/class-post-types.php

includes/class-activitypub.php

…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

includes/class-blocks.php

obenland · 2025-09-19T15:20:51Z

@pfefferle Should prevent_empty_post_meta() and default_post_metadata() also be moved into Post_Types?

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

pfefferle · 2025-09-19T15:46:34Z

If we also move every meta registration I am not sure if Post_Type is still the right name!?

obenland · 2025-09-19T15:49:03Z

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

Copilot

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.}

Copilot · 2025-09-19T16:08:26Z

includes/class-post-types.php

+						throw new \Exception( 'Error message is no valid string' );
+					}
+
+					return \esc_sql( $value );


Using esc_sql for post meta sanitization is incorrect. esc_sql is for preparing values for SQL queries, not for sanitizing stored data. Use sanitize_text_field or appropriate sanitization function instead.

Copilot AI review requested due to automatic review settings September 19, 2025 15:07

obenland self-assigned this Sep 19, 2025

obenland requested a review from pfefferle September 19, 2025 15:07

github-actions bot added the [Focus] Editor Changes to the ActivityPub experience in the block editor label Sep 19, 2025

obenland added the Skip Changelog Disables the "Changelog Updated" action for PRs where changelog entries are not necessary. label Sep 19, 2025

Copilot AI reviewed Sep 19, 2025

View reviewed changes

pfefferle reviewed Sep 19, 2025

View reviewed changes

includes/class-post-types.php Show resolved Hide resolved

obenland commented Sep 19, 2025

View reviewed changes

includes/class-activitypub.php Show resolved Hide resolved

obenland force-pushed the consolidate-post-type-registrations branch from 5b5c632 to 72167b4 Compare September 19, 2025 15:18

pfefferle reviewed Sep 19, 2025

View reviewed changes

includes/class-blocks.php Show resolved Hide resolved

obenland added 2 commits September 19, 2025 10:22

Remove extra blank line in Activitypub class

6088b0a

Deleted an unnecessary blank line for improved code cleanliness in the Activitypub class.

Fix regex pattern in coverage warning detection for escaped backslashes

710e0c3

The regex pattern was failing to match @Covers warnings with escaped backslashes in class names. Changed from .* to [^"]* to properly capture content within quotes.

github-actions bot added the Actions label Sep 19, 2025

Simplify warning detection to just look for WARNINGS! marker

81fc691

- 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

obenland added 2 commits September 19, 2025 11:03

obenland requested review from Copilot and pfefferle September 19, 2025 16:07

Copilot AI reviewed Sep 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Consolidate custom post type and meta registrations into Post_Types class #2215

Consolidate custom post type and meta registrations into Post_Types class #2215

obenland commented Sep 19, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 19, 2025

Uh oh!

obenland Sep 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

obenland commented Sep 19, 2025

Uh oh!

pfefferle commented Sep 19, 2025

Uh oh!

obenland commented Sep 19, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 19, 2025

Uh oh!

Uh oh!

	'sanitize_callback' => 'esc_sql',
	'sanitize_callback' => 'sanitize_text_field',

Consolidate custom post type and meta registrations into Post_Types class #2215

Are you sure you want to change the base?

Consolidate custom post type and meta registrations into Post_Types class #2215

Conversation

obenland commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes:

Other information:

Testing instructions:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

obenland Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

obenland commented Sep 19, 2025

Uh oh!

pfefferle commented Sep 19, 2025

Uh oh!

obenland commented Sep 19, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

obenland commented Sep 19, 2025 •

edited

Loading