Add DNS record management feature for mapped domains #308
Conversation
This feature allows customers and network admins to manage DNS records (A, AAAA, CNAME, MX, TXT) for domains mapped via hosting provider APIs. ## Core Infrastructure - Add DNS_Record value object for consistent record handling - Add DNS_Provider_Interface for DNS-capable host providers - Add DNS_Record_Manager for centralized operations and AJAX handlers - Update Base_Host_Provider with default DNS method implementations ## Provider Implementations - Cloudflare: Full CRUD via REST API with zone lookup - cPanel: Full CRUD via UAPI DNS module - Hestia: Full CRUD via CLI commands ## Customer UI - Add DNS management modal in domain mapping element - Add Vue.js component for dynamic record table - Add forms for add/edit/delete DNS records ## Admin UI - Add DNS management widget on domain edit page - Add admin forms for record CRUD operations ## Tests - 70 unit tests covering DNS_Record, DNS_Record_Manager, and all provider DNS methods (Cloudflare, cPanel, Hestia) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughA comprehensive DNS management system is added to the platform, featuring a DNS_Provider_Interface for standardized DNS operations, implementations across multiple host providers (Cloudflare, cPanel, Hestia), a DNS_Record_Manager for orchestrating operations, admin and customer-facing UI components, and client-side Vue.js integration with full test coverage. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (17)
views/dashboard-widgets/domain-mapping.php (1)
71-84: Unused permission check variable$can_manage_dns.The
$can_manage_dnsvariable is retrieved on line 74 but not used in the conditional on line 76. The current logic shows the DNS action when either a provider exists OR the setting is enabled, regardless of whether the user has permission.Consider using the permission check to gate the action visibility:
🔎 Proposed fix
// Check if DNS management is available $dns_manager = \WP_Ultimo\Managers\DNS_Record_Manager::get_instance(); $dns_provider = $dns_manager->get_dns_provider(); $can_manage_dns = $dns_manager->customer_can_manage_dns(get_current_user_id(), $item->get_domain()); -if ($dns_provider || wu_get_setting('enable_customer_dns_management', false)) { +if ($can_manage_dns && ($dns_provider || wu_get_setting('enable_customer_dns_management', false))) { $second_row_actions['manage_dns'] = [views/domain/dns-record-form.php (1)
183-190: TTL selection default logic may not work as intended.The TTL
selected()calls use inconsistent default values. Line 184 uses($record['ttl'] ?? 1)for Auto, but lines 185-188 use($record['ttl'] ?? 0)for other values. This means if no TTL is set, "Auto" will be selected (correct), but the comparison for other values will never match since0doesn't equal300,3600, etc.This works by accident but is confusing. Consider using a consistent default:
🔎 Proposed fix for clarity
<select name="record[ttl]" id="record_ttl" class="wu-w-full"> - <option value="1" <?php selected(($record['ttl'] ?? 1), 1); ?>><?php esc_html_e('Auto', 'ultimate-multisite'); ?></option> - <option value="300" <?php selected(($record['ttl'] ?? 0), 300); ?>><?php esc_html_e('5 minutes', 'ultimate-multisite'); ?></option> - <option value="3600" <?php selected(($record['ttl'] ?? 0), 3600); ?>><?php esc_html_e('1 hour', 'ultimate-multisite'); ?></option> - <option value="14400" <?php selected(($record['ttl'] ?? 0), 14400); ?>><?php esc_html_e('4 hours', 'ultimate-multisite'); ?></option> - <option value="86400" <?php selected(($record['ttl'] ?? 0), 86400); ?>><?php esc_html_e('1 day', 'ultimate-multisite'); ?></option> + <?php $ttl = $record['ttl'] ?? 1; ?> + <option value="1" <?php selected($ttl, 1); ?>><?php esc_html_e('Auto', 'ultimate-multisite'); ?></option> + <option value="300" <?php selected($ttl, 300); ?>><?php esc_html_e('5 minutes', 'ultimate-multisite'); ?></option> + <option value="3600" <?php selected($ttl, 3600); ?>><?php esc_html_e('1 hour', 'ultimate-multisite'); ?></option> + <option value="14400" <?php selected($ttl, 14400); ?>><?php esc_html_e('4 hours', 'ultimate-multisite'); ?></option> + <option value="86400" <?php selected($ttl, 86400); ?>><?php esc_html_e('1 day', 'ultimate-multisite'); ?></option> </select>views/domain/dns-management-modal.php (1)
75-79: Error state only displays whenreadonlyis true.The error message on line 76 uses
v-if="!loading && error && readonly", which means errors won't be displayed when the provider supports management (readonlyis false). Consider showing errors regardless of readonly status:🔎 Proposed fix
<!-- Error State --> -<div v-if="!loading && error && readonly" class="wu-bg-yellow-50 wu-border wu-border-yellow-200 wu-rounded wu-p-3 wu-mb-4"> +<div v-if="!loading && error" class="wu-bg-yellow-50 wu-border wu-border-yellow-200 wu-rounded wu-p-3 wu-mb-4"> <span class="dashicons dashicons-warning wu-text-yellow-600 wu-mr-2"></span> {{ error }} </div>tests/WP_Ultimo/Integrations/Host_Providers/CPanel_DNS_Test.php (1)
194-207: Testtest_default_ttldoesn't verify actual TTL behavior.This test creates a record without TTL and asserts the type is 'A', but doesn't actually test any TTL-related functionality. Consider either removing this test or making it meaningful by verifying the provider applies a default TTL.
🔎 Example of a meaningful test
public function test_default_ttl() { - // cPanel typically uses 14400 as default TTL - $record = [ - 'type' => 'A', - 'name' => 'test', - 'content' => '192.168.1.1', - ]; - - // This tests that the record data is properly structured - $this->assertEquals('A', $record['type']); + // Test that cPanel uses 14400 as default TTL + // This would require mocking or accessing the default TTL constant + $this->markTestIncomplete('TODO: Test default TTL application in cPanel provider'); }tests/WP_Ultimo/Integrations/Host_Providers/DNS_Record_Test.php (1)
314-326: Minor alignment inconsistency in test variables.Static analysis flagged alignment issues on lines 315-316. This is a minor style nit.
🔎 Proposed fix for consistent alignment
-$a_record = new DNS_Record(['type' => 'A', 'name' => 'test', 'content' => '1.1.1.1']); -$aaaa_record = new DNS_Record(['type' => 'AAAA', 'name' => 'test', 'content' => '::1']); +$a_record = new DNS_Record(['type' => 'A', 'name' => 'test', 'content' => '1.1.1.1']); +$aaaa_record = new DNS_Record(['type' => 'AAAA', 'name' => 'test', 'content' => '::1']); $cname_record = new DNS_Record(['type' => 'CNAME', 'name' => 'test', 'content' => 'target.com']); -$mx_record = new DNS_Record(['type' => 'MX', 'name' => 'test', 'content' => 'mail.test.com', 'priority' => 10]); -$txt_record = new DNS_Record(['type' => 'TXT', 'name' => 'test', 'content' => 'test']); +$mx_record = new DNS_Record(['type' => 'MX', 'name' => 'test', 'content' => 'mail.test.com', 'priority' => 10]); +$txt_record = new DNS_Record(['type' => 'TXT', 'name' => 'test', 'content' => 'test']);tests/WP_Ultimo/Integrations/Host_Providers/Cloudflare_DNS_Test.php (2)
83-90: Weak assertion doesn't validate expected behavior.The assertion
is_wp_error($result) || is_array($result)accepts both error and success states, which doesn't verify the intended "not configured" behavior. Since credentials are missing, the test should assertWP_Errorspecifically, similar totest_get_dns_records_not_configuredinHestia_DNS_Test.php.Proposed fix
public function test_get_dns_records_not_configured() { - // Without proper API credentials, should return WP_Error $result = $this->provider->get_dns_records('example.com'); - - // This will fail as we don't have real credentials - // In a real test environment, you'd mock the API - $this->assertTrue(is_wp_error($result) || is_array($result)); + // Without proper API credentials, should return WP_Error + $this->assertInstanceOf(\WP_Error::class, $result); }
125-132: Use Yoda condition for comparison.Per the project's coding standards, use Yoda-style comparisons.
Proposed fix
- $this->assertTrue(is_wp_error($result) || $result === true); + $this->assertTrue(is_wp_error($result) || true === $result);tests/WP_Ultimo/Integrations/Host_Providers/Hestia_DNS_Test.php (1)
169-196: Tests don't validate provider behavior.
test_hestia_record_structureandtest_root_domain_indicatorcreate local arrays and assert on them without invoking any provider methods. These tests validate PHP array syntax rather than Hestia provider behavior.Consider either removing these or refactoring to test actual provider output when mocked.
inc/integrations/host-providers/class-cpanel-host-provider.php (2)
330-338: Error check may not catch empty error arrays.The condition
isset($result->errors)will be true even when$result->errorsis an empty array[]. Consider also checking if the array is non-empty.Proposed fix
- if (! $result || isset($result->errors) || ! isset($result->result->data)) { - $error_message = isset($result->errors) && is_array($result->errors) + if (! $result || ! empty($result->errors) || ! isset($result->result->data)) { + $error_message = ! empty($result->errors) && is_array($result->errors) ? implode(', ', $result->errors) : __('Failed to fetch DNS records from cPanel.', 'ultimate-multisite');
488-504: Missing default case for unsupported record types.Unlike
create_dns_record(line 425-431),update_dns_recordlacks a default case to reject unsupported record types. An unsupported type would proceed with incomplete parameters.Proposed fix
switch (strtoupper($record['type'])) { case 'A': case 'AAAA': $params['address'] = $record['content']; break; case 'CNAME': $params['cname'] = $this->ensure_trailing_dot($record['content']); break; case 'MX': $params['exchange'] = $this->ensure_trailing_dot($record['content']); $params['preference'] = (int) ($record['priority'] ?? 10); break; case 'TXT': $params['txtdata'] = $record['content']; break; + default: + return new \WP_Error( + 'unsupported-type', + /* translators: %s: record type */ + sprintf(__('Unsupported record type: %s', 'ultimate-multisite'), $record['type']) + ); }inc/managers/class-dns-record-manager.php (1)
71-77: Unused loop variable$class.The
$classvariable is assigned but never used in the foreach loops. Consider using_or omitting the value if only keys are needed.Proposed fix
- foreach ($integrations as $id => $class) { + foreach (array_keys($integrations) as $id) { $instance = $domain_manager->get_integration_instance($id);Also applies to: 95-101
inc/integrations/host-providers/class-hestia-host-provider.php (1)
612-639: Duplicated zone extraction logic across providers.The
extract_zone_namemethod is nearly identical toextract_zone_nameinCPanel_Host_Provider(lines 591-610). Consider extracting this toBase_Host_Provideror a shared trait to reduce duplication.inc/admin-pages/class-domain-edit-admin-page.php (1)
718-720: Consider sanitizing record data before passing to provider.The
record_datais obtained from the request and passed directly to the DNS provider. While providers may validate the data, it would be safer to sanitize the input at this layer as well. Consider sanitizing the array values usingsanitize_text_field()or similar functions.🔎 Suggested approach
$record_data = wu_request('record', []); + +// Sanitize record data +$record_data = array_map(function($value) { + return is_string($value) ? sanitize_text_field($value) : $value; +}, $record_data); $result = $dns_provider->create_dns_record($domain->get_domain(), $record_data);inc/integrations/host-providers/class-cloudflare-host-provider.php (1)
732-752: The multi-part TLD list is incomplete.The hardcoded list of multi-part TLDs is limited. Many other TLDs exist (e.g.,
.com.mx,.co.za,.ac.uk,.gov.uk). Consider using a more comprehensive approach or documenting this as a known limitation.For a more robust solution, you could use the Public Suffix List via a library, though the current approach works for common cases.
tests/WP_Ultimo/Managers/DNS_Record_Manager_Test.php (2)
283-290: Use nowdoc syntax instead of heredoc.Since the heredoc content has no variable interpolation, use nowdoc syntax (
<<<'BIND') instead of heredoc (<<<BIND) for clarity and slight performance improvement.🔎 Suggested fix
- $bind_content = <<<BIND + $bind_content = <<<'BIND' \$ORIGIN example.com. @ 3600 IN A 192.168.1.1 www 3600 IN CNAME example.com.
322-328: Avoid using reserved keywordobjectas parameter name.The parameter
$objectuses a reserved keyword in PHP. Consider renaming to$instanceor$target.🔎 Suggested fix
-private function invoke_private_method($object, string $method, array $parameters = []) { - $reflection = new \ReflectionClass(get_class($object)); +private function invoke_private_method($instance, string $method, array $parameters = []) { + $reflection = new \ReflectionClass(get_class($instance)); $method = $reflection->getMethod($method); $method->setAccessible(true); - return $method->invokeArgs($object, $parameters); + return $method->invokeArgs($instance, $parameters); }inc/integrations/host-providers/class-dns-record.php (1)
383-391: CNAME validation could be more robust.The CNAME validation only checks that the content is not an IP address, but doesn't validate that it's a valid hostname. Invalid hostnames like
example..comor--testwould pass validation.Consider adding hostname format validation, though providers may also validate this.
🔎 Suggested enhancement
case 'CNAME': // CNAME should be a hostname, not an IP if (filter_var($this->content, FILTER_VALIDATE_IP)) { return new \WP_Error( 'invalid_cname', __('CNAME record requires a hostname, not an IP address.', 'ultimate-multisite') ); } + // Basic hostname validation + if (!preg_match('/^([a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?\.)*[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?\.?$/', $this->content)) { + return new \WP_Error( + 'invalid_hostname', + __('CNAME record requires a valid hostname.', 'ultimate-multisite') + ); + } break;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
assets/js/dns-management.jsinc/admin-pages/class-domain-edit-admin-page.phpinc/integrations/host-providers/class-base-host-provider.phpinc/integrations/host-providers/class-cloudflare-host-provider.phpinc/integrations/host-providers/class-cpanel-host-provider.phpinc/integrations/host-providers/class-dns-record.phpinc/integrations/host-providers/class-hestia-host-provider.phpinc/integrations/host-providers/interface-dns-provider.phpinc/managers/class-dns-record-manager.phpinc/managers/class-domain-manager.phpinc/ui/class-domain-mapping-element.phptests/WP_Ultimo/Integrations/Host_Providers/CPanel_DNS_Test.phptests/WP_Ultimo/Integrations/Host_Providers/Cloudflare_DNS_Test.phptests/WP_Ultimo/Integrations/Host_Providers/DNS_Record_Test.phptests/WP_Ultimo/Integrations/Host_Providers/Hestia_DNS_Test.phptests/WP_Ultimo/Managers/DNS_Record_Manager_Test.phpviews/dashboard-widgets/domain-mapping.phpviews/domain/admin-dns-management.phpviews/domain/dns-management-modal.phpviews/domain/dns-record-form.php
🧰 Additional context used
🧬 Code graph analysis (7)
views/domain/dns-management-modal.php (1)
inc/functions/settings.php (1)
wu_get_setting(37-40)
inc/integrations/host-providers/class-cloudflare-host-provider.php (3)
inc/integrations/host-providers/class-base-host-provider.php (6)
get_dns_records(682-688)get_zone_id(771-774)get_supported_record_types(755-758)create_dns_record(701-707)update_dns_record(721-727)delete_dns_record(740-746)inc/integrations/host-providers/interface-dns-provider.php (5)
get_dns_records(71-71)get_supported_record_types(114-114)create_dns_record(82-82)update_dns_record(94-94)delete_dns_record(105-105)inc/integrations/host-providers/class-dns-record.php (2)
DNS_Record(20-508)from_provider(453-507)
views/domain/admin-dns-management.php (1)
inc/functions/url.php (1)
wu_network_admin_url(53-60)
views/dashboard-widgets/domain-mapping.php (2)
inc/managers/class-dns-record-manager.php (2)
get_dns_provider(66-80)customer_can_manage_dns(115-146)inc/functions/settings.php (1)
wu_get_setting(37-40)
inc/integrations/host-providers/class-base-host-provider.php (1)
inc/integrations/host-providers/interface-dns-provider.php (9)
supports_dns_management(34-34)get_dns_records(71-71)create_dns_record(82-82)update_dns_record(94-94)delete_dns_record(105-105)get_supported_record_types(114-114)is_dns_enabled(43-43)enable_dns(52-52)disable_dns(61-61)
inc/integrations/host-providers/class-cpanel-host-provider.php (4)
inc/integrations/host-providers/class-base-host-provider.php (5)
get_dns_records(682-688)get_supported_record_types(755-758)create_dns_record(701-707)update_dns_record(721-727)delete_dns_record(740-746)inc/integrations/host-providers/interface-dns-provider.php (5)
get_dns_records(71-71)get_supported_record_types(114-114)create_dns_record(82-82)update_dns_record(94-94)delete_dns_record(105-105)inc/integrations/host-providers/cpanel-api/class-cpanel-api.php (1)
uapi(262-271)inc/integrations/host-providers/class-dns-record.php (2)
DNS_Record(20-508)from_provider(453-507)
inc/integrations/host-providers/class-dns-record.php (1)
assets/js/dns-management.js (1)
classes(139-145)
🪛 GitHub Actions: Code Quality
tests/WP_Ultimo/Integrations/Host_Providers/CPanel_DNS_Test.php
[error] 1-1: Filenames should be all lowercase with hyphens as word separators. Expected cpanel-dns-test.php, but found CPanel_DNS_Test.php.
🪛 GitHub Actions: Unit & Integration Tests
inc/managers/class-dns-record-manager.php
[error] 156-156: Too few arguments to function WP_Ultimo\Managers\DNS_Record_Manager::get_allowed_record_types(), 0 passed in /__w/ultimate-multisite/ultimate-multisite/tests/WP_Ultimo/Managers/DNS_Record_Manager_Test.php on line 49 and exactly 1 expected
tests/WP_Ultimo/Managers/DNS_Record_Manager_Test.php
[error] 324-324: ReflectionException: Method WP_Ultimo\Managers\DNS_Record_Manager::validate_dns_record() does not exist
[error] 106-106: ReflectionException: Method WP_Ultimo\Managers\DNS_Record_Manager::validate_dns_record() does not exist
[error] 122-122: ReflectionException: Method WP_Ultimo\Managers\DNS_Record_Manager::validate_dns_record() does not exist
[error] 138-138: ReflectionException: Method WP_Ultimo\Managers\DNS_Record_Manager::validate_dns_record() does not exist
[error] 154-154: ReflectionException: Method WP_Ultimo\Managers\DNS_Record_Manager::validate_dns_record() does not exist
[error] 170-170: ReflectionException: Method WP_Ultimo\Managers\DNS_Record_Manager::validate_dns_record() does not exist
[error] 186-186: ReflectionException: Method WP_Ultimo\Managers\DNS_Record_Manager::validate_dns_record() does not exist
[error] 201-201: ReflectionException: Method WP_Ultimo\Managers\DNS_Record_Manager::validate_dns_record() does not exist
[error] 216-216: ReflectionException: Method WP_Ultimo\Managers\DNS_Record_Manager::validate_dns_record() does not exist
[error] 248-248: ReflectionException: Method WP_Ultimo\Managers\DNS_Record_Manager::validate_dns_record() does not exist
[error] 273-273: TypeError: WP_Ultimo\Managers\DNS_Record_Manager::export_to_bind(): Argument #1 ($domain) must be of type string, array given
[error] 292-292: ArgumentCountError: Too few arguments to function WP_Ultimo\Managers\DNS_Record_Manager::parse_bind_format(), 1 passed
🪛 GitHub Check: Code Quality Checks
tests/WP_Ultimo/Integrations/Host_Providers/CPanel_DNS_Test.php
[failure] 157-157:
Use Yoda Condition checks, you must.
[warning] 87-87:
This comment is 43% valid code; is this commented out code?
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-cpanel-dns-test.php, but found CPanel_DNS_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected cpanel-dns-test.php, but found CPanel_DNS_Test.php.
tests/WP_Ultimo/Integrations/Host_Providers/Hestia_DNS_Test.php
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-hestia-dns-test.php, but found Hestia_DNS_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected hestia-dns-test.php, but found Hestia_DNS_Test.php.
tests/WP_Ultimo/Integrations/Host_Providers/DNS_Record_Test.php
[warning] 316-316:
Equals sign not aligned with surrounding assignments; expected 2 spaces but found 1 space
[warning] 315-315:
Equals sign not aligned with surrounding assignments; expected 5 spaces but found 1 space
tests/WP_Ultimo/Integrations/Host_Providers/Cloudflare_DNS_Test.php
[failure] 131-131:
Use Yoda Condition checks, you must.
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-cloudflare-dns-test.php, but found Cloudflare_DNS_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected cloudflare-dns-test.php, but found Cloudflare_DNS_Test.php.
tests/WP_Ultimo/Managers/DNS_Record_Manager_Test.php
[warning] 322-322:
It is recommended not to use reserved keyword "object" as function parameter name. Found: $object
[warning] 284-284:
Detected heredoc without interpolation or expressions. Use nowdoc syntax instead
[warning] 268-268:
Array double arrow not aligned correctly; expected 6 space(s) between "'ttl'" and double arrow, but found 5.
[warning] 267-267:
Array double arrow not aligned correctly; expected 2 space(s) between "'content'" and double arrow, but found 1.
[warning] 266-266:
Array double arrow not aligned correctly; expected 5 space(s) between "'name'" and double arrow, but found 4.
[warning] 265-265:
Array double arrow not aligned correctly; expected 5 space(s) between "'type'" and double arrow, but found 4.
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-dns-record-manager-test.php, but found DNS_Record_Manager_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected dns-record-manager-test.php, but found DNS_Record_Manager_Test.php.
🪛 PHPMD (2.15.0)
inc/managers/class-dns-record-manager.php
71-71: Avoid unused local variables such as '$class'. (undefined)
(UnusedLocalVariable)
95-95: Avoid unused local variables such as '$class'. (undefined)
(UnusedLocalVariable)
inc/integrations/host-providers/class-base-host-provider.php
682-682: Avoid unused parameters such as '$domain'. (undefined)
(UnusedFormalParameter)
701-701: Avoid unused parameters such as '$domain'. (undefined)
(UnusedFormalParameter)
701-701: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
721-721: Avoid unused parameters such as '$domain'. (undefined)
(UnusedFormalParameter)
721-721: Avoid unused parameters such as '$record_id'. (undefined)
(UnusedFormalParameter)
721-721: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
740-740: Avoid unused parameters such as '$domain'. (undefined)
(UnusedFormalParameter)
740-740: Avoid unused parameters such as '$record_id'. (undefined)
(UnusedFormalParameter)
771-771: Avoid unused parameters such as '$domain'. (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). (3)
- GitHub Check: Build Plugin for Testing
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (17)
inc/integrations/host-providers/class-base-host-provider.php (1)
660-829: Well-structured DNS provider interface implementation.The default DNS method implementations follow a clean template pattern. Child classes that support DNS will override these methods while others get sensible WP_Error defaults. The static analysis warnings about unused parameters are expected for this interface default implementation pattern and can be safely ignored.
The enable/disable DNS methods correctly manage state via network options, maintaining consistency with the existing
enable()/disable()pattern for host integrations.inc/managers/class-domain-manager.php (1)
142-143: LGTM!The DNS Record Manager initialization is correctly hooked at priority 11 to ensure host provider integrations are loaded first (priority 10 default). The singleton pattern usage is appropriate for this manager.
Also applies to: 190-200
views/domain/dns-record-form.php (1)
41-52: LGTM on form security.The form properly includes a nonce field for CSRF protection and uses hidden inputs to pass domain context. The edit mode handling with
record_idis correctly conditional.views/domain/dns-management-modal.php (1)
145-152: LGTM on inline script configuration.The nonce and URLs are properly escaped with
esc_js()for safe JavaScript output. Usingwindow.wu_dns_configas a config object is a reasonable pattern for passing server-side data to client-side code.views/domain/admin-dns-management.php (1)
25-56: LGTM on provider-managed header section.The provider header correctly shows the provider name and provides appropriate CRUD action buttons. The refresh button properly calls the Vue instance method.
tests/WP_Ultimo/Integrations/Host_Providers/DNS_Record_Test.php (1)
1-487: Excellent test coverage for DNS_Record value object.The test suite comprehensively covers:
- Record creation for all supported types (A, AAAA, CNAME, MX, TXT)
- Validation logic including type-specific rules
- Provider-specific parsing (Cloudflare, cPanel, Hestia)
- Edge cases like type normalization, missing priority, and meta data
inc/integrations/host-providers/interface-dns-provider.php (1)
25-115: Well-designed interface contract.The
DNS_Provider_Interfaceprovides a clean, consistent API for DNS operations across different providers. The method signatures are well-documented with proper PHPDoc annotations including return types and parameter descriptions.assets/js/dns-management.js (1)
34-79: Well-structured Vue component initialization.The component properly:
- Guards against missing container or Vue
- Prevents duplicate initialization via
container.__vue__check- Reads configuration from data attributes
- Handles loading states appropriately
inc/admin-pages/class-domain-edit-admin-page.php (3)
96-122: LGTM! Well-structured form registration.The DNS management forms are properly registered with separate render and handler callbacks following the existing pattern in the codebase.
146-186: LGTM! Proper script registration and localization.The DNS management script is correctly enqueued with appropriate dependencies, and the configuration is properly localized with nonces for AJAX security.
852-876: LGTM! Well-structured delete confirmation form.The delete modal includes proper confirmation messaging with the record identifier, hidden fields for the required data, and follows the established form patterns.
inc/integrations/host-providers/class-cloudflare-host-provider.php (3)
55-58: LGTM! DNS management capability properly declared.The
dns-managementsupport flag is correctly added to the$supportsarray, enabling the DNS management features for Cloudflare.
462-478: LGTM! Proper payload construction for DNS records.The record payload is correctly constructed with type normalization, TTL defaults, and proper handling of MX priority. The removal of
proxiedfor MX/TXT records correctly follows Cloudflare's API requirements.
698-722: LGTM! Zone resolution with proper fallback.The zone ID resolution correctly attempts to find the zone by domain name first, then falls back to the configured default zone. This provides flexibility for multi-zone setups while maintaining backward compatibility.
inc/integrations/host-providers/class-dns-record.php (3)
20-77: LGTM! Well-designed DNS_Record value object.The class is well-structured with typed public properties, sensible defaults, and comprehensive documentation. The use of public typed properties is appropriate for a value object in PHP 8+.
431-442: LGTM! CSS classes consistent with JavaScript.The type-to-class mapping matches the JavaScript implementation in
dns-management.js, ensuring consistent styling across server and client rendering.
453-507: LGTM! Comprehensive provider normalization.The
from_providerfactory method properly normalizes data from different DNS providers:
- Cloudflare: Handles zone metadata
- cPanel: Strips trailing dots and maps various content field names
- Hestia: Generates synthetic IDs when not present
The default fallback ensures forward compatibility with new providers.
| } else { | ||
| self.error = response.data?.message || 'Failed to load DNS records.'; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
WordPress JavaScript optional chaining ES2020 support transpilation requirements
💡 Result:
Short answer
- Optional chaining (?.) is an ES2020 feature. If you use WordPress’s official build tools (@wordpress/build, @wordpress/babel-preset-default or the Gutenberg browserslist config) your code will be transpiled so you normally do NOT need to add a separate optional‑chaining plugin. [1][2][3].
- If you use a custom toolchain you must ensure the parser/transpiler runs before a tool that cannot parse ?. (common pitfalls: webpack4’s parser). Options: use @wordpress/build or @wordpress/babel-preset-default, add @babel/plugin-transform-optional-chaining (or the plugin-proposal), compile with esbuild/TypeScript target ≤ ES2019, or upgrade to a parser that understands ES2020 (e.g. Webpack 5). [4][5][6]
Key references
- @wordpress/wp-build (uses esbuild and produces transpiled output). [1]
- @wordpress/babel-preset-default (WordPress’s Babel preset; extendable). [3]
- @wordpress/browserslist-config (controls target browsers that determine what gets transpiled). [2]
- Babel docs: transform/syntax/optional‑chaining and that preset‑env includes the transform for ES2020. [5]
- Webpack/Babel discussions: webpack4 parser may choke on untransformed ?. — either transpile earlier or upgrade to Webpack 5. [6][4]
If you want, tell me which build setup you use (WordPress tools, create-react-app, webpack config, TypeScript, @wordpress/scripts) and I’ll give exact config lines to add.
🏁 Script executed:
# Look for build configuration files
fd -type f -name "webpack.config.*" -o -name "babel.config.*" -o -name ".babelrc*" -o -name "package.json" | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 251
🏁 Script executed:
# Check the directory structure to understand the project layout
git ls-files | head -30Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 908
🏁 Script executed:
# Find build configuration files
find . -maxdepth 3 -type f \( -name "webpack.config.js" -o -name "webpack.config.json" -o -name "babel.config.js" -o -name ".babelrc" -o -name "package.json" \) 2>/dev/null | grep -v node_modules | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 95
🏁 Script executed:
# Check if dns-management.js is in the assets directory and look at the project structure
find . -name "dns-management.js" -o -name "package.json" | head -10Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 125
🏁 Script executed:
# Read package.json to check build dependencies and configuration
cat -n package.jsonRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 4082
🏁 Script executed:
# Check the uglify build script to understand how JavaScript is processed
cat -n scripts/uglify.js | head -50Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 644
🏁 Script executed:
# Check if there are any other JavaScript preprocessing steps
cat -n scripts/clean-js.js | head -30Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 180
🏁 Script executed:
# Check if there's any browserslist or browser support documentation
find . -maxdepth 2 -type f \( -name ".browserslistrc" -o -name "browserslist" \) 2>/dev/nullRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Check the plugin header or readme for browser requirements
head -50 ultimate-multisite.php | grep -i "requires\|browser\|support"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 323
🏁 Script executed:
# Check README for browser/support information
cat -n README.md 2>/dev/null | head -100Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 6204
Replace optional chaining with explicit null checks or add Babel transpilation.
The build process uses UglifyJS for minification only, which does not transpile ES2020 syntax like optional chaining (?.). This code will fail in older browsers (IE11, Safari <14, etc.) that don't support optional chaining. Either add @babel/plugin-transform-optional-chaining to your Babel configuration, or replace response.data?.message with response.data && response.data.message.
Also applies to: 296-298
🤖 Prompt for AI Agents
In assets/js/dns-management.js around lines 114 to 116 (and also apply the same
fix at 296-298), the use of optional chaining (response.data?.message) is not
compatible with the current build/minifier; replace the optional chaining with
an explicit null/undefined check such as using response.data &&
response.data.message (or response.data ? response.data.message : undefined), or
alternatively add @babel/plugin-transform-optional-chaining to the project's
Babel config so optional chaining is transpiled during the build.
| $supported_types = implode(',', $this->get_supported_record_types()); | ||
|
|
||
| $response = $this->cloudflare_api_call( | ||
| "client/v4/zones/{$zone_id}/dns_records", | ||
| 'GET', | ||
| [ | ||
| 'per_page' => 100, | ||
| 'type' => $supported_types, | ||
| ] | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n inc/integrations/host-providers/class-cloudflare-host-provider.php | sed -n '379,436p'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1845
🌐 Web query:
Cloudflare API DNS records pagination per_page response format
💡 Result:
Short answer — how it works:
- Request: use query params per_page (min=1, default=100, max=5,000,000 for DNS records) and page. [1]
- Response: JSON with result (array of records) plus result_info containing pagination metadata — typically: page, per_page, count (items in this page), total_count (total matching items). Many endpoints also include total_pages; some collection endpoints use cursor-based pagination and expose cursors under result_info.cursors. Example fields: {"page":1,"per_page":20,"count":20,"total_count":123,"total_pages":7}. [2][3]
Sources:
- Cloudflare DNS Records API (list) — query params and per_page limits. [1]
- Cloudflare API pagination & example result_info (count, page, per_page, total_count). [2]
- Example responses showing total_pages and result_info. [3]
Implement pagination to fetch all DNS records.
The code calls the Cloudflare API with per_page=100 but only processes the first page of results. Domains with more than 100 DNS records will silently have records truncated. The Cloudflare API response includes result_info with pagination metadata (total_count, page, total_pages); implement a loop to fetch all pages or document this limitation explicitly if intentional.
| public function to_array(): array { | ||
|
|
||
| return [ | ||
| 'id' => $this->id, | ||
| 'type' => $this->type, | ||
| 'name' => $this->name, | ||
| 'content' => $this->content, | ||
| 'ttl' => $this->ttl, | ||
| 'priority' => $this->priority, | ||
| 'proxied' => $this->proxied, | ||
| 'meta' => $this->meta, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Missing get_id() accessor method.
The admin page code (lines 759, 845 in class-domain-edit-admin-page.php) calls $r->get_id() on DNS_Record objects, but this class only has the public $id property without a corresponding getter method. This will cause a fatal error.
🔎 Add the missing getter method
+ /**
+ * Get the record ID.
+ *
+ * @since 2.3.0
+ *
+ * @return string
+ */
+ public function get_id(): string {
+
+ return $this->id;
+ }
+
/**
* Get the record type.
*🤖 Prompt for AI Agents
In inc/integrations/host-providers/class-dns-record.php around lines 130 to 142,
the class exposes a public $id property but lacks the get_id() accessor which
causes calls to $r->get_id() to fail; add a public method named get_id() that
returns the record id (simply return $this->id) and declare an appropriate
return type (e.g., int|string|null or no type if uncertain) to match how id is
used elsewhere, keeping method visibility public so existing callers continue to
work.
| return [ | ||
| 'id' => time(), // Hestia doesn't return the new ID | ||
| 'type' => $type, | ||
| 'name' => $name, | ||
| 'content' => $value, | ||
| 'ttl' => $ttl, | ||
| 'priority' => '' !== $priority ? (int) $priority : null, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Using time() as record ID may cause issues.
Since Hestia doesn't return the new record ID, using time() as a placeholder prevents immediate edits/deletes of newly created records. Consider re-fetching records after creation to obtain the actual ID, or documenting this limitation in the UI.
🤖 Prompt for AI Agents
In inc/integrations/host-providers/class-hestia-host-provider.php around lines
473–481, the code returns an ID generated with time() when Hestia doesn't
provide the new record ID; replace this by re-fetching the zone's records
immediately after creation to locate the newly created record and use its real
ID (match on type/name/content/ttl/priority and return that record's ID), and if
a matching record cannot be found set the ID to null and ensure the calling
code/UI treats records with null ID as non-editable/deletable (or surface a
user-facing message documenting the limitation).
| public function get_allowed_record_types(int $user_id): array { | ||
|
|
||
| // Super admins get all types | ||
| if (is_super_admin($user_id)) { | ||
| return DNS_Record::VALID_TYPES; | ||
| } | ||
|
|
||
| // Get allowed types from settings | ||
| $allowed = wu_get_setting('dns_record_types_allowed', ['A', 'CNAME', 'TXT']); | ||
|
|
||
| return array_intersect($allowed, DNS_Record::VALID_TYPES); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "*DNS_Record_Manager_Test*" -o -name "*test*dns*" | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 135
🏁 Script executed:
rg -n "get_allowed_record_types" --type phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 990
🏁 Script executed:
# Check the tests directory structure
find ./tests -type f -name "*.php" | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1165
🏁 Script executed:
cat -n ./tests/WP_Ultimo/Managers/DNS_Record_Manager_Test.php | sed -n '40,60p'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 818
Test method missing required user ID argument.
The test at line 49 calls get_allowed_record_types() without the required int $user_id parameter. Update the call to pass a user ID, for example: $this->manager->get_allowed_record_types(1) or a test user ID appropriate for your test setup.
🧰 Tools
🪛 GitHub Actions: Unit & Integration Tests
[error] 156-156: Too few arguments to function WP_Ultimo\Managers\DNS_Record_Manager::get_allowed_record_types(), 0 passed in /__w/ultimate-multisite/ultimate-multisite/tests/WP_Ultimo/Managers/DNS_Record_Manager_Test.php on line 49 and exactly 1 expected
🤖 Prompt for AI Agents
In inc/managers/class-dns-record-manager.php around lines 156 to 167 the test
calling get_allowed_record_types() is missing the required int $user_id
argument; update the test to pass a valid user ID (for example 1 or a test
fixture user ID) when invoking the method (e.g.
$this->manager->get_allowed_record_types(1)) so the call matches the method
signature and the test runs correctly.
| <?php | ||
| /** | ||
| * Test case for DNS_Record_Manager. | ||
| * | ||
| * @package WP_Ultimo | ||
| * @subpackage Tests | ||
| */ | ||
|
|
||
| namespace WP_Ultimo\Tests\Managers; | ||
|
|
||
| use WP_Ultimo\Managers\DNS_Record_Manager; | ||
| use WP_UnitTestCase; | ||
|
|
||
| /** | ||
| * Test DNS_Record_Manager functionality. | ||
| */ | ||
| class DNS_Record_Manager_Test extends WP_UnitTestCase { |
There was a problem hiding this comment.
File naming convention violation.
Per static analysis, test file names should follow the pattern class-dns-record-manager-test.php (lowercase with hyphens). The current filename DNS_Record_Manager_Test.php doesn't match the expected convention.
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-dns-record-manager-test.php, but found DNS_Record_Manager_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected dns-record-manager-test.php, but found DNS_Record_Manager_Test.php.
🤖 Prompt for AI Agents
In tests/WP_Ultimo/Managers/DNS_Record_Manager_Test.php around lines 1 to 17,
the test filename violates the project's naming convention; rename the file to
the lowercase-hyphenated pattern class-dns-record-manager-test.php and update
any references (autoload, test suite, or includes) to the new filename so the
test runner and static analysis detect it correctly.
| public function test_validate_dns_record_valid_a_record() { | ||
| $record = [ | ||
| 'type' => 'A', | ||
| 'name' => 'test', | ||
| 'content' => '192.168.1.1', | ||
| 'ttl' => 3600, | ||
| ]; | ||
|
|
||
| $result = $this->invoke_private_method($this->manager, 'validate_dns_record', [$record]); | ||
|
|
||
| $this->assertTrue($result); | ||
| } |
There was a problem hiding this comment.
Critical: validate_dns_record method does not exist on DNS_Record_Manager.
The pipeline failures indicate that validate_dns_record is not a method on DNS_Record_Manager. The tests are attempting to invoke a non-existent method, causing ReflectionException.
Based on the DNS_Record class in this PR, validation appears to be on the DNS_Record object itself via its validate() method, not on the manager.
🔎 Suggested fix - use DNS_Record::validate() instead
public function test_validate_dns_record_valid_a_record() {
- $record = [
+ $record = new \WP_Ultimo\Integrations\Host_Providers\DNS_Record([
'type' => 'A',
'name' => 'test',
'content' => '192.168.1.1',
'ttl' => 3600,
- ];
+ ]);
- $result = $this->invoke_private_method($this->manager, 'validate_dns_record', [$record]);
+ $result = $record->validate();
$this->assertTrue($result);
}🤖 Prompt for AI Agents
In tests/WP_Ultimo/Managers/DNS_Record_Manager_Test.php around lines 83 to 94,
the test calls a non-existent validate_dns_record method on DNS_Record_Manager
causing a ReflectionException; update the test to construct a DNS_Record object
(using the same record array) and call its validate() method instead, then
assertTrue on the result (or adjust assertions if validate() throws/returns
differently), removing the invoke_private_method call and using the public
DNS_Record API for validation.
| public function test_export_to_bind() { | ||
| $records = [ | ||
| new \WP_Ultimo\Integrations\Host_Providers\DNS_Record([ | ||
| 'type' => 'A', | ||
| 'name' => '@', | ||
| 'content' => '192.168.1.1', | ||
| 'ttl' => 3600, | ||
| ]), | ||
| new \WP_Ultimo\Integrations\Host_Providers\DNS_Record([ | ||
| 'type' => 'MX', | ||
| 'name' => '@', | ||
| 'content' => 'mail.example.com', | ||
| 'ttl' => 3600, | ||
| 'priority' => 10, | ||
| ]), | ||
| ]; | ||
|
|
||
| $bind = $this->manager->export_to_bind($records, 'example.com'); | ||
|
|
||
| $this->assertStringContainsString('$ORIGIN example.com.', $bind); | ||
| $this->assertStringContainsString('@ 3600 IN A 192.168.1.1', $bind); | ||
| $this->assertStringContainsString('@ 3600 IN MX 10 mail.example.com.', $bind); | ||
| } |
There was a problem hiding this comment.
Critical: export_to_bind method signature mismatch.
The pipeline failure shows export_to_bind(): Argument #1 ($domain) must be of type string, array given. The test passes records as the first argument and domain as second, but the actual method expects domain first.
🔎 Suggested fix - swap argument order
- $bind = $this->manager->export_to_bind($records, 'example.com');
+ $bind = $this->manager->export_to_bind('example.com', $records);📝 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.
| public function test_export_to_bind() { | |
| $records = [ | |
| new \WP_Ultimo\Integrations\Host_Providers\DNS_Record([ | |
| 'type' => 'A', | |
| 'name' => '@', | |
| 'content' => '192.168.1.1', | |
| 'ttl' => 3600, | |
| ]), | |
| new \WP_Ultimo\Integrations\Host_Providers\DNS_Record([ | |
| 'type' => 'MX', | |
| 'name' => '@', | |
| 'content' => 'mail.example.com', | |
| 'ttl' => 3600, | |
| 'priority' => 10, | |
| ]), | |
| ]; | |
| $bind = $this->manager->export_to_bind($records, 'example.com'); | |
| $this->assertStringContainsString('$ORIGIN example.com.', $bind); | |
| $this->assertStringContainsString('@ 3600 IN A 192.168.1.1', $bind); | |
| $this->assertStringContainsString('@ 3600 IN MX 10 mail.example.com.', $bind); | |
| } | |
| public function test_export_to_bind() { | |
| $records = [ | |
| new \WP_Ultimo\Integrations\Host_Providers\DNS_Record([ | |
| 'type' => 'A', | |
| 'name' => '@', | |
| 'content' => '192.168.1.1', | |
| 'ttl' => 3600, | |
| ]), | |
| new \WP_Ultimo\Integrations\Host_Providers\DNS_Record([ | |
| 'type' => 'MX', | |
| 'name' => '@', | |
| 'content' => 'mail.example.com', | |
| 'ttl' => 3600, | |
| 'priority' => 10, | |
| ]), | |
| ]; | |
| $bind = $this->manager->export_to_bind('example.com', $records); | |
| $this->assertStringContainsString('$ORIGIN example.com.', $bind); | |
| $this->assertStringContainsString('@ 3600 IN A 192.168.1.1', $bind); | |
| $this->assertStringContainsString('@ 3600 IN MX 10 mail.example.com.', $bind); | |
| } |
🧰 Tools
🪛 GitHub Actions: Unit & Integration Tests
[error] 273-273: TypeError: WP_Ultimo\Managers\DNS_Record_Manager::export_to_bind(): Argument #1 ($domain) must be of type string, array given
🪛 GitHub Check: Code Quality Checks
[warning] 268-268:
Array double arrow not aligned correctly; expected 6 space(s) between "'ttl'" and double arrow, but found 5.
[warning] 267-267:
Array double arrow not aligned correctly; expected 2 space(s) between "'content'" and double arrow, but found 1.
[warning] 266-266:
Array double arrow not aligned correctly; expected 5 space(s) between "'name'" and double arrow, but found 4.
[warning] 265-265:
Array double arrow not aligned correctly; expected 5 space(s) between "'type'" and double arrow, but found 4.
🤖 Prompt for AI Agents
In tests/WP_Ultimo/Managers/DNS_Record_Manager_Test.php around lines 256 to 278,
the test calls export_to_bind($records, 'example.com') but the method signature
expects the domain string first; update the test to call
export_to_bind('example.com', $records) (swap the arguments) so the first
parameter is the domain string and the second is the records array, leaving
assertions unchanged.
| /** | ||
| * Test parse_bind_format. | ||
| */ | ||
| public function test_parse_bind_format() { | ||
| $bind_content = <<<BIND | ||
| \$ORIGIN example.com. | ||
| @ 3600 IN A 192.168.1.1 | ||
| www 3600 IN CNAME example.com. | ||
| @ 3600 IN MX 10 mail.example.com. | ||
| @ 3600 IN TXT "v=spf1 mx ~all" | ||
| BIND; | ||
|
|
||
| $records = $this->manager->parse_bind_format($bind_content); | ||
|
|
||
| $this->assertIsArray($records); | ||
| $this->assertGreaterThanOrEqual(4, count($records)); | ||
|
|
||
| // Find A record | ||
| $a_record = array_filter($records, fn($r) => $r['type'] === 'A'); | ||
| $this->assertNotEmpty($a_record); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the DNS_Record_Manager class file
find . -type f -name "*.php" | grep -i "DNS_Record_Manager" | grep -v Test | head -5Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 218
🏁 Script executed:
ast-grep --pattern $'function parse_bind_format($$$) {
$$$
}'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
grep -A 10 "function parse_bind_format" path/to/DNS_Record_Manager.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 144
🏁 Script executed:
find . -type f -name "*DNS_Record_Manager*.php" -o -name "*dns_record_manager*.php" | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 135
🏁 Script executed:
rg "function parse_bind_format" --type php -B 2 -A 15Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1297
parse_bind_format() requires a domain parameter.
The method signature requires two arguments: parse_bind_format(string $content, string $domain). The test currently passes only the bind content, missing the required domain parameter.
Update line 295:
- $records = $this->manager->parse_bind_format($bind_content);
+ $records = $this->manager->parse_bind_format($bind_content, 'example.com');📝 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.
| /** | |
| * Test parse_bind_format. | |
| */ | |
| public function test_parse_bind_format() { | |
| $bind_content = <<<BIND | |
| \$ORIGIN example.com. | |
| @ 3600 IN A 192.168.1.1 | |
| www 3600 IN CNAME example.com. | |
| @ 3600 IN MX 10 mail.example.com. | |
| @ 3600 IN TXT "v=spf1 mx ~all" | |
| BIND; | |
| $records = $this->manager->parse_bind_format($bind_content); | |
| $this->assertIsArray($records); | |
| $this->assertGreaterThanOrEqual(4, count($records)); | |
| // Find A record | |
| $a_record = array_filter($records, fn($r) => $r['type'] === 'A'); | |
| $this->assertNotEmpty($a_record); | |
| } | |
| /** | |
| * Test parse_bind_format. | |
| */ | |
| public function test_parse_bind_format() { | |
| $bind_content = <<<BIND | |
| \$ORIGIN example.com. | |
| @ 3600 IN A 192.168.1.1 | |
| www 3600 IN CNAME example.com. | |
| @ 3600 IN MX 10 mail.example.com. | |
| @ 3600 IN TXT "v=spf1 mx ~all" | |
| BIND; | |
| $records = $this->manager->parse_bind_format($bind_content, 'example.com'); | |
| $this->assertIsArray($records); | |
| $this->assertGreaterThanOrEqual(4, count($records)); | |
| // Find A record | |
| $a_record = array_filter($records, fn($r) => $r['type'] === 'A'); | |
| $this->assertNotEmpty($a_record); | |
| } |
🧰 Tools
🪛 GitHub Actions: Unit & Integration Tests
[error] 292-292: ArgumentCountError: Too few arguments to function WP_Ultimo\Managers\DNS_Record_Manager::parse_bind_format(), 1 passed
🪛 GitHub Check: Code Quality Checks
[warning] 284-284:
Detected heredoc without interpolation or expressions. Use nowdoc syntax instead
🤖 Prompt for AI Agents
In tests/WP_Ultimo/Managers/DNS_Record_Manager_Test.php around lines 280 to 300,
the call to parse_bind_format only passes the bind content but the method
requires a second domain parameter; update the test to call parse_bind_format
with both the $bind_content and the domain string (e.g., "example.com") so the
signature parse_bind_format(string $content, string $domain) is satisfied and
the test uses the correct domain for parsing.
| <tbody v-if="!loading && error"> | ||
| <tr> | ||
| <td colspan="4"> | ||
| <div class="wu-mt-0 wu-p-4 wu-bg-red-100 wu-border wu-border-solid wu-border-red-200 wu-rounded-sm wu-text-red-500" v-html="error[0].message"></div> | ||
| </td> | ||
| </tr> | ||
| </tbody> |
There was a problem hiding this comment.
Potential XSS via v-html with error message.
Line 180 uses v-html="error[0].message" to render error content. If the error message contains user-controlled input or is not properly sanitized on the server side, this could lead to XSS vulnerabilities.
🔎 Proposed safer alternative
<tbody v-if="!loading && error">
<tr>
<td colspan="4">
- <div class="wu-mt-0 wu-p-4 wu-bg-red-100 wu-border wu-border-solid wu-border-red-200 wu-rounded-sm wu-text-red-500" v-html="error[0].message"></div>
+ <div class="wu-mt-0 wu-p-4 wu-bg-red-100 wu-border wu-border-solid wu-border-red-200 wu-rounded-sm wu-text-red-500">{{ error[0].message }}</div>
</td>
</tr>
</tbody>🤖 Prompt for AI Agents
In views/domain/admin-dns-management.php around lines 177 to 183 the template
uses v-html="error[0].message", which can introduce XSS if the message contains
untrusted input; change this to render escaped text instead (use mustache
interpolation or v-text="error[0].message") so the message is output as plain
text, and if HTML rendering is required only after safe whitelisting, run the
message through a trusted sanitizer (e.g., DOMPurify) before binding. Also add a
defensive check that error[0] exists before attempting to display its message.
Summary
This feature allows customers and network admins to manage DNS records (A, AAAA, CNAME, MX, TXT) for domains mapped via hosting provider APIs.
Core Infrastructure
DNS_Recordvalue object for consistent record handlingDNS_Provider_Interfacefor DNS-capable host providersDNS_Record_Managerfor centralized operations and AJAX handlersBase_Host_Providerwith default DNS method implementationsProvider Implementations
Customer UI
Admin UI
New Files
inc/integrations/host-providers/interface-dns-provider.php- Interface for DNS providersinc/integrations/host-providers/class-dns-record.php- DNS record value objectinc/managers/class-dns-record-manager.php- Central DNS managementassets/js/dns-management.js- Vue.js DNS componentviews/domain/dns-management-modal.php- Customer DNS modalviews/domain/dns-record-form.php- Add/edit formviews/domain/admin-dns-management.php- Admin widget viewTest Plan
vendor/bin/phpunit --testdoxTests
70 unit tests covering:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.