Make WordPress Core

Opened 7 years ago

Last modified 16 months ago

#41977 assigned enhancement

media_handle_upload() un required second parameter

Reported by: tkama's profile Tkama Owned by: antpb's profile antpb
Milestone: Future Release Priority: normal
Severity: normal Version: 2.5
Component: Media Keywords:
Focuses: Cc:

Description

Why second parameter of media_handle_upload( $file_id, $post_id ) is required?

It's better to define it as 0 and make it optional. In some cases it's more convenient! And we lose nothing if do so...

Attachments (1)

41977.diff (1.4 KB) - added by costdev 16 months ago.
Make the $post_id parameter optional. Default 0.

Download all attachments as: .zip

Change History (9)

#1 @desrosj
5 years ago

  • Component changed from General to Media
  • Keywords close 2nd-opinion added
  • Version changed from 4.8.2 to 2.5

Hi @Tkama,

I don't know that there's any real benefit to making this change. Future function calls would no longer require the parameter, but requiring the parameter also requires the developer to make a conscious decision about where the image should be attached, if at all.

Marking as a close candidate, but also for a 2nd-opinion.

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


18 months ago

#3 follow-up: @antpb
18 months ago

  • Milestone changed from Awaiting Review to 6.2
  • Owner set to antpb
  • Status changed from new to assigned

Moving this to 6.2 as this change improves the ease of using the function with little to no potential issues from setting default to 0.

This change should save a developer from having to read the docs to figure out why the function is failing.

#4 @desrosj
17 months ago

  • Keywords close 2nd-opinion removed

Removing the close keyword since @antpb has volunteered to own this one.

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


16 months ago

@costdev
16 months ago

Make the $post_id parameter optional. Default 0.

#6 in reply to: ↑ 3 @azaozz
16 months ago

Replying to antpb:

This change should save a developer from having to read the docs...

Actually I tend to agree with @desrosj comment from 4 years ago:

requiring the parameter also requires the developer to make a conscious decision about where the image should be attached

WP adds context to images and other uploads by saving the post they were uploaded to. This context can be used for many things like permission settings, etc.

Imho would be nice to remind developers to add the post_ID for new uploads if possible. Perhaps this ticket can be repurposed to expand the docs for media_handle_upload() and explain in more details why it is good to add $post_id when possible.

Last edited 16 months ago by azaozz (previous) (diff)

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


16 months ago

#8 @costdev
16 months ago

  • Milestone changed from 6.2 to Future Release

This ticket was discussed during the bug scrub. As this ticket still needs some work and 6.2 Beta 1 is being released today, I'll move this to Future Release.

As the next step seems to be a docs-only change, if this is ready later in the cycle, feel free to pull it back into the milestone.

Additional props: @mukesh27

Note: See TracTickets for help on using tickets.