Prevent optimizing post previews by default

[hbhalodia]

Summary

Fixes #1841

Relevant technical choices

• This PR adds the condition is_preview() in od_can_optimize_response function to exclude the post/page preview, because it won't make sense to gather the URL metric data for the post that is not yet published.
• See - Optimization Detective should be disabled by default on post previews #1841

[github-actions]

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: hbhalodia <[email protected]>
Co-authored-by: westonruter <[email protected]>

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.98%. Comparing base (68c6deb) to head (e961c37).
Report is 7 commits behind head on trunk.

Additional details and impacted files

@@           Coverage Diff           @@
##            trunk    #1848   +/-   ##
=======================================
  Coverage   65.97%   65.98%           
=======================================
  Files          88       88           
  Lines        6895     6896    +1     
=======================================
+ Hits         4549     4550    +1     
  Misses       2346     2346           

Flag	Coverage Δ
multisite	65.98% <100.00%> (+<0.01%)	⬆️
single	38.18% <100.00%> (+<0.01%)	⬆️

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.

[westonruter]

Thank you! Could you please add a test case to make sure that this condition is working as expected?

[hbhalodia]

Hi @westonruter, I had tried to add the test case for the same, Could you please check if below is the correct way to add a test case for this?

'singular_as_post_preview'  => array(
  'set_up'   => static function (): string {
	  $post_id = self::factory()->post->create( array( 'post_title' => 'Hello', 'post_status' => 'draft' ) );
	  return get_permalink( $post_id );
  },
  'expected' => false,
),

We can add the above here after this,

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

Lines 284 to 292 in 68c6deb

	'home_customizer_preview_as_anonymous' => array(
	'set_up' => static function (): string {
	global $wp_customize;
	require_once ABSPATH . 'wp-includes/class-wp-customize-manager.php';
	$wp_customize = new WP_Customize_Manager();
	$wp_customize->start_previewing_theme();
	return home_url( '/' );
	},
	'expected' => false,

If yes, I would update the PR with the changes, I can update the test case name as per the standards.

I do have run the test locally, by mocking this as publish and draft and it results in correct output as expected after adding the condition.

Thank You,

[westonruter]

@hbhalodia that mostly looks good, but probably should use get_preview_post_link().

[hbhalodia]

Hi @westonruter, Thanks for the suggestion.

I have one issue, when I try to commit this, phpstan is giving issue related to return value as it should only be string, but function return either string|null. I tried adding the return value to accept string|null, but it throws error related native union type, I committed it with --no-verify, Can you please suggest how can I pass this failed action.

Thank You,

[hbhalodia]

Hi @westonruter, This is now fixed. Updated the PR with the changes and test cases.

Thank You,

[westonruter]

I tried commenting out this line to see if the new test would start to fail as expected, but it still passed. This is because the test's set_up logic wasn't setting the current user to be able to view the post preview in the first place. When this happens, the $wp_query->posts loop is then empty. This results in od_get_cache_purge_post_id() returning null which as can be seen below also results in this od_can_optimize_response() function returning false. So I update the test in e961c37 to explicitly set the current user so that is_preview() will return true.

[hbhalodia]

Thanks @westonruter, for the fix and insights on this.

[westonruter]

Thank you!