Skip to content

Commit

Permalink
Fix/missing wp user type (#988)
Browse files Browse the repository at this point in the history 
* fix: preventing loss of fact that a guest author might also be a WP_User

* fix: making the update operation dependent on $append flag.

This might be a problematic decision. But the way I justify this change is that if you are appending co-authors, there may already be a WP_User set as the author. So we don't really have to care whether one is passed or not. Because of this, we do not need to forcibly return a `false` flag since that is confusing to the caller, especially because we actually do save the guest authors which are given in the call! Instead, if the $append flag is false, we should expect that at least one user will be a WP_User. In that case, if none is passed in, then there is a mismatch of the intended authors. Because now, the `wp_posts.post_author` column will have an old `wp_users.ID` which remains set and most likely isn't the intent of the caller.

* fix: attempting DB update only when $new_author is not empty.

Also, returning the actual response from the DB, to make this call even more accurate in terms of what is actually happen at the DB layer.

* fix: need to ensure pure WP_User is processed correctly as post_author.

A pure WP_User (i.e. a WP_User that IS NOT linked to a Guest Author) needs to be handled specially.

* fix: a necessary refactor of the `get_coauthor_by` function.

This refactor is absolutely necessary in order for all the previous fixes to work as expected. Without this fix, what happens is that when you use `get_coauthor_by` by 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, `get_coauthor_by` will now check if the `linked_account` attribute is set. If so, it will attempt to find the corresponding user for the Guest Account. It still gives priority to returning a Guest Author. When a Guest Author is not found, it will 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.

* fix: returning a plain WP_User if guest authors is not enabled.

I forgot to run tests on my previous commit. This satisfies the test Test_CoAuthors_Plus::test_get_coauthor_by_when_guest_authors_not_enabled which is expecting a WP_User when the plugin is not enabled.

* feat: adding additional tests for co-authors-plus.php functionality.

* fix: preventing loss of fact that a guest author might also be a WP_User

* fix: making the update operation dependent on $append flag.

This might be a problematic decision. But the way I justify this change is that if you are appending co-authors, there may already be a WP_User set as the author. So we don't really have to care whether one is passed or not. Because of this, we do not need to forcibly return a `false` flag since that is confusing to the caller, especially because we actually do save the guest authors which are given in the call! Instead, if the $append flag is false, we should expect that at least one user will be a WP_User. In that case, if none is passed in, then there is a mismatch of the intended authors. Because now, the `wp_posts.post_author` column will have an old `wp_users.ID` which remains set and most likely isn't the intent of the caller.

* fix: attempting DB update only when $new_author is not empty.

Also, returning the actual response from the DB, to make this call even more accurate in terms of what is actually happen at the DB layer.

* fix: need to ensure pure WP_User is processed correctly as post_author.

A pure WP_User (i.e. a WP_User that IS NOT linked to a Guest Author) needs to be handled specially.

* fix: a necessary refactor of the get_coauthor_by function.

This refactor is absolutely necessary in order for all the previous fixes to work as expected. Without this fix, what happens is that when you use `get_coauthor_by` by searching with a Guest Author, any link to a WP_User the Guest Author may have 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 currently returning an expected value is when you search by a WP_User account/field 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, `get_coauthor_by` will now check if the `linked_account` attribute is set. If so, it will then attempt to find the corresponding WP_User for the Guest Author. Crucially, it still gives priority to returning a Guest Author. When a Guest Author is not found, it will then attempt to 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.

* fix: renaming user_login's for new authors introduced for new tests.

These user_login's were causing other tests to fail because you cannot create another user with the same user_login.

* fix: removing use of assertObjectHasProperty

Older version of PHPUnit do not have this function available. Updating to workaround: `assertTrue( property_exists( $obj, 'prop' ) )`

* fix: typo in function call

* fix: using strict comparison instead of function call `is_null`

* fix: using more descriptive assertion for array validation.

* fix: using `create_and_get` post factory func, to avoid query call.

* fix: removing use of newly introduced is_wp_user property.

Relying instead on wp_user property which has already been used before.

* fix: PHPCS fixes and added commentary/descriptions to docblocks.

* fix: some small quick fixes for formatting and documentation

* fix: removing repetitive test.

* add: new assertion func that determines if an obj is not a WP_User class

* add: new assertion to help determine if a Post has the correct Authors

* add: new test solely for CoAuthorPlus::get_coauthor_by().

By fully testing CoAuthorPlus::get_coauthor_by(), we can remove some repetitive assertions that don't directly relate to what's being tested.

* fix: was passing string values when I should've been passing Author objs

* fix: using a data provider for very similar tests

---------

Co-authored-by: Gary Jones <[email protected]>
  • Loading branch information
@eddiesshop @GaryJones
eddiesshop and GaryJones authored Oct 9, 2024
1 parent d5f9404 commit fea7157
Show file tree
Hide file tree
Showing 2 changed files with 1,304 additions and 43 deletions.
131 changes: 92 additions & 39 deletions php/class-coauthors-plus.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,51 +307,88 @@ public function is_guest_authors_enabled(): bool {
public function get_coauthor_by( $key, $value, $force = false ) {

// If Guest Authors are enabled, prioritize those profiles
if ( isset( $this->guest_authors ) && $this->is_guest_authors_enabled() ) {
if ( $this->is_guest_authors_enabled() && isset( $this->guest_authors ) ) {
$guest_author = $this->guest_authors->get_guest_author_by( $key, $value, $force );
if ( is_object( $guest_author ) ) {
return $guest_author;
}
}
if ( isset( $guest_author->linked_account ) ) {
$user = $this->get_user_by( 'login', $guest_author->linked_account );

switch ( $key ) {
case 'id':
case 'login':
case 'user_login':
case 'email':
case 'user_nicename':
case 'user_email':
if ( 'user_login' === $key ) {
$key = 'login';
}
if ( 'user_email' === $key ) {
$key = 'email';
}
if ( 'user_nicename' === $key ) {
$key = 'slug';
}
$user = get_user_by( $key, $value );
if ( ! $user && ( 'login' === $key || 'slug' === $key ) ) {
// Re-try lookup without prefixed value if no results found.
$value = preg_replace( '#^cap\-#', '', $value );
$user = get_user_by( $key, $value );
if ( null !== $user ) {
$guest_author->wp_user = $user;
}
}
if ( ! $user ) {

return $guest_author;
} else {
// Guest Author was not found, so let's see if we are searching for a WP_User.
$user = $this->get_user_by( $key, $value );

if ( null === $user ) {
return false;
}

// At this point we have a valid $user.
$user->type = 'wpuser';
// However, if guest authors are enabled and there's a guest author linked to this
// user account, we want to use that instead
if ( isset( $this->guest_authors ) && $this->is_guest_authors_enabled() ) {
$guest_author = $this->guest_authors->get_guest_author_by( 'linked_account', $user->user_login );
if ( is_object( $guest_author ) ) {
$user = $guest_author;
}

$guest_author = $this->guest_authors->get_guest_author_by( 'linked_account', $user->user_login );
if ( is_object( $guest_author ) ) {
$guest_author->wp_user = $user;
$user = $guest_author;
}

return $user;
}
} else {
$user = $this->get_user_by( $key, $value );

if ( null === $user ) {
return false;
}

$user->type = 'wpuser';

return $user;
}
return false;
}

/**
* Searches for authors by way of the WP_User table using a specific list of data points. If login or slug
* are provided as search parameters, this function will remove `cap-` from the search value, if present.
*
* @param string $key Key to search by, i.e. 'id', 'login', 'user_login', 'email', 'user_email', 'user_nicename'.
* @param string $value Value to search for.
*
* @return WP_User|null
*/
protected function get_user_by( $key, $value ) {
$acceptable_keys = [
'id' => 'id',
'login' => 'login',
'user_login' => 'login',
'email' => 'email',
'user_email' => 'email',
'user_nicename' => 'slug',
];

if ( ! array_key_exists( $key, $acceptable_keys ) ) {
return null;
}

$key = $acceptable_keys[ $key ];

$user = get_user_by( $key, $value );

if ( ! $user && ( 'login' === $key || 'slug' === $key ) ) {
// Re-try lookup without prefixed value if no results found.
$value = preg_replace( '#^cap\-#', '', $value );
$user = get_user_by( $key, $value );
}

if ( false === $user ) {
return null;
}

return $user;
}

/**
Expand Down Expand Up @@ -1016,18 +1053,34 @@ public function add_coauthors( $post_id, $coauthors, $append = false, $query_typ
if ( empty( $post_author_user )
|| ! in_array( $post_author_user->user_login, $coauthors ) ) {
foreach ( $coauthor_objects as $coauthor_object ) {
if ( 'wpuser' === $coauthor_object->type ) {
if ( $coauthor_object instanceof WP_User ) {
$new_author = $coauthor_object;
break;
} elseif ( isset( $coauthor_object->wp_user ) && $coauthor_object->wp_user instanceof WP_User ) {
$new_author = $coauthor_object->wp_user;
break;
}
}
// Uh oh, no WP_Users assigned to the post
if ( empty( $new_author ) ) {

/*
* If setting a fresh group of authors for a post, (i.e. $append === false),
* then perhaps one of those authors should be a WP_USER. However,
* if $append === true, and we are perhaps unable to find a
* WP_USER (perhaps none was given), we don't really
* care whether post_author should be updated.
* */
if ( false === $append && empty( $new_author ) ) {
return false;
}

$wpdb->update( $wpdb->posts, array( 'post_author' => $new_author->ID ), array( 'ID' => $post_id ) );
clean_post_cache( $post_id );
if ( ! empty( $new_author ) ) {
$update = $wpdb->update( $wpdb->posts, array( 'post_author' => $new_author->ID ), array( 'ID' => $post_id ) );
clean_post_cache( $post_id );

if ( is_bool( $update ) ) {
return $update;
}
}
}
return true;

Expand Down
Loading

0 comments on commit fea7157

Please sign in to comment.