feat: content distribution class

[miguelpeixe]

1208510338230630-as-1208621948575042/f

Introduces the main class for the Network's content distribution with basic functionality for a post to be distributed across the network according to a specified configuration.

It uses Newspack Network implementation on top of data events for that, so it creates 2 new data events:

network_post_updated

Fires once a distributed post updates, with the following payload:

{
	"site_url": "https://node1.com",
	"post_id": 1,
	"config": {
		"enabled": true,
		"site_urls": [ "https://hub.com", "https://node2.com" ],
		"network_post_id": "1234567890abcdef1234567890abcdef"
	},
	"post_data": {
		"title": "Hello World",
		"date": "2021-01-01 00:00:00",
		"slug": "hello-world",
		"post_type": "post",
		"raw_content": "Content",
		"content": "<p>Content</p>",
		"excerpt": "Excerpt"
	}
}

$post_payload['config'] contains the required information for distribution:

• enabled: Whether distribution is enabled
• site_urls: Sites to distribute the post to
• network_post_id: An md5 composed of site URL and post ID to be unique across the network

The network post ID is used to match an existing linked post to perform updates:

newspack-network/includes/class-content-distribution.php

Lines 278 to 304 in 2d3f9ca

	/**
	* Get a linked post given the distributed post hash.
	*
	* @param string $post_type The post type.
	* @param string $network_post_id The network post ID for the distributed post.
	*
	* @return WP_Post|null The linked post or null if not found.
	*/
	protected static function get_linked_post( $post_type, $network_post_id ) {
	$posts = get_posts(
	[
	'post_type' => $post_type,
	'post_status' => [ 'publish', 'pending', 'draft', 'auto-draft', 'future', 'private', 'inherit', 'trash' ],
	'posts_per_page' => 1,
	'meta_query' => [ // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_query
	[
	'key' => self::NETWORK_POST_ID_META,
	'value' => $network_post_id,
	],
	],
	]
	);
	if ( empty( $posts ) ) {
	return null;
	}
	return $posts[0];
	}

network_linked_post_inserted

Fires once a linked post is updated, with the following payload:

{
	"origin": {
		"site_url": "https://node1.com",
		"post_id": 1
	},
	"destination": {
		"site_url": "https://node2.com",
		"post_id": 123,
		"is_unlinked": false
	}
}

This event is not yet used but should allow the origin site (or hub) to be aware of its linked posts across the network. This can be for UI or system purposes.

Testing

