Skip to content
This repository has been archived by the owner on Sep 24, 2018. It is now read-only.

Add Embeddable Taxonomy Term Links to the Post Response #1048

Merged
merged 24 commits into from
Apr 17, 2015

Conversation

rachelbaker
Copy link
Member

Fixes #900

The _links in the Post response now include embeddable references for each object taxonomy.

@rachelbaker
Copy link
Member Author

In v1 the taxonomy terms were included in the Post response, this moves them to embeddable resource links. Any taxonomy that is returned by wp_get_object_taxonomies() is shown, which matches v1 behavior.

$terms = get_terms( $taxonomy, $prepared_args );
if ( isset( $request['post'] ) ) {
$post_id = absint( $request['post'] );
$terms = wp_get_object_terms( $post_id, $taxonomy, $prepared_args );
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we validate the taxonomy for the post type?

@rachelbaker
Copy link
Member Author

@danielbachhuber db2c077 adds checks to be sure a post exists and that the taxonomy is related to the post.


$valid_taxonomies = get_object_taxonomies( $post->post_type );
if ( in_array( $taxonomy, $valid_taxonomies ) ) {
return $taxonomy;
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this just returning bool? I don't understand why you'd need $taxonomy returned when you're passing it in the first place.

@@ -1133,6 +1133,19 @@ protected function prepare_links( $post ) {
);
}

$taxonomies = get_object_taxonomies( $post->post_type );
if ( ! empty( $taxonomies ) ) {
foreach ( $taxonomies as $tax ) {
Copy link
Member

Choose a reason for hiding this comment

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

Need to check the taxonomy isn't private. This would benefit from a test.

Copy link
Member Author

Choose a reason for hiding this comment

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

WordPress doesn't have private taxonomies: https://core.trac.wordpress.org/ticket/21949 Related #419

Copy link
Member

Choose a reason for hiding this comment

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

We still need to check. Otherwise we will be exposing private data.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added in line 1156

@danielbachhuber
Copy link
Member

Just a few nits.

@rachelbaker
Copy link
Member Author

I found a big hole in my logic here. I need to check that a user has permission to read a post before we return the terms associated with it. Still a WIP.

@rachelbaker
Copy link
Member Author

Includes:

  • checks to make sure a user can read a post before returning terms related to a given post_id
  • checks to make sure a taxonomy is public
  • added permission_callback methods.
  • unit tests
  • wraps responses in json_ensure_response

*
* @param string $taxonomy
* @param integer $post_id
* @return null|WP_Error
Copy link
Member

Choose a reason for hiding this comment

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

Why return null?

Copy link
Member Author

Choose a reason for hiding this comment

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

It just returns....what would you do?

Copy link
Member

Choose a reason for hiding this comment

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

True?

@rachelbaker
Copy link
Member Author

@danielbachhuber Both pieces of recent feedback have been addressed in 82ac7d2 and de7b818

danielbachhuber added a commit that referenced this pull request Apr 17, 2015
Add Embeddable Taxonomy Term Links to the Post Response
@danielbachhuber danielbachhuber merged commit cd6e309 into develop Apr 17, 2015
@danielbachhuber danielbachhuber deleted the add-tag-and-cat-links branch April 17, 2015 21:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Posts should include _links to associated taxonomy terms
2 participants