Skip to content
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

feat(content-distribution): partial payload #205

Merged
merged 13 commits into from
Jan 29, 2025

Conversation

miguelpeixe
Copy link
Member

@miguelpeixe miguelpeixe commented Jan 28, 2025

Implements support for distribution of partial payloads, given a list of post data keys to include in the payload.

The Content_Distribution::distribute_post_partial( $post, $post_data_keys ) can be called directly to trigger a partial update, but it should be most commonly used in the context of distribute_queued_posts(), which will decide on PHP shutdown whether to execute a partial or full distribution of a given post, based on the contents of the $queued_distributions var, populated via the queue_post_distribution( $post_id, $post_data_key ); method.

E.g., if a post is saved and a post meta is updated in the same execution, the following will be called:

Content_Distribution::queue_post_distribution( <post_id> ); // via wp_after_insert_post hook
Content_Distribution::queue_post_distribution( <post_id>, 'post_meta' ); // via updated_post_meta hook

The above will trigger a single full post update on shutdown.

If only a post meta is updated on the execution:

Content_Distribution::queue_post_distribution( <post_id>, 'post_meta' ); // via updated_post_meta hook

A single partial update will run on shutdown.

This PR also improves the post meta coverage by hooking into added_post_meta and deleted_post_meta.

Testing

  1. Distribute a post to a node
  2. Update a post meta via CLI wp post meta update {post_id} test_meta test_value
  3. Confirm in the hub's event log that the payload's post data only includes post_meta, date_gmt, and modified_gmt
  4. Delete the post meta via CLI wp post meta delete {post_id} test_meta
  5. Confirm in the hub's event log table that the payload includes the post_meta and test_meta is not there
  6. In the dashboard, edit the post and save
  7. Confirm in the hub's event log that the full payload is sent

@miguelpeixe miguelpeixe self-assigned this Jan 28, 2025
@miguelpeixe miguelpeixe marked this pull request as ready for review January 28, 2025 22:00
@miguelpeixe miguelpeixe requested a review from a team as a code owner January 28, 2025 22:00
Copy link
Member

@naxoc naxoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this and it works great. Thanks!

I only added a small nit on return type.

$current_payload = $this->get_post_payload();
$current_payload_error = self::get_payload_error( $current_payload );
if ( is_wp_error( $current_payload_error ) ) {
return $current_payload_error->get_error_message();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error should be returned here I think –not a string.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Fixed in ac91081

@miguelpeixe miguelpeixe merged commit b844d06 into trunk Jan 29, 2025
4 checks passed
@miguelpeixe miguelpeixe deleted the feat/content-distribution-partial-payload branch January 29, 2025 12:13
Copy link

Hey @miguelpeixe, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants