-
-
Notifications
You must be signed in to change notification settings - Fork 66
Add rocket integration #293
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughIntroduces a new Rocket.net host provider integration that extends the base host provider class with full domain lifecycle management, API authentication, connectivity testing, and user-facing configuration and instruction views. The integration is initialized in the domain manager's load sequence. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
views/wizards/host-integrations/rocket-instructions.php (1)
1-118: Rocket instructions template looks solid; consider simplifying one i18n string.Markup, escaping, and link attributes all look good and consistent with the rest of the plugin. The only minor nit is Line 12: the sentence is composed from several small
esc_html_e()fragments, which can make accurate translations harder in languages with different word order.If you care about top‑quality translations, consider switching that first paragraph to a single translatable string with placeholders (e.g., via
printf), so translators see the whole sentence at once. This is purely optional; the current code is safe and functional.inc/integrations/host-providers/class-rocket-host-provider.php (3)
70-80:detect()may break on PHP 7.4 due tostr_contains; consider astrpos-based implementation.
str_contains()is only available on PHP 8+. If Ultimate Multisite still supports PHP 7.4, this will cause a fatal error when the file is loaded. A simple, compatible alternative is to base this onstrpos():public function detect(): bool { - return str_contains(ABSPATH, 'rocket.net') || str_contains(ABSPATH, 'rocketdotnet'); + $path = (string) ABSPATH; + + return (false !== strpos($path, 'rocket.net')) || (false !== strpos($path, 'rocketdotnet')); }If the plugin has officially dropped support for PHP < 8.0, you can ignore this, but it’s worth double‑checking your stated minimum PHP version.
119-175: Use$site_idin domain logs to address PHPMD warnings and improve diagnostics.
$site_idis currently unused inon_add_domain()andon_remove_domain(), which PHPMD correctly flags. It’s also useful context in logs when troubleshooting mapping issues. You can incorporate it into the existing log messages, e.g.:public function on_add_domain($domain, $site_id): void { @@ - if (is_wp_error($response)) { - wu_log_add('integration-rocket', sprintf('[Add Domain] %s: %s', $domain, $response->get_error_message()), LogLevel::ERROR); - } else { + if (is_wp_error($response)) { + wu_log_add( + 'integration-rocket', + sprintf('[Add Domain] %s (site %d): %s', $domain, $site_id, $response->get_error_message()), + LogLevel::ERROR + ); + } else { @@ - if (200 === $response_code || 201 === $response_code) { - wu_log_add('integration-rocket', sprintf('[Add Domain] %s: Success - %s', $domain, $response_body)); - } else { - wu_log_add('integration-rocket', sprintf('[Add Domain] %s: Failed (HTTP %d) - %s', $domain, $response_code, $response_body), LogLevel::ERROR); + if (200 === $response_code || 201 === $response_code) { + wu_log_add( + 'integration-rocket', + sprintf('[Add Domain] %s (site %d): Success - %s', $domain, $site_id, $response_body) + ); + } else { + wu_log_add( + 'integration-rocket', + sprintf('[Add Domain] %s (site %d): Failed (HTTP %d) - %s', $domain, $site_id, $response_code, $response_body), + LogLevel::ERROR + ); } } } @@ public function on_remove_domain($domain, $site_id): void { @@ - if (! $domain_id) { - wu_log_add('integration-rocket', sprintf('[Remove Domain] %s: Domain not found on Rocket.net', $domain), LogLevel::WARNING); + if (! $domain_id) { + wu_log_add( + 'integration-rocket', + sprintf('[Remove Domain] %s (site %d): Domain not found on Rocket.net', $domain, $site_id), + LogLevel::WARNING + ); @@ - if (is_wp_error($response)) { - wu_log_add('integration-rocket', sprintf('[Remove Domain] %s: %s', $domain, $response->get_error_message()), LogLevel::ERROR); - } else { + if (is_wp_error($response)) { + wu_log_add( + 'integration-rocket', + sprintf('[Remove Domain] %s (site %d): %s', $domain, $site_id, $response->get_error_message()), + LogLevel::ERROR + ); + } else { @@ - if (200 === $response_code || 204 === $response_code) { - wu_log_add('integration-rocket', sprintf('[Remove Domain] %s: Success - %s', $domain, $response_body)); - } else { - wu_log_add('integration-rocket', sprintf('[Remove Domain] %s: Failed (HTTP %d) - %s', $domain, $response_code, $response_body), LogLevel::ERROR); + if (200 === $response_code || 204 === $response_code) { + wu_log_add( + 'integration-rocket', + sprintf('[Remove Domain] %s (site %d): Success - %s', $domain, $site_id, $response_body) + ); + } else { + wu_log_add( + 'integration-rocket', + sprintf('[Remove Domain] %s (site %d): Failed (HTTP %d) - %s', $domain, $site_id, $response_code, $response_body), + LogLevel::ERROR + ); } } }This both satisfies the static analysis warning and gives you more actionable logs when something goes wrong. Based on static analysis hints.
187-205: Log subdomain no‑op handlers to use parameters and clarify behavior.
on_add_subdomain()andon_remove_subdomain()intentionally do nothing, but PHPMD warns about unused$subdomainand$site_id. Adding a debug‑level log clarifies the no‑op behavior and marks parameters as used:public function on_add_subdomain($subdomain, $site_id) { - // Rocket.net manages subdomains automatically via wildcard SSL - // No action needed + // Rocket.net manages subdomains automatically via wildcard SSL. + // Log and return so behavior is explicit and parameters are used. + wu_log_add( + 'integration-rocket', + sprintf('[Add Subdomain] %s (site %d): no action required, handled via wildcard SSL.', $subdomain, $site_id), + LogLevel::DEBUG + ); } @@ public function on_remove_subdomain($subdomain, $site_id) { - // Rocket.net manages subdomains automatically via wildcard SSL - // No action needed + // Rocket.net manages subdomains automatically via wildcard SSL. + // Log and return so behavior is explicit and parameters are used. + wu_log_add( + 'integration-rocket', + sprintf('[Remove Subdomain] %s (site %d): no action required, handled via wildcard SSL.', $subdomain, $site_id), + LogLevel::DEBUG + ); }This should resolve the PHPMD
UnusedFormalParameterwarnings and provide useful traceability for subdomain events. Based on static analysis hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/img/hosts/rocket.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
inc/integrations/host-providers/class-rocket-host-provider.php(1 hunks)inc/managers/class-domain-manager.php(1 hunks)views/wizards/host-integrations/rocket-instructions.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
inc/managers/class-domain-manager.php (1)
inc/integrations/host-providers/class-rocket-host-provider.php (1)
Rocket_Host_Provider(20-429)
inc/integrations/host-providers/class-rocket-host-provider.php (2)
inc/integrations/host-providers/class-base-host-provider.php (1)
supports(337-340)inc/functions/helper.php (1)
wu_log_add(208-211)
🪛 PHPMD (2.15.0)
inc/integrations/host-providers/class-rocket-host-provider.php
119-119: Avoid unused parameters such as '$site_id'. (undefined)
(UnusedFormalParameter)
151-151: Avoid unused parameters such as '$site_id'. (undefined)
(UnusedFormalParameter)
187-187: Avoid unused parameters such as '$subdomain'. (undefined)
(UnusedFormalParameter)
187-187: Avoid unused parameters such as '$site_id'. (undefined)
(UnusedFormalParameter)
202-202: Avoid unused parameters such as '$subdomain'. (undefined)
(UnusedFormalParameter)
202-202: Avoid unused parameters such as '$site_id'. (undefined)
(UnusedFormalParameter)
⏰ 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). (2)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
🔇 Additional comments (2)
inc/managers/class-domain-manager.php (1)
1008-1011: Rocket provider registration inload_integrations()is consistent.The new
Rocket_Host_Provider::get_instance()call follows the same pattern and ordering as the existing host providers and should integrate cleanly with the existing discovery/filter mechanisms.inc/integrations/host-providers/class-rocket-host-provider.php (1)
403-428: Instructions, description, and logo wiring look correct.
get_instructions(),get_description(), andget_logo()align with existing host providers: they point at the new template, return a translated description, and usewu_get_assetfor the host logo. Nothing to change here.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.