Skip to content
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

Eliminate varying URL Metrics by logged-in state and discontinue disabling optimization by default for admins #1788

Merged
merged 2 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions plugins/optimization-detective/docs/hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ Filters whether the current response can be optimized. By default, detection and
2. It’s not a post embed template (`is_embed()`).
3. It’s not the Customizer preview (`is_customize_preview()`)
4. It’s not the response to a `POST` request.
5. The user is not an administrator (`current_user_can( 'customize' )`), unless you're in plugin development mode (`wp_is_development_mode( 'plugin' )`).
6. There is at least one queried post on the page. This is used to facilitate the purging of page caches after a new URL Metric is stored.
5. There is at least one queried post on the page. This is used to facilitate the purging of page caches after a new URL Metric is stored.

To force every response to be optimized regardless of the conditions above, you can do:

Expand Down
2 changes: 1 addition & 1 deletion plugins/optimization-detective/docs/introduction.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ When no more URL Metrics are needed for a URL due to the sample size being obtai

URL Metrics have a “freshness TTL” after which they will be stale and the JavaScript will be served again to start gathering metrics again to ensure that the right elements continue to get their loading prioritized. When a URL Metrics custom post type hasn't been touched in a while, it is automatically garbage-collected.

👉 **Note:** This plugin optimizes pages for actual visitors, and it depends on visitors to optimize pages (since URL Metrics need to be collected). As such, you won't see optimizations applied immediately after activating the plugin (and dependent plugin(s)). And since administrator users are not normal visitors typically, optimizations are not applied for admins by default (but this can be overridden with the `od_can_optimize_response` filter below). URL Metrics are not collected for administrators because it is likely that additional elements will be present on the page which are not also shown to non-administrators, meaning the URL Metrics could not reliably be reused between them.
👉 **Note:** This plugin optimizes pages for actual visitors, and it depends on visitors to optimize pages (since URL Metrics need to be collected). As such, you won't see optimizations applied immediately after activating the plugin (and dependent plugin(s)).

When the `WP_DEBUG` constant is enabled, additional logging for Optimization Detective is added to the browser console.
6 changes: 0 additions & 6 deletions plugins/optimization-detective/optimization.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,6 @@ function od_can_optimize_response(): bool {
is_customize_preview() ||
// Since the images detected in the response body of a POST request cannot, by definition, be cached.
( isset( $_SERVER['REQUEST_METHOD'] ) && 'GET' !== $_SERVER['REQUEST_METHOD'] ) ||
// The aim is to optimize pages for the majority of site visitors, not for those who administer the site, unless
// in 'plugin' development mode. For admin users, additional elements will be present, like the script from
// wp_customize_support_script(), which will interfere with the XPath indices. Note that
// od_get_normalized_query_vars() is varied by is_user_logged_in(), so membership sites and e-commerce sites
// will still be able to be optimized for their normal visitors.
( current_user_can( 'customize' ) && ! wp_is_development_mode( 'plugin' ) ) ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the development mode condition here? That seems unrelated to what this PR is doing, so shouldn't it be kept?

Copy link
Member Author

@westonruter westonruter Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The development mode condition was added to force optimization to be performed even when the user is an administrator. However, now optimization applies by default for everyone including administrators, so this is obsolete.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context, see #1423 fixed via #1700.

// Page caching plugins can only reliably be told to invalidate a cached page when a post is available to trigger
// the relevant actions on.
null === od_get_cache_purge_post_id()
Expand Down
5 changes: 0 additions & 5 deletions plugins/optimization-detective/storage/data.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,6 @@ function od_get_normalized_query_vars(): array {
);
}

// Vary URL Metrics by whether the user is logged in since additional elements may be present.
if ( is_user_logged_in() ) {
$normalized_query_vars['user_logged_in'] = true;
}

return $normalized_query_vars;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public function data_provider_test_od_get_normalized_query_vars(): array {
'set_up' => function (): array {
wp_set_current_user( self::factory()->user->create( array( 'role' => 'subscriber' ) ) );
$this->go_to( home_url( '/' ) );
return array( 'user_logged_in' => true );
return array();
},
),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ public function data_provider_test_od_can_optimize_response(): array {
wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) );
$this->go_to( home_url( '/' ) );
},
'expected' => false,
'expected' => true,
),
);
}
Expand Down
Loading