class/class-mainwp-child.php (3)

74-82: Unnecessary pass‑by‑reference in callback arrays

The &$this notation is only needed when you modify the referenced variable, which is not the case when registering callbacks with add_action(). Removing the ampersand makes the intent clearer and avoids the (minor) overhead caused by creating references.

-        add_action( 'init', array( &$this, 'init_check_login' ), 1 );
-        add_action( 'init', array( &$this, 'parse_init' ), 9999 );
-        add_action( 'init', array( &$this, 'localization' ), 33 );
-        add_action( 'init', array( &$this, 'init_hooks' ), 9 );
-        add_action( 'admin_init', array( &$this, 'admin_init' ) );
+        add_action( 'init', array( $this, 'init_check_login' ), 1 );
+        add_action( 'init', array( $this, 'parse_init' ), 9999 );
+        add_action( 'init', array( $this, 'localization' ), 33 );
+        add_action( 'init', array( $this, 'init_hooks' ), 9 );
+        add_action( 'admin_init', array( $this, 'admin_init' ) );

---

522-524: Wrapper is_plugin_active() adds no value

The helper simply forwards the call to WordPress’ global function:

private function is_plugin_active( $plugin ) {
    return is_plugin_active( $plugin );
}

Unless you plan to stub or override this in unit tests, the extra indirection is unnecessary. Removing the wrapper avoids a function call per check and simplifies the class.

---

531-551: Minor security / robustness tweak for $_GET['page'] usage

strpos( $_GET['page'], 'mainwp' ) is safe in this context because you only read the value, but casting to string prevents the edge case where page is an array (possible if someone crafts the query string).

-        $is_mainwp_page = (isset($_GET['page']) && strpos($_GET['page'], 'mainwp') !== false);
+        $is_mainwp_page = (isset($_GET['page']) && strpos( (string) $_GET['page'], 'mainwp' ) !== false);

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

class/class-mainwp-child.php (2)

111-121: ⚠️ Potential issue

Move performance-intensive component initialization to the current_screen action

The is_mainwp_pages() method relies on get_current_screen() which isn't available during the constructor. This could cause these components to never be initialized, even on MainWP-specific pages.

Move this initialization to a callback on the current_screen action or at least to admin_init:

- // These are only needed in admin area and are performance-intensive
- if ($this->is_mainwp_pages()) {
-     // Lazy load these only when on MainWP pages
-     MainWP_Child_Plugins_Check::instance();
-     MainWP_Child_Themes_Check::instance();
- 
-     // Initiate MainWP Cache Control class
-     MainWP_Child_Cache_Purge::instance();
- 
-     // Initiate MainWP Child API Backups class
-     MainWP_Child_Api_Backups::instance();
- }
+ // Initialize performance-intensive components only when needed
+ add_action(
+     'current_screen',
+     function( $screen ) {
+         // List of screens where MainWP Child functionality is needed
+         $mainwp_screens = array(
+             'settings_page_mainwp_child_tab',
+             'dashboard',
+             'update-core',
+         );
+         
+         // Also check if we're on a page with mainwp in the query string
+         $is_mainwp_page = false;
+         if ( isset( $_GET['page'] ) ) {
+             $page = sanitize_text_field( wp_unslash( $_GET['page'] ) );
+             $is_mainwp_page = ( false !== strpos( $page, 'mainwp' ) );
+         }
+         
+         if ( in_array( $screen->id, $mainwp_screens, true ) || $is_mainwp_page ) {
+             // Lazy load these only when on MainWP pages
+             MainWP_Child_Plugins_Check::instance();
+             MainWP_Child_Themes_Check::instance();
+             MainWP_Child_Cache_Purge::instance();
+             MainWP_Child_Api_Backups::instance();
+         }
+     }
+ );

---

84-93: ⚠️ Potential issue

Use current_screen action instead of direct get_current_screen() call

The code attempts to check the current screen in the constructor, but get_current_screen() returns null before the current_screen action has fired. This will likely cause these hooks to never be registered, even when on the plugins or update-core pages.

Refactor this code to use the current_screen action instead:

