Add filter to prevent thumbnail orphan deletion

[kraftner]

What feature are you requesting?

First: I have to admit this is probably a "won't fix" since the use case is somewhat obscure so please feel free to just close this. Anyway:

Currently the routine that checks for unused thumbnails and deletes them checks for standard embeds and usage in ACF fields, but you can't add a custom check:

https://github.com/epiphyt/embed-privacy/blob/8c6143b26a99b603cc3a4eaf2dde5bfae9af012b/inc/class-thumbnails.php#L97-L104

Why is this feature necessary?

I have a rather complex integration though that sometimes replaces a shortcode* that might contain a YouTube embed. To keep things consistent I re-use the default YouTube Embed definition and manually add the Thumbnails using Thumbnails::get_instance()->set_youtube_thumbnail();.

The problem now though is that on every save of the post the thumbnail gets deleted and re-downloaded. Having a filter for missing_id in the snippet above to add a custom check if the thumbnail should be deleted would solve this for me.

Right now I either need to live with the constant re-creation or create a duplicate YouTube Embed definition just for this particular use.

---

* It's a shortcode for a H5P Interactive Video that might sometimes contain a YouTube video and only then needs to be blocked. So I manually filter the shortcode output and conditionally block it using Embed_Privacy::get_instance()->get_output_template().

Are you expecing side-effects?

Not really. The worst thing I can think of is someone misusing the filter and all orphans being kept forever.

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[MatzeKitt]

You’re right, this should be addressed since maybe other plugins aside ACF store the data elsewhere, even another postmeta field would be enough.

[kraftner]

@MatzeKitt I am really struggling to understand how this is supposed to be used. :grimacing:

If I add an action to embed_privacy_pre_thumbnail_delete_orphaned_delete the default deletion routine is fully bypassed:

https://github.com/epiphyt/embed-privacy/blob/87658b280094cb4b45cc53421a0f31d779ea7a33/inc/thumbnail/class-thumbnail.php#L123-L129

So if in my action I decide that the thumbnail is indeed orphaned and should be deleted I can do one of two things:

1. Try to recreate the deletion routine by rebuilding the variables $meta_value and $meta_key and duplicating the code from self::delete() because that is a private method I can't call from outside.
2. Or, and this is a quite awkward thing to do: Let the filter remove itself so the default routine runs just to re-add it again on embed_privacy_thumbnail_checked_orphaned for any other thumbnails being processed after this one to be handled the same.

At least those are they only ways I could come up with. So, either I need some help understand the intended use or this needs to be handled differently.

[MatzeKitt]

Yeah, that indeed seems to be a bit weird. An actual filter instead of an action would be better.

Should I also add the possibility to add the parameters for $meta_value and $meta_key? And should I makeThumbnail::delete()` a public method?

[kraftner]

Okay, glad I didn't just misunderstand something. :smile:

To answer your questions:

Personally I wouldn't make Thumbnail::delete() public and keep the actual deletion an internal function. I also wouldn't add those parameters then. I really don't need to know the internal structure of the meta fields and the filename etc as long as I have a clear interface to control the deletion process. For future maintenance you'll also have it easier if such things aren't public API surfaces.

So I'd basically rewrite the thing something like this:

$should_delete = $missing_id && $missing_url && ! self::is_in_use( $meta_value, $post_id, $global_metadata );

if ( $should_delete ) {

    $should_delete = apply_filters( 'embed_privacy_should_thumbnail_delete_orphaned_delete', $should_delete, $id, $url, $post_id, $provider );

}

if ( $should_delete ) {
    self::delete( $meta_value );
    \delete_post_meta( $post_id, $meta_key );
    \delete_post_meta( $post_id, $meta_key . '_url' );
}

[MatzeKitt]

@kraftner Please check out the latest changes. 🙂

[kraftner]

Looks good to me!

I especially like your careful work of keeping the weird default behavior in place while properly deprecating it. :+1: