WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 2 days ago

#50972 reviewing defect (bug)

media_handle_sideload() does not allow $post_data to override the post_date that gets used by wp_upload_dir

Reported by: jamesgol Owned by: SergeyBiryukov
Milestone: 5.7 Priority: normal
Severity: minor Version: trunk
Component: Media Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

Now that $post_id is an optional attribute to media_handle_sideload() it should be possible to override the $time passed to wp_handle_sideload() via the optional $post_data array.

My use case is calling media_handle_sideload() via a custom plugin, only after the existing library is checked for duplicates is the post actually created so there is no $post_id to reference (yet).

Attachments (2)

media_handle_sideload_date.diff (430 bytes) - added by jamesgol 3 months ago.
50972.patch (435 bytes) - added by mukesh27 3 months ago.
Updated patch.

Download all attachments as: .zip

Change History (10)

#1 @mukesh27
3 months ago

#50971 was marked as a duplicate.

#2 @SergeyBiryukov
3 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 5.6
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#3 @mukesh27
3 months ago

Hi there!

isset( $post_data ) value is always true when it was empty array or non empty array.

Update the patch without checking it.

@mukesh27
3 months ago

Updated patch.

#4 @jamesgol
3 months ago

I always assume that someone's going to do something wrong like try to pass in null or false as an argument. So in my opinion it's safer to at least check. One step further would be a check for is_array().

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


9 days ago

#6 @hellofromTonya
9 days ago

  • Keywords needs-unit-tests added

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


2 days ago

#8 @antpb
2 days ago

  • Milestone changed from 5.6 to 5.7

It was mentioned in the recent Media meeting that the sideloading functions could benefit from a refactor to allow testing as it is currently hard to test the same way that media_sideload_image was and did not receive tests.

Being so late in beta cycle it's probably best to move this out of the 5.6 milestone. If anyone feels differently please feel free to move it back in.

Note: See TracTickets for help on using tickets.