Make WordPress Core

Opened 9 months ago

Closed 8 months ago

Last modified 8 months ago

#58392 closed enhancement (fixed)

Add $post parameter to the wp_trash_post action.

Reported by: mujuonly's profile mujuonly Owned by: audrasjb's profile audrasjb
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Posts, Post Types Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

The $post variable is already available in the wp_trash_post() function so it can just be passed.

Should be the same for the below functions as well to maintain the consistency

trashed_post
untrash_post
untrashed_post

Attachments (2)

58392.diff (1.4 KB) - added by dhruvishah2203 9 months ago.
diff file uploaded
58392.2.diff (3.1 KB) - added by SergeyBiryukov 9 months ago.

Download all attachments as: .zip

Change History (22)

This ticket was mentioned in PR #4499 on WordPress/wordpress-develop by @mujuonly.


9 months ago
#1

  • Keywords has-patch added

Add $post parameter to wp_trash_post action

Trac ticket: https://core.trac.wordpress.org/ticket/58392

#2 @mukesh27
9 months ago

  • Keywords changes-requested added

Hi there! thanks for the ticket and PR.

Left one minor feedback.

#3 @nihar007
9 months ago

  • Keywords dev-feedback added; has-patch changes-requested removed

Hi,
I checked the source code and I saw trashed_post, untrash_post, untrashed_post methods require post id not post and used multiple places. I think post id is the best.

@dhruvishah2203
9 months ago

diff file uploaded

#4 @SergeyBiryukov
9 months ago

  • Milestone changed from Awaiting Review to 6.3

Hi there, thanks for the ticket!

Taking a closer look here:

  • pre_trash_post receives $post.
  • wp_trash_post receives $post_id.
  • trashed_post receives $post_id.
  • pre_untrash_post receives $post and $previous_status.
  • untrash_post receives $post_id and $previous_status.
  • untrashed_post receives $post_id and $previous_status.

See [49125] / #23022 for reference.

It appears that passing $previous_status to the former three hooks would bring more consistency, so 58392.2.diff does just that.

Would that be enough here, or do we still need to pass $post to wp_trash_post and untrash_post? Could you share the use case for that?

This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.


9 months ago

#6 @costdev
9 months ago

  • Keywords needs-unit-tests added

This ticket was discussed during the bug scrub. As this is new code and to ensure that the parameters are always passed in future, we agreed that this should have a unit test to verify the expected parameters are received.

Additional props: @mukesh27 @oglekler

#7 @oglekler
9 months ago

  • Keywords has-patch added

This ticket was mentioned in Slack in #core by oglekler. View the logs.


9 months ago

This ticket was mentioned in PR #4580 on WordPress/wordpress-develop by @hugod.


9 months ago
#9

  • Keywords has-unit-tests added; needs-unit-tests removed

Trac ticket: https://core.trac.wordpress.org/ticket/58392

This PR adds unit tests to the previously proposed diff.

#10 @hugod
9 months ago

Hi,
I added tests in https://github.com/WordPress/wordpress-develop/pull/4580.
Tell me if it lacks of anything.

This ticket was mentioned in Slack in #core by hugod. View the logs.


9 months ago

#12 @audrasjb
9 months ago

Thanks for the unit tests @hugod!
I added some feedback in the PR :)

@hugod commented on PR #4580:


9 months ago
#13

Hi @audrasjb and @costdev 👋
Thanks for the feedback!
I've split the test and added some docblocks as you suggested.

@hugod commented on PR #4580:


9 months ago
#14

Thanks for the insight @costdev! Nice to see how standardization is going 🙂

#15 @audrasjb
9 months ago

  • Owner set to audrasjb
  • Status changed from new to reviewing

Self assigning for final review.

This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.


9 months ago

#17 @oglekler
8 months ago

  • Keywords needs-testing added; dev-feedback removed

I am adding needs-testing to highlight that this patch is ready for review.

#18 @audrasjb
8 months ago

  • Keywords needs-testing removed

#19 @audrasjb
8 months ago

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

In 56043:

Posts, Post Types: Add a $previous_status parameter to wp_trash_post() related hooks.

This adds a $previous_status parameter to the pre_trash_post, wp_trash_post, and trashed_post hooks.

Props mujuonly, mukesh27, nihar007, dhruvishah2203, SergeyBiryukov, costdev, hugod, audrasjb, oglekler.
Fixes #58392.

Note: See TracTickets for help on using tickets.