Make WordPress Core

Opened 7 months ago

Last modified 4 days ago

#57897 new enhancement

REST attachments controller does not handle terms on creation

Reported by: swissspidy's profile swissspidy Owned by:
Milestone: 6.4 Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch has-unit-tests has-testing-info commit
Focuses: rest-api Cc:

Description

\WP_REST_Attachments_Controller::create_item() does not call parent::create_item(), thus it also doesn't call \WP_REST_Posts_Controller::handle_terms(). That means it's impossible to assign terms to the freshly uploaded attachment.

rest_after_insert_attachment can be used to work around this, but I think it would make sense to have this built-in.

Requires only a few lines of code:

$terms_update = $this->handle_terms( $post_id, $request );

if ( is_wp_error( $terms_update ) ) {
        return $terms_update;
}

Change History (17)

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


7 months ago
#1

  • Keywords has-patch added; needs-patch removed

This PR fixes an issue where terms could not be assigned to newly uploaded attachments through the REST API, due to the \WP_REST_Attachments_Controller::create_item() function not calling parent::create_item() and thus not calling \WP_REST_Posts_Controller::handle_terms(). This PR adds the necessary lines of code to call handle_terms() in create_item(), allowing terms to be assigned to attachments through the REST API. The changes have been tested and work as expected.

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

#2 @swissspidy
4 months ago

  • Keywords needs-testing needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release

@TimothyBlynJacobs commented on PR #4209:


4 months ago
#3

Thanks for the patch @tanjimTC! This change looks good, could you add unit tests demonstrating that we can create an attachment with terms?

@spacedmonkey commented on PR #4209:


4 months ago
#4

Let's add a unit test.

#5 @swissspidy
2 months ago

  • Milestone changed from Future Release to 6.4

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


3 weeks ago

#7 @oglekler
3 weeks ago

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

This ticket has unit tests and is ready for testing.

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


3 weeks ago

#9 @Ankit K Gupta
2 weeks ago

  • Keywords needs-testing-info added

Hi @talldanwp Thanks for the PR, could you please add manual testing instructions to verify this fix?

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


12 days ago

#11 @oglekler
12 days ago

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

This ticket was discussed during a bug scrub, Unit-tests are still missing, and we have 12 days left until Beta 1 when the opportunity window will be closed. Due to lack of activity, this is a candidate for Future release.

Add props to @mukesh27

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


8 days ago
#12

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

PR adds handle terms in the REST attachments controller to assign terms to the freshly created attachment.

Currently, \WP_REST_Attachments_Controller::create_item() does not call parent::create_item(), thus it also doesn't call \WP_REST_Posts_Controller::handle_terms(). That means it's impossible to assign terms to the freshly uploaded attachment. As suggested in the ticket description, PR adds handle_terms() in WP_REST_Attachments_Controller to fix this issue.

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

#13 @Dharm1025
7 days ago

  • Keywords has-testing-info needs-testing added; needs-testing-info removed

Testing Instructions

Steps to test

  1. Apply PR 5238
  2. Register a new taxonomy for the attachment object type with show_in_rest set to true OR add an already registered taxonomy to an attachment. For example:
    <?php
    register_taxonomy_for_object_type( 'category', 'attachment' );
    
  1. Send a REST API request to create an attachment with taxonomy terms. (For more information, refer to the WordPress REST API documentation).

Expected Results

  • ✅ Taxonomy terms included in the API request should be assigned to the newly created attachment.

cc: @ankit-k-gupta (as you requested testing instructions earlier)

@mukesh27 commented on PR #5238:


7 days ago
#14

Thank you, @iamdharmesh, for the unit tests. They appear to be solid to me. Let's wait for feedback from some of the maintainers before marking it for commit.

#15 @Ankit K Gupta
7 days ago

Test / Verification Report. ✅

Patch tested: https://github.com/WordPress/wordpress-develop/pull/5238

Env

  • WordPress - 6.4-alpha-20230918.155338
  • Web Server: Nginx
  • Chrome Version - 116
  • OS - macOS
  • Theme: Storefront
  • PHP - 7.4.0
  • Active Plugin - JSON Basic Authentication

Actual Results

  • ✅ Issue resolved with PR.


Test result

This pull request addresses an issue where terms could not be assigned to newly uploaded attachments through the REST API. Tested by following the above instructions.
Without/before the PR, taxonomies were not getting assigned when uploading media via REST API. After the PR, categories are assigned to the newly added attachment.

Video Demostration

https://www.loom.com/share/a933fead39624c3186143e1b29b43a70

After the patch

https://i.imgur.com/ZGc3yqp.jpg

Before the patch

https://i.imgur.com/hztJ9kj.jpg

#16 @Ankit K Gupta
7 days ago

  • Keywords needs-testing removed

#17 @oglekler
4 days ago

  • Keywords commit added

@mukesh27, let's mark it for commit and ping @spacedmonkey as a maintainer :)

Note: See TracTickets for help on using tickets.