Conversation
…rdless of experiment is on/off
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #973 +/- ##
=============================================
- Coverage 74.57% 74.54% -0.04%
- Complexity 3132 3141 +9
=============================================
Files 132 133 +1
Lines 12213 12237 +24
=============================================
+ Hits 9108 9122 +14
- Misses 3105 3115 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
dkotter
left a comment
There was a problem hiding this comment.
Things look good to me, thanks! Holding on merging this for now though as we add a new upgrade routine here that has the file name hardcoded to v1.4.0. If we end up doing a minor release (v1.3.1 as an example) prior to releasing 1.4.0, this code will be inaccurate. Will merge this once we know for sure the version we'll be releasing this as
dugyen
left a comment
There was a problem hiding this comment.
Reviewed this in detail (diff, tests, and interaction with the surrounding code). This looks like a solid, correct fix — didn't find a blocking issue.
What it does right, matching both parts of #970:
- Adds a
DAY_IN_SECONDSTTL to the positive cache inSEO_Integration.php, and now also caches a'none'sentinel for "no SEO plugin active" so the front-endoutput_meta_description()path stops re-scanning every plugin on every request. - Moves cache invalidation off
Meta_Description::register()and onto an unconditionalSEO_Integration::register_cache_invalidation()called fromMain::load()(hooked toplugins_loaded), so the cache now self-heals viaactivated_plugin/deactivated_pluginregardless of whether the experiment is enabled — exactly the "invalidation independent of experiment toggle" fix the issue asked for. - Goes a bit further than requested: the new
V1_4_0upgrade routine deletes the old non-expiring transient once on update, so sites with an already-stale infinite cache get a clean slate rather than waiting on the new logic. Also handles multisite network-wide activation by clearing the cache on every site in the network. - Test coverage is thorough: positive/negative caching, TTL-independent invalidation on both hooks, the upgrade routine's version-gating (skips on fresh installs and already-upgraded sites), and multisite network-wide vs. single-site invalidation scope.
- I checked for stale references to the removed
clear_active_plugin_cache()method/hook elsewhere in the codebase, including the pre-existingMeta_DescriptionTest.php— none found, so no regression there.
Two minor, non-blocking observations:
- The multisite network-wide path (
clear_cache_for_network()) fetches every site (get_sites(['number' => 0])) and does aswitch_to_blog()round-trip per site on every plugin activate/deactivate, not just SEO-relevant ones. Fine for typical networks; could be slow on a network with many thousands of sites. - The
phpcs:ignorecomment onswitch_to_blog()names a specific VIPMinimum sniff — worth confirming CI is happy with that exact sniff name for this repo's ruleset, but harmless either way.
Nice work, @hbhalodia.
AI assistance: Yes. Tool(s): Claude Code (Sonnet 5). Used for: reading the diff and tracing it against the rest of the codebase to write this review; I read and take responsibility for the assessment above.
What?
Closes #970
Why?
How?
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Code, Unit Test under the direction provided by me. Implementation reviwed by me.
Testing Instructions
Changelog Entry