feat(content-distribution): handle post thumbnail #157

miguelpeixe · 2024-12-04T15:25:36Z

Add support for syncing the post thumbnail when distributing content.

Testing

Make sure you have a network setup with at least 1 hub and 1 node
On the hub, create a new post with a featured image, grab the post ID, and use the WP shell to distribute to the node:

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

Visit the media library and confirm the post thumbnail was created
Confirm the post is added as a draft and the thumbnail is assigned
In the original post, change the featured image and save
Visit the media library in node1 and confirm that the previous featured image has been deleted and the new one was created
Confirm the new featured image was also updated in the post
In the original post, delete the featured image and save
Back in node1, confirm in the media library that the featured image has been deleted
Confirm in the linked post that no featured image is assigned

miguelpeixe · 2024-12-06T13:16:55Z

includes/class-content-distribution.php

+		// Delete existing thumbnail attachment.
+		if ( $current_thumbnail_id ) {
+			wp_delete_attachment( $current_thumbnail_id, true );
+		}


When syncing the post thumbnail, whether the distributed post is updating the featured image or deleting it, the destination site will permanently delete the orphan image.

I'm not sure if this is too aggressive, but I also don't like the idea of lingering orphan images.

The current approach can be problematic if:

The site starts using the synced attachment in other content

The site unlinked the post, changed the featured image to something local, and re-linked the post - deleting their local attachment

We can prevent the last one above by adding a meta to the synced attachment so only those get deleted.

It's probably a pretty expensive query, but do you think it's worth checking if other posts are using the image before deleting it?

There is also the use case that the image is used in the post body.

I've decided to not delete the attachment for now. Instead, we set a post meta to not lose track of the attachments that came via distribution and we can decide later how to deal with them. WDYT? e5d1418

I think a post content query is not viable, even if executed via cron. It's very expensive...

I think that makes sense. I like the idea with the postmeta. Better have a bit too much media than media missing.

…on-featured-image

naxoc

This looks good and tests well. (The unit-test is broken, but that is from the updated-date-PR I think).

I added some comments about the image deletion though. I think there are use cases we can get in trouble with.

naxoc · 2024-12-09T15:05:59Z

includes/class-content-distribution.php

+		// Delete existing thumbnail attachment.
+		if ( $current_thumbnail_id ) {
+			wp_delete_attachment( $current_thumbnail_id, true );
+		}


It's probably a pretty expensive query, but do you think it's worth checking if other posts are using the image before deleting it?

There is also the use case that the image is used in the post body.

naxoc · 2024-12-09T15:08:52Z

includes/class-content-distribution.php

