Opened 4 months ago
Last modified 4 months ago
#63667 new defect (bug)
Delete oEmbed postmeta if the related embed block is deleted / no longer present in the content
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Awaiting Review | Priority: | normal |
| Severity: | normal | Version: | 2.9 |
| Component: | Embeds | Keywords: | has-patch has-unit-tests has-test-info dev-feedback |
| Focuses: | Cc: |
Description
While the post is updated/saved, oEmbed cache can be evaluated and updated.
I want to use information about embeds, and it is much more convenient to use post meta than parsing blocks.
Change History (5)
This ticket was mentioned in PR #9203 on WordPress/wordpress-develop by @sainathpoojary.
4 months ago
#1
- Keywords has-patch has-unit-tests added
#2
@
4 months ago
This ticket should go live as soon as possible.
I’m working on a site that contains a large amount of unnecessary embed data, which I need to clean up. This can be done either via a plugin or manually using the following function:
WP_Embed::delete_oembed_caches( int $post_id )
I’ll test this patch and share an update shortly.
This ticket was mentioned in PR #9224 on WordPress/wordpress-develop by @SirLouen.
4 months ago
#3
I've been reviewing PR #9203, but maybe I'm wrong, but I feel it's too convoluted, so a full review won't be possible. After further reviewing, I think that caches should be refreshed after each update or delete action, which include first a deletion before regeneration. I'm taking the already existing function delete_oembed_caches although I'm not 100% confident if maybe a more targeted function rather than this should be added for this purpose.
Trac ticket: https://core.trac.wordpress.org/ticket/63667
#4
@
4 months ago
- Keywords has-test-info dev-feedback added
- Version set to 2.9
Testing Information
- Create a new Post
- Add any Oembed, for example a YouTube video. You can add 2 if you want to play around
- Save the post
- Go to the database, table
wp_postmetaand check for the lastmeta_keycreated that should be like_oembed_<long_md5_id>for example:_oembed_9b8701b5644e2d4823a2beee2049cd0e - Now open that new post again
- Remove the oembed and save again
- Before the patch, the previous
_oembed_key should be there. After the past, it should be gone. - For extra testing, if you added two oembeds, the second one that you did not delete should still be there.
Additional Information
- I have done a little refactoring. After some extra review, I can't really see why in [12023] it was chosen to hook to
maybe_run_ajax_cacheinstead of directly tocache_oembed. It's a way longer route without any clear advantage (given that the whole code is already being executed PHP level), but it can bring some troubles.
Needs further review.
cc @peterwilsoncc
#5
@
4 months ago
More info
After further testing by adding and editing new embeds, I've not really been able to get pass through this line. It's true that I've been only playing with Gutenberg. Maybe with Classic Editor there is a different song.
What I've been trying to figure out is the utility of maybe_run_ajax_cache with this message parameter supposedly received somehow because I cannot see other references in the code and I can't really trigger it to debug what it could be receiving.
This oEmbed logic has been like this since [12023] completely unchanged. I can't find anything enlightening reading through #10337 either. In fact, this was first introduced in this patch in that ticket and all the information we have about this decision is:
Move oEmbed caching to an AJAX request after the post is saved
@azaozz was there 16 years ago. Possibly, he can figure out what is this all about.
For now, preemptively, I'm going to leave untouched both calls to this AJAX thing, in case there is some backwards compatibility topic with Classic Editor, but for the current status of the oEmbed, I can't really see the utility of calling such function, instead of calling directly cache_oembed which is the function that maybe_run_ajax_cache ends calling after those unknown checks.
Trac ticket: #63667