Make WordPress Core

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: oglekler's profile oglekler 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

Trac ticket: #63667

#2 @gulamdastgir04
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.

Last edited 4 months ago by gulamdastgir04 (previous) (diff)

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 @SirLouen
4 months ago

  • Keywords has-test-info dev-feedback added
  • Version set to 2.9

Testing Information

  1. Create a new Post
  2. Add any Oembed, for example a YouTube video. You can add 2 if you want to play around
  3. Save the post
  4. Go to the database, table wp_postmeta and check for the last meta_key created that should be like _oembed_<long_md5_id> for example: _oembed_9b8701b5644e2d4823a2beee2049cd0e
  5. Now open that new post again
  6. Remove the oembed and save again
  7. Before the patch, the previous _oembed_ key should be there. After the past, it should be gone.
  8. 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_cache instead of directly to cache_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 @SirLouen
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.

Note: See TracTickets for help on using tickets.