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.

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

Fixing double posts counts for users with linked accounts #567

Merged
merged 1 commit into from
Jul 17, 2018
Merged

Fixing double posts counts for users with linked accounts #567

merged 1 commit into from
Jul 17, 2018

Conversation

TheCrowned
Copy link
Contributor

This fixes #561.

On a correct, up-to-date CAP installation, posts counts for authors with guest authors profile are absolutely off - basically, they are double what they should.

This is an example with CAP enabled - clearly off since the Mine posts is more than the total:
w cap

With CAP disabled:
wo cap

Prior to this PR, the code which filtered the WP posts count was this:

$term = $this->get_author_term( $user );
$guest_term = get_term_by( 'slug', 'cap-' . $user->user_nicename, $this->coauthor_taxonomy );

// Only modify the count if it has a linked account with posts or the author exists as a term
if ( $user->linked_account && $guest_term->count ) {
	if ( $term && ! is_wp_error( $term )) {
		$count = $guest_term->count + $term->count;
	} else {
		$count = $guest_term->count;
	}
} elseif ( $term && ! is_wp_error( $term ) ) {
	$count = $term->count;
}

Both $term and $guest_term are considered, guessing that $term would hold the non-prefixed version of the CAP author term, while $guest_term the prefixed cap- one. The counts are later summed under the assumption that they are complementary. The flaw is that $this->get_author_term() already fetches the one term that exists, giving priority to the prefixed cap- version of it. So what happens is that, if cap-$username exists, both $term and $guest_term will contain its content, and $count will then be doubled.

The fix is thus to get rid of the $guest_term var and only retain the final conditional:

if ( $term && ! is_wp_error( $term ) ) {
	$count = $term->count;
}

But aren't we losing some counts on the way? Aren't we discarding the non-prefixed version of the term? Is it just useless? No, we don't lose counts; No, we are discarding it, and no, it is not useless. The pivot is $this->update_author_term(): it will update any of the terms that exist, also suggesting that it should never happen that they co-exist: either there is the prefixed cap- term, or the non-prefixed one. In any case, get_author_term() will fetch the one that exists for the author, which will already contain the total count, and we don't need to sum anything to it.

(I think we should also think about migrating terms to all having the cap- prefix, but this is another issue #566)

@TheCrowned TheCrowned changed the title Only using new cap- prefixed term when counting user posts. Fixing double posts counts for users with linked accounts Jul 17, 2018
@sboisvert
Copy link
Contributor

This is an awesome Fix and an awesome explanation of the fix!

@sboisvert sboisvert merged commit 68de11d into Automattic:master Jul 17, 2018
@rebeccahum
Copy link
Contributor

Just a follow-up on this, since it merges over the fixes in PR #461, the edge case of what is described in #460 is present again.

rebeccahum pushed a commit that referenced this pull request Mar 26, 2019
…t-authors

Fixing double posts counts for users with linked accounts
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.

Wrong post count for guest authors with terms
3 participants