WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#42030 closed enhancement (fixed)

Introduce pre_trash_post filter

Reported by: bor0 Owned by: SergeyBiryukov
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.9
Component: Posts, Post Types Keywords: has-patch
Focuses: Cc:
PR Number:

Description

In wp_delete_post we have this neat filter pre_delete_post whether to proceed with deletion. I was wondering why we don't have that for wp_trash_post, i.e. pre_trash_post.

It can cause some issues, for example in our extensions we're querying custom post types based on their trash status, and there are cases where we don't want to allow them to be trashed (e.g. they have another post type that links to the to-be-trashed one). However, once they're trashed, our queries based on their trash status are erroneous.

Another point that was made by Marius Jensen on #core:

I see no issue with introducing it to maintain parity with the delete feature (trashing is a soft delete after all)

Attachments (4)

42030.patch (620 bytes) - added by bor0 2 years ago.
42030.2.patch (2.4 KB) - added by bor0 2 years ago.
42030.3.patch (2.5 KB) - added by bor0 2 years ago.
Can't use wp_slash on object
42030.4.patch (5.3 KB) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (16)

@bor0
2 years ago

#1 @bor0
2 years ago

  • Keywords has-patch added

#2 @bor0
2 years ago

@SergeyBiryukov can you triage this? Thanks!

#3 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 4.9
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#4 @SergeyBiryukov
2 years ago

  • Component changed from General to Posts, Post Types

#5 @SergeyBiryukov
2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 41638:

Posts, Post Types: Introduce pre_trash_post and pre_untrash_post filters to allow for short-circuiting wp_trash_post() and wp_untrash_post().

This brings parity with pre_delete_post filter in wp_delete_post(), introduced in [34082].

Props bor0.
Fixes #42030.

#6 follow-up: @bor0
2 years ago

@SergeyBiryukov little nitpick, we use:
$post_trash = get_post( $id, ARRAY_A ); for wp_trash_post
and
$post_delete = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM $wpdb->posts WHERE ID = %d", $id ) ); for wp_delete_post

Thus, we have type array in wp_trash_post, and type WP_Post in wp_delete_post.

I figured it would be easy to just improves the comment on the filter, but maybe even better to bring a little consistency between wp_trash_post and wp_delete_post.

If you think the proposed patch has a high impact, we can just go ahead and update the filter comments on wp_trash_post and wp_untrash_post to say array instead of WP_Post.

@bor0
2 years ago

#7 @SergeyBiryukov
2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@bor0
2 years ago

Can't use wp_slash on object

#8 @ramiy
2 years ago

Why not using pre_trash_{$post_type}?

#9 in reply to: ↑ 6 @SergeyBiryukov
2 years ago

Replying to bor0:

little nitpick, we use:
$post_trash = get_post( $id, ARRAY_A ); for wp_trash_post
and
$post_delete = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM $wpdb->posts WHERE ID = %d", $id ) ); for wp_delete_post

Thus, we have type array in wp_trash_post, and type WP_Post in wp_delete_post.

Good catch. Luckily, wp_trash_post() already has WP_Post as one of the documented return values, as it falls back to wp_delete_post() if Trash is disabled. wp_untrash_post() also has WP_Post as a return value, which is not exactly accurate at the moment. Both function only pass the post ID to their hooks, not an array or object, so the internal representation should be relatively safe to change.

On a related note, wp_delete_post() has WP_Post as a documented return value, but returns a regular object instead. Same for pre_delete_post filter, $post parameter type is inaccurate there.

I think it makes sense to standardize on WP_Post here.

Replying to ramiy:

Why not using pre_trash_{$post_type}?

This was meant to complement the existing pre_delete_post filter added in [34082]. There is no corresponding pre_delete_{$post_type} filter, it can be explored in a new ticket though.

#10 @SergeyBiryukov
2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 41642:

Posts, Post Types: In wp_delete_post(), wp_trash_post(), wp_untrash_post(), and wp_delete_attachment(), standardize on WP_Post as a return value and internal representation of the post data.

Props bor0, SergeyBiryukov.
Fixes #42030.

#11 @SergeyBiryukov
2 years ago

In 41644:

Posts, Post Types: Correct test_submitting_comment_to_trashed_post_returns_error().

wp_trash_post() accepts a post ID, not a WP_Post object.

See #42030.

#12 @westonruter
2 years ago

In 41824:

Customize: Introduce WP_Customize_Manager::trash_changeset_post() to reduce duplication and ensure proper changeset trashing logic.

Trashing a changeset via wp_trash_post() does not have the desired result since it mutates post_content (via Kses) and the post_name (even though it is a UUID).

Props dlh.
See #39896, #42030.
Fixes #42175.

Note: See TracTickets for help on using tickets.