- // Only add these hooks when on the plugins page or updates page
- if (is_admin() && function_exists('get_current_screen')) {
-     $screen = get_current_screen();
-     if ($screen && ($screen->id === 'plugins' || $screen->id === 'update-core')) {
-         // Support for better detection for premium plugins
-         add_action('pre_current_active_plugins', array(MainWP_Child_Updates::get_instance(), 'detect_premium_themesplugins_updates'));
-         // Support for better detection for premium themes
-         add_action('core_upgrade_preamble', array(MainWP_Child_Updates::get_instance(), 'detect_premium_themesplugins_updates'));
-     }
- }
+ // Register conditional hooks for plugins/updates pages
+ add_action(
+     'current_screen',
+     function( $screen ) {
+         if ( $screen && in_array( $screen->id, array( 'plugins', 'update-core' ), true ) ) {
+             // Support for better detection for premium plugins/themes
+             add_action( 'pre_current_active_plugins', array( MainWP_Child_Updates::get_instance(), 'detect_premium_themesplugins_updates' ) );
+             add_action( 'core_upgrade_preamble', array( MainWP_Child_Updates::get_instance(), 'detect_premium_themesplugins_updates' ) );
+         }
+     }
+ );

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

class/class-mainwp-child.php (2)

470-471: Inconsistent extension initialization pattern.

Most extensions follow the pattern of calling both instance() and init(), but MainWP_Child_Back_Up_Buddy only calls instance() without init(). This could be intentional, but it's inconsistent with other extensions.

Consider using the same pattern for all extensions:

-            MainWP_Child_Back_Up_Buddy::instance();
+            MainWP_Child_Back_Up_Buddy::instance()->init();

---

534-535: Duplicate comment block for incorrect method.

These lines contain a duplicate comment block that appears to be for the is_mainwp_pages() method, but it's placed above the get_current_admin_page() method.

Remove the duplicate comment block:

-    /**
-     * Helper method to check if we're on a MainWP admin page.
-     *
-     * @return bool True if on a MainWP admin page, false otherwise.
-     */

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

class/class-mainwp-child.php (2)

548-575: ⚠️ Potential issue

is_mainwp_pages() inherits the same timing flaw

The helper again calls get_current_screen() directly. When invoked from init_full() (on plugins_loaded) it will always return false, so the performance‑heavy classes are never instantiated—even on genuine MainWP pages.

Reuse the current_screen callback suggested above, or call is_mainwp_pages() inside such a callback to ensure $screen is populated.
Otherwise critical functionality (Plugins_Check, Themes_Check, cache purge, API backups) will remain disabled.

---

124-135: 🛠️ Refactor suggestion

⚠️ Potential issue

get_current_screen() still queried too early – hooks will never be added

init_full() is executed on plugins_loaded, which fires long before WordPress sets up the current admin screen (current_screen action).
At this moment get_current_screen() returns null, so the two hooks intended for the Plugins and Updates pages are never registered.
The exact same timing problem was pointed out in an earlier review but remains unresolved.

Proposed fix – defer the screen check:

-        // Cache the screen object to avoid multiple calls to get_current_screen()
-        $screen = null;
-        if ( function_exists( 'get_current_screen' ) ) {
-            $screen = get_current_screen();
-        }
-
-        // Only add these hooks when on the plugins page or updates page
-        if ( $screen && ( 'plugins' === $screen->id || 'update-core' === $screen->id ) ) {
-            add_action( 'pre_current_active_plugins', array( MainWP_Child_Updates::get_instance(), 'detect_premium_themesplugins_updates' ) );
-            add_action( 'core_upgrade_preamble',      array( MainWP_Child_Updates::get_instance(), 'detect_premium_themesplugins_updates' ) );
-        }
+        // Defer until WP knows the current screen.
+        add_action(
+            'current_screen',
+            static function ( $screen ) {
+                if ( in_array( $screen->id, array( 'plugins', 'update-core' ), true ) ) {
+                    add_action( 'pre_current_active_plugins', array( MainWP_Child_Updates::get_instance(), 'detect_premium_themesplugins_updates' ) );
+                    add_action( 'core_upgrade_preamble',      array( MainWP_Child_Updates::get_instance(), 'detect_premium_themesplugins_updates' ) );
+                }
+            }
+        );

