Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

More term ordering fixes #406

Merged
merged 11 commits into from
Apr 3, 2017
Merged

More term ordering fixes #406

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2086458
Introduce helper function for grabbing cached list of coauthor terms.
Mar 20, 2017
4aea6b1
Use the new helper function for grabbing cached co-author terms lists.
Mar 20, 2017
0a640ad
Explicitly check for a non-empty array and tidy up the code a little.
Mar 20, 2017
8101750
Must. Remember. Semicolons.
Mar 20, 2017
d5305b5
Bust the coauthors terms order cache when the objects terms are updat…
Mar 20, 2017
d58d3ae
Cache empty arrays when wp_get_object_terms() returns a WP_Error
Mar 21, 2017
b1fe8c2
Move cap_get_coauthor_terms_for_post() logic into the CoAuthors_Plus …
Mar 21, 2017
934a261
Don't create test tables as temporary
trepmal Mar 23, 2017
fe48900
Don't cache when get_object_terms fails
mjangda Mar 28, 2017
3248dfc
Update travis config to handle phpunit + php7
mjangda Mar 28, 2017
f53d8f7
Update Travis environment matrix
mjangda Mar 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions co-authors-plus.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ function __construct() {
// Delete CoAuthor Cache on Post Save & Post Delete
add_action( 'save_post', array( $this, 'clear_cache') );
add_action( 'delete_post', array( $this, 'clear_cache') );
add_action( 'set_object_terms', array( $this, 'clear_cache_on_terms_set' ), 10, 6 );
}