+
+		$attachment_id = media_sideload_image( $thumbnail_url, $post_id, '', 'id' );
+		if ( is_wp_error( $attachment_id ) ) {
+			return;


A silent error here is probably fine, so this is a question more than a request – but do we have a way to log stuff like this?

Good call! We have a Debugger class, added a log in 23b93e6

miguelpeixe · 2024-12-09T18:53:46Z

Thank you for the review, @naxoc!

I've addressed the comments and fixed the tests in 9c7212b

naxoc

Good to go!

* feat: content distribution class * feat: insert linked post * refactor: use site url instead of ID * chore: lint * feat: return errors on post insert * chore: lint * feat: support webhooks request priority * feat: unlink functionality and unit tests * chore: lint * feat: introduce 'linked_post_inserted' hook and listener * chore: lint * fix: listener hook name * fix: typo * chore: better docblocks * chore: remove redundant code * test: persist post hash * test: remove unnecessary assertion * refactor: distribute_post() to use handle_post_updated() method * refactor: post hash is now network post id * chore: update comment * chore: Add CLI command for distribute * feat: content distribution class (#155) * feat: content distribution class * feat: insert linked post * refactor: use site url instead of ID * chore: lint * feat: return errors on post insert * chore: lint * feat: support webhooks request priority * feat: unlink functionality and unit tests * chore: lint * feat: introduce 'linked_post_inserted' hook and listener * chore: lint * fix: listener hook name * fix: typo * chore: better docblocks * chore: remove redundant code * test: persist post hash * test: remove unnecessary assertion * refactor: distribute_post() to use handle_post_updated() method * refactor: post hash is now network post id * chore: update comment * feat(content-distribution): prevent older content updating linked posts (#156) * feat(content-distribution): handle post thumbnail (#157) * Add network util class * Almost done except for a few todos * refactor: OOP for distributed and linked posts (#160) * chore: Require php 8.1 * chore: Update to use util class Also use new incoming/outgoing classes * fix: Include correct classname The old classname "Linked_Post" was still used instead of "Incoming_Post" causing sync to fail. ## How to test Probably eyeballing is enough * chore: Add validation of outgoing post Also move inclusion of the content distribution CLI class to better respect the flag we are introdocuing * chore: phpcs * fix(content-distribution): debug post update and remove deprecated config (#165) * fix: Don't return payload on all posts * fix: post handling * chore: move try-catch * chore: Move check for networked urls * Move more * Move things again * chore: Add a mock networked node * chore: add one more node to the mock nodes * Add a test --------- Co-authored-by: Miguel Peixe <miguel.peixe@automattic.com>

# [2.4.0-alpha.1](v2.3.4...v2.4.0-alpha.1) (2025-01-10) ### Bug Fixes * **content-distribution:** post insertion hook and additional meta for incoming post event ([#173](#173)) ([48df13c](48df13c)) * load text domain on init hook ([#171](#171)) ([01fb89c](01fb89c)) ### Features * content distribution - experimental ([#168](#168)) ([dc837d8](dc837d8)) * **content-distribution:** add CLI command for distribute post ([#159](#159)) ([7a43b86](7a43b86)), closes [#155](#155) [#156](#156) [#157](#157) [#160](#160) [#165](#165) * **content-distribution:** canonical url ([#177](#177)) ([5ca60ce](5ca60ce)) * **content-distribution:** capability and admin page ([#176](#176)) ([5285285](5285285)) * **content-distribution:** control distribution meta and prevent multiple dispatches ([#170](#170)) ([e76a2dc](e76a2dc)) * **content-distribution:** editor plugin for distribution ([#167](#167)) ([e10aef4](e10aef4)) * **content-distribution:** handle status changes ([#166](#166)) ([4af5da1](4af5da1)) * **content-distribution:** log incoming post errors ([#182](#182)) ([74c9119](74c9119)) * **content-distribution:** posts column ([#178](#178)) ([8e07640](8e07640)) * **content-distribution:** reserved taxonomies ([#174](#174)) ([a2c54d2](a2c54d2)) * **content-distribution:** sync comment and ping statuses ([#179](#179)) ([90c5425](90c5425)) * **content-distribution:** sync post meta ([#163](#163)) ([353a3d8](353a3d8)) * **event-log:** collapse data ([#180](#180)) ([956219d](956219d)) * limit purchase of a network membership ([#169](#169)) ([deb2683](deb2683))

# [2.4.0](v2.3.4...v2.4.0) (2025-01-20) ### Bug Fixes * **content-distribution:** post insertion hook and additional meta for incoming post event ([#173](#173)) ([48df13c](48df13c)) * load text domain on init hook ([#171](#171)) ([01fb89c](01fb89c)) ### Features * content distribution - experimental ([#168](#168)) ([dc837d8](dc837d8)) * **content-distribution:** add CLI command for distribute post ([#159](#159)) ([7a43b86](7a43b86)), closes [#155](#155) [#156](#156) [#157](#157) [#160](#160) [#165](#165) * **content-distribution:** canonical url ([#177](#177)) ([5ca60ce](5ca60ce)) * **content-distribution:** capability and admin page ([#176](#176)) ([5285285](5285285)) * **content-distribution:** control distribution meta and prevent multiple dispatches ([#170](#170)) ([e76a2dc](e76a2dc)) * **content-distribution:** editor plugin for distribution ([#167](#167)) ([e10aef4](e10aef4)) * **content-distribution:** handle status changes ([#166](#166)) ([4af5da1](4af5da1)) * **content-distribution:** log incoming post errors ([#182](#182)) ([74c9119](74c9119)) * **content-distribution:** posts column ([#178](#178)) ([8e07640](8e07640)) * **content-distribution:** reserved taxonomies ([#174](#174)) ([a2c54d2](a2c54d2)) * **content-distribution:** sync comment and ping statuses ([#179](#179)) ([90c5425](90c5425)) * **content-distribution:** sync post meta ([#163](#163)) ([353a3d8](353a3d8)) * **event-log:** collapse data ([#180](#180)) ([956219d](956219d)) * limit purchase of a network membership ([#169](#169)) ([deb2683](deb2683))

miguelpeixe added 24 commits November 28, 2024 12:24

feat: content distribution class

9997fd5

feat: insert linked post

7a5153d

refactor: use site url instead of ID

4230692

chore: lint

0561d77

feat: return errors on post insert

e1aee3f

chore: lint

6d5e931

feat: support webhooks request priority

34dcbf3

feat: unlink functionality and unit tests

5e47bbe

chore: lint

ca10658

feat: introduce 'linked_post_inserted' hook and listener

136ed6a

chore: lint

7738de3

fix: listener hook name

6095800

fix: typo

699228a

chore: better docblocks

0bf9c37

chore: remove redundant code

20d5dc0

test: persist post hash

fb501d4

test: remove unnecessary assertion

548a571

refactor: distribute_post() to use handle_post_updated() method

b0c0a0d

refactor: post hash is now network post id

2d3f9ca

chore: update comment

78c7f01

feat(content-distribution): handle post thumbnail

c05d5fa

refactor: use media_sideload_image function

4741f7a

test: thumbnail insertion, update and removal

90745b6

test: simplify tests

b04fcfa

miguelpeixe commented Dec 6, 2024

View reviewed changes

miguelpeixe self-assigned this Dec 6, 2024

miguelpeixe marked this pull request as ready for review December 6, 2024 13:18

miguelpeixe requested a review from a team as a code owner December 6, 2024 13:18

miguelpeixe added the [Status] Needs Review label Dec 6, 2024

miguelpeixe requested a review from naxoc December 6, 2024 13:18

Base automatically changed from feat/content-distribution-class to epic/content-distribution December 9, 2024 11:52

miguelpeixe added 2 commits December 9, 2024 08:55

Merge branch 'epic/content-distribution' into feat/content-distributi…

cb072cf

…on-featured-image

Merge branch 'epic/content-distribution' into feat/content-distributi…

4d861db

…on-featured-image

naxoc reviewed Dec 9, 2024

View reviewed changes

miguelpeixe added 3 commits December 9, 2024 14:45

test: fix tests after merge

9c7212b

fix: do not delete attachment and set meta

e5d1418

fix: add debugger log for failed attachment

23b93e6

miguelpeixe requested a review from naxoc December 9, 2024 18:53

naxoc approved these changes Dec 9, 2024

View reviewed changes

miguelpeixe merged commit f2de08c into epic/content-distribution Dec 9, 2024

miguelpeixe deleted the feat/content-distribution-featured-image branch December 9, 2024 21:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(content-distribution): handle post thumbnail #157

feat(content-distribution): handle post thumbnail #157

Uh oh!

miguelpeixe commented Dec 4, 2024 •

edited

Loading

Uh oh!

miguelpeixe Dec 6, 2024

Uh oh!

naxoc Dec 9, 2024

Uh oh!

miguelpeixe Dec 9, 2024 •

edited

Loading

Uh oh!

naxoc Dec 9, 2024

Uh oh!

naxoc left a comment

Uh oh!

naxoc Dec 9, 2024

Uh oh!

naxoc Dec 9, 2024

Uh oh!

miguelpeixe Dec 9, 2024

Uh oh!

miguelpeixe commented Dec 9, 2024

Uh oh!

naxoc left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat(content-distribution): handle post thumbnail #157

feat(content-distribution): handle post thumbnail #157

Uh oh!

Conversation

miguelpeixe commented Dec 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Uh oh!

miguelpeixe Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

naxoc Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

miguelpeixe Dec 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

naxoc Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

naxoc left a comment

Choose a reason for hiding this comment

Uh oh!

naxoc Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

naxoc Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

miguelpeixe Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

miguelpeixe commented Dec 9, 2024

Uh oh!

naxoc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

miguelpeixe commented Dec 4, 2024 •

edited

Loading

miguelpeixe Dec 9, 2024 •

edited

Loading