Fix changing template and better use of singletons#263
Conversation
|
Warning Rate limit exceeded@superdav42 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request refactors initialization patterns across multiple classes by converting constructor-based initialization to explicit init() methods, removes singleton pattern implementation from Dashboard_Statistics, refactors error handling in Template_Switching_Element to use JSON responses and singleton accessors, adds a WP_List_Table import, and introduces comprehensive unit tests for singleton pattern enforcement. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes Areas requiring attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/class-dashboard-statistics.php (1)
56-64: Guard missing keys before dereferencing$argsLine 58 dereferences
$args['start_date'],$args['end_date'], and$args['types']without first normalizing the input. If a caller provides a partial overrides array, PHP will emit notices and$this->typescan becomenull, breaking thestatistics_data()loop. Please parse the arguments with safe defaults before assigning them.Apply this diff to harden the constructor:
public function __construct($args = []) { if ($args) { - $this->start_date = $args['start_date']; - - $this->end_date = $args['end_date']; - - $this->types = $args['types']; + $args = wp_parse_args( + $args, + [ + 'start_date' => null, + 'end_date' => null, + 'types' => [], + ] + ); + + $this->start_date = $args['start_date']; + $this->end_date = $args['end_date']; + $this->types = is_array($args['types']) ? $args['types'] : []; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
inc/admin-pages/class-list-admin-page.php(1 hunks)inc/class-dashboard-statistics.php(2 hunks)inc/class-user-switching.php(1 hunks)inc/class-wp-ultimo.php(0 hunks)inc/ui/class-jumper.php(1 hunks)inc/ui/class-template-switching-element.php(4 hunks)inc/ui/class-toolbox.php(1 hunks)inc/ui/class-tours.php(1 hunks)tests/WP_Ultimo/Traits/Singleton_Test.php(1 hunks)
💤 Files with no reviewable changes (1)
- inc/class-wp-ultimo.php
🧰 Additional context used
🧬 Code graph analysis (6)
inc/ui/class-jumper.php (2)
inc/class-wp-ultimo.php (1)
init(115-213)inc/ui/class-base-element.php (1)
init(220-252)
inc/ui/class-tours.php (3)
inc/ui/class-template-switching-element.php (1)
init(104-109)inc/class-wp-ultimo.php (1)
init(115-213)inc/traits/trait-singleton.php (1)
init(48-51)
inc/class-user-switching.php (4)
inc/admin-pages/class-list-admin-page.php (1)
init(96-104)inc/ui/class-jumper.php (1)
init(48-53)inc/ui/class-template-switching-element.php (1)
init(104-109)inc/class-wp-ultimo.php (1)
init(115-213)
tests/WP_Ultimo/Traits/Singleton_Test.php (1)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-938)
inc/ui/class-toolbox.php (6)
inc/admin-pages/class-list-admin-page.php (1)
init(96-104)inc/class-user-switching.php (1)
init(32-35)inc/ui/class-jumper.php (1)
init(48-53)inc/ui/class-template-switching-element.php (1)
init(104-109)inc/ui/class-tours.php (1)
init(38-45)inc/class-wp-ultimo.php (1)
init(115-213)
inc/ui/class-template-switching-element.php (2)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-938)inc/deprecated/deprecated.php (1)
get_instance(353-356)
🪛 GitHub Check: Code Quality Checks
tests/WP_Ultimo/Traits/Singleton_Test.php
[failure] 59-59:
Blank line found at start of control structure
[warning] 51-51:
file_get_contents() is discouraged. Use wp_remote_get() for remote URLs instead.
[failure] 37-37:
Use Yoda Condition checks, you must.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: PHP 7.4
🔇 Additional comments (6)
inc/admin-pages/class-list-admin-page.php (1)
20-20: Import keeps the global list table accessible.Pulling
WP_List_Tableinto scope avoids accidental namespacing and keeps the property annotations honest. Nicely done.inc/ui/class-tours.php (1)
33-45: Init refactor stays faithful to the old behavior.Mirroring the constructor logic inside
init()keeps the AJAX and enqueue hooks intact while aligning with the revised singleton lifecycle.inc/class-user-switching.php (1)
27-35: Singleton init keeps plugin boot timing unchanged.Moving the hook registration into
init()preserves the plugins_loaded behavior while matching the new singleton convention.inc/ui/class-jumper.php (1)
43-52: Jumper still wires itself at the same lifecycle points.The explicit
init()preserves bothinitcallbacks and respects the priority, so behavior remains consistent with the constructor version.inc/ui/class-toolbox.php (1)
25-33: Toolbox init follows the new singleton pattern cleanly.Hook registration mirrors the old constructor, so toolbox availability is unaffected while matching the standardized initialization approach.
inc/class-dashboard-statistics.php (1)
26-26: Verification confirmed: no lingering singleton API callsThe codebase sweep confirms all callers have been successfully migrated. There are zero instances of
Dashboard_Statistics::get_instance()anywhere. The sole instantiation pattern found (./inc/admin-pages/class-dashboard-admin-page.php:489) correctly usesnew Dashboard_Statistics(...).
| if (strpos($content, 'use \WP_Ultimo\Traits\Singleton;') !== false || | ||
| strpos($content, 'use \\WP_Ultimo\\Traits\\Singleton;') !== false) { | ||
|
|
||
| // Extract namespace and class name | ||
| if (preg_match('/namespace\s+([^;]+);/i', $content, $namespace_match) && | ||
| preg_match('/class\s+(\w+)/i', $content, $class_match)) { | ||
|
|
||
| $namespace = trim($namespace_match[1]); | ||
| $class_name = trim($class_match[1]); | ||
| $full_class = $namespace . '\\' . $class_name; | ||
|
|
||
| // Verify the class exists and uses the trait | ||
| if (class_exists($full_class)) { | ||
| $reflection = new ReflectionClass($full_class); | ||
| $traits = $reflection->getTraitNames(); | ||
|
|
||
| if (in_array('WP_Ultimo\Traits\Singleton', $traits, true)) { | ||
| $singleton_classes[] = $full_class; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove the blank line immediately inside the control structure
Lines 55-56 trigger “Blank line found at start of control structure.” Drop the empty line so the in-block comment is the first statement.
Apply this diff to comply:
if (strpos($content, 'use \WP_Ultimo\Traits\Singleton;') !== false ||
strpos($content, 'use \\WP_Ultimo\\Traits\\Singleton;') !== false) {
-
// Extract namespace and class name
if (preg_match('/namespace\s+([^;]+);/i', $content, $namespace_match) &&
preg_match('/class\s+(\w+)/i', $content, $class_match)) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (strpos($content, 'use \WP_Ultimo\Traits\Singleton;') !== false || | |
| strpos($content, 'use \\WP_Ultimo\\Traits\\Singleton;') !== false) { | |
| // Extract namespace and class name | |
| if (preg_match('/namespace\s+([^;]+);/i', $content, $namespace_match) && | |
| preg_match('/class\s+(\w+)/i', $content, $class_match)) { | |
| $namespace = trim($namespace_match[1]); | |
| $class_name = trim($class_match[1]); | |
| $full_class = $namespace . '\\' . $class_name; | |
| // Verify the class exists and uses the trait | |
| if (class_exists($full_class)) { | |
| $reflection = new ReflectionClass($full_class); | |
| $traits = $reflection->getTraitNames(); | |
| if (in_array('WP_Ultimo\Traits\Singleton', $traits, true)) { | |
| $singleton_classes[] = $full_class; | |
| } | |
| } | |
| } | |
| } | |
| } | |
| if (strpos($content, 'use \WP_Ultimo\Traits\Singleton;') !== false || | |
| strpos($content, 'use \\WP_Ultimo\\Traits\\Singleton;') !== false) { | |
| // Extract namespace and class name | |
| if (preg_match('/namespace\s+([^;]+);/i', $content, $namespace_match) && | |
| preg_match('/class\s+(\w+)/i', $content, $class_match)) { | |
| $namespace = trim($namespace_match[1]); | |
| $class_name = trim($class_match[1]); | |
| $full_class = $namespace . '\\' . $class_name; | |
| // Verify the class exists and uses the trait | |
| if (class_exists($full_class)) { | |
| $reflection = new ReflectionClass($full_class); | |
| $traits = $reflection->getTraitNames(); | |
| if (in_array('WP_Ultimo\Traits\Singleton', $traits, true)) { | |
| $singleton_classes[] = $full_class; | |
| } | |
| } | |
| } | |
| } | |
| } |
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 59-59:
Blank line found at start of control structure
🤖 Prompt for AI Agents
In tests/WP_Ultimo/Traits/Singleton_Test.php around lines 54 to 76, there is an
empty blank line immediately after the opening brace of the if (strpos(...))
control structure; remove that blank line so the in-block comment becomes the
first statement inside the block, ensuring no extra blank line exists at the
start of the control structure and preserving existing indentation and comments.
1 participant
Summary by CodeRabbit
Refactor
Tests