/**
Expand Down Expand Up @@ -1473,6 +1474,38 @@ public function filter_jetpack_open_graph_tags( $og_tags, $image_dimensions ) {
return apply_filters( 'coauthors_open_graph_tags', $og_tags );
}

/**
* Retrieve a list of coauthor terms for a single post.
*
* Grabs a correctly ordered list of authors for a single post, appropriately
* cached because it requires `wp_get_object_terms()` to succeed.
*
* @param int $post_id ID of the post for which to retrieve authors.
* @return array Array of coauthor WP_Term objects
*/
public function get_coauthor_terms_for_post( $post_id ) {

if ( ! $post_id ) {
return array();
}

$cache_key = 'coauthors_post_' . $post_id;
$coauthor_terms = wp_cache_get( $cache_key, 'co-authors-plus' );

if ( false === $coauthor_terms ) {
$cached = wp_get_object_terms( $post_id, $this->coauthor_taxonomy, array(
Copy link
Member

Choose a reason for hiding this comment

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

Minor: $cached is not quite the best name here. We should probably swap the var names $coauthor_terms and $cached.

 + $cache_coauthors = wp_cache_get( $cache_key, 'co-authors-plus' );
 +
 +		if ( false === $coauthor_terms ) {
 +			$coauthor_terms = wp_get_object_terms( $post_id, $this->coauthor_taxonomy, array(
...

'orderby' => 'term_order',
'order' => 'ASC',
) );
// Cache an empty array if the taxonomy doesn't exist.
$coauthor_terms = ( is_wp_error( $cached ) ) ? array() : $cached;
wp_cache_set( $cache_key, $coauthor_terms, 'co-authors-plus' );
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should be caching when there is an error.

Copy link
Member

Choose a reason for hiding this comment

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

Addressed in fe48900

Copy link
Member

Choose a reason for hiding this comment

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

@philipjohn let me know if that changeset makes sense to you and we can merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My worry about not caching errors is that a simple typo could easily result in wp_get_object_terms() triggering a lot more than we'd like, and the uncached queries that comes with it.

}

return $coauthor_terms;

}

/**
* Callback to clear the cache on post save and post delete.
*
Expand All @@ -1481,6 +1514,23 @@ public function filter_jetpack_open_graph_tags( $og_tags, $image_dimensions ) {
public function clear_cache( $post_id ) {
wp_cache_delete( 'coauthors_post_' . $post_id, 'co-authors-plus' );
}

/**
* Callback to clear the cache when an object's terms are changed.
*
* @param $post_id The Post ID.
*/
public function clear_cache_on_terms_set( $object_id, $terms, $tt_ids, $taxonomy, $append, $old_tt_ids ) {

// We only care about the coauthors taxonomy
if ( $this->coauthor_taxonomy !== $taxonomy ) {
return;
}

wp_cache_delete( 'coauthors_post_' . $object_id, 'co-authors-plus' );

}

}

global $coauthors_plus;
Expand Down Expand Up @@ -1624,3 +1674,17 @@ function cap_filter_comment_moderation_email_recipients( $recipients, $comment_i
}
return $recipients;
}

/**
* Retrieve a list of coauthor terms for a single post.
*
* Grabs a correctly ordered list of authors for a single post, appropriately
* cached because it requires `wp_get_object_terms()` to succeed.
*
* @param int $post_id ID of the post for which to retrieve authors.
* @return array Array of coauthor WP_Term objects
*/
function cap_get_coauthor_terms_for_post( $post_id ) {
global $coauthors_plus;
return $coauthors_plus->get_coauthor_terms_for_post( $post_id );
}
26 changes: 15 additions & 11 deletions php/class-wp-cli.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,9 @@ public function create_terms_for_posts() {

$count++;

$terms = get_the_terms( $single_post->ID, $coauthors_plus->coauthor_taxonomy );

if ( is_wp_error( $terms ) ) {
WP_CLI::error( $terms->get_error_message() );
$terms = cap_get_coauthor_terms_for_post( $single_post->ID );
if ( empty( $terms ) ) {
WP_CLI::error( sprintf( 'No co-authors found for post #%d.', $single_post->ID ) );
}

if ( ! empty( $terms ) ) {
Expand Down Expand Up @@ -236,8 +235,13 @@ public function assign_user_to_coauthor( $args, $assoc_args ) {
$posts = $wpdb->get_col( $wpdb->prepare( "SELECT ID FROM $wpdb->posts WHERE post_author=%d AND post_type IN ('$post_types')", $user->ID ) );
$affected = 0;
foreach ( $posts as $post_id ) {
if ( $coauthors = get_the_terms( $post_id, $coauthors_plus->coauthor_taxonomy ) ) {
WP_CLI::line( sprintf( __( 'Skipping - Post #%d already has co-authors assigned: %s', 'co-authors-plus' ), $post_id, implode( ', ', wp_list_pluck( $coauthors, 'slug' ) ) ) );
$coauthors = cap_get_coauthor_terms_for_post( $post_id );
if ( ! empty( $coauthors ) ) {
WP_CLI::line( sprintf(
__( 'Skipping - Post #%d already has co-authors assigned: %s', 'co-authors-plus' ),
$post_id,
implode( ', ', wp_list_pluck( $coauthors, 'slug' ) )
) );
continue;
}

Expand Down Expand Up @@ -545,8 +549,8 @@ public function list_posts_without_terms( $args, $assoc_args ) {
while ( $posts->post_count ) {

foreach ( $posts->posts as $single_post ) {
$terms = get_the_terms( $single_post->ID, $coauthors_plus->coauthor_taxonomy );

$terms = cap_get_coauthor_terms_for_post( $single_post->ID );
if ( empty( $terms ) ) {
$saved = array(
$single_post->ID,
Expand Down Expand Up @@ -698,9 +702,9 @@ public function remove_terms_from_revisions() {
WP_CLI::line( 'Found ' . count( $ids ) . ' revisions to look through' );
$affected = 0;
foreach ( $ids as $post_id ) {
$terms = get_the_terms( $post_id, $coauthors_plus->coauthor_taxonomy );
if ( ! $terms ) {

$terms = cap_get_coauthor_terms_for_post( $post_id );
if ( empty( $terms ) ) {
continue;
}

Expand Down
6 changes: 1 addition & 5 deletions template-tags.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,7 @@ function get_coauthors( $post_id = 0 ) {
}

if ( $post_id ) {
$cache_key = 'coauthors_post_' . $post_id;
if ( false === ( $coauthor_terms = wp_cache_get( $cache_key, 'co-authors-plus' ) ) ) {
$coauthor_terms = wp_get_object_terms( $post_id, $coauthors_plus->coauthor_taxonomy, array( 'orderby' => 'term_order', 'order' => 'ASC' ) );
wp_cache_set( $cache_key, $coauthor_terms, 'co-authors-plus' );
}
$coauthor_terms = cap_get_coauthor_terms_for_post( $post_id );
if ( is_array( $coauthor_terms ) && ! empty( $coauthor_terms ) ) {
foreach ( $coauthor_terms as $coauthor ) {
$coauthor_slug = preg_replace( '#^cap\-#', '', $coauthor->slug );
Expand Down
14 changes: 14 additions & 0 deletions tests/test-author-queried-object.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@

class Test_Author_Queried_Object extends CoAuthorsPlus_TestCase {

/**
* Set up for test
*
* Don't create tables as 'temporary'.
*
* @see https://github.com/Automattic/Co-Authors-Plus/issues/398
*/
function setUp() {
parent::setUp();

remove_filter( 'query', array( $this, '_create_temporary_tables' ) );
remove_filter( 'query', array( $this, '_drop_temporary_tables' ) );
}

/**
* On author pages, the queried object should only be set
* to a user that's not a member of the blog if they
Expand Down