-
Notifications
You must be signed in to change notification settings - Fork 875
Sync: Introduce helper function and CPT for Activity Log entries #48567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
coder-karen
wants to merge
13
commits into
trunk
Choose a base branch
from
add/sync-custom-post-types-for-activity-log
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
f20a6b8
Add new helper function to aid in syncing custom post types specifica…
coder-karen ce6d4a0
changelog
coder-karen 38f7b82
Update composer.lock files
coder-karen 83bf4e5
changelog
coder-karen ec33021
Merge branch 'trunk' into add/sync-custom-post-types-for-activity-log
coder-karen 6f9d525
Update composer.lock files
coder-karen e1d6e74
Additional validation, sanitization, cleanup
coder-karen 93f77fe
Adding tests
coder-karen 876740d
Change approach to use an Activity_Log_Event class
coder-karen 6a0bde2
Prevent supporting new line chars
coder-karen 813be65
Cleanup, inc new tests
coder-karen aebafc0
Additional defensiveness and simplifying checks, also based on reviews
coder-karen 27bbf37
Additional comments, clarify validation, ensure tests and changelog a…
coder-karen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
4 changes: 4 additions & 0 deletions
4
projects/packages/sync/changelog/add-sync-custom-post-types-for-activity-log
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Significance: minor | ||
| Type: added | ||
|
|
||
| Add Activity Log custom event support with a private custom post type and class API for creating entries. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
344 changes: 344 additions & 0 deletions
344
projects/packages/sync/src/class-activity-log-event.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,344 @@ | ||
| <?php | ||
| /** | ||
| * Activity Log custom event support. | ||
| * | ||
| * @package automattic/jetpack-sync | ||
| */ | ||
|
|
||
| namespace Automattic\Jetpack\Sync; | ||
|
|
||
| /** | ||
| * Handles Activity Log custom event creation and validation. | ||
| */ | ||
| class Activity_Log_Event { | ||
|
|
||
| /** | ||
| * Post type name for Activity Log custom entries. | ||
| */ | ||
| const POST_TYPE = 'jp_act_log_event'; | ||
|
|
||
| /** | ||
| * Default event severity. | ||
| */ | ||
| const DEFAULT_SEVERITY = 'info'; | ||
|
|
||
| /** | ||
| * Allowed event severities. | ||
| */ | ||
| const ALLOWED_SEVERITIES = array( | ||
| 'info' => true, | ||
| 'success' => true, | ||
| 'warning' => true, | ||
| 'error' => true, | ||
| ); | ||
|
|
||
| /** | ||
| * Maximum title length. | ||
| */ | ||
| const MAX_TITLE_LENGTH = 200; | ||
|
|
||
| /** | ||
| * Maximum content length. | ||
| */ | ||
| const MAX_CONTENT_LENGTH = 5000; | ||
|
|
||
| /** | ||
| * Maximum source length. | ||
| */ | ||
| const MAX_SOURCE_LENGTH = 100; | ||
|
|
||
| /** | ||
| * Initialize Activity Log custom event hooks. | ||
| */ | ||
| public static function init() { | ||
| add_action( 'init', array( __CLASS__, 'register_post_type' ) ); | ||
| add_filter( 'wp_insert_post_empty_content', array( __CLASS__, 'prevent_invalid_post_insert' ), 10, 2 ); | ||
| add_filter( 'wp_insert_post_data', array( __CLASS__, 'normalize_post_data' ), 10, 2 ); | ||
| add_filter( 'publicize_should_publicize_published_post', array( __CLASS__, 'prevent_publicize' ), 10, 2 ); | ||
| add_filter( 'jetpack_sitemap_post_types', array( __CLASS__, 'filter_sitemap_post_types' ) ); | ||
| } | ||
|
|
||
| /** | ||
| * Registers the Activity Log CPT with hardened defaults that prevent leakage | ||
| * to front-end queries, RSS, REST, search, sitemaps, and exports. | ||
| */ | ||
| public static function register_post_type() { | ||
| register_post_type( | ||
| self::POST_TYPE, | ||
| array( | ||
| 'labels' => array( | ||
| 'name' => __( 'Activity Log Events', 'jetpack-sync' ), | ||
| 'singular_name' => __( 'Activity Log Event', 'jetpack-sync' ), | ||
| ), | ||
| 'public' => false, | ||
| 'publicly_queryable' => false, | ||
| 'show_ui' => false, | ||
| 'show_in_menu' => false, | ||
| 'show_in_nav_menus' => false, | ||
| 'show_in_rest' => false, | ||
| 'show_in_admin_bar' => false, | ||
| 'exclude_from_search' => true, | ||
| 'has_archive' => false, | ||
| 'rewrite' => false, | ||
| 'query_var' => false, | ||
| 'can_export' => false, | ||
| 'supports' => array( 'title', 'editor' ), | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Logs a custom event to the Jetpack Activity Log. | ||
| * | ||
| * Prefer calling this on or after the `init` action so Sync listeners are registered. | ||
| * The Activity Log post type is registered defensively if needed before insert. | ||
| * | ||
| * @param array $args { | ||
| * Activity log event arguments. | ||
| * | ||
| * @type string $title Required. Plain-text title, truncated to 200 chars. | ||
| * @type string $content Required. Plain-text body, truncated to 5000 chars. | ||
| * @type string $source Optional. Identifier for the source of the event, e.g. 'mc'. | ||
| * @type string $severity Optional. 'info', 'success', 'warning', or 'error'. Defaults to 'info'. | ||
| * } | ||
| * @return int|false Post ID on success, false if validation fails. | ||
| */ | ||
| public static function create( array $args ) { | ||
| $payload = self::build_payload( $args ); | ||
| if ( false === $payload ) { | ||
| return false; | ||
| } | ||
|
|
||
| if ( ! post_type_exists( self::POST_TYPE ) ) { | ||
| self::register_post_type(); | ||
| } | ||
|
|
||
| $post_content = wp_json_encode( $payload, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE ); | ||
| if ( false === $post_content ) { | ||
| return false; | ||
| } | ||
|
|
||
| $post_id = wp_insert_post( | ||
| wp_slash( | ||
| array( | ||
| 'post_type' => self::POST_TYPE, | ||
| 'post_title' => $payload['title'], | ||
| 'post_content' => $post_content, | ||
| 'post_status' => 'publish', | ||
| ) | ||
| ), | ||
| true | ||
| ); | ||
|
|
||
| return is_wp_error( $post_id ) ? false : $post_id; | ||
| } | ||
|
|
||
| /** | ||
| * Checks that an Activity Log custom event has a valid payload before enqueueing it for sync, | ||
| * in case data bypasses the Activity_Log_Event::create() helper. | ||
| * | ||
| * @param \WP_Post $post Activity Log post. | ||
| * @return bool | ||
| */ | ||
| public static function is_valid_post( $post ) { | ||
| if ( ! $post instanceof \WP_Post || self::POST_TYPE !== $post->post_type ) { | ||
| return false; | ||
| } | ||
|
|
||
| // Build a sanitized candidate to validate the payload contract without mutating the stored post. | ||
| return false !== self::build_payload_from_post_content( $post->post_content ); | ||
| } | ||
|
|
||
| /** | ||
| * Prevents invalid Activity Log event posts from being inserted or updated via wp_insert_post(). | ||
| * | ||
| * @param bool $maybe_empty Whether the post should be considered empty. | ||
| * @param array $postarr Post data passed to wp_insert_post(). | ||
| * @return bool | ||
| */ | ||
| public static function prevent_invalid_post_insert( $maybe_empty, $postarr ) { | ||
| if ( ! is_array( $postarr ) || self::POST_TYPE !== self::get_postarr_post_type( $postarr ) ) { | ||
| return $maybe_empty; | ||
| } | ||
|
|
||
| return false === self::build_payload_from_post_content( $postarr['post_content'] ?? '' ); | ||
| } | ||
|
|
||
| /** | ||
| * Normalizes Activity Log event posts before they are inserted or updated via wp_insert_post(). | ||
| * | ||
| * @param array $data Slashed, sanitized post data. | ||
| * @param array $postarr Post data passed to wp_insert_post(). | ||
| * @return array | ||
| */ | ||
| public static function normalize_post_data( $data, $postarr ) { | ||
| if ( ! is_array( $data ) || ! is_array( $postarr ) || self::POST_TYPE !== self::get_postarr_post_type( $postarr ) ) { | ||
| return $data; | ||
| } | ||
|
|
||
| $payload = self::build_payload_from_post_content( $data['post_content'] ?? '' ); | ||
| if ( false === $payload ) { | ||
| return $data; | ||
| } | ||
|
|
||
| $post_content = wp_json_encode( $payload, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE ); | ||
| if ( false === $post_content ) { | ||
| return $data; | ||
| } | ||
|
|
||
| $data['post_title'] = wp_slash( $payload['title'] ); | ||
| $data['post_content'] = wp_slash( $post_content ); | ||
|
|
||
| return $data; | ||
| } | ||
|
|
||
| /** | ||
| * Never auto-share Activity Log entries via Jetpack Social, | ||
| * even if a third party adds 'publicize' post-type support to this CPT. | ||
| * | ||
| * @param bool $should_publicize Publicize status prior to this filter running. | ||
| * @param \WP_Post $post The post to test for Publicizability. | ||
| * @return bool | ||
| */ | ||
| public static function prevent_publicize( $should_publicize, $post ) { | ||
| return ( $post && self::POST_TYPE === $post->post_type ) ? false : $should_publicize; | ||
| } | ||
|
|
||
| /** | ||
| * Never include Activity Log entries in Jetpack sitemaps, | ||
| * even if a third party adds this CPT to the sitemap post-type list. | ||
| * | ||
| * @param string[] $types Sitemap post types. | ||
| * @return string[] | ||
| */ | ||
| public static function filter_sitemap_post_types( $types ) { | ||
| return array_values( array_diff( (array) $types, array( self::POST_TYPE ) ) ); | ||
| } | ||
|
|
||
| /** | ||
| * Builds an Activity Log event payload from raw input. | ||
| * | ||
| * @param array $args Raw event arguments. | ||
| * @return array|false Sanitized payload, or false if validation fails. | ||
| */ | ||
| private static function build_payload( array $args ) { | ||
| $title = self::sanitize_string( $args['title'] ?? '', self::MAX_TITLE_LENGTH ); | ||
| $content = self::sanitize_string( $args['content'] ?? '', self::MAX_CONTENT_LENGTH ); | ||
|
|
||
| if ( '' === $title || '' === $content ) { | ||
| return false; | ||
| } | ||
|
|
||
| $severity = self::sanitize_severity( $args['severity'] ?? self::DEFAULT_SEVERITY ); | ||
| if ( false === $severity ) { | ||
| return false; | ||
| } | ||
|
|
||
| $payload = array( | ||
| 'title' => $title, | ||
| 'content' => $content, | ||
| 'severity' => $severity, | ||
| ); | ||
|
|
||
| $source = self::sanitize_string( $args['source'] ?? '', self::MAX_SOURCE_LENGTH ); | ||
| if ( '' !== $source ) { | ||
| $payload['source'] = $source; | ||
| } | ||
|
|
||
| return $payload; | ||
| } | ||
|
|
||
| /** | ||
| * Builds an Activity Log event payload from post content. | ||
| * | ||
| * @param mixed $post_content Raw post content. | ||
| * @return array|false Sanitized payload, or false if validation fails. | ||
| */ | ||
| private static function build_payload_from_post_content( $post_content ) { | ||
| $data = self::decode_payload( $post_content ); | ||
| if ( ! is_array( $data ) ) { | ||
| return false; | ||
| } | ||
|
|
||
| return self::build_payload( $data ); | ||
| } | ||
|
|
||
| /** | ||
| * Decodes an Activity Log event payload from post content. | ||
| * | ||
| * @param mixed $post_content Raw post content. | ||
| * @return array|false | ||
| */ | ||
| private static function decode_payload( $post_content ) { | ||
| $data = json_decode( (string) $post_content, true ); | ||
| if ( ! is_array( $data ) ) { | ||
| $data = json_decode( wp_unslash( (string) $post_content ), true ); | ||
| } | ||
|
|
||
| return is_array( $data ) ? $data : false; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the post type from wp_insert_post() input. | ||
| * | ||
| * @param array $postarr Post data passed to wp_insert_post(). | ||
| * @return string | ||
| */ | ||
| private static function get_postarr_post_type( array $postarr ) { | ||
| if ( isset( $postarr['post_type'] ) ) { | ||
| return (string) $postarr['post_type']; | ||
| } | ||
|
|
||
| if ( ! empty( $postarr['ID'] ) ) { | ||
| return (string) get_post_type( (int) $postarr['ID'] ); | ||
| } | ||
|
|
||
| return ''; | ||
| } | ||
|
|
||
| /** | ||
| * Strips HTML/PHP from a value and truncates it to a maximum character length, multibyte-safe. | ||
| * | ||
| * @param mixed $value Raw value. | ||
| * @param int $max Maximum length in characters. | ||
| * @return string | ||
| */ | ||
| private static function sanitize_string( $value, $max ) { | ||
| if ( is_array( $value ) || is_object( $value ) ) { | ||
| return ''; | ||
| } | ||
|
|
||
| $value = wp_strip_all_tags( (string) $value, true ); | ||
| $value = preg_replace( '/\s+/', ' ', $value ); | ||
| if ( null === $value ) { | ||
| return ''; | ||
| } | ||
|
|
||
| $value = trim( $value ); | ||
|
|
||
| if ( function_exists( 'mb_substr' ) ) { | ||
| return mb_substr( $value, 0, $max ); | ||
| } | ||
|
|
||
| return substr( $value, 0, $max ); | ||
| } | ||
|
|
||
| /** | ||
| * Sanitizes an Activity Log severity value. | ||
| * | ||
| * @param mixed $severity Raw severity. | ||
| * @return string|false Sanitized severity, or false if invalid. | ||
| */ | ||
| private static function sanitize_severity( $severity ) { | ||
| if ( is_array( $severity ) || is_object( $severity ) ) { | ||
| return false; | ||
| } | ||
|
|
||
| $severity = strtolower( trim( (string) $severity ) ); | ||
| if ( '' === $severity ) { | ||
| return self::DEFAULT_SEVERITY; | ||
| } | ||
|
|
||
| return isset( self::ALLOWED_SEVERITIES[ $severity ] ) ? $severity : false; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry it just occurred to me that we need this to be
truein order to be able to create custom Activity Log entries via the REST API. Probably also need to add'rest_base' => 'jp-activity-log-events'or similarThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this more, it seems there are two options:
POST /wp-json/jetpack/v4/activity-log/event, with something like permission_callback:is_signed_with_blog_token(), and callback:Activity_Log_Event::create( $args ). Benefits include not exposing the raw CPT as a public REST post type, avoiding clients needing to know about JSON in post_content, permission can be very specific, future storage can change without breaking callers. Cons could be that internal tools might need different logic for Simple site if they can't use the Jetpack endpoint.jp-activity-log-events. The pros are the simplicity, re-using WPcom REST post machinery,rest_pre_insert_jp_act_log_eventas you mentioned in Slack can normalize the payload, but cons include making the CPT REST API part of the contract, needing to be more careful about permissions, needing to define/guard read, update, delete, response shape, and backward compatibility expectations, and lastly potentially exposing more WP post semantics than we really need.So it seems as though the differentiation is whether we want to be indicating we are 'creating an Activity Log event' or 'creating a REST CPT post'.
Additionally the other decider, to me, is how third party users would add AL entries to a site. For the most part, you'd assume it would be via the new class or via
wp_insert_post. If, due to some custom integration outside of the context of a WP site, API access is needed, then an event shaped endpoint (creating an Activity Log event) still makes more sense.For the second option, I'd say that should be applied in a new PR. The existing Linear endpoint task could be re-written to focus on an endpoint that posts to the site itself (originally meant for direct AL entry creation, if we were to take that route). I have actually tested creating the endpoint locally, and adding an entry via WPcom, and it works well (both options do).
Based on this, what do you think now?