Unnecessary default category code remains in wp_insert_attachment

[jeremyfelt]

After the default arguments are set, post_type is forced to 'attachment', negating any requirement for default categories to be added.

Quick patch removes:

(1) The test performed for 'post' post_type in order to assign default categories to an array. Result was always an empty array.

(2) The call to wp_set_post_categories with this empty array, which sends the post_ID through a bunch of taxonomy related code that passes over it. This does cause the action 'set_object_terms' to be skipped, which should be okay as attachments don't have terms.

[jeremyfelt]

Clarification on the 'set_object_terms' action. It's possible that a plugin could use this to manage taxonomy on attachments as they are uploaded, so it may be useful to add the same do_action that would normally be run to the patch to replace the removed wp_set_post_categories.

do_action('set_object_terms', $object_id, $terms, $tt_ids, $taxonomy, $append, $old_tt_ids);

Would obviously need to change the arguments a bit.

I can redo the patch if it's determined that this action hook is necessary.

[dd32]

Attachments have been added to the default taxonomy for quit awhile now, there's been previous tickets on this as well. From memory, the basic consensus was that some plugins that categorize attachments will expect the default category to be added, and it remains consistent with being backwards compatible.

[jeremyfelt]

I may be confused, and this patch was a result of something I noticed while hunting out another issue, but I don't see default taxonomy being provided for attachments at all.

From the current post.php code, if the post_type in wp_insert_attachment is set to 'attachment', which it always is, the $post_categories array is left empty. Only if post_type matches 'post' will it retrieve a default option for category.

Because of this, wp_set_post_categories is always called with an empty $post_categories array, which then leads to wp_set_object_terms being called with an empty $terms array. This in turn calls wp_get_object_terms at a possible 2 different points for the original attachment ID - only returning a result if a plugin has previously added associated a term with an attachment ID - unlikely in this scenario as we're still in wp_insert_attachment.

I could see moving do_action( 'set_object_terms'.... to post.php so that a plugin could hook in and add taxonomy to an attachment as it wishes, but I can't see how the remainder of the code is relevant.

[jeremyfelt]

Further clarification on what is passed to the 'set_object_terms' hook, located a few functions away in wp_set_object_terms, when an attachment is added.

$terms = array();
$tt_ids = array();
$taxonomy = 'category';
$append = false;
$old_tt_ids = array();

do_action('set_object_terms', $object_id, $terms, $tt_ids, $taxonomy, $append, $old_tt_ids);

It's entirely possible a plugin is using this hook to automatically add category information on wp_insert_attachment, but unlikely. In reality, the same information ($post_ID) is passed to the 'add_attachment' and 'edit_attachment' hooks immediately after, which would be a more reliable place to hook in.

I'm going to leave the patch as is, but definitely open to this action hook being added directly to wp_insert_attachment as a replacement of wp_set_post_categories.

Also, this is mislabeled as a bug, switching to enhancement as it's more for optimization & cleanup.

[ericlewis]

Consider the following.

I for some reason want categories to apply to attachments. So, I add this code somewhere (functions.php, mu-plugin, plugin, pick your poison):

<?php
add_action( 'init', function() {
        register_taxonomy_for_object_type( 'category', 'attachment' );
});

I then set the category while creating an attachment somewhere else:

<?php
wp_insert_attachment( 
        array( 'post_status' => 'publish', 
                'post_title' => 'Robin Williams dancing to Thriller with Jim Carrey',
                'post_category' => array( 3, 5 ) ) );

The code that's on the chopping block here will be needed to cover this edge case. Suggest a close here as wontfix. @jeremyfelt, thoughts?

[wonderboymusic]

I recently annotated wp_insert_attachment() - it shows where it diverges from wp_insert_post() and how it relates to post_category

[helen]

Related, and we should do this please: #21963

[wonderboymusic]

There is no more substance inside wp_insert_attachment() after [28788]. No unnecessary category code runs unless category is enabled for the attachment post type.