Consider multisite for posts abstraction. #1004
Conversation
includes/classes/DistributorPost.php
Outdated
| } | ||
| } | ||
|
|
||
| return $thumbnail[ $hash ]; | ||
| return get_the_post_thumbnail( $this->post, $size, $attr ); |
There was a problem hiding this comment.
Need to remove this return statement
includes/classes/DistributorPost.php
Outdated
|
|
||
| $this->site_id = get_current_blog_id(); | ||
|
|
||
| // Pre-populate the post data to account for switching sites. |
There was a problem hiding this comment.
Can you expound on what the problem is? I guess I'm not quite understanding why all the data below needs to be set when we are on a multisite. I'm assuming it has something to do with ensuring we have the right information from the right site but I guess not completely following without being able to see in what situations this comes into play
There was a problem hiding this comment.
ensuring we have the right information from the right site
This is the goal.
We we distribute network sites, we run switch_to_blog() fairly early on (I'll probably try to delay it) and calling the helper methods was returning either false or the wrong post's information if there was a post ID collision.
In the DistributorPost::post_data() method, we currently call get_the_title() and the ::get_media(), ::get_terms() and ::get_meta() methods.
I'm wanting to make sure they always return data related to the post and site we set up the object with.
I guess I need to improve the comment.
includes/classes/DistributorPost.php
Outdated
| @@ -314,7 +335,12 @@ public function get_the_id() { | |||
| * @return string Post permalink. | |||
| */ | |||
| public function get_permalink() { | |||
| return get_permalink( $this->post ); | |||
| static $permalink = null; | |||
There was a problem hiding this comment.
Not opposed to using statics here but does this actually help in practice? Are we calling these methods multiple times in the same execution?
There was a problem hiding this comment.
We may... some of the canonical filters call get_permalink.
I'm not sure what's better or worse: making sure the site matches and switching if needs be or using statics. The former is slow but the latter may hit order of operations issues if other plugins add or remove filters.
|
Closing this off as an interesting experiment. Replaced with #1010 |
Description of the Change
Accounts for multisite in the post abstraction helper methods.
Ensures the site switched correctly when using accessing post data. This prevents incorrect data during post ID collisions, or missing data if the post does not exist on the switched site.
Uses a lot of statics to avoid needless switching. This might not be worth it.
Closes #1007
How to test the Change
Changelog Entry
Credits
Props @peterwilsoncc
Checklist: