Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#37813 closed enhancement (fixed)

Missing $wp_error parameter for wp_insert_attachment

Reported by: grapplerulrich's profile grapplerulrich Owned by: johnbillion's profile johnbillion
Milestone: 4.7 Priority: normal
Severity: normal Version: 2.0
Component: Media Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

wp_insert_attachment() uses wp_insert_post() which has a parameter whether to allow return of WP_Error on failure.

It is not possible to do the same for wp_insert_attachment().

This would be my suggested patch.

<?php
function wp_insert_attachment( $args, $file = false, $parent = 0, $wp_error = false ) {
    $defaults = array(
        'file'        => $file,
        'post_parent' => 0
    );
 
    $data = wp_parse_args( $args, $defaults );
 
    if ( ! empty( $parent ) ) {
        $data['post_parent'] = $parent;
    }
 
    $data['post_type'] = 'attachment';
 
    return wp_insert_post( $data, $wp_error );
}

Related: #32318

Attachments (4)

37813.patch (1.2 KB) - added by grapplerulrich 8 years ago.
37813.diff (1.2 KB) - added by mrahmadawais 8 years ago.
Enhancement: wp_insert_attachment can now return WP_Error on failure
37813.1.patch (1.4 KB) - added by grapplerulrich 8 years ago.
37813.2.patch (2.8 KB) - added by johnbillion 8 years ago.

Download all attachments as: .zip

Change History (18)

#1 @johnbillion
8 years ago

  • Focuses administration removed
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.7
  • Type changed from defect (bug) to enhancement

#2 @wonderboymusic
8 years ago

  • Keywords good-first-bug added

#3 @grapplerulrich
8 years ago

  • Keywords has-patch added; needs-patch removed

#4 @DrewAPicture
8 years ago

  • Owner set to grapplerulrich
  • Status changed from new to assigned

Assigning to mark the good-first-bug as "claimed".

@mrahmadawais
8 years ago

Enhancement: wp_insert_attachment can now return WP_Error on failure

#5 @mrahmadawais
8 years ago

  • Keywords needs-testing added

Oh, looks like I forgot to upload and when I did, there was already a patch. Anywho, I added the docs as well. Looks, like @grapplerulrich your patch has some issue as well. It's removing what you intend to add. Test my patch to see what I am talking about.

#6 follow-up: @grapplerulrich
8 years ago

I have updated my patch. I followed the docs in https://make.wordpress.org/core/handbook/contribute/git/
Could there be a mistake there in step 4?

#7 in reply to: ↑ 6 @mrahmadawais
8 years ago

Replying to grapplerulrich:

I have updated my patch. I followed the docs in https://make.wordpress.org/core/handbook/contribute/git/
Could there be a mistake there in step 4?

I am using SVN diff at the moment. Will check that later.

@since 4.7.0 Add fourth paramater `$wp_error` to allow  return of WP_Error on failure.

@since is used to depict when was this function introduced. It's not a changelog. So, the line above should not be there. Take a look at my patch https://core.trac.wordpress.org/attachment/ticket/37813/37813.diff and see if there is anything wrong with it. You can use Grunt WP patch https://github.com/aaronjorbin/grunt-patch-wordpress to apply the patch.

Let me know.

Last edited 8 years ago by mrahmadawais (previous) (diff)

#8 follow-up: @grapplerulrich
8 years ago

I have been asked in the past to add @since to be used as a changelog. e.g. https://core.trac.wordpress.org/ticket/37076#comment:4

#9 in reply to: ↑ 8 @mrahmadawais
8 years ago

  • Status changed from assigned to closed

Replying to grapplerulrich:

I have been asked in the past to add @since to be used as a changelog. e.g. https://core.trac.wordpress.org/ticket/37076#comment:4

You are right. I was not aware of this. I think it is a new change. https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#since-section-changelogs

#10 follow-up: @johnbillion
8 years ago

  • Status changed from closed to reopened

@mrahmadawais Tickets remain open until they've been committed to core. Thanks!

#11 @johnbillion
8 years ago

  • Keywords needs-testing removed
  • Owner changed from grapplerulrich to johnbillion
  • Status changed from reopened to reviewing

@johnbillion
8 years ago

#12 @johnbillion
8 years ago

  • Keywords has-unit-tests added; good-first-bug removed

37813.2.patch improves the phrasing and adds a unit test.

#13 @johnbillion
8 years ago

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

In 38408:

Media: Add a $wp_error parameter to wp_insert_attachment() to give it parity with wp_insert_post().

Fixes #37813
Props grapplerulrich, mrahmadawais

#14 in reply to: ↑ 10 @mrahmadawais
8 years ago

Replying to johnbillion:

@mrahmadawais Tickets remain open until they've been committed to core. Thanks!

Sorry about that. I just wrote the comment and pressed submit. Must have mistakenly clicked at the closed option. My bad! Thanks for the props!

Note: See TracTickets for help on using tickets.