-
-
Notifications
You must be signed in to change notification settings - Fork 67
Fix enhance api requests #309
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
📝 WalkthroughWalkthroughThis PR modifies five host provider integration classes to standardize error handling, add defensive API checks, reorganize setup flow logic, and adjust domain handling patterns. Changes include moving dependency loading under setup guards, standardizing return types to arrays, fixing domain error comparisons, adding WPE_API availability checks, reworking domain lookups, and expanding Gridpane configuration constants. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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: 2
🧹 Nitpick comments (1)
inc/integrations/host-providers/class-closte-host-provider.php (1)
108-108: Fix the domain error comparison.The comparison is reversed—it should compare the response error against the expected string, not the other way around. While functionally equivalent in this case, the current Yoda condition reduces readability.
🔎 Suggested fix
- } elseif (isset($domain_response['error']) && 'Invalid or empty domain: ' . $domain === $domain_response['error']) { + } elseif (isset($domain_response['error']) && $domain_response['error'] === 'Invalid or empty domain: ' . $domain) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
inc/integrations/host-providers/class-base-host-provider.phpinc/integrations/host-providers/class-closte-host-provider.phpinc/integrations/host-providers/class-enhance-host-provider.phpinc/integrations/host-providers/class-gridpane-host-provider.phpinc/integrations/host-providers/class-wpengine-host-provider.php
🧰 Additional context used
🧬 Code graph analysis (3)
inc/integrations/host-providers/class-enhance-host-provider.php (1)
inc/functions/helper.php (1)
wu_get_isset(66-73)
inc/integrations/host-providers/class-closte-host-provider.php (1)
inc/functions/helper.php (1)
wu_log_add(208-211)
inc/integrations/host-providers/class-base-host-provider.php (2)
inc/integrations/host-providers/class-wpengine-host-provider.php (1)
load_dependencies(89-97)inc/class-sunrise.php (1)
load_dependencies(106-164)
⏰ 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: Build Plugin for Testing
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (9)
inc/integrations/host-providers/class-base-host-provider.php (1)
103-113: LGTM! Improved setup flow logic.Moving dependency loading and hook registration into the
is_setup()guard ensures they only execute when the provider is properly configured. This prevents potential issues from loading dependencies or registering hooks when required constants are missing.inc/integrations/host-providers/class-gridpane-host-provider.php (1)
65-67: LGTM! Constants list now matches actual usage.These constants are already referenced in the implementation (lines 116, 148-149, 168-169), so adding them to the required constants list ensures proper setup validation. This prevents runtime errors when these constants are undefined.
inc/integrations/host-providers/class-wpengine-host-provider.php (3)
108-110: LGTM! Defensive guard against missing WPE_API class.This check prevents fatal errors if
load_dependencies()fails to load the WPE_API class (e.g., whenWPE_PLUGIN_DIRis undefined or the file is unreadable). The early return gracefully handles the missing dependency.
129-131: LGTM! Consistent defensive guard.Applies the same protection as in
on_add_domain, ensuring safe operation when the WPE_API class is unavailable.
205-207: LGTM! Improved error reporting for test connection.Unlike the domain methods that silently return,
test_connectionappropriately provides user feedback via JSON error response when the WPE_API class is missing.inc/integrations/host-providers/class-closte-host-provider.php (1)
218-218: LGTM! Standardized return types to arrays.Consolidating return values to consistently use arrays improves error handling predictability and makes the API easier to consume. The docblock updates accurately reflect the implementation.
Also applies to: 265-265, 271-274, 279-282, 325-329, 347-351, 355-358
inc/integrations/host-providers/class-enhance-host-provider.php (3)
103-103: LGTM! Improved UX for API token field.Adding
autocomplete="new-password"prevents browsers from auto-filling the API token field with saved passwords, which is appropriate for bearer tokens.
181-187: LGTM! Domain lookup updated for new API response shape.The field name changes (
domain→domainName,id→domainId) are consistent with the response field change inon_add_domain. The logic correctly handles cases where the domain is not found.
262-263: LGTM! Improved error handling and documentation.The error message now includes the actual API error from the response, and the docblock accurately reflects that
$datacan be either an array or a string (as demonstrated by the domain addition changes).Also applies to: 272-274
| [ | ||
| 'domain' => $domain, | ||
| ] | ||
| $domain |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n inc/integrations/host-providers/class-enhance-host-provider.php | sed -n '130,155p'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1278
🌐 Web query:
Enhance Control Panel API documentation adding domain to server
💡 Result:
Which control panel (cPanel, Plesk, Cloud provider, or a custom Control Panel) and which doc format do you want (OpenAPI YAML/JSON, Markdown, or HTML)? Also confirm the required fields for the domain (e.g., domain name, server_id, DNS zone, SSL cert, aliases) and auth method (API key, OAuth, etc.).
Meanwhile — a concise OpenAPI-style endpoint template and minimal doc checklist you can drop into docs:
OpenAPI endpoint template (YAML — adapt names/types to your schema)
paths:
/servers/{server_id}/domains:
post:
summary: Add a domain to a server
tags: [Domains]
security:
- ApiKeyAuth: []
parameters:
- name: server_id
in: path
required: true
schema:
type: string
description: Server identifier
requestBody:
required: true
content:
application/json:
schema:
type: object
required: [domain]
properties:
domain:
type: string
format: hostname
example: example.com
zone_template:
type: string
description: Optional DNS zone/template name
enable_ssl:
type: boolean
default: true
aliases:
type: array
items:
type: string
examples:
addDomain:
value: { "domain": "example.com", "enable_ssl": true }
responses:
'201':
description: Domain added
content:
application/json:
schema:
$ref: '#/components/schemas/Domain'
'400':
description: Invalid request (validation error)
'401':
description: Unauthorized
'404':
description: Server not found
'409':
description: Domain already exists on server
components:
securitySchemes:
ApiKeyAuth:
type: apiKey
in: header
name: X-API-Key
schemas:
Domain:
type: object
properties:
id:
type: string
domain:
type: string
server_id:
type: string
dns_status:
type: string
ssl_status:
type: string
created_at:
type: string
format: date-time
Minimal documentation checklist (recommended)
- Purpose/behavior summary (what “adding a domain” does: create DNS records, bind vhost, enable SSL, etc.). [1][2]
- URL, HTTP method, path parameters, and required headers/auth. [2]
- Request body schema, validation rules (hostname format, max length), and examples. [2]
- Success and error responses with status codes and example payloads (201, 400, 401, 404, 409, 500). [2]
- Side effects & asynchronous behaviour (is SSL provisioning async? provide status endpoint). [4]
- Security notes (who can add domains; rate limits). [1][3]
- CLI / curl and SDK examples (one-liner curl, JS/Python snippets). [3]
- OpenAPI validation / linting step and where the spec is stored (CI). [1]
If you confirm control panel and desired format, I’ll produce a finalized OpenAPI YAML or Markdown doc section (including curl and SDK examples) tailored to your API, plus sample error payloads and a small CI lint workflow.
Sources: OpenAPI best practices and API design guidance used above. [1] OpenAPI Best Practices. [2] API Design / REST best-practices. [3] Internal doc-generation & examples. (learn.openapis.org)
Citations:
🏁 Script executed:
grep -n "function send_enhance_api_request" inc/integrations/host-providers/class-enhance-host-provider.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 168
🏁 Script executed:
grep -n "send_enhance_api_request" inc/integrations/host-providers/class-enhance-host-provider.php | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 390
🏁 Script executed:
ast-grep --pattern $'function send_enhance_api_request($_) {
$$$
}'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
cat -n inc/integrations/host-providers/class-enhance-host-provider.php | sed -n '277,320p'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1768
🏁 Script executed:
# Search for any comments or documentation about the Enhance API contract
rg -A 5 -B 5 "domainid|domain_id" inc/integrations/host-providers/class-enhance-host-provider.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 1516
🏁 Script executed:
# Check git log for recent changes to understand what was changed
git log --oneline -20 -- inc/integrations/host-providers/class-enhance-host-provider.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 113
🏁 Script executed:
git diff 0e6b983^..0e6b983 -- inc/integrations/host-providers/class-enhance-host-provider.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 122
🏁 Script executed:
# Also search for any Enhance API documentation or references in the codebase
fd -e md -e txt -e json | xargs grep -l "Enhance\|enhance" 2>/dev/null | head -10Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 167
🏁 Script executed:
grep -A 10 -B 2 -i "enhance.*api\|domain.*add" README.md readme.txt 2>/dev/null | head -50Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Search for any comments or docs about the API payload structure
rg -i "domain.*payload|payload.*domain|api.*contract" inc/integrations/Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Check if there's a git show for the specific commit to see what changed
git show --stat 0e6b983Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 50393
🏁 Script executed:
cat .wiki/Enhance-Integration.md | head -100Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 3860
🏁 Script executed:
# Look for specific sections about domain addition
rg -A 20 "add.*domain|POST.*domain" .wiki/Enhance-Integration.mdRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 2641
🏁 Script executed:
# Check what the exact payload format should be according to docs
grep -A 5 "POST /servers" .wiki/Enhance-Integration.mdRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 178
🏁 Script executed:
# Verify the exact behavior - when a plain string is json_encoded
python3 << 'EOF'
import json
# Current code behavior
domain = "example.com"
payload = json.dumps(domain)
print(f"Current payload (plain string): {payload}")
print(f"Type: {type(domain)}")
# What it should be
correct_payload = json.dumps({"domain": domain})
print(f"Expected payload (object): {correct_payload}")
EOFRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 167
Fix the API request payload format — domain must be wrapped in an object.
The Enhance API documentation specifies the POST body should be {"domain": "example.com"}, but the current code passes $domain as a plain string, which JSON-encodes to just "example.com". This will cause the API call to fail. Wrap the domain in an object: ['domain' => $domain] before passing to send_enhance_api_request().
The response field domainid appears correct based on the integration patterns, though the API documentation does not explicitly document the POST response schema.
🤖 Prompt for AI Agents
In inc/integrations/host-providers/class-enhance-host-provider.php around line
141, the POST payload currently passes $domain as a plain string which
JSON-encodes to "\"example.com\"" and breaks the API; change the payload to an
associative array wrapping the domain (e.g. ['domain' => $domain]) before
calling send_enhance_api_request(), then verify the call still reads the
response.domainid as before.
| $response = $this->send_enhance_api_request( | ||
| '/servers/' . $server_id . '/domains', | ||
| 'GET' | ||
| '/servers/' . $server_id |
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.
🧩 Analysis chain
🏁 Script executed:
cd inc/integrations/host-providers && wc -l class-enhance-host-provider.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 116
🏁 Script executed:
cd inc/integrations/host-providers && sed -n '240,270p' class-enhance-host-provider.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 1009
🏁 Script executed:
cd inc/integrations/host-providers && sed -n '1,50p' class-enhance-host-provider.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 1179
🏁 Script executed:
git -C . log --oneline --all -- inc/integrations/host-providers/class-enhance-host-provider.php | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 151
🏁 Script executed:
git -C . show HEAD:inc/integrations/host-providers/class-enhance-host-provider.php | sed -n '240,270p'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1009
🏁 Script executed:
git -C . log -p --all -- inc/integrations/host-providers/class-enhance-host-provider.php | grep -A 5 -B 5 "servers.*domains\|servers.*{id}" | head -100Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1784
🏁 Script executed:
git -C . show 0e6b983 --statRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 50393
🏁 Script executed:
git -C . show 0e6b983 -- inc/integrations/host-providers/class-enhance-host-provider.php | grep -A 20 "test_connection"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 690
🏁 Script executed:
rg "servers.*domains" inc/integrations/host-providers/class-enhance-host-provider.php -A 2 -B 2Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 564
🏁 Script executed:
sed -n '1,100p' inc/integrations/host-providers/class-enhance-host-provider.php | head -50Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1179
🏁 Script executed:
sed -n '300,423p' inc/integrations/host-providers/class-enhance-host-provider.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 3585
🏁 Script executed:
git -C . show 0e6b983:inc/integrations/host-providers/class-enhance-host-provider.php | sed -n '240,260p'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 669
🏁 Script executed:
git -C . show 1ddcfc1:inc/integrations/host-providers/class-enhance-host-provider.php 2>/dev/null | sed -n '240,260p'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 685
Update inline comment and simplify success condition.
The endpoint change from /servers/{id}/domains to /servers/{id} is confirmed—the function now tests API connectivity by fetching server info rather than listing domains. However, the inline comment saying "Test by attempting to list domains" is stale and should reflect that it now retrieves server information. Additionally, the success condition's check for isset($response['items']) is dead code; the new endpoint returns a server object with an id field, not an array with items. Consider simplifying the condition to just isset($response['id']) or, if backward compatibility is desired, add a clarifying comment explaining why both checks remain.
🤖 Prompt for AI Agents
In inc/integrations/host-providers/class-enhance-host-provider.php around line
252, update the stale inline comment and simplify the success condition: replace
the comment that says "Test by attempting to list domains" with one that says
the function fetches server information from /servers/{id}, and change the
response check from isset($response['items']) to isset($response['id']) (or, if
you need backward compatibility, add a brief comment explaining you accept
either a server object with id or a legacy items array and use
isset($response['id']) || isset($response['items'])).
Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.