Automattic / Co-Authors-Plus

Multiple bylines and Guest Authors for WordPress
https://wordpress.org/plugins/co-authors-plus/
GNU General Public License v2.0
287 stars 205 forks source link

Fix/missing wp user type #988

Open eddiesshop opened 11 months ago

eddiesshop commented 11 months ago

Description

I came across this bug when attempting to do the following:

$author_data = [
    'wp_user_nicename', // WP User who also happens to be a Guest Author
    'guest_author_only_nicename'
];
$coauthors_plus->add_coauthors( $post_id, $author_data, $append = false );

I placed my WP_User first in the array because of this block of code. It will update the wp_posts.post_author column for the first WP_User it finds.

However, I noticed that the wp_posts.post_author column was not being updated as I was expecting.

I started digging through the code and found that this block of code was essentially being skipped, due to the fact that this function always returns a Guest Author object.

This is actually causing the add_coauthors function to always respond with false for instances where wp_posts.post_author = 0 or where the user no longer exists on the system. Even if you're passing in a valid WP_User who is also a Guest Author, it would respond with false, as was my case.

With this fix, we retain the fact that the original user_nicename we passed is also a WP_User. In this way, we don't waste compute cycles in that foreach loop where it would never find a valid WP_User it's trying to look for.

Deploy Notes

Are there any new dependencies added that should be taken into account when deploying to WordPress.org? No

Steps to Test

Outline the steps to test and verify the PR here.

You will need 5 authors:

  1. WP_User who is not linked to Guest Author ( user_nicename: wp_user_1 )
  2. WP_User who is not linked to Guest Author ( user_nicename: wp_user_2 )
  3. Guest Author who is not linked ( user_nicename: ga_1 )
  4. Guest Author who is not linked ( user_nicename: ga_2 )
  5. WP_User who is linked to Guest Author ( user_nicename: wp_user_and_ga_1 )

Example:

  1. Check out PR.
  2. Instantiate CoAuthor_Plus: $cap = new CoAuthor_plus();
  3. Execute $cap->add_coauthors( $post_id, [ 'wp_user_1' ] ); result should be true. wp_posts.post_author should be equal to wp_users.ID
  4. Execute $cap->add_coauthors( $post_id, [ 'ga_1', 'ga_2' ] ); result should be false. This is because $append = false and there was no wp_user passed as a CoAuthor. Notice also that wp_posts.post_author remains untouched.
  5. Execute Step 1 again, then execute Step 4 again, this time passing $append = true, like so: $cap->add_coauthors( $post_id, [ 'ga_1', 'ga_2' ], true ); result should be true. Notice, that all 3 authors are correctly displayed.
  6. Execute $cap->add_coauthors( $post_id, [ 'ga_1', 'wp_user_2' ], true ); result should be true. Notice that wp_posts.post_author is not overwritten. This is because we are appending authors to the post.
  7. Execute $cap->add_coauthors( $post_id, [ 'ga_2', 'wp_user_and_ga_1' ] ); result should be true. Notice here that the WP_User is placed second in the array, but it is correctly picked up now, and the wp_posts.post_author column is correctly updated to match the user's wp_users.ID column.
eddiesshop commented 10 months ago

The previous 2 commits address another issue I found during some internal testing.

What was happening was that using get_coauthor_by and searching with a Guest Author, if that Guest Author has a valid link to a WP_User, it is summarily ignored. Functions like add_coauthors expect at least one coauthor to be a valid WP_User so that the wp_posts.post_author column can be appropriately updated. The only case where this function is returning an expected value is when you search by the WP_User first. When it arrives at $guest_author = $this->guest_authors->get_guest_author_by( $key, $value, $force );, $guest_author === false. It is then forced to move to the switch statement to find a user via their WP_User data.

With this refactor, when you arrive at $guest_author = $this->guest_authors->get_guest_author_by( $key, $value, $force ); and a valid $guest_author is obtained, it will now check if the linked_account attribute is set. If so, it will attempt to find the corresponding user for the Guest Account. Crucially, this change still gives priority to returning a Guest Author first and foremost. Only when a Guest Author is not found, will it search for a WP_User. If found, it will also search to see if a linked Guest Author account exists. If it does, it will return that Guest Author object instead, without losing the fact that this account also has a WP_User associated with it. If the plugin is disabled, then it will attempt to find a WP_User.

So using the examples outlined in the description, the below scenario should be tested as well:

  1. Execute $cap->add_coauthors( $post_id, [ 'wp_user_2', 'ga_1', 'wp_user_and_ga_1' ] ); result should be true. Notice that wp_posts.post_author is equal to the ID for wp_user_2. Also, all 3 authors can be found when looking at the terms associated to $post_id.
  2. Execute $cap->add_coauthors( $post_id, [ 'wp_user_and_ga_1', 'ga_1', 'wp_user_2' ] ); result should be same as above, except now wp_posts.post_author is linked to wp_user_and_ga_1.
  3. Execute $cap->add_coauthors( $post_id, [ 'ga_1' ] ); result should be false. Since this is a pure GA account, wp_post.post_author is not updated, however we can see a term-taxonomy associated with this $post_id. Follow this up by executing $cap->add_coauthors( $post_id, [ 'wp_user_2', 'wp_user_and_ga_1' ], true ); the result should be true. Notice that wp_posts.post_author is equal to wp_user_2's user ID.
GaryJones commented 10 months ago

Thanks @eddiesshop. A couple of quick meta questions:

  1. Should/Can the fix for the extra issue go into a new PR?
  2. Since most of the steps are to execute code, could you please add suitable tests (happy and sad paths) for each fix?