This guarantees the hooks are added only when the correct screen is active while avoiding premature get_current_screen() calls.

mainwp-child.php (1)

111-129: Minor: use wp_doing_cron() / wp_doing_rest() helpers for clarity

WordPress 5.8+ ships with wp_doing_cron() and wp_doing_rest() which convey intent more clearly than raw constant checks and avoid false negatives when the constants are defined but false.

Not blocking, but adopting them improves readability:

&& ! wp_doing_cron()
&& ! wp_doing_rest()

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

class/class-mainwp-child.php (1)

275-348: 💡 Verification agent

❓ Verification inconclusive

Individual option queries: ensure values are maybe_unserialize()‑ed

Fetching each option with raw SQL bypasses the normal get_option() logic that unserialises complex values.
Storing the raw (still‑serialised) string in the alloptions cache may break consumers expecting decoded arrays/objects and can trigger fatal errors when they attempt array access.

-                $result = $wpdb->get_row(
+                $result = $wpdb->get_row(
                     $wpdb->prepare(
                         "SELECT option_name, option_value FROM $wpdb->options WHERE option_name = %s",
                         $option
                     )
                 );
                 if ( $result ) {
-                    $alloptions_db[] = $result;
+                    // Mirror WordPress core behaviour.
+                    $result->option_value = maybe_unserialize( $result->option_value );
+                    $alloptions_db[]      = $result;
                 }

Please grep for any places that assume mainwp_child_* options are arrays/objects; those callers will break if the value remains serialised.

---

Fix unserialization of options loaded via raw SQL

By bypassing get_option(), you’re inserting serialized strings into the options cache. Mirror core behavior by running maybe_unserialize() on each $result->option_value before storing it.

Suggested diff in class/class-mainwp-child.php (around line 300):

--- a/class/class-mainwp-child.php
+++ b/class/class-mainwp-child.php
@@ foreach ( $mainwp_options as $option ) {
-                if ( $result ) {
-                    $alloptions_db[] = $result;
-                }
+                if ( $result ) {
+                    // Mirror WordPress core: unserialize complex values.
+                    $result->option_value = maybe_unserialize( $result->option_value );
+                    $alloptions_db[] = $result;
+                }

Action items:

• Apply the above change in class/class-mainwp-child.php.

• Search the codebase for places that consume mainwp_child_* options and assume they’re arrays/objects. Update any callers that directly expect an array or object:

  rg -n "mainwp_child_" .

class/class-mainwp-child.php (3)

100-128: Minor duplication of hooks between init_frontend_only() and register_essential_hooks()

template_redirect and the localisation hook are added both here (lines 101‑105) and again inside register_essential_hooks() (lines 231‑236).
In practice only one of the init‑methods will run, so this is harmless, but removing the duplication keeps intent crystal‑clear.

---

230-245: Spelling of filter mainwp_child_verify_authed_acion_nonce

The hook name is missing the “t” in “action”. If this is intentional for backward compatibility, please add an inline comment; otherwise, consider correcting it to avoid future typos and confusion.

---

610-648: is_mainwp_pages() could shortcut earlier & avoid extra get_current_screen() calls

Great helper, but you can avoid the relatively expensive get_current_screen() call on every non‑MainWP admin page by early‑returning when $_GET['page'] (sanitised) does not contain mainwp.

$page = filter_input( INPUT_GET, 'page', FILTER_SANITIZE_FULL_SPECIAL_CHARS );
if ( empty( $page ) || stripos( $page, 'mainwp' ) === false ) {
    // Nothing indicates a MainWP page; bail out without calling get_current_screen().
    return false;
}

This micro‑optimisation saves a DB query in multisite setups where get_current_screen() is comparatively heavy.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

class/class-mainwp-child.php (1)

270-336: 🛠️ Refactor suggestion

load_all_options() now issues ~30 individual SQL queries – consider a single prepared IN (…) query

Security‑wise the per‑option prepare( …, %s ) loop is sound, but it replaces one query with up to 30, which is noticeably slower on database servers with higher latency.

A safe and performant alternative is to build the placeholders dynamically:

$placeholders = implode( ',', array_fill( 0, count( $mainwp_options ), '%s' ) );
$sql         = "SELECT option_name, option_value
                FROM $wpdb->options
                WHERE option_name IN ( $placeholders )";

$alloptions_db = $wpdb->get_results( $wpdb->prepare( $sql, $mainwp_options ) );

$wpdb->prepare() accepts a variable‑length argument list, so injection is still impossible while reducing round‑trips to one.

class/class-mainwp-child.php (4)

69-71: Defer the automatic update() check to admin‑only contexts

update() executes an get_option() on every request, including normal frontend page views.
Although the early‑return makes the call inexpensive after the first run, it is still an avoidable DB lookup that provides no benefit outside of the admin/CLI/cron surface area.

-    $this->update();
+    // Defer version‑migration logic to full initialisation only.

Consider moving the call to the top of init_full() so version‑migration runs only when strictly necessary.
This further reduces the plugin’s footprint for anonymous visitors.

---

79-85: init_frontend_only() still wires up heavy admin hooks

register_essential_hooks() attaches several admin‑only callbacks (admin_init, plugin_action_links_*, etc.).
Those hooks will never fire on the frontend, but WordPress still has to store the callbacks and evaluate them on every request.

If the goal is to be as cheap as possible on the frontend, create a trimmed‑down hook set (e.g. only template_redirect and security filters) for init_frontend_only(), and register the full set in init_full().

This keeps memory footprint & callback traversal time minimal.

---

151-183: Avoid double get_current_screen() calls and leverage the passed $screen

Inside the second current_screen callback we call $this->is_mainwp_pages(), which in turn executes get_current_screen() again.
Since the $screen object is already available as the callback parameter, we can eliminate the redundant global call:

-            function () {
-                if ( $this->is_mainwp_pages() ) {
+            function ( $screen ) {
+                if ( $this->is_mainwp_pages( $screen ) ) {

and adjust is_mainwp_pages() to accept an optional $screen parameter.
This saves one function call for every admin page load and keeps the helper pure.

---

617-644: is_mainwp_pages() could accept a $screen parameter and normalise early exit

By taking an optional WP_Screen $screen = null argument we can:

1. Re‑use the screen object when it is already known (see comment above),
2. Reduce global calls,
3. Improve testability as the method becomes pure.

No behaviour changes, just a cleaner contract.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

class/class-mainwp-child.php (1)

270-336: 🛠️ Refactor suggestion

Security gain acknowledged, but 29 individual queries hurt performance

Looping over $mainwp_options with a separate SELECT … WHERE option_name = %s call is immune to SQL‑injection, yet it triggers one additional database round‑trip per option.

A middle ground keeps security while restoring performance:

$placeholders = implode( ',', array_fill( 0, count( $mainwp_options ), '%s' ) );
$sql         = "SELECT option_name, option_value FROM $wpdb->options WHERE option_name IN ($placeholders)";
$prepared    = $wpdb->prepare( $sql, ...$mainwp_options );
$results     = $wpdb->get_results( $prepared );

All identifiers stay parameterised; no manual quoting is needed and the query runs in one go.

class/class-mainwp-child.php (2)

73-85: Front‑end requests still register several admin‑only hooks

init_frontend_only() wires up heavy‑weight hooks such as admin_init, plugin_action_links_* and a high‑priority init callback (parse_init) that is mainly used for authenticated MainWP calls.

Loading these on every anonymous front‑end hit slightly increases memory footprint and hook dispatch time.

Consider guarding admin‑only hooks, e.g.:

-        $this->register_essential_hooks();
+        // Only wire front‑end safe hooks.
+        add_action( 'template_redirect', array( $this, 'template_redirect' ) );
+        add_action( 'init', array( $this, 'localization' ), 33 );

---

611-644: Helper now safe to call – minor nitpick on duplicate logic

is_mainwp_pages() first checks the page query‑var, then (if needed) falls back to get_current_screen().
You already hard‑code 'update-core' in $mainwp_screens, while the first callback in register_screen_hooks() performs a separate check for 'update-core' again.

To avoid future divergence, consider exposing is_mainwp_pages() as a public util and re‑use it in both places, or extend the helper to accept a list of extra screen‑IDs.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro