-
-
Notifications
You must be signed in to change notification settings - Fork 513
Question: How should we treat the co-authors-plus integration wrt Factories? #2155
Description
NOTE: A first-stab PR for this is forthcoming, but as I call out below I think this deserves its own issue.
The main issue: which User class to instantiate?
In the Integrations\CoAuthorsPlus, the authors() method (hooked onto the timber/post/authors filter) determines which class to instantiate:
public function authors( $author, $post ) {
$authors = array();
$cauthors = get_coauthors($post->ID);
foreach ( $cauthors as $author ) {
$uid = $this->get_user_uid( $author );
if ( $uid ) {
$authors[] = new \Timber\User($uid);
} else {
$authors[] = new CoAuthorsPlusUser($author);
}
}
return $authors;
}
protected function get_user_uid( $cauthor ) {
// if is guest author
if( is_object($cauthor) && isset($cauthor->type) && $cauthor->type == 'guest-author'){
// if have have a linked user account
global $coauthors_plus;
if( !$coauthors_plus->force_guest_authors && isset($cauthor->linked_account) && !empty($cauthor->linked_account ) ){
return $cauthor->linked_account;
} else {
return null;
}
} else {
return $cauthor->ID;
}
}The logic around whether $uid is truthy appears to depend on whether it's for a "guest author" (I gather this is just arbitrary data submitted at post creation time - not corresponding to a wp_users row), whether guest authors are being "forced" (no idea what this means - plugin setting?), etc. In other words, I'm not terribly certain what this code does. :)
So my main question is: how should this interface with UserFactory and the user class map? I'm taking a stab at implementing what I think should happen, but I'd like to get a review specially for this.
Handling stdClass instances
Another concern is that get_coauthors() appears to sometimes return instances of stdClass. I think these represent "guest authors." Trouble is, stdClass is not supported by UserFactory::from() - in this case it calls from_post_object (copy/pasta, will fix) which eventually throws an InvalidArgumentException. The reasoning here is that at the heart of @gchtr's design there's an assumption that the callback to the timber/class/classmap filter eventually gets a WP_User. This is a strong guarantee and the entire class map API gets weaker if we throw this away.
We could add a case to handle stdClass inside the factory, but I'm hesitant to do that because it's a good design and I do not think that Factories should have to know about special stuff like Integrations. I think it's perfectly reasonable for integrations to know how to bootstrap their own objects that Factories know how to deal with. So the approach I'm going to take to this is to have Integrations\CoAuthorsPlus instantiate CoAuthorsPlusUser objects directly for (what I assume are) guest authors.
Linked Authors
Apparently "linked authors" are a thing. Appears to have something to do with guest authors, and it's causing testLinkedGuestAuthor() to fail. My head is already swimming from all this abstraction. Halp?! ;)