Skip to content

Conversation

@iamdharmesh
Copy link
Member

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


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@iamdharmesh iamdharmesh marked this pull request as ready for review September 18, 2023 08:50
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
$request->set_param( 'categories', array( $category['term_id'] ) );
$response = rest_get_server()->dispatch( $request );
$attachment = $response->get_data();
$this->assertSame( array( $category['term_id'] ), wp_list_pluck( wp_get_post_terms( $attachment['id'], 'category' ), 'term_id' ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$this->assertSame( array( $category['term_id'] ), wp_list_pluck( wp_get_post_terms( $attachment['id'], 'category' ), 'term_id' ) );
$term = wp_get_post_terms( $attachment['id'], 'category' );
$this->assertSame( $category['term_id'], $term[0]->term_id );

Simplified version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 4c7bbca

@mukeshpanchal27
Copy link
Member

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.

@swissspidy
Copy link
Member

@swissspidy swissspidy closed this Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants