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

Fix logic inversion in od_can_optimize_response() #1659

Merged
merged 1 commit into from
Nov 17, 2024

Conversation

westonruter
Copy link
Member

I was testing the latest plugins on my blog and I was perplexed to find that Optimization Detective wasn't working on any URL. I found out it was due to a logic inversion I introduced in #1641. I didn't notice the issue when testing locally because I have the following dev-mode plugin code running, including:

add_filter( 'od_can_optimize_response', '__return_true' );

This means that the checking for od_get_cache_purge_post_id() I added to od_can_optimize_response() wasn't having any effect. Note that each of the conditions in that function are meant to be negative conditions for which responses should not be optimized. The entire boolean chain of conditions is then negated. So this is why the logic looks correct at first glance, but not if you look at the other conditions in the boolean chain.

The issue also was not detected in unit tests because (1) there are no posts created initially when unit tests run so null !== od_get_cache_purge_post_id() was always returning false, and (2) I hadn't explicitly checked the condition for when this is expected to return null.

These are rectified in this PR.

@westonruter westonruter added [Type] Bug An existing feature is broken [Plugin] Optimization Detective Issues for the Optimization Detective plugin skip changelog PRs that should not be mentioned in changelogs labels Nov 17, 2024
Copy link

github-actions bot commented Nov 17, 2024

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: westonruter <westonruter@git.wordpress.org>
Co-authored-by: felixarntz <flixos90@git.wordpress.org>

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

@westonruter westonruter changed the base branch from trunk to publish/3.6.0 November 17, 2024 04:18
@westonruter westonruter changed the base branch from publish/3.6.0 to release/3.6.0 November 17, 2024 04:19
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Good catch!

@westonruter westonruter merged commit 89e4080 into release/3.6.0 Nov 17, 2024
22 checks passed
@westonruter westonruter deleted the fix/od-can-optimize-check branch November 17, 2024 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Optimization Detective Issues for the Optimization Detective plugin skip changelog PRs that should not be mentioned in changelogs [Type] Bug An existing feature is broken
Projects
Status: Done 😃
Development

Successfully merging this pull request may close these issues.

2 participants