-
Notifications
You must be signed in to change notification settings - Fork 880
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
Add Indexable visibility columns #17757
base: trunk
Are you sure you want to change the base?
Changes from 41 commits
b3fd2cd
87c11c3
2eef6e2
b5bc8db
6c015ba
3d96968
acdf97a
e18c9ac
58d6a71
b7cfa04
c2d121e
38b552d
2fdce2f
3a18f72
5e89640
0a38f49
81e7421
1564f53
ecae28e
ea0a46e
6ccacd5
ccf7f9b
af7b219
510f9de
ed46abb
ff247f9
ca85a07
faf1180
f6a8adf
274df53
d51829e
5c1193a
661e9a3
82f8a78
66d6e4d
cf56df4
68aba31
3329a1b
e906ace
29739ee
66c9433
9eb9263
0c9ac1a
2059186
b95492e
101edcf
5d095a5
d8f754f
f613dfc
3e7e755
c24488e
4ed8b38
275c423
08c1f5f
e1c5055
6cf15bf
bad2929
148d47c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,22 +87,35 @@ public function build( $user_id, Indexable $indexable ) { | |
$indexable->is_robots_noarchive = null; | ||
$indexable->is_robots_noimageindex = null; | ||
$indexable->is_robots_nosnippet = null; | ||
$indexable->is_public = ( $indexable->is_robots_noindex ) ? false : null; | ||
$indexable->has_public_posts = $this->author_archive->author_has_public_posts( $user_id ); | ||
$indexable->blog_id = \get_current_blog_id(); | ||
$indexable->set_deprecated_property( 'is_public', ( $indexable->is_robots_noindex ) ? false : null ); | ||
|
||
$this->reset_social_images( $indexable ); | ||
$this->handle_social_images( $indexable ); | ||
|
||
$timestamps = $this->get_object_timestamps( $user_id ); | ||
$indexable->object_published_at = $timestamps->published_at; | ||
$indexable->object_last_modified = $timestamps->last_modified; | ||
$indexable = $this->set_aggregate_values( $indexable ); | ||
|
||
$indexable->version = $this->version; | ||
|
||
return $indexable; | ||
} | ||
|
||
/** | ||
* Sets the aggregate values for an author indexable. | ||
* | ||
* @param Indexable $indexable The indexable to set the aggregates for. | ||
* | ||
* @return Indexable The indexable with set aggregates. | ||
*/ | ||
public function set_aggregate_values( Indexable $indexable ) { | ||
$aggregates = $this->get_public_post_archive_aggregates( $indexable->object_id ); | ||
$indexable->object_published_at = $aggregates->first_published_at; | ||
$indexable->object_last_modified = max( $indexable->object_last_modified, $aggregates->most_recent_last_modified ); | ||
$indexable->number_of_publicly_viewable_posts = $aggregates->number_of_public_posts; | ||
Comment on lines
+112
to
+114
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block is repeated 4 times throughout the builders. Maybe create a separate function to avoid Code Duplication? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did consider it, but decided to not change more than I had to. I want to better abstract this when we revise the builders entirely. Do you think that's ok? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeap, for sure it's ok 👍 :) |
||
|
||
return $indexable; | ||
} | ||
|
||
/** | ||
* Retrieves the meta data for this indexable. | ||
* | ||
|
@@ -168,24 +181,28 @@ protected function find_alternative_image( Indexable $indexable ) { | |
} | ||
|
||
/** | ||
* Returns the timestamps for a given author. | ||
* Returns public post aggregates for a given author. | ||
* | ||
* @param int $author_id The author ID. | ||
* @param int $author_id The author ID. | ||
* | ||
* @return object An object with last_modified and published_at timestamps. | ||
* @return object An object with the number of public posts, most recent last modified and first published at timestamps. | ||
*/ | ||
protected function get_object_timestamps( $author_id ) { | ||
protected function get_public_post_archive_aggregates( $author_id ) { | ||
$post_statuses = $this->post_helper->get_public_post_statuses(); | ||
$post_types = $this->author_archive->get_author_archive_post_types(); | ||
|
||
$sql = " | ||
SELECT MAX(p.post_modified_gmt) AS last_modified, MIN(p.post_date_gmt) AS published_at | ||
SELECT | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idea: how about getting post ID's for an author first and iterating over those - using primary keys seems more efficient than where-ing over a set of fields. |
||
COUNT(p.ID) as number_of_public_posts, | ||
MAX(p.post_modified_gmt) AS most_recent_last_modified, | ||
MIN(p.post_date_gmt) AS first_published_at | ||
FROM {$this->wpdb->posts} AS p | ||
WHERE p.post_status IN (" . implode( ', ', array_fill( 0, count( $post_statuses ), '%s' ) ) . ") | ||
AND p.post_password = '' | ||
WHERE p.post_status IN (" . implode( ', ', array_fill( 0, count( $post_statuses ), '%s' ) ) . ') | ||
AND p.post_author = %d | ||
"; | ||
AND p.post_type IN (' . implode( ', ', array_fill( 0, count( $post_types ), '%s' ) ) . ') | ||
'; | ||
|
||
$replacements = \array_merge( $post_statuses, [ $author_id ] ); | ||
$replacements = \array_merge( $post_statuses, [ $author_id ], $post_types ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe a check if $post_types is array here, before merging it to the $replacement? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! that could've been a nasty bug. I added validation higher up in the get_author_post_types so we're sure that we get an array. |
||
|
||
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- We are using wpdb prepare. | ||
return $this->wpdb->get_row( $this->wpdb->prepare( $sql, $replacements ) ); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that
select_one()
is only used atcolumn_info()
which is turn is used only atrename_column()
which is not actively used anywhere in our org.