1. Make sure you have a working Network setup with at least 1 hub and 2 nodes (let's call them hub, node1, node2)
2. On node1, create a new post with various blocks (paragraph, image, quote, button, HPP...) and grab the post ID
3. Open wp shell and configure the post distribution to hub and node2 by running with the following:

$post_id = 0; // Change to your post ID.
$site_urls = [ 'http://hub.local', 'http://node2.local' ]; // Change to match your setup urls.
Newspack_Network\Content_Distribution::set_post_distribution( $post_id, $site_urls );

4. The data event is attached to wp_after_insert_post, so you can either hit "Save" on the post or trigger it manually via shell with:

Newspack_Network\Content_Distribution::distribute_post( $post_id );

5. Once all the events are dealt with, you should see the post with the correct title, date, and content in both hub and node2. Author and taxonomies are not covered in this PR (see 1208510338230630-as-1208621948575089/f and 1208510338230630-as-1208621948575091/f)
6. Let's unlink the post in node2 by running in node2's shell:

$linked_post_id = 0; // Change to node2's linked post ID
Newspack_Network\Content_Distribution::set_post_unlinked( $linked_post_id );

7. In node1, make changes to the post and save
8. Once the events are done, confirm the post has been updated in the hub but remained unchanged in node2
9. Make changes to the post in node2 and save
10. Relink the post in node2:

$linked_post_id = 0; // Change to node2's linked post ID
Newspack_Network\Content_Distribution::set_post_unlinked( $linked_post_id, false );

11. Confirm the post in node2 now reflects the most recent content from the distributed post

[miguelpeixe]

This requires Automattic/newspack-plugin#3587, which is currently up for review.

[leogermani]

double negative variables are usually pretty confusing to read. "If Unlinked = false it means that yes, it's linked".. Maybe consider having a LINKED state, with straight forward boolean value that will not be confusing

[miguelpeixe]

I explored alternatives and I'm not sure it improves readability nor maitainability. Ultimately, the post is always linked and the insert_linked_post() method is still called when "unlinked".

Having unlinked as the post meta also makes it easier to manage the different state, as we'll replace the content once the flag is removed. Having it the other way around adds some complexity that I don't think justifies the change.

Perhaps we can find a different name for unlinked that's not a negative so we can preserve the logic?

[leogermani]

Perhaps we can find a different name for unlinked that's not a negative so we can preserve the logic?

Yeah, that would be nice, but I can't think of anything good.

Not the end of the world, if you've explored alternatives and think this is more readable let's go with it

[miguelpeixe]

How about standalone?

[leogermani]

yeah... meh

What can help as well is if we had helper methods like set_post_as_linked ... is_post_linked, that would be shortcuts for the ...unlinked( ..., false) methods

[leogermani]

Maybe get the payload from handle_post_update so you have one single source of truth about the data payload. (If anything changes in that method, you might end up with discrepancies between the two ways this event is dispatched

[miguelpeixe]

Good call! Updated in b0c0a0d. In this case, we check on $data because the dispatch call does not bail on empty data, like data event listeners.

[leogermani]

When I read post-hash I was expecting something like a hash of site-url plus post ID, so we can verify that it's the same post on both ends. Since you're only doing an ID, maybe it can be called ID? Network id? Also, you can use uniqid()

[miguelpeixe]

I like that. Changed to network_post_id using an md5 of site URL and post ID in 2d3f9ca

[leogermani]

Everything works as described and the implementation is great :)

post meta is also not coming, but I guess that's still expected.

The only comment I have is that this class is geting big and multi-purpose already. For example we have the registering and handling of the data event, we have the methods to distribute a post, and we also have the methods to ingest a distributed post... I believe this would look (and grow) much better if we had a few smaller classes...

Maybe I would even create a Distributed_Post class that you could instantiate and call methods like get_processed_content(), unlink(), get_config(), etc...

[miguelpeixe]

Thanks for the review, @leogermani!

The only comment I have is that this class is geting big and multi-purpose already.

I agree, if you look at the next PRs it's about to get bigger and I'd like to split it up. Once the other PRs are merged, I'll prep a PR solely for organization purposes, with no functional changes, for easier review.

Maybe I would even create a Distributed_Post class that you could instantiate and call methods like get_processed_content(), unlink(), get_config(), etc...

Yes, some OOP might be good here. I'll see how it looks.

[naxoc]

I'm a bit late to the review party, but I have a few ideas:

• The set_post_distribution() function naming: I wonder if it would be easier to understand if it was called set_post_linked() to match the set_post_unlinked() naming?
• In set_post_distribution() – should we validate that the site_urls are valid networked sites?

[miguelpeixe]

Thanks for the comments, @naxoc!

The set_post_distribution() function naming: I wonder if it would be easier to understand if it was called set_post_linked() to match the set_post_unlinked() naming?

I think that would be the case if the method received the same boolean param to toggle link/unlinked. With set_post_distribution() we are configuring how the post distribution works.

From Leo's suggestion, I think it'll be better once we move these methods to a Distributed_Post with a set_config() method. WDYT?

In set_post_distribution() – should we validate that the site_urls are valid networked sites?

Good call! I know the hub is aware of all the nodes, but I'm not sure it's on node to nodes. If that check requires an API fetch, I'd rather not do it. I'll check!

[naxoc]

From Leo's suggestion, I think it'll be better once we move these methods to a Distributed_Post with a set_config() method. WDYT?

I like that 👍

I wrote a validation thingy in this PR: https://github.com/Automattic/newspack-network/pull/159/files (line 70-ish) – and then thought that we should maybe centralize it. I can also just do that in the PR if you are ok with that (note that the PR is work in progress).

[miguelpeixe]

That's a great idea! I'm working on a refactor for the OOP approach in #160. I can add support for it there so we do as you suggest, a single point of true for this check.