Fix: Defer Conversion Tracking class and localization initialization#70
Fix: Defer Conversion Tracking class and localization initialization#70
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPlugin initialization was changed to defer class and tracker setup to the WordPress init hook; the constructor no longer calls init_classes() directly and init_tracker() is invoked at the start of init_classes() during the staged initialization. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant WP as "WordPress"
participant PL as "Plugin::__construct"
participant INIT as "init hook"
participant CL as "init_classes"
participant TR as "init_tracker"
participant IG as "Integrations / Translations"
WP->>PL: load plugin (plugins_loaded)
PL->>INIT: register init handler (add_action 'init', init_classes)
WP->>INIT: triggers 'init'
INIT->>CL: call init_classes()
CL->>TR: init_tracker() (first)
CL->>IG: include integrations / load translations
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
conversion-tracking.php (1)
186-189:⚠️ Potential issue | 🔴 Critical
plugin_upgradeswill silently be skipped, and activation hook never registered — deferred constructor breaks timing.The constructor now runs during
plugins_loadedviawcct_init()(priority 10, default). Wheninit_hooks()is called, it registersplugin_upgradesonplugins_loadedat the same priority (10) that is already executing. Per WordPress'sWP_Hookbehavior, callbacks added at an already-executing priority do not run in that same invocation—they run on the nextplugins_loadeddispatch. Result:plugin_upgradesis silently skipped on every page load.Additionally,
register_activation_hook()inside the constructor (line 83) will not run during plugin activation, since WordPress firesactivate_{plugin}beforeplugins_loaded. Theactivate()method setswcct_installedandwcct_versionoptions, which will never be initialized on first activation.Proposed fixes:
Raise
plugin_upgradesto a laterplugins_loadedpriority so it fires afterwcct_initcompletes:- add_action( 'plugins_loaded', array( $this, 'plugin_upgrades' ) ); + add_action( 'plugins_loaded', array( $this, 'plugin_upgrades' ), 20 );Register the activation hook outside the constructor (e.g., at the file level after the class definition, or in a static initialization method) so it is registered before
plugins_loaded:- // Inside __construct(): - register_activation_hook( __FILE__, array( $this, 'activate' ) ); + // After class definition: + register_activation_hook( __FILE__, [ 'WeDevs_WC_Conversion_Tracking', 'activate_plugin' ] );Create a static
activate_plugin()that calls the singleton'sactivate()method, or register the hook inline beforewcct_initis hooked.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@conversion-tracking.php` around lines 186 - 189, The init_hooks registration currently adds plugin_upgrades to plugins_loaded at the same priority as wcct_init so it will be skipped; change the add_action for plugin_upgrades inside init_hooks to use a later priority (e.g., 20) so plugin_upgrades runs after wcct_init completes. Also remove register_activation_hook from the class constructor and register it earlier (file-level or static initializer) by adding a static activate_plugin() wrapper that calls the singleton's activate() (or call activate() directly from that static wrapper) and register_activation_hook(...) against that static function so activation is registered before plugins_loaded.
🧹 Nitpick comments (1)
conversion-tracking.php (1)
74-89: Consider consolidating allinithook registrations insideinit_hooks().
includesandinit_classesare registered oninitdirectly in the constructor body (lines 80–81), whilelocalization_setupandinit_trackerare registered oninitinsideinit_hooks(). The split makes the full execution order non-obvious to maintainers.♻️ Suggested consolidation
public function __construct() { require_once __DIR__ . '/vendor/autoload.php'; $this->define_constants(); $this->init_hooks(); - - // Defer heavy includes and class instantiation until WP 'init' - add_action( 'init', array( $this, 'includes' ), 10 ); - add_action( 'init', array( $this, 'init_classes' ), 20 ); // Add High Performance Order Storage Support add_action( 'before_woocommerce_init', [ $this, 'declare_woocommerce_feature_compatibility' ] ); do_action( 'wcct_loaded' ); }public function init_hooks() { add_action( 'plugins_loaded', array( $this, 'plugin_upgrades' ), 20 ); add_action( 'init', array( $this, 'localization_setup' ) ); + // Defer heavy includes and class instantiation until WP 'init' + add_action( 'init', array( $this, 'includes' ), 10 ); + add_action( 'init', array( $this, 'init_classes' ), 20 ); add_action( 'admin_notices', array( $this, 'check_woocommerce_exist' ) ); add_filter( 'plugin_action_links_' . plugin_basename( __FILE__ ), array( $this, 'plugin_action_links' ) ); add_action( 'admin_notices', array( $this, 'happy_addons_ads_banner' ) ); // Defer tracker initialization until after localization and integrations add_action( 'init', array( $this, 'init_tracker' ), 30 ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@conversion-tracking.php` around lines 74 - 89, The constructor currently registers includes() and init_classes() on the 'init' hook while localization_setup() and init_tracker() are registered inside init_hooks(), splitting init registrations and hiding execution order; move the add_action calls for includes (priority 10) and init_classes (priority 20) from the __construct() into init_hooks() so all 'init' hook registrations are consolidated in one place (preserve the original priorities), remove the duplicate registrations in the constructor, and keep other constructor responsibilities (define_constants(), init_hooks(), activation hook, before_woocommerce_init registration, and do_action('wcct_loaded')) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@conversion-tracking.php`:
- Line 83: The register_activation_hook call is being registered inside the
class constructor (which runs on plugins_loaded), so the activate() callback
(which sets wcct_installed and wcct_version) is missed during the activation
request; move the register_activation_hook out of the constructor to file scope
so it runs at require-time (before plugins_loaded) and binds activate()
correctly, keeping the plugins_loaded add_action for normal initialization
inside the constructor.
---
Outside diff comments:
In `@conversion-tracking.php`:
- Around line 186-189: The init_hooks registration currently adds
plugin_upgrades to plugins_loaded at the same priority as wcct_init so it will
be skipped; change the add_action for plugin_upgrades inside init_hooks to use a
later priority (e.g., 20) so plugin_upgrades runs after wcct_init completes.
Also remove register_activation_hook from the class constructor and register it
earlier (file-level or static initializer) by adding a static activate_plugin()
wrapper that calls the singleton's activate() (or call activate() directly from
that static wrapper) and register_activation_hook(...) against that static
function so activation is registered before plugins_loaded.
---
Nitpick comments:
In `@conversion-tracking.php`:
- Around line 74-89: The constructor currently registers includes() and
init_classes() on the 'init' hook while localization_setup() and init_tracker()
are registered inside init_hooks(), splitting init registrations and hiding
execution order; move the add_action calls for includes (priority 10) and
init_classes (priority 20) from the __construct() into init_hooks() so all
'init' hook registrations are consolidated in one place (preserve the original
priorities), remove the duplicate registrations in the constructor, and keep
other constructor responsibilities (define_constants(), init_hooks(), activation
hook, before_woocommerce_init registration, and do_action('wcct_loaded'))
intact.
| add_action( 'init', array( $this, 'includes' ), 10 ); | ||
| add_action( 'init', array( $this, 'init_classes' ), 20 ); | ||
|
|
||
| register_activation_hook( __FILE__, array( $this, 'activate' ) ); |
There was a problem hiding this comment.
activate() never fires on plugin activation — register_activation_hook is now too late.
WordPress fires activate_{plugin_basename} during the activation request, immediately after loading the plugin file but before plugins_loaded. Since the constructor (and therefore register_activation_hook) is now deferred to plugins_loaded, the activation callback is never registered when the activate_ hook fires. activate() — which writes wcct_installed and wcct_version options — is silently skipped on every fresh install or re-activation.
Move register_activation_hook outside the constructor, to file scope where it executes at require-time:
🐛 Proposed fix
Remove it from the constructor:
public function __construct() {
require_once __DIR__ . '/vendor/autoload.php';
$this->define_constants();
$this->init_hooks();
// Defer heavy includes and class instantiation until WP 'init'
add_action( 'init', array( $this, 'includes' ), 10 );
add_action( 'init', array( $this, 'init_classes' ), 20 );
- register_activation_hook( __FILE__, array( $this, 'activate' ) );
// Add High Performance Order Storage Support
add_action( 'before_woocommerce_init', [ $this, 'declare_woocommerce_feature_compatibility' ] );
do_action( 'wcct_loaded' );
}Register it at file scope, alongside the plugins_loaded hook:
function wcct_init() {
return WeDevs_WC_Conversion_Tracking::init();
}
// Instantiate plugin on 'plugins_loaded' to allow localization to be registered
add_action( 'plugins_loaded', 'wcct_init' );
+ register_activation_hook( __FILE__, function() {
+ wcct_init()->activate();
+ } );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@conversion-tracking.php` at line 83, The register_activation_hook call is
being registered inside the class constructor (which runs on plugins_loaded), so
the activate() callback (which sets wcct_installed and wcct_version) is missed
during the activation request; move the register_activation_hook out of the
constructor to file scope so it runs at require-time (before plugins_loaded) and
binds activate() correctly, keeping the plugins_loaded add_action for normal
initialization inside the constructor.
…improved performance
2 participants
Closes #64
Changelog
Fixed an issue where the woocommerce-conversion-tracking text domain was loaded too early, causing PHP notices.
Deferred class and integration initialization to the init hook to ensure proper localization.
Summary by CodeRabbit