Enhance Optimization Detective meta generator tag with all disabled reasons #1979

ShyamGadde · 2025-04-09T07:59:11Z

Summary

Fixes #1977

Relevant technical choices

Created a new helper function od_get_disabled_reasons() that centralizes the logic for determining why Optimization Detective is disabled
Updated the meta generator tag to include all disabled reason flags (can_optimize_response_false, rest_api_unavailable, and query_param_disabled)
Refactored the existing code in od_maybe_add_template_output_buffer_filter() to use this helper function

Signed-off-by: Shyamsundar Gadde <[email protected]>

codecov · 2025-04-09T08:07:54Z

Codecov Report

Attention: Patch coverage is 96.07843% with 2 lines in your changes missing coverage. Please review.

Project coverage is 72.58%. Comparing base (9ac71ce) to head (291f732).

Files with missing lines	Patch %	Lines
plugins/optimization-detective/optimization.php	60.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk    #1979      +/-   ##
==========================================
+ Coverage   72.51%   72.58%   +0.07%     
==========================================
  Files          85       85              
  Lines        7023     7041      +18     
==========================================
+ Hits         5093     5111      +18     
  Misses       1930     1930

Flag	Coverage Δ
multisite	`72.58% <96.07%> (+0.07%)`	⬆️
single	`39.72% <0.00%> (-0.62%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

plugins/optimization-detective/helper.php

Co-authored-by: Weston Ruter <[email protected]>

Signed-off-by: Shyamsundar Gadde <[email protected]>

github-actions · 2025-04-09T22:28:56Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ShyamGadde <[email protected]>
Co-authored-by: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

westonruter · 2025-04-09T23:04:36Z

plugins/optimization-detective/helper.php

+function od_get_disabled_reasons(): array {
+	$conditions = array(
+		'can_optimize_response_false' => array(
+			'test'   => od_can_optimize_response(),


I just realized that the disabled reasons are somewhat nested here. There are 7 different possible reasons for OD being disabled in od_can_optimize_response(). Should the logic in that function be similarly structured in an array in another function so that its reasons can be exposed as well? Maybe this is overkill. But I just wanted to mention it since it came to mind.

I implemented this in bae1198. Please let me know if this matches what you had in mind.

…tion Detective disabled state Signed-off-by: Shyamsundar Gadde <[email protected]>

westonruter · 2025-04-10T15:54:33Z

plugins/optimization-detective/optimization.php

 */
-function od_can_optimize_response(): bool {


We should add this function back as a wrapper for the new function, like:

function od_can_optimize_response(): bool { return count( od_get_cannot_optimize_reasons() ) === 0; }

Granted, the function is private, but still... we're technically in beta so we shouldn't break any back-compat.

Done in 9c097f6.

westonruter · 2025-04-10T15:55:53Z

plugins/optimization-detective/tests/test-optimization.php

 	 *
-	 * @covers ::od_can_optimize_response
+	 * @covers ::od_get_cannot_optimize_reasons


This test could then test both od_get_cannot_optimize_reasons() and od_can_optimize_response()

plugins/optimization-detective/helper.php

…ptimize_reasons Signed-off-by: Shyamsundar Gadde <[email protected]>

…unction Signed-off-by: Shyamsundar Gadde <[email protected]>

westonruter · 2025-04-10T19:55:34Z

plugins/optimization-detective/helper.php

+	// > Access to script at '.../detect.js?ver=0.4.1' from origin 'null' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.
+	// So it's better to just avoid attempting to optimize Post Embed responses (which don't need optimization anyway).
+	if ( is_embed() ) {
+		$reasons['is_embed'] = __( 'Page is not optimized because it is an embed.', 'optimization-detective' );


Let's add a test for this for good measure.

Signed-off-by: Shyamsundar Gadde <[email protected]>

westonruter · 2025-04-10T20:48:28Z

plugins/optimization-detective/helper.php

+	 * @param bool $able Whether response can be optimized.
+	 */
+	$can_optimize_after_filter = (bool) apply_filters( 'od_can_optimize_response', $can_optimize_by_default );


Idea: How about we pass the $reasons into the od_can_optimize_response filter as the second argument? This would allow od_can_optimize_response filter callbacks to see any existing reasons for why OD was disabled, and then add their own considerations on top.

So for example, the above $reasons array could instead be an $disabled_flags (or maybe there is a better name) which is array<string, bool>, this could then be passed as the second argument to the filter.

The array of booleans can then be passed through array_filter to remove the non-disabled flags, and then the resulting can be intersected with associative array array<string, string> that has the disabled reasons provided as the string array values. This resulting intersection can then be returned as $reasons.

Why would this be useful to add? Consider the case where you want to selectively enable the search template to be optimized. In order to do this, you'd have to re-compute all of the conditions to flip the false value of $can_optimize_by_default back to true. But if all of the flags were passed in, then they could be examined before flipping that boolean. For example:

add_filter( 'od_can_optimize_response', static function ( $can_optimize, $disabled_flags ) { if ( ! $can_optimize && is_search() ) { unset( $disabled_flags['is_search'] ); $can_optimize = count( array_filter( $disabled_flags ) ) === 0; } return $can_optimize; } );

So in this way, the $disabled_flags would not just be array<string, bool> but rather it should be an array shape that defines all of the keys.

Implemented it in 28264af.

Signed-off-by: Shyamsundar Gadde <[email protected]>

westonruter

Sorry for the delay in adding another review. 😞

westonruter · 2025-04-18T16:44:15Z

plugins/optimization-detective/helper.php

+		$disabled_flags['is_preview'] = true;
+	}
+
+	// Since injection of inline-editing controls interfere with breadcrumbs, while also just not necessary in this context.


Suggested change

// Since injection of inline-editing controls interfere with breadcrumbs, while also just not necessary in this context.

// Disable in Customizer preview since injection of inline-editing controls can interfere with XPath. Optimization is also not necessary in this context.

westonruter · 2025-04-18T16:44:41Z

plugins/optimization-detective/helper.php

+		'no_cache_purge_post_id' => false,
+	);
+
+	// Since there is no predictability in whether posts in the loop will have featured images assigned or not. If a


Suggested change

// Since there is no predictability in whether posts in the loop will have featured images assigned or not. If a

// Disable the search template since there is no predictability in whether posts in the loop will have featured images assigned or not. If a

westonruter · 2025-04-18T16:45:58Z

plugins/optimization-detective/helper.php

+		$disabled_flags['is_customize_preview'] = true;
+	}
+
+	// Since the images detected in the response body of a POST request cannot, by definition, be cached.


Suggested change

// Since the images detected in the response body of a POST request cannot, by definition, be cached.

// Disable for POST responses since since they cannot, by definition, be cached.

westonruter · 2025-04-18T16:46:29Z

plugins/optimization-detective/helper.php

+		$disabled_flags['not_get_request'] = true;
+	}
+
+	// Page caching plugins can only reliably be told to invalidate a cached page when a post is available to trigger


Suggested change

// Page caching plugins can only reliably be told to invalidate a cached page when a post is available to trigger

// Disable when there is no post ID available for cache purging. Page caching plugins can only reliably be told to invalidate a cached page when a post is available to trigger

westonruter · 2025-04-18T16:50:43Z

plugins/optimization-detective/helper.php

+		foreach ( $disabled_flags as $flag => $is_disabled ) {
+			if ( $is_disabled && isset( $reason_messages[ $flag ] ) ) {
+				$reasons[ $flag ] = $reason_messages[ $flag ];
+			}
+		}


Alternative which uses wp_array_slice_assoc():

Suggested change

foreach ( $disabled_flags as $flag => $is_disabled ) {

if ( $is_disabled && isset( $reason_messages[ $flag ] ) ) {

$reasons[ $flag ] = $reason_messages[ $flag ];

}

}

$reasons = wp_array_slice_assoc( $reason_messages, array_keys( array_filter( $disabled_flags ) ) );

westonruter · 2025-04-18T16:51:46Z

plugins/optimization-detective/helper.php

+ * @since n.e.x.t
+ * @access private
+ *
+ * @return array<string, string> Array of disabled reason codes and their messages.


How about adding an explicit array shape for the return value here, defining all of the possible (nullable) keys mapping to boolean values?

Maybe this is overkill since we're never . If we do it here, then it should also be done in the param for the od_can_optimize_response filter.

Add all disabled reasons to Optimization Detective meta generator tag

d5bf984

Signed-off-by: Shyamsundar Gadde <[email protected]>

ShyamGadde added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Optimization Detective Issues for the Optimization Detective plugin labels Apr 9, 2025

ShyamGadde added this to the optimization-detective 1.1.0 milestone Apr 9, 2025

westonruter reviewed Apr 9, 2025

View reviewed changes

plugins/optimization-detective/helper.php Outdated Show resolved Hide resolved

ShyamGadde and others added 4 commits April 10, 2025 03:44

Fix function documentation wording

2a65043

Co-authored-by: Weston Ruter <[email protected]>

Merge branch 'trunk' into update/od-meta-generator-tag

10161e5

Add test coverage annotation for od_get_disabled_reasons helper function

74a7607

Signed-off-by: Shyamsundar Gadde <[email protected]>

Add tests for meta generator tag with different disabled reasons

c0214cf

Signed-off-by: Shyamsundar Gadde <[email protected]>

ShyamGadde marked this pull request as ready for review April 9, 2025 22:28

ShyamGadde requested a review from felixarntz as a code owner April 9, 2025 22:28

ShyamGadde requested a review from westonruter April 9, 2025 22:28

westonruter reviewed Apr 9, 2025

View reviewed changes

Enhance meta generator tag to include all nested reasons for Optimiza…

bae1198

…tion Detective disabled state Signed-off-by: Shyamsundar Gadde <[email protected]>

westonruter reviewed Apr 10, 2025

View reviewed changes

plugins/optimization-detective/helper.php Show resolved Hide resolved

Reintroduce od_can_optimize_response as a wrapper for od_get_cannot_o…

9c097f6

…ptimize_reasons Signed-off-by: Shyamsundar Gadde <[email protected]>

ShyamGadde force-pushed the update/od-meta-generator-tag branch from f3712cd to 9c097f6 Compare April 10, 2025 19:14

Consolidate all disabled reason checks into od_get_disabled_reasons f…

9467eea

…unction Signed-off-by: Shyamsundar Gadde <[email protected]>

westonruter reviewed Apr 10, 2025

View reviewed changes

Add test for is_embed

57969e1

Signed-off-by: Shyamsundar Gadde <[email protected]>

ShyamGadde requested a review from westonruter April 10, 2025 20:23

westonruter reviewed Apr 10, 2025

View reviewed changes

Enhance od_can_optimize_response filter with disabled_flags parameter

28264af

Signed-off-by: Shyamsundar Gadde <[email protected]>

This comment was marked as resolved.

Sign in to view

felixarntz mentioned this pull request Apr 15, 2025

PHPUnit tests failing in trunk #1987

Closed

This comment was marked as resolved.

Sign in to view

Merge branch 'trunk' into update/od-meta-generator-tag

291f732

westonruter reviewed Apr 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance Optimization Detective meta generator tag with all disabled reasons #1979

Enhance Optimization Detective meta generator tag with all disabled reasons #1979

ShyamGadde commented Apr 9, 2025 •

edited

Loading

codecov bot commented Apr 9, 2025 •

edited

Loading

github-actions bot commented Apr 9, 2025 •

edited

Loading

westonruter Apr 9, 2025

ShyamGadde Apr 10, 2025

westonruter Apr 10, 2025

westonruter Apr 10, 2025

ShyamGadde Apr 10, 2025

westonruter Apr 10, 2025

westonruter Apr 10, 2025

westonruter Apr 10, 2025

westonruter Apr 10, 2025

westonruter Apr 10, 2025

ShyamGadde Apr 14, 2025

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

westonruter left a comment

westonruter Apr 18, 2025

westonruter Apr 18, 2025

westonruter Apr 18, 2025

westonruter Apr 18, 2025

westonruter Apr 18, 2025

westonruter Apr 18, 2025

westonruter Apr 18, 2025

	// Since injection of inline-editing controls interfere with breadcrumbs, while also just not necessary in this context.
	// Disable in Customizer preview since injection of inline-editing controls can interfere with XPath. Optimization is also not necessary in this context.

	// Since there is no predictability in whether posts in the loop will have featured images assigned or not. If a
	// Disable the search template since there is no predictability in whether posts in the loop will have featured images assigned or not. If a

	// Since the images detected in the response body of a POST request cannot, by definition, be cached.
	// Disable for POST responses since since they cannot, by definition, be cached.

	// Page caching plugins can only reliably be told to invalidate a cached page when a post is available to trigger
	// Disable when there is no post ID available for cache purging. Page caching plugins can only reliably be told to invalidate a cached page when a post is available to trigger

Enhance Optimization Detective meta generator tag with all disabled reasons #1979

Are you sure you want to change the base?

Enhance Optimization Detective meta generator tag with all disabled reasons #1979

Conversation

ShyamGadde commented Apr 9, 2025 • edited Loading

Summary

Relevant technical choices

codecov bot commented Apr 9, 2025 • edited Loading

Codecov Report

github-actions bot commented Apr 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

westonruter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShyamGadde commented Apr 9, 2025 •

edited

Loading

codecov bot commented Apr 9, 2025 •

edited

Loading

github-actions bot commented Apr 9, 2025 •

edited

Loading