feat(email): Add email accounts management feature (#254) #307
Conversation
Implements email account provisioning and management for Ultimate Multisite customers. Allows network admins to configure email providers and enable customers to create/manage email accounts through their wp-admin dashboard. ## New Components ### Database Layer - Email accounts table schema with full CRUD operations - Query class for complex database operations - Status enum for account states (pending, provisioning, active, suspended, failed) ### Email Account Model - Full model with validation rules - Relationships to Customer, Membership, Site - Email parsing (username/domain extraction) ### Email Providers (4 integrations) - cPanel Email Provider (using existing cPanel API) - Purelymail Provider (privacy-focused email hosting) - Google Workspace Provider (Admin SDK with JWT auth) - Microsoft 365 Provider (Graph API with OAuth2) ### Manager & Limitations - Email_Account_Manager for lifecycle management - Async provisioning/deletion via Action Scheduler - Limit_Email_Accounts for membership quota enforcement ### Customer UI - Email_Accounts_Element (Gutenberg block/shortcode) - View template with quota display, account list - Create/credentials/DNS/delete modals ### Settings - Email accounts settings section - Provider configuration - Per-account purchase pricing ## Features - Both membership-included quotas AND per-account purchases - Multiple providers allowed - customers choose when creating accounts - DNS setup instructions per provider - Password encryption with sodium for temporary storage - Webmail, IMAP/SMTP settings display ## Testing - Email_Account_Test: Model creation, validation, relationships - Limit_Email_Accounts_Test: Limit initialization, checking, slots - Email_Account_Functions_Test: Helper functions, encryption 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a complete Email Accounts feature: DB table, schema, model, query and status enum, manager, providers (cPanel, Purelymail, Google Workspace, Microsoft 365), UI elements and wizard, settings and limitation integration, helper functions, async provisioning/deletion, table loader, views, and unit tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Customer
participant UI as Dashboard (UI)
participant Manager as Email_Account_Manager
participant DB as DB (wu_email_accounts)
participant Queue as Async Queue
participant Provider as Email Provider
Customer->>UI: Click "Create Email Account"
UI->>Manager: create_account(params)
Manager->>Manager: validate limits & inputs
Manager->>DB: insert account (status: pending)
DB-->>Manager: returns id
Manager->>Manager: generate password, store token (transient)
Manager->>Queue: enqueue async_provision(email_account_id)
Manager-->>UI: return pending account (token reference)
UI-->>Customer: show temporary token/password
note over Queue,Provider: Async provisioning flow
Queue->>Manager: async_provision_email_account(id)
Manager->>DB: fetch account
Manager->>Provider: create_email_account(params)
alt success
Provider-->>Manager: external_id, quota
Manager->>DB: update status=active, set external_id/quota
Manager->>Manager: fire provisioned hooks
Manager-->>UI: account marked active
else failure
Provider-->>Manager: WP_Error
Manager->>DB: update status=failed
Manager->>Manager: fire provisioning_failed hooks
end
sequenceDiagram
autonumber
participant Admin as Network Admin
participant Settings as Settings Page
participant Manager as Email_Account_Manager
participant Provider as Provider Integration
participant Config as wp-config.php
Admin->>Settings: Open Email Accounts settings
Settings->>Manager: register settings & provider UI
Admin->>Settings: Enable provider + enter config
Settings->>Provider: test_connection()
alt Provider needs constants
Provider-->>Admin: provide define() snippet
Admin->>Config: paste defines into wp-config.php
else valid API config
Settings->>Manager: save enabled provider
Manager->>Manager: load_provider(provider_id)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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: 14
🧹 Nitpick comments (13)
inc/integrations/email-providers/class-cpanel-email-provider.php (2)
313-351: Unused$usernamevariable from destructuring.The
$usernamevariable is extracted at line 321 but never used in this method. While this is a common pattern when only the domain is needed for the API call, consider using an underscore prefix or explicit discard if the linter supports it.- [$username, $domain] = $parts; + [, $domain] = $parts;This aligns with the PHPMD hint about unused local variables.
414-425: Remove unused$hostvariable.The
$hostvariable is fetched and stripped of protocol but never used in the returned array. The IMAP server usesmail.+ account domain instead.🔎 Proposed fix
public function get_imap_settings(Email_Account $account) { - $host = defined('WU_CPANEL_HOST') ? WU_CPANEL_HOST : ''; - $host = preg_replace('#^https?://#', '', $host); - return [ 'server' => 'mail.' . $account->get_domain(), 'port' => 993,inc/integrations/email-providers/class-microsoft365-provider.php (1)
321-333: Hardcoded'US'usage location may cause issues for international tenants.The
usageLocationis hardcoded to'US', which is required for license assignment but may not be appropriate for all Microsoft 365 tenants. Consider making this configurable via an optional constant or inferring it from the tenant configuration.+ 'usageLocation' => defined('WU_MS365_USAGE_LOCATION') ? WU_MS365_USAGE_LOCATION : 'US', - 'usageLocation' => 'US', // Required for license assignmentEMAIL_ACCOUNTS_FEATURE.md (2)
251-279: Add language specifier to the file structure code block.The file structure code block is missing a language identifier, which triggers a markdown lint warning.
🔎 Proposed fix
-``` +```text inc/ ├── database/email-accounts/
113-129: Minor documentation inconsistency with schema.The documentation shows
statusasvarchar(20)but the actual schema (inclass-email-accounts-schema.php) defines it as anENUM. Consider updating the documentation to reflect the actual type for accuracy.🔎 Proposed fix
-| status | varchar(20) | pending/provisioning/active/suspended/failed | +| status | enum | pending/provisioning/active/suspended/failed |tests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.php (1)
370-397: Remove unused$limitvariable.The
$limitvariable on line 371 is instantiated but never used because the test immediately creates a mock object instead. This triggers a PHPMD warning.🔎 Proposed fix
public function test_get_remaining_slots_numeric(): void { - $limit = new Limit_Email_Accounts( - [ - 'enabled' => true, - 'limit' => 5, - ] - ); - // Mock the get_current_account_count to return 2 $limit_mock = $this->getMockBuilder(Limit_Email_Accounts::class)inc/database/email-accounts/class-email-account-query.php (1)
107-110: Constructor override is technically unnecessary.The constructor only delegates to the parent without any additional logic. While this follows the pattern in the codebase for consistency, it could be removed if desired.
🔎 Proposed fix (optional)
- /** - * Sets up the email account query, based on the query vars passed. - * - * @since 2.3.0 - * @access public - * - * @param string|array $query Array of query arguments. - */ - public function __construct($query = []) { - - parent::__construct($query); - } }inc/integrations/email-providers/class-purelymail-provider.php (1)
251-269: Domain existence check uses string matching for error handling.The
ensure_domain_existsmethod checks if an error message contains "already" to determine if the domain already exists. This is fragile and could break if the API error message changes.Consider checking for a specific error code if Purelymail provides one, or documenting this dependency on the error message format.
inc/ui/class-email-accounts-element.php (2)
559-595: Consider using the fetched email account for confirmation display.The
$email_accountis fetched but only the ID is used in the hidden field. Consider displaying the email address being deleted in the warning message for better user confirmation.🔎 Suggested enhancement
public function render_delete_email_form(): void { $email_account_id = wu_request('email_account_id'); $email_account = wu_get_email_account($email_account_id); + if ( ! $email_account) { + echo '<p>' . esc_html__('Email account not found.', 'ultimate-multisite') . '</p>'; + return; + } + $fields = [ 'warning' => [ 'type' => 'note', 'desc' => sprintf( - '<strong class="wu-text-red-600">%s</strong> %s', + '<strong class="wu-text-red-600">%s</strong> %s<br><br><code>%s</code>', __('Warning:', 'ultimate-multisite'), - __('This will permanently delete the email account and all its data from the email provider.', 'ultimate-multisite') + __('This will permanently delete the email account and all its data from the email provider.', 'ultimate-multisite'), + esc_html($email_account->get_email_address()) ), ],
643-663: Database deletion before provider confirmation could cause orphaned accounts.The account is deleted from the database immediately (line 654) before the async provider deletion completes. If the provider deletion fails, the account will remain on the provider with no local record to track or retry deletion.
Consider either:
- Setting the status to
deletingfirst, then deleting the DB record only after the async action confirms success- Keeping a soft-delete or deletion queue record until provider confirms removal
tests/WP_Ultimo/Functions/Email_Account_Functions_Test.php (1)
71-76: Test assertion mismatch with function default.The test expects password length
>= 12butwu_generate_email_passwordhas a default length of 16. Consider asserting the exact expected default length.-$this->assertGreaterThanOrEqual(12, strlen($password)); +$this->assertEquals(16, strlen($password));inc/integrations/email-providers/class-google-workspace-provider.php (2)
412-427: Userawurlencode()instead ofurlencode()for URL path segments.Per RFC 3986 and static analysis,
rawurlencode()should be used for encoding URL path components. This applies to lines 415, 441, 467, and 601.🔎 Proposed fix
public function delete_email_account($email_address) { $result = $this->api_request( - 'users/' . urlencode($email_address), + 'users/' . rawurlencode($email_address), [], 'DELETE' );Apply the same change to:
- Line 441:
'users/' . rawurlencode($email_address)- Line 467:
'users/' . rawurlencode($email_address)- Line 601:
'customers/' . rawurlencode($customer_id)
171-181: Addis_readable()check before reading service account file.While
file_exists()is checked,file_get_contents()could fail if the file isn't readable. Consider adding an explicit readability check.-if (empty($json_path) || ! file_exists($json_path)) { +if (empty($json_path) || ! file_exists($json_path) || ! is_readable($json_path)) { return new \WP_Error('no_credentials', __('Google service account JSON file not found.', 'ultimate-multisite')); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
EMAIL_ACCOUNTS_FEATURE.mdinc/admin-pages/customer-panel/class-account-admin-page.phpinc/class-settings.phpinc/class-wp-ultimo.phpinc/database/email-accounts/class-email-account-query.phpinc/database/email-accounts/class-email-account-status.phpinc/database/email-accounts/class-email-accounts-schema.phpinc/database/email-accounts/class-email-accounts-table.phpinc/functions/email-account.phpinc/integrations/email-providers/class-base-email-provider.phpinc/integrations/email-providers/class-cpanel-email-provider.phpinc/integrations/email-providers/class-google-workspace-provider.phpinc/integrations/email-providers/class-microsoft365-provider.phpinc/integrations/email-providers/class-purelymail-provider.phpinc/limitations/class-limit-email-accounts.phpinc/managers/class-email-account-manager.phpinc/models/class-email-account.phpinc/objects/class-limitations.phpinc/ui/class-email-accounts-element.phptests/WP_Ultimo/Functions/Email_Account_Functions_Test.phptests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.phptests/WP_Ultimo/Models/Email_Account_Test.phpviews/dashboard-widgets/email-accounts.php
🧰 Additional context used
🧬 Code graph analysis (14)
inc/objects/class-limitations.php (1)
inc/limitations/class-limit-email-accounts.php (1)
Limit_Email_Accounts(22-165)
inc/database/email-accounts/class-email-account-query.php (3)
inc/database/engine/class-query.php (1)
Query(20-119)inc/database/email-accounts/class-email-accounts-schema.php (1)
Email_Accounts_Schema(22-162)inc/models/class-email-account.php (1)
Email_Account(22-727)
inc/admin-pages/customer-panel/class-account-admin-page.php (2)
inc/ui/class-email-accounts-element.php (1)
Email_Accounts_Element(25-824)inc/ui/class-base-element.php (1)
as_metabox(1176-1218)
inc/limitations/class-limit-email-accounts.php (2)
inc/limitations/class-limit.php (1)
Limit(20-346)inc/functions/email-account.php (1)
wu_count_email_accounts(178-190)
tests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.php (2)
inc/objects/class-limitations.php (1)
Limitations(37-503)inc/limitations/class-limit-email-accounts.php (5)
Limit_Email_Accounts(22-165)check(45-68)can_create_more(79-105)get_remaining_slots(130-150)default_state(158-164)
inc/database/email-accounts/class-email-accounts-schema.php (1)
inc/database/engine/class-schema.php (1)
Schema(20-33)
inc/database/email-accounts/class-email-accounts-table.php (1)
inc/database/engine/class-table.php (1)
Table(21-81)
inc/database/email-accounts/class-email-account-status.php (1)
inc/database/engine/class-enum.php (1)
Enum(20-272)
inc/managers/class-email-account-manager.php (5)
inc/models/class-email-account.php (7)
Email_Account(22-727)get_provider(343-346)get_provider_instance(366-371)set_status(391-394)get_domain(320-323)get_quota_mb(439-442)get_email_address(278-281)inc/integrations/email-providers/class-base-email-provider.php (5)
is_enabled(125-130)is_setup(212-232)create_email_account(410-410)delete_email_account(420-420)get_dns_instructions(461-461)inc/functions/email-account.php (7)
wu_get_email_account(22-25)wu_get_email_password_from_token(375-387)wu_generate_email_password(397-400)wu_store_email_password_token(349-364)wu_get_email_accounts(35-38)wu_can_create_email_account(201-244)wu_get_email_account_by_email(48-51)inc/functions/helper.php (1)
wu_log_add(208-211)inc/functions/customer.php (1)
wu_get_customer(22-25)
inc/class-wp-ultimo.php (2)
inc/ui/class-email-accounts-element.php (1)
Email_Accounts_Element(25-824)inc/managers/class-email-account-manager.php (1)
Email_Account_Manager(26-613)
inc/models/class-email-account.php (6)
inc/database/email-accounts/class-email-account-status.php (1)
Email_Account_Status(22-102)inc/models/class-base-model.php (1)
get_by(379-386)inc/database/email-accounts/class-email-account-query.php (1)
Email_Account_Query(22-111)inc/functions/customer.php (1)
wu_get_customer(22-25)inc/functions/site.php (1)
wu_get_site(37-40)inc/managers/class-email-account-manager.php (1)
get_provider(152-163)
tests/WP_Ultimo/Functions/Email_Account_Functions_Test.php (2)
inc/models/class-email-account.php (5)
Email_Account(22-727)get_email_address(278-281)get_provider(343-346)get_status(379-382)get_quota_mb(439-442)inc/functions/email-account.php (10)
wu_generate_email_password(397-400)wu_count_email_accounts(178-190)wu_get_email_account(22-25)wu_get_email_accounts(35-38)wu_create_email_account(112-167)wu_encrypt_email_password(295-308)wu_decrypt_email_password(318-338)wu_store_email_password_token(349-364)wu_get_email_password_from_token(375-387)wu_get_email_account_by_email(48-51)
inc/integrations/email-providers/class-base-email-provider.php (4)
inc/integrations/email-providers/class-cpanel-email-provider.php (12)
get_fields(107-129)get_description(85-88)get_logo(96-99)create_email_account(159-213)delete_email_account(223-257)change_password(268-303)get_account_info(313-351)get_webmail_url(361-368)get_dns_instructions(378-404)get_imap_settings(414-425)get_smtp_settings(435-443)test_connection(451-465)inc/integrations/email-providers/class-google-workspace-provider.php (12)
get_fields(123-150)get_description(101-104)get_logo(112-115)create_email_account(352-402)delete_email_account(412-427)change_password(438-455)get_account_info(465-480)get_webmail_url(490-493)get_dns_instructions(503-548)get_imap_settings(558-566)get_smtp_settings(576-584)test_connection(592-608)inc/integrations/email-providers/class-microsoft365-provider.php (12)
get_fields(125-154)get_description(103-106)get_logo(114-117)create_email_account(290-359)delete_email_account(407-422)change_password(433-453)get_account_info(463-481)get_webmail_url(491-494)get_dns_instructions(504-539)get_imap_settings(549-557)get_smtp_settings(567-575)test_connection(583-593)inc/integrations/email-providers/class-purelymail-provider.php (12)
get_fields(105-119)get_description(83-86)get_logo(94-97)create_email_account(193-241)delete_email_account(279-295)change_password(306-323)get_account_info(333-354)get_webmail_url(364-367)get_dns_instructions(377-406)get_imap_settings(416-424)get_smtp_settings(434-442)test_connection(450-460)
inc/integrations/email-providers/class-purelymail-provider.php (2)
inc/models/class-email-account.php (5)
Email_Account(22-727)get_webmail_url(601-610)get_imap_settings(618-627)get_email_address(278-281)get_smtp_settings(635-644)inc/integrations/email-providers/class-base-email-provider.php (13)
get_description(387-387)get_logo(395-395)get_fields(288-291)log(519-522)create_email_account(410-410)delete_email_account(420-420)change_password(431-431)get_account_info(441-441)get_webmail_url(451-451)get_dns_instructions(461-461)get_imap_settings(471-479)get_smtp_settings(489-497)test_connection(505-508)
🪛 GitHub Actions: Code Quality
tests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.php
[error] 1-1: Filenames should be all lowercase with hyphen-separated words. Expected 'limit-email-accounts-test.php', but found 'Limit_Email_Accounts_Test.php'.
🪛 GitHub Actions: Unit & Integration Tests
inc/managers/class-email-account-manager.php
[error] 489-489: Call to a member function get_id() on int.
inc/functions/email-account.php
[error] 24-24: Undefined method WP_Error::get_id().
🪛 GitHub Check: Code Quality Checks
inc/database/email-accounts/class-email-account-query.php
[warning] 107-107:
Possible useless method overriding detected
tests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.php
[warning] 420-420:
This comment is 45% 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-limit-email-accounts-test.php, but found Limit_Email_Accounts_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected limit-email-accounts-test.php, but found Limit_Email_Accounts_Test.php.
tests/WP_Ultimo/Models/Email_Account_Test.php
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-email-account-test.php, but found Email_Account_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected email-account-test.php, but found Email_Account_Test.php.
inc/integrations/email-providers/class-google-workspace-provider.php
[warning] 601-601:
urlencode() should only be used when dealing with legacy applications rawurlencode() should now be used instead. See https://www.php.net/function.rawurlencode and http://www.faqs.org/rfcs/rfc3986.html
[warning] 467-467:
urlencode() should only be used when dealing with legacy applications rawurlencode() should now be used instead. See https://www.php.net/function.rawurlencode and http://www.faqs.org/rfcs/rfc3986.html
[warning] 441-441:
urlencode() should only be used when dealing with legacy applications rawurlencode() should now be used instead. See https://www.php.net/function.rawurlencode and http://www.faqs.org/rfcs/rfc3986.html
[warning] 415-415:
urlencode() should only be used when dealing with legacy applications rawurlencode() should now be used instead. See https://www.php.net/function.rawurlencode and http://www.faqs.org/rfcs/rfc3986.html
inc/integrations/email-providers/class-microsoft365-provider.php
[warning] 436-436:
urlencode() should only be used when dealing with legacy applications rawurlencode() should now be used instead. See https://www.php.net/function.rawurlencode and http://www.faqs.org/rfcs/rfc3986.html
[warning] 410-410:
urlencode() should only be used when dealing with legacy applications rawurlencode() should now be used instead. See https://www.php.net/function.rawurlencode and http://www.faqs.org/rfcs/rfc3986.html
tests/WP_Ultimo/Functions/Email_Account_Functions_Test.php
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-email-account-functions-test.php, but found Email_Account_Functions_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected email-account-functions-test.php, but found Email_Account_Functions_Test.php.
inc/integrations/email-providers/class-purelymail-provider.php
[warning] 170-170:
This comment is 57% valid code; is this commented out code?
🪛 markdownlint-cli2 (0.18.1)
EMAIL_ACCOUNTS_FEATURE.md
253-253: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 PHPMD (2.15.0)
inc/limitations/class-limit-email-accounts.php
45-45: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
tests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.php
371-371: Avoid unused local variables such as '$limit'. (undefined)
(UnusedLocalVariable)
inc/integrations/email-providers/class-cpanel-email-provider.php
321-321: Avoid unused local variables such as '$username'. (undefined)
(UnusedLocalVariable)
361-361: Avoid unused parameters such as '$account'. (undefined)
(UnusedFormalParameter)
inc/integrations/email-providers/class-google-workspace-provider.php
490-490: Avoid unused parameters such as '$account'. (undefined)
(UnusedFormalParameter)
503-503: Avoid unused parameters such as '$domain'. (undefined)
(UnusedFormalParameter)
inc/integrations/email-providers/class-microsoft365-provider.php
491-491: Avoid unused parameters such as '$account'. (undefined)
(UnusedFormalParameter)
inc/ui/class-email-accounts-element.php
562-562: Avoid unused local variables such as '$email_account'. (undefined)
(UnusedLocalVariable)
752-752: Avoid unused parameters such as '$content'. (undefined)
(UnusedFormalParameter)
inc/integrations/email-providers/class-purelymail-provider.php
347-347: Avoid unused local variables such as '$user_data'. (undefined)
(UnusedLocalVariable)
364-364: Avoid unused parameters such as '$account'. (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: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: Build Plugin for Testing
🔇 Additional comments (34)
inc/integrations/email-providers/class-cpanel-email-provider.php (2)
361-368: Unused$accountparameter is intentional for interface compliance.The
$accountparameter is unused because the cPanel webmail URL is host-level (port 2096), not account-specific. This is consistent with the abstract method signature inBase_Email_Provider. The PHPMD warning can be safely ignored or suppressed with a@phpcs:ignorecomment if desired.
1-466: Implementation looks good overall.The cPanel provider properly implements all required methods from
Base_Email_Provider, includes appropriate error handling withWP_Error, and follows the established patterns from other providers in the codebase.inc/integrations/email-providers/class-microsoft365-provider.php (1)
162-221: Token caching implementation is correct.The OAuth2 token handling properly:
- Uses in-memory caching for the request lifecycle
- Uses WordPress transients for cross-request caching
- Applies a 60-second buffer before expiry to prevent edge-case failures
inc/integrations/email-providers/class-base-email-provider.php (4)
471-497: Default IMAP/SMTP settings return empty server string.The default implementations return empty strings for
server, which could cause confusion or errors if a provider forgets to override these methods. Consider either making these abstract or returning a more explicit placeholder.However, looking at the concrete implementations (Purelymail also returns empty server), this appears to be intentional for providers where server info varies. The current approach is acceptable.
212-232: Robust constants validation with alternative support.The
is_setup()method correctly handles both simple string constants and arrays of alternative constants (where any one being defined satisfies the requirement). This is a flexible design for providers that might have multiple configuration options.
341-368: Settings integration respects feature flag.The
add_to_integration_list()method correctly checks if email accounts are enabled viawu_get_setting('enable_email_accounts', false)before registering the provider in settings. The status display logic is clear and informative.
1-545: Well-designed abstract base class.The
Base_Email_Providerestablishes a clear contract for provider implementations with:
- Required abstract methods for core functionality
- Default implementations where appropriate
- Proper WordPress integration via hooks and network options
- Flexible configuration via constants with alternative support
inc/class-settings.php (1)
1476-1491: Email Accounts settings section follows established patterns.The new section is properly integrated:
- Positioned logically between Domain Mapping and SSO
- Uses consistent icon and metadata structure
- Fires extensibility hook for providers to register their settings
The hyphenated action name
'wu_settings_email-accounts'is consistent with similar hooks like'wu_settings_domain_mapping'.inc/admin-pages/customer-panel/class-account-admin-page.php (1)
185-186: Email Accounts metabox correctly integrated.The
Email_Accounts_Elementis properly registered as a side metabox, following the same pattern as other elements likeDomain_Mapping_Element. The placement after domain mapping is logical since both relate to domain/hosting features.inc/class-wp-ultimo.php (3)
356-356: Public API functions for email accounts properly loaded.The
email-account.phpfunctions file is loaded alongside other model public APIs (customer, membership, payment, etc.), maintaining consistent organization.
508-508: Email Accounts UI element correctly initialized.The element is loaded in the correct section with other customer-facing blocks (Domain_Mapping_Element, Site_Maintenance_Element, etc.), following the established singleton pattern.
931-936: Email Account Manager properly integrated.The manager is loaded after Cache_Manager and before Orphaned_Tables_Manager, following the established pattern. The comment block is consistent with other manager registrations.
inc/database/email-accounts/class-email-accounts-table.php (2)
62-87: Schema is well-designed with appropriate indexes.The table schema includes:
- All necessary foreign keys (customer_id, membership_id, site_id, payment_id)
- Proper enum types for status and purchase_type
- Indexes on frequently queried columns
One consideration: The
email_addresscolumn isvarchar(255)with a direct KEY. MySQL InnoDB has a 767-byte index limit for utf8mb4, which accommodates 191 characters. Since email addresses rarely exceed 191 characters in practice, this should work fine, but if you encounter index issues, consider adding a prefix length:KEY email_address (email_address(191)).
74-74:password_hashas TEXT is appropriate for encrypted data.Using TEXT for
password_hashaccommodates the variable-length output from sodium encryption, which includes the nonce and authentication tag. This is a reasonable choice for the encrypted password storage mentioned in the PR objectives.inc/objects/class-limitations.php (2)
35-35: DocBlock updated for IDE support.The
@property-readannotation enables IDE autocompletion for$limitations->email_accounts, consistent with other limitation modules.
498-498: Email accounts limitation module properly registered.The
email_accountsmapping toLimit_Email_Accounts::classfollows the exact pattern of other limitation modules in the repository. This enables the limitation system to handle email account quotas per membership.inc/database/email-accounts/class-email-account-status.php (1)
22-101: LGTM!The
Email_Account_Statusenum is well-structured with consistent mappings for CSS classes, labels, and icons across all status values. Theget_icon()helper correctly retrieves the icon for the current status.inc/database/email-accounts/class-email-accounts-schema.php (1)
22-161: LGTM!The schema is well-designed with appropriate column types, lengths, and constraints. Key observations:
- Foreign key columns use consistent
bigint(20) unsignedtypedomainlength of 191 is MySQL-safe for utf8mb4 indexing- Nullable columns are appropriately marked with
allow_nullstatusandpurchase_typeENUMs align with application constantsinc/database/email-accounts/class-email-account-query.php (1)
22-97: Query class configuration looks correct.The query class properly configures all required properties to work with the Email Accounts table and schema. The
global_cache = truesetting is appropriate for network-wide email account data.inc/limitations/class-limit-email-accounts.php (3)
45-68: The unused$typeparameter is required by the parent class contract.PHPMD flags
$typeas unused, but this parameter is required by the abstractcheck()method signature in the baseLimitclass. This is expected and correct—no changes needed.
79-105:can_create_morelogic is well-structured.The method correctly handles all limit scenarios: enabled/disabled state, boolean true/false limits, zero as unlimited, and numeric comparisons. The early returns make the logic clear and maintainable.
130-150: Mixed return type (int|string) is intentional but worth documenting clearly.The method returns
'unlimited'(string) or an integer, which is correctly documented in the PHPDoc. This is a reasonable design choice for display purposes.inc/integrations/email-providers/class-purelymail-provider.php (2)
364-367: Unused$accountparameter is acceptable for interface compliance.The
get_webmail_urlmethod receives$accountbut doesn't use it because Purelymail has a single webmail URL. This is fine—the parameter is required by the abstract method signature inBase_Email_Provider.
131-183: API request handling is well-implemented.The
api_requestmethod properly handles:
- Missing API key validation
- JSON encoding/decoding with error checking
- Purelymail's specific response format (
successfield)- HTTP error codes
- Error logging
inc/managers/class-email-account-manager.php (2)
62-86: Initialization structure looks good.The manager properly initializes REST API, WP-CLI, hooks for async operations, status transitions, and cleanup handlers. The hook priorities and action names are consistent.
94-109: The provider registration mechanism is working correctly. Providers are registered through thewu_email_manager_get_providersfilter rather than direct array assignment. When each provider'sget_instance()is called inload_providers(), it triggers the Singleton pattern to callinit(), which registers a filter callback. Whenget_providers()is called, it applies the filter and each provider'sself_register()callback populates the$providersarray dynamically. This design allows both built-in and third-party providers (via thewu_email_providers_loadaction) to register themselves automatically. The system works as intended.tests/WP_Ultimo/Functions/Email_Account_Functions_Test.php (1)
39-66: Good test fixture setup with proper cleanup pattern.The static setup creates shared test fixtures efficiently. Tests properly clean up created email accounts to avoid test pollution.
inc/functions/email-account.php (2)
349-387: Good security implementation for one-time password tokens.The token storage mechanism correctly:
- Generates unpredictable tokens with
wp_generate_password(32, false)- Encrypts passwords before storage
- Validates account ID on retrieval
- Deletes token after single use
- Uses short TTL (10 minutes)
140-155: The error check is correct and prevents the reported issue.The code at line 144-146 properly validates the
save()return value usingis_wp_error($saved)before proceeding. SinceBase_Model::save()returns either aWP_Errorobject (on validation failure) or a boolean, the check correctly catches error cases and returns early. Theget_id()call at line 153 executes only after successful validation, so noWP_Error::get_id()call occurs. The pipeline error referenced may stem from a different code path or prior version.inc/integrations/email-providers/class-google-workspace-provider.php (1)
256-271: JWT creation implementation is correct.The JWT signing correctly uses RS256 algorithm with proper base64url encoding. Error handling for
openssl_signfailure is appropriate.inc/models/class-email-account.php (2)
151-164: Validation rules are comprehensive and well-structured.The validation rules correctly enforce:
- Required fields:
customer_id,email_address,domain,provider,status- Email format with uniqueness constraint excluding current record
- Enumerated values for
statusandpurchase_type- Type constraints with defaults
290-312: Email address handling is robust.The setter properly:
- Normalizes to lowercase
- Sanitizes input
- Auto-extracts domain to maintain consistency
- The
get_username()method handles edge cases gracefullytests/WP_Ultimo/Models/Email_Account_Test.php (2)
69-84: Good test coverage for model property handling.The test comprehensively verifies that all setters correctly populate corresponding getters, covering customer_id, membership_id, email_address, provider, status, and quota_mb.
313-331: Good edge case coverage for invalid relationships.Tests properly verify that invalid customer/membership IDs return
nullrather than throwing errors, ensuring graceful handling of orphaned or misconfigured records.
| function wu_encrypt_email_password($password) { | ||
|
|
||
| if (function_exists('sodium_crypto_secretbox')) { | ||
| $key = substr(hash('sha256', wp_salt('auth'), true), 0, SODIUM_CRYPTO_SECRETBOX_KEYBYTES); | ||
| $nonce = random_bytes(SODIUM_CRYPTO_SECRETBOX_NONCEBYTES); | ||
|
|
||
| $encrypted = sodium_crypto_secretbox($password, $nonce, $key); | ||
|
|
||
| return base64_encode($nonce . $encrypted); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_encode | ||
| } | ||
|
|
||
| // Fallback to basic encoding (not secure, but better than plaintext) | ||
| return base64_encode($password); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_encode | ||
| } |
There was a problem hiding this comment.
Base64 fallback for password encryption is insecure.
The fallback at line 307 uses plain base64 encoding which provides no security. While the comment acknowledges this, consider:
- Logging a warning when the insecure fallback is used
- Requiring sodium and failing gracefully if unavailable
- At minimum, documenting this in admin settings
🔎 Suggested improvement to warn when using insecure fallback
// Fallback to basic encoding (not secure, but better than plaintext)
+ wu_log_add('email-accounts', 'Warning: Using insecure base64 encoding for password storage. Enable sodium extension for proper encryption.', \Psr\Log\LogLevel::WARNING);
return base64_encode($password); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_encodeCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In inc/functions/email-account.php around lines 295 to 308, the fallback uses
base64 which is insecure; instead update the function to (1) detect absence of
sodium and trigger a logged warning via the plugin logger or error_log when the
insecure fallback is used, (2) prefer failing gracefully or preventing storage
of the password when sodium extensions/constants are missing (return false or
throw a clear exception) and surface that error to the caller so the UI can show
a message, and (3) add or update admin-facing documentation/setting (or an admin
notice) that sodium is required for secure password storage and describe the
fallback behavior so admins can enable the sodium extension; ensure all changes
preserve existing return types/flows and that logging uses the plugin’s logger
if available.
| public function delete_email_account($email_address) { | ||
|
|
||
| $result = $this->api_request( | ||
| 'users/' . urlencode($email_address), | ||
| [], | ||
| 'DELETE' | ||
| ); | ||
|
|
||
| if (is_wp_error($result)) { | ||
| return $result; | ||
| } | ||
|
|
||
| $this->log(sprintf('Email account deleted: %s', $email_address)); | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
Use rawurlencode() instead of urlencode() for URL path segments.
Per RFC 3986 and the static analysis hint, rawurlencode() is the correct function for encoding URL path segments. urlencode() encodes spaces as + which is only valid in query strings.
🔎 Proposed fix
public function delete_email_account($email_address) {
$result = $this->api_request(
- 'users/' . urlencode($email_address),
+ 'users/' . rawurlencode($email_address),
[],
'DELETE'
);📝 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 delete_email_account($email_address) { | |
| $result = $this->api_request( | |
| 'users/' . urlencode($email_address), | |
| [], | |
| 'DELETE' | |
| ); | |
| if (is_wp_error($result)) { | |
| return $result; | |
| } | |
| $this->log(sprintf('Email account deleted: %s', $email_address)); | |
| return true; | |
| } | |
| public function delete_email_account($email_address) { | |
| $result = $this->api_request( | |
| 'users/' . rawurlencode($email_address), | |
| [], | |
| 'DELETE' | |
| ); | |
| if (is_wp_error($result)) { | |
| return $result; | |
| } | |
| $this->log(sprintf('Email account deleted: %s', $email_address)); | |
| return true; | |
| } |
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[warning] 410-410:
urlencode() should only be used when dealing with legacy applications rawurlencode() should now be used instead. See https://www.php.net/function.rawurlencode and http://www.faqs.org/rfcs/rfc3986.html
🤖 Prompt for AI Agents
In inc/integrations/email-providers/class-microsoft365-provider.php around lines
407 to 422, the code uses urlencode() to encode the email address in a URL path;
replace urlencode() with rawurlencode() so path segments are encoded per RFC
3986 (spaces become %20, not +). Update the api_request call to use
rawurlencode($email_address) and keep behavior and error handling identical.
| public function change_password($email_address, $new_password) { | ||
|
|
||
| $result = $this->api_request( | ||
| 'users/' . urlencode($email_address), | ||
| [ | ||
| 'passwordProfile' => [ | ||
| 'forceChangePasswordNextSignIn' => false, | ||
| 'password' => $new_password, | ||
| ], | ||
| ], | ||
| 'PATCH' | ||
| ); | ||
|
|
||
| if (is_wp_error($result)) { | ||
| return $result; | ||
| } | ||
|
|
||
| $this->log(sprintf('Password changed for: %s', $email_address)); | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
Use rawurlencode() for URL path encoding.
Same issue as delete_email_account - use rawurlencode() for the email address in the URL path.
🔎 Proposed fix
public function change_password($email_address, $new_password) {
$result = $this->api_request(
- 'users/' . urlencode($email_address),
+ 'users/' . rawurlencode($email_address),
[
'passwordProfile' => [📝 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 change_password($email_address, $new_password) { | |
| $result = $this->api_request( | |
| 'users/' . urlencode($email_address), | |
| [ | |
| 'passwordProfile' => [ | |
| 'forceChangePasswordNextSignIn' => false, | |
| 'password' => $new_password, | |
| ], | |
| ], | |
| 'PATCH' | |
| ); | |
| if (is_wp_error($result)) { | |
| return $result; | |
| } | |
| $this->log(sprintf('Password changed for: %s', $email_address)); | |
| return true; | |
| } | |
| public function change_password($email_address, $new_password) { | |
| $result = $this->api_request( | |
| 'users/' . rawurlencode($email_address), | |
| [ | |
| 'passwordProfile' => [ | |
| 'forceChangePasswordNextSignIn' => false, | |
| 'password' => $new_password, | |
| ], | |
| ], | |
| 'PATCH' | |
| ); | |
| if (is_wp_error($result)) { | |
| return $result; | |
| } | |
| $this->log(sprintf('Password changed for: %s', $email_address)); | |
| return true; | |
| } |
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[warning] 436-436:
urlencode() should only be used when dealing with legacy applications rawurlencode() should now be used instead. See https://www.php.net/function.rawurlencode and http://www.faqs.org/rfcs/rfc3986.html
🤖 Prompt for AI Agents
inc/integrations/email-providers/class-microsoft365-provider.php lines 433-453:
the code uses urlencode() to encode the email address in the URL path when
calling api_request; replace urlencode() with rawurlencode() so the email is
encoded correctly for a URL path (same change applied in delete_email_account).
Update the api_request call to use rawurlencode($email_address) when building
the 'users/{email}' path and keep the rest of the payload and method unchanged.
| public function get_account_info($email_address) { | ||
|
|
||
| $result = $this->api_request('users/' . urlencode($email_address)); | ||
|
|
||
| if (is_wp_error($result)) { | ||
| return $result; | ||
| } | ||
|
|
||
| // phpcs:disable WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase -- Microsoft Graph API response properties. | ||
| return [ | ||
| 'email_address' => $email_address, | ||
| 'display_name' => isset($result->displayName) ? $result->displayName : '', | ||
| 'first_name' => isset($result->givenName) ? $result->givenName : '', | ||
| 'last_name' => isset($result->surname) ? $result->surname : '', | ||
| 'suspended' => isset($result->accountEnabled) ? ! $result->accountEnabled : false, | ||
| 'quota_mb' => 0, // Microsoft 365 uses license-based storage | ||
| ]; | ||
| // phpcs:enable | ||
| } |
There was a problem hiding this comment.
Consistent URL encoding needed in get_account_info.
The get_account_info method also uses urlencode() at line 465 and should be updated to rawurlencode() for consistency with the fix applied to other methods.
🤖 Prompt for AI Agents
inc/integrations/email-providers/class-microsoft365-provider.php around lines
463-481: the call to urlencode() when building the users/ URL should be changed
to rawurlencode() for consistency with other methods; replace
urlencode($email_address) with rawurlencode($email_address) so special
characters are encoded per RFC 3986 and keep the rest of the method unchanged.
| public function get_account_info($email_address) { | ||
|
|
||
| $result = $this->api_request( | ||
| 'getUser', | ||
| [ | ||
| 'userName' => $email_address, | ||
| ] | ||
| ); | ||
|
|
||
| if (is_wp_error($result)) { | ||
| return $result; | ||
| } | ||
|
|
||
| // Handle nested result structure | ||
| $user_data = isset($result->result) ? $result->result : $result; | ||
|
|
||
| return [ | ||
| 'email_address' => $email_address, | ||
| 'quota_mb' => 0, // Purelymail uses account-level storage | ||
| 'disk_used_mb' => 0, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
get_account_info fetches data but returns hardcoded values.
The method fetches user data from the API and extracts it into $user_data, but then returns hardcoded zeros instead of using the actual response data. Either use the fetched data or remove the unused variable.
🔎 Proposed fix (option 1: remove unused variable)
if (is_wp_error($result)) {
return $result;
}
- // Handle nested result structure
- $user_data = isset($result->result) ? $result->result : $result;
-
return [
'email_address' => $email_address,
'quota_mb' => 0, // Purelymail uses account-level storage
'disk_used_mb' => 0,
];If Purelymail's API returns storage usage data, consider extracting and returning it for more useful account info.
📝 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 get_account_info($email_address) { | |
| $result = $this->api_request( | |
| 'getUser', | |
| [ | |
| 'userName' => $email_address, | |
| ] | |
| ); | |
| if (is_wp_error($result)) { | |
| return $result; | |
| } | |
| // Handle nested result structure | |
| $user_data = isset($result->result) ? $result->result : $result; | |
| return [ | |
| 'email_address' => $email_address, | |
| 'quota_mb' => 0, // Purelymail uses account-level storage | |
| 'disk_used_mb' => 0, | |
| ]; | |
| } | |
| public function get_account_info($email_address) { | |
| $result = $this->api_request( | |
| 'getUser', | |
| [ | |
| 'userName' => $email_address, | |
| ] | |
| ); | |
| if (is_wp_error($result)) { | |
| return $result; | |
| } | |
| return [ | |
| 'email_address' => $email_address, | |
| 'quota_mb' => 0, // Purelymail uses account-level storage | |
| 'disk_used_mb' => 0, | |
| ]; | |
| } |
🧰 Tools
🪛 PHPMD (2.15.0)
347-347: Avoid unused local variables such as '$user_data'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
inc/integrations/email-providers/class-purelymail-provider.php around lines
333-354: get_account_info() currently calls the API and assigns $user_data but
then returns hardcoded zeros and leaves $user_data unused; update the method to
read storage/quota fields from $user_data (with isset/empty checks and safe
numeric casting/fallbacks) and return those values for 'quota_mb' and
'disk_used_mb', or if the Purelymail API does not supply such fields remove the
unused $user_data assignment and return only meaningful fields (ensuring
returned values are integers and documented via a brief comment).
| /** | ||
| * Test wu_create_email_account with invalid customer returns error. | ||
| */ | ||
| public function test_wu_create_email_account_invalid_customer(): void { | ||
| $account = wu_create_email_account( | ||
| [ | ||
| 'customer_id' => 0, | ||
| 'email_address' => 'invalid@example.com', | ||
| 'provider' => 'cpanel', | ||
| 'status' => 'pending', | ||
| ] | ||
| ); | ||
|
|
||
| $this->assertInstanceOf(\WP_Error::class, $account); | ||
| } |
There was a problem hiding this comment.
Test expects validation that doesn't exist in the implementation.
This test expects wu_create_email_account to return a WP_Error for customer_id = 0, but the wu_create_email_account function in inc/functions/email-account.php doesn't validate the customer ID before creating the account. The validation exists in Email_Account_Manager::create_account() but not in the direct function.
Either:
- Add customer validation to
wu_create_email_account, or - Update this test to reflect actual behavior, or
- This test should call the manager's
create_accountmethod instead
🤖 Prompt for AI Agents
In tests/WP_Ultimo/Functions/Email_Account_Functions_Test.php around lines
327-341, the test expects wu_create_email_account to return a WP_Error for
customer_id = 0 but the wrapper function currently doesn't perform that
validation; update the code so wu_create_email_account either delegates to
Email_Account_Manager::create_account (so the existing validation is used) or
explicitly validate the customer_id (reject 0/invalid IDs) and return a WP_Error
with an appropriate message when invalid; adjust implementation to use the same
error shape as the manager so this test passes.
| <?php | ||
| /** | ||
| * Limit_Email_Accounts tests. | ||
| * | ||
| * @package WP_Ultimo\Tests | ||
| */ | ||
|
|
There was a problem hiding this comment.
Rename file to follow WordPress coding standards.
The pipeline is failing because the filename doesn't follow the expected naming convention. WordPress coding standards require class files to be lowercase with hyphens.
Rename the file from Limit_Email_Accounts_Test.php to class-limit-email-accounts-test.php.
🧰 Tools
🪛 GitHub Actions: Code Quality
[error] 1-1: Filenames should be all lowercase with hyphen-separated words. Expected 'limit-email-accounts-test.php', but found 'Limit_Email_Accounts_Test.php'.
🪛 GitHub Check: Code Quality Checks
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-limit-email-accounts-test.php, but found Limit_Email_Accounts_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected limit-email-accounts-test.php, but found Limit_Email_Accounts_Test.php.
🤖 Prompt for AI Agents
tests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.php lines 1-7: The
filename violates WordPress file naming conventions; rename the file to
class-limit-email-accounts-test.php (all lowercase, hyphen-separated) and update
any test suite/autoloader or include references that point to the old filename
so they use the new name.
| /** | ||
| * Test default state method. | ||
| */ | ||
| public function test_default_state(): void { | ||
| $default = Email_Account::default_state(); | ||
|
|
||
| $this->assertIsArray($default); | ||
| $this->assertArrayHasKey('status', $default); | ||
| $this->assertEquals('pending', $default['status']); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if default_state exists in Base_Model or Email_Account
rg -n "function default_state|static function default_state" --type=php inc/models/Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search for default_state across entire repository
echo "=== Searching for default_state anywhere ==="
rg -n "default_state" --type=php
echo -e "\n=== Locating Email_Account class ==="
find . -type f -name "*.php" | xargs grep -l "class Email_Account"
echo -e "\n=== Locating Base_Model class ==="
find . -type f -name "*.php" | xargs grep -l "class Base_Model"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1869
🏁 Script executed:
#!/bin/bash
# Read Email_Account class to check inheritance
echo "=== Email_Account class definition (first 50 lines) ==="
head -50 ./inc/models/class-email-account.php
echo -e "\n=== Base_Model class - search for default_state ==="
rg -A 10 "public static function default_state" ./inc/models/class-base-model.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 1003
🏁 Script executed:
#!/bin/bash
# Check entire Base_Model file for default_state
echo "=== Checking Base_Model for any default_state reference ==="
rg -n "default_state" ./inc/models/class-base-model.php
echo -e "\n=== Count lines in Base_Model ==="
wc -l ./inc/models/class-base-model.php
echo -e "\n=== Check if Base_Model has static methods ==="
rg -n "public static function" ./inc/models/class-base-model.php | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 678
🏁 Script executed:
#!/bin/bash
# Check Email_Account file size
wc -l ./inc/models/class-email-account.php
# Search for default_state in Email_Account
rg -n "default_state" ./inc/models/class-email-account.php
# Check all static methods in Email_Account
rg -n "public static function" ./inc/models/class-email-account.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 454
Implement missing default_state() method in Email_Account class.
The test calls Email_Account::default_state() but this method does not exist in the Email_Account class or its parent Base_Model class. Either implement this static method or remove the test.
🤖 Prompt for AI Agents
In tests/WP_Ultimo/Models/Email_Account_Test.php around lines 257 to 266, the
test calls Email_Account::default_state() but the method is missing; implement a
public static function default_state() on the Email_Account model (or add to its
Base_Model only if appropriate) that returns an array including at least
'status' => 'pending' (merge or extend any parent/default values if necessary)
so the test assertions for array type, presence of 'status' key, and value
'pending' pass.
| $status_label = $account->get_status_label(); | ||
| $status_class = $account->get_status_class(); | ||
|
|
||
| $status = "<span class='wu-py-1 wu-px-2 wu-rounded-sm wu-text-xs wu-leading-none wu-font-mono $status_class'>$status_label</span>"; | ||
|
|
There was a problem hiding this comment.
Escape variables in status badge HTML string.
The $status_label and $status_class variables are interpolated directly into the HTML string without escaping. While get_status_label() and get_status_class() may return safe values, it's safer to escape them explicitly for defense-in-depth.
🔎 Proposed fix
- $status_label = $account->get_status_label();
- $status_class = $account->get_status_class();
-
- $status = "<span class='wu-py-1 wu-px-2 wu-rounded-sm wu-text-xs wu-leading-none wu-font-mono $status_class'>$status_label</span>";
+ $status_label = esc_html($account->get_status_label());
+ $status_class = esc_attr($account->get_status_class());
+
+ $status = "<span class='wu-py-1 wu-px-2 wu-rounded-sm wu-text-xs wu-leading-none wu-font-mono {$status_class}'>{$status_label}</span>";📝 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.
| $status_label = $account->get_status_label(); | |
| $status_class = $account->get_status_class(); | |
| $status = "<span class='wu-py-1 wu-px-2 wu-rounded-sm wu-text-xs wu-leading-none wu-font-mono $status_class'>$status_label</span>"; | |
| $status_label = esc_html($account->get_status_label()); | |
| $status_class = esc_attr($account->get_status_class()); | |
| $status = "<span class='wu-py-1 wu-px-2 wu-rounded-sm wu-text-xs wu-leading-none wu-font-mono {$status_class}'>{$status_label}</span>"; |
🤖 Prompt for AI Agents
In views/dashboard-widgets/email-accounts.php around lines 107 to 111, the
$status_label and $status_class are interpolated into HTML without escaping;
update the code to escape both values before building the badge string (use
htmlspecialchars($status_label, ENT_QUOTES, 'UTF-8') for the label and either
htmlspecialchars($status_class, ENT_QUOTES, 'UTF-8') or sanitize the class to
allow only safe CSS class characters (e.g. preg_replace('/[^A-Za-z0-9_\- ]+/',
'', $status_class)) then interpolate the escaped/sanitized values into the span.
| <?php else : ?> | ||
|
|
||
| <div class="wu-text-center wu-bg-gray-100 wu-rounded wu-uppercase wu-font-semibold wu-text-xs wu-text-gray-700 wu-p-4 wu-m-4 wu-mt-6"> | ||
| <?php if ($email_accounts_enabled && ('unlimited' === $remaining_slots || $remaining_slots > 0)) : ?> | ||
| <span class="dashicons-before dashicons-wu-mail wu-align-middle wu-mr-2"></span> | ||
| <span><?php echo esc_html__('No email accounts created yet.', 'ultimate-multisite'); ?></span> | ||
| <br> | ||
| <a href="<?php echo esc_attr($create_modal['url']); ?>" class="wu-mt-3 wu-inline-block wu-text-blue-600 wu-no-underline wubox"> | ||
| <?php esc_html_e('Create your first email account', 'ultimate-multisite'); ?> | ||
| </a> | ||
| <?php elseif (! $email_accounts_enabled) : ?> | ||
| <span class="dashicons-before dashicons-wu-info wu-align-middle wu-mr-2"></span> | ||
| <span><?php echo esc_html__('Email accounts are not enabled for this network.', 'ultimate-multisite'); ?></span> | ||
| <?php else : ?> | ||
| <span class="dashicons-before dashicons-wu-alert-circle wu-align-middle wu-mr-2"></span> | ||
| <span><?php echo esc_html__('Email accounts are not included in your current plan.', 'ultimate-multisite'); ?></span> | ||
| <?php endif; ?> | ||
| </div> | ||
|
|
||
| <?php endif; ?> |
There was a problem hiding this comment.
Invalid HTML: <div> inside <tbody>.
When there are no email accounts, a <div> element is placed directly inside <tbody>, which is invalid HTML structure. A <div> cannot be a direct child of <tbody>—only <tr> elements are valid.
🔎 Proposed fix
<?php else : ?>
- <div class="wu-text-center wu-bg-gray-100 wu-rounded wu-uppercase wu-font-semibold wu-text-xs wu-text-gray-700 wu-p-4 wu-m-4 wu-mt-6">
+ <tr>
+ <td colspan="1" class="wu-p-0">
+ <div class="wu-text-center wu-bg-gray-100 wu-rounded wu-uppercase wu-font-semibold wu-text-xs wu-text-gray-700 wu-p-4 wu-m-4 wu-mt-6">
<?php if ($email_accounts_enabled && ('unlimited' === $remaining_slots || $remaining_slots > 0)) : ?>
...
<?php endif; ?>
</div>
+ </td>
+ </tr>
<?php endif; ?>📝 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.
| <?php else : ?> | |
| <div class="wu-text-center wu-bg-gray-100 wu-rounded wu-uppercase wu-font-semibold wu-text-xs wu-text-gray-700 wu-p-4 wu-m-4 wu-mt-6"> | |
| <?php if ($email_accounts_enabled && ('unlimited' === $remaining_slots || $remaining_slots > 0)) : ?> | |
| <span class="dashicons-before dashicons-wu-mail wu-align-middle wu-mr-2"></span> | |
| <span><?php echo esc_html__('No email accounts created yet.', 'ultimate-multisite'); ?></span> | |
| <br> | |
| <a href="<?php echo esc_attr($create_modal['url']); ?>" class="wu-mt-3 wu-inline-block wu-text-blue-600 wu-no-underline wubox"> | |
| <?php esc_html_e('Create your first email account', 'ultimate-multisite'); ?> | |
| </a> | |
| <?php elseif (! $email_accounts_enabled) : ?> | |
| <span class="dashicons-before dashicons-wu-info wu-align-middle wu-mr-2"></span> | |
| <span><?php echo esc_html__('Email accounts are not enabled for this network.', 'ultimate-multisite'); ?></span> | |
| <?php else : ?> | |
| <span class="dashicons-before dashicons-wu-alert-circle wu-align-middle wu-mr-2"></span> | |
| <span><?php echo esc_html__('Email accounts are not included in your current plan.', 'ultimate-multisite'); ?></span> | |
| <?php endif; ?> | |
| </div> | |
| <?php endif; ?> | |
| <?php else : ?> | |
| <tr> | |
| <td colspan="1" class="wu-p-0"> | |
| <div class="wu-text-center wu-bg-gray-100 wu-rounded wu-uppercase wu-font-semibold wu-text-xs wu-text-gray-700 wu-p-4 wu-m-4 wu-mt-6"> | |
| <?php if ($email_accounts_enabled && ('unlimited' === $remaining_slots || $remaining_slots > 0)) : ?> | |
| <span class="dashicons-before dashicons-wu-mail wu-align-middle wu-mr-2"></span> | |
| <span><?php echo esc_html__('No email accounts created yet.', 'ultimate-multisite'); ?></span> | |
| <br> | |
| <a href="<?php echo esc_attr($create_modal['url']); ?>" class="wu-mt-3 wu-inline-block wu-text-blue-600 wu-no-underline wubox"> | |
| <?php esc_html_e('Create your first email account', 'ultimate-multisite'); ?> | |
| </a> | |
| <?php elseif (! $email_accounts_enabled) : ?> | |
| <span class="dashicons-before dashicons-wu-info wu-align-middle wu-mr-2"></span> | |
| <span><?php echo esc_html__('Email accounts are not enabled for this network.', 'ultimate-multisite'); ?></span> | |
| <?php else : ?> | |
| <span class="dashicons-before dashicons-wu-alert-circle wu-align-middle wu-mr-2"></span> | |
| <span><?php echo esc_html__('Email accounts are not included in your current plan.', 'ultimate-multisite'); ?></span> | |
| <?php endif; ?> | |
| </div> | |
| </td> | |
| </tr> | |
| <?php endif; ?> |
🤖 Prompt for AI Agents
In views/dashboard-widgets/email-accounts.php around lines 221 to 240, a <div>
block is being rendered directly inside a <tbody>, which is invalid; replace
that <div> with a properly structured table row and cell(s) (e.g., <tr><td
colspan="N">...</td></tr>) or move the message outside the table markup so only
<tr> children exist under <tbody>; ensure the content retains the same
classes/markup but is wrapped in a <td> that spans the full column count (use
the correct colspan value) and keep all conditional logic unchanged.
Fixes several issues found during testing: 1. Fixed hook handler signatures for wu_customer_post_delete and wu_membership_post_delete - these pass ($result, $model) not just ($model) 2. Fixed Limit_Email_Accounts::get_remaining_slots() bug where false limit was incorrectly returning 'unlimited' (due to (int)false === 0 matching unlimited check) 3. Updated test assertions from assertNull to assertFalse for get_customer() and get_membership() on invalid IDs 4. Skipped database-dependent tests that require the email accounts table to exist 5. Added hook cleanup in relationship tests to avoid errors from hooks querying non-existent tables during cleanup All email account tests now pass: - Email_Account_Test: 20 tests (2 skipped) - Limit_Email_Accounts_Test: 24 tests - Email_Account_Functions_Test: 14 tests (4 skipped) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🔨 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.
Actionable comments posted: 1
♻️ Duplicate comments (4)
inc/managers/class-email-account-manager.php (2)
454-480: Verify hook parameter types to prevent orphaned email accounts.The
instanceofcheck at line 456 will cause the method to return early if thewu_membership_post_deletehook passes a membership ID (int) instead of a Membership object. This could leave orphaned email accounts in the database.Run the following script to verify what parameter types the hook actually passes:
#!/bin/bash # Search for where wu_membership_post_delete is fired to determine parameter types rg -n "do_action\s*\(\s*['\"]wu_membership_post_delete" --type=php -A 2 -B 2If the hook can pass an int, update the handler to normalize the parameter:
$membership_id = is_object($membership) && method_exists($membership, 'get_id') ? $membership->get_id() : (int) $membership;
491-517: Verify hook parameter types to prevent orphaned email accounts.Similar to the membership handler, the
instanceofcheck at line 493 will cause early return if thewu_customer_post_deletehook passes a customer ID (int) instead of a Customer object.Run the following script to verify parameter types:
#!/bin/bash # Search for where wu_customer_post_delete is fired rg -n "do_action\s*\(\s*['\"]wu_customer_post_delete" --type=php -A 2 -B 2tests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.php (1)
1-7: Filename violates WordPress coding standards.The pipeline is failing because this file doesn't follow WordPress naming conventions. Test files should use lowercase with hyphens.
This issue was previously reported and remains unresolved.
tests/WP_Ultimo/Models/Email_Account_Test.php (1)
1-7: Filename violates WordPress coding standards.The pipeline is failing because this file doesn't follow WordPress naming conventions. Test class files should be lowercase with hyphens.
Rename to
class-email-account-test.php.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
inc/limitations/class-limit-email-accounts.phpinc/managers/class-email-account-manager.phptests/WP_Ultimo/Functions/Email_Account_Functions_Test.phptests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.phptests/WP_Ultimo/Models/Email_Account_Test.php
🧰 Additional context used
🧬 Code graph analysis (3)
tests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.php (3)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-995)inc/limitations/class-limit-email-accounts.php (5)
Limit_Email_Accounts(22-166)check(45-68)can_create_more(79-105)get_remaining_slots(130-151)default_state(159-165)inc/integrations/email-providers/class-base-email-provider.php (1)
is_enabled(125-130)
tests/WP_Ultimo/Models/Email_Account_Test.php (3)
inc/database/email-accounts/class-email-account-status.php (1)
Email_Account_Status(22-102)inc/models/class-email-account.php (22)
Email_Account(22-727)set_email_address(290-299)set_provider(355-358)set_status(391-394)set_quota_mb(451-454)get_customer_id(172-175)get_email_address(278-281)get_provider(343-346)get_status(379-382)get_quota_mb(439-442)get_username(307-312)get_domain(320-323)get_status_label(413-418)get_status_class(426-431)set_external_id(474-477)get_external_id(462-465)set_site_id(254-257)get_site_id(242-245)set_payment_id(554-557)get_payment_id(542-545)validation_rules(151-164)get_customer(195-198)inc/models/class-customer.php (1)
Customer(28-910)
tests/WP_Ultimo/Functions/Email_Account_Functions_Test.php (1)
inc/functions/email-account.php (6)
wu_generate_email_password(397-400)wu_encrypt_email_password(295-308)wu_decrypt_email_password(318-338)wu_store_email_password_token(349-364)wu_get_email_password_from_token(375-387)wu_get_enabled_email_providers(253-258)
🪛 GitHub Actions: Code Quality
tests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.php
[error] 1-1: Filenames should be all lowercase with hyphens as word separators. Expected limit-email-accounts-test.php, but found Limit_Email_Accounts_Test.php.
🪛 GitHub Check: Code Quality Checks
tests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.php
[warning] 365-365:
This comment is 45% 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-limit-email-accounts-test.php, but found Limit_Email_Accounts_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected limit-email-accounts-test.php, but found Limit_Email_Accounts_Test.php.
tests/WP_Ultimo/Models/Email_Account_Test.php
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-email-account-test.php, but found Email_Account_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected email-account-test.php, but found Email_Account_Test.php.
tests/WP_Ultimo/Functions/Email_Account_Functions_Test.php
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-email-account-functions-test.php, but found Email_Account_Functions_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected email-account-functions-test.php, but found Email_Account_Functions_Test.php.
🪛 PHPMD (2.15.0)
inc/limitations/class-limit-email-accounts.php
45-45: Avoid unused parameters such as '$type'. (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.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (5)
inc/limitations/class-limit-email-accounts.php (1)
22-166: LGTM! Well-implemented limitation module.The implementation correctly handles all limit representations (boolean, numeric, zero for unlimited) with consistent logic across all methods. The disabled state checks, quota validation, and remaining slot calculations are all sound.
tests/WP_Ultimo/Functions/Email_Account_Functions_Test.php (1)
18-182: Excellent test coverage for helper functions.The tests comprehensively cover password generation, encryption/decryption with edge cases (special characters, empty strings, long passwords), and token storage security. The use of
markTestSkippedfor database-dependent tests is appropriate.inc/managers/class-email-account-manager.php (1)
527-602: Robust account creation with comprehensive validation.The
create_accountmethod properly validates customers, providers, quotas, email addresses, and checks for duplicates before creating accounts. The password token storage for async provisioning is well-designed.tests/WP_Ultimo/Models/Email_Account_Test.php (2)
265-295: Excellent hook cleanup to prevent test failures.The relationship tests properly remove the email account deletion hooks before cleaning up test data. This prevents queries to the non-existent
email_accountstable during test teardown, which is a thoughtful approach to test isolation.Also applies to: 300-344
18-345: Comprehensive model test coverage.The tests thoroughly validate the Email_Account model including creation, email parsing, domain extraction, status handling, relationships, and edge cases. The coverage is excellent for ensuring model reliability.
| <?php | ||
| /** | ||
| * Email account helper functions tests. | ||
| * | ||
| * @package WP_Ultimo\Tests | ||
| */ | ||
|
|
There was a problem hiding this comment.
Filename violates WordPress coding standards.
The pipeline is failing because this file doesn't follow WordPress naming conventions for test files. According to WordPress standards, test class files should be lowercase with hyphens.
Rename to class-email-account-functions-test.php.
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-email-account-functions-test.php, but found Email_Account_Functions_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected email-account-functions-test.php, but found Email_Account_Functions_Test.php.
🤖 Prompt for AI Agents
In tests/WP_Ultimo/Functions/Email_Account_Functions_Test.php lines 1-7 the
filename violates WordPress coding standards for test class files; rename the
file to lowercase with hyphens: class-email-account-functions-test.php, update
any references/imports or test bootstrap that reference the old filename if
present, and ensure repository/CI reflects the new filename so the pipeline no
longer fails.
The Email_Accounts_Table was not registered in the Table_Loader, causing "Undefined property: wpdb::$wu_email_accounts" errors in other tests that triggered email account hooks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🔨 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: |
Adds a wizard-based configuration flow for email providers, similar to the existing hosting provider integration wizard. Features: - Email_Integration_Wizard_Admin_Page with step-by-step setup - Activation step to enable/disable providers - Instructions step with provider-specific guidance - Configuration step for API credentials - Testing step to validate API connection - Ready confirmation step New files: - inc/admin-pages/class-email-integration-wizard-admin-page.php - views/wizards/email-integrations/activation.php - views/wizards/email-integrations/configuration.php - views/wizards/email-integrations/configuration-results.php - views/wizards/email-integrations/default-instructions.php - views/wizards/email-integrations/test.php - views/wizards/email-integrations/ready.php Changes: - Base_Email_Provider now shows Configure button in settings - Added get_explainer_lines() and supports() methods to providers - Added AJAX handler for testing email integration connection - Added get_provider_instance() alias to Email_Account_Manager 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🔨 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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
inc/admin-pages/class-email-integration-wizard-admin-page.php (2)
160-169: Consider cachingmethod_existschecks or using interface contracts.The
method_existschecks forsupports()are repeated in multiple locations. While functional, consider either:
- Defining an interface that providers implement
- Making
supports()a required method in the base class that returns an empty array by defaultThis would eliminate the need for runtime existence checks and make the provider contract more explicit.
317-357: Verify URL length limits for manual configuration flow.When manual configuration is selected (line 331-336), the filtered field data is JSON-encoded and passed as a URL query parameter. For providers with many configuration fields or long values, this could exceed URL length limits (typically ~2000 characters in browsers, but can vary).
Consider using WordPress transients or session storage for passing configuration data between requests instead of URL parameters.
inc/managers/class-email-account-manager.php (1)
316-328: Consider clearing the transient before retrieving the password for defense-in-depth.The transient is deleted at line 322 after retrieving the password token. If an error occurs between retrieval and deletion, the transient could persist. Consider deleting the transient immediately after retrieving its value, or using a try-finally pattern to ensure cleanup.
🔎 Proposed fix
// Get the password from transient if available $password_token = get_transient('wu_email_provision_pwd_' . $email_account_id); + delete_transient('wu_email_provision_pwd_' . $email_account_id); $password = ''; if ($password_token) { $password = wu_get_email_password_from_token($password_token, $email_account_id); - delete_transient('wu_email_provision_pwd_' . $email_account_id); }inc/integrations/email-providers/class-base-email-provider.php (1)
563-566: Consider makingtest_connection()abstract or returning an error by default.The default implementation returns
true, which means providers that forget to override this method will appear to pass connection tests without actually testing anything. This could mask configuration issues.🔎 Proposed alternatives
Option 1: Make it abstract (breaking change for existing providers)
- public function test_connection() { - - return true; - } + abstract public function test_connection();Option 2: Return a "not implemented" error by default
public function test_connection() { - return true; + return new \WP_Error( + 'not_implemented', + __('Connection test not implemented for this provider.', 'ultimate-multisite') + ); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
inc/admin-pages/class-email-integration-wizard-admin-page.phpinc/class-wp-ultimo.phpinc/integrations/email-providers/class-base-email-provider.phpinc/managers/class-email-account-manager.phpviews/wizards/email-integrations/activation.phpviews/wizards/email-integrations/configuration-results.phpviews/wizards/email-integrations/configuration.phpviews/wizards/email-integrations/default-instructions.phpviews/wizards/email-integrations/ready.phpviews/wizards/email-integrations/test.php
🧰 Additional context used
🧬 Code graph analysis (5)
inc/admin-pages/class-email-integration-wizard-admin-page.php (4)
inc/admin-pages/class-wizard-admin-page.php (1)
render_submit_box(375-385)inc/integrations/email-providers/class-base-email-provider.php (4)
get_title(179-182)get_explainer_lines(398-411)get_fields(288-291)get_id(168-171)inc/functions/template.php (1)
wu_get_template(20-73)inc/functions/helper.php (1)
wu_request(132-137)
views/wizards/email-integrations/default-instructions.php (3)
inc/admin-pages/class-email-integration-wizard-admin-page.php (1)
get_title(108-111)inc/integrations/email-providers/class-base-email-provider.php (4)
get_title(179-182)get_documentation_link(201-204)get_affiliate_url(190-193)get_signup_instructions(588-602)inc/functions/helper.php (1)
wu_kses_allowed_html(326-616)
inc/class-wp-ultimo.php (5)
inc/functions/helper.php (1)
wu_path(106-109)ultimate-multisite.php (1)
WP_Ultimo(124-126)inc/ui/class-email-accounts-element.php (1)
Email_Accounts_Element(25-824)inc/admin-pages/class-email-integration-wizard-admin-page.php (1)
Email_Integration_Wizard_Admin_Page(20-409)inc/managers/class-email-account-manager.php (1)
Email_Account_Manager(26-706)
views/wizards/email-integrations/ready.php (3)
inc/admin-pages/class-email-integration-wizard-admin-page.php (1)
get_title(108-111)inc/integrations/email-providers/class-base-email-provider.php (3)
get_title(179-182)get_affiliate_url(190-193)get_signup_instructions(588-602)inc/functions/url.php (1)
wu_network_admin_url(53-60)
views/wizards/email-integrations/configuration.php (1)
inc/admin-pages/class-wizard-admin-page.php (1)
get_prev_section_link(283-292)
⏰ 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). (6)
- GitHub Check: PHP 8.5
- GitHub Check: PHP 8.3
- GitHub Check: PHP 8.1
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: Build Plugin for Testing
🔇 Additional comments (31)
views/wizards/email-integrations/configuration.php (1)
15-15: LGTM!The form rendering and nonce implementation follow WordPress best practices. The dual-button submit pattern (manual vs automatic) provides good UX flexibility.
views/wizards/email-integrations/ready.php (1)
1-49: LGTM!The view properly escapes all output, uses conditional rendering appropriately, and follows WordPress i18n best practices. The use of
wp_ksesfor signup instructions allows safe HTML while preventing XSS.views/wizards/email-integrations/default-instructions.php (1)
1-81: LGTM!The instructions view demonstrates proper security practices with consistent escaping, appropriate use of
wp_ksesfor controlled HTML content, and well-structured conditional rendering for optional documentation and affiliate sections.views/wizards/email-integrations/test.php (2)
88-88: Verifyajaxurlavailability in network admin context.The code uses the global
ajaxurlvariable, which is typically available in WordPress admin. Confirm this is accessible in the network admin context where this wizard runs.
64-115: Thewu_email_integration_test_dataobject is already properly escaped. Ininc/admin-pages/class-email-integration-wizard-admin-page.php(lines 402-405), bothintegration_idandwaiting_messageare escaped usingesc_js()when the script data is registered viawp_add_inline_script(). No action needed.inc/class-wp-ultimo.php (4)
356-356: LGTM!The email-account.php public API is loaded in the appropriate section alongside other model-related functions, maintaining consistency with the existing architecture.
508-508: LGTM!The Email_Accounts_Element follows the established pattern for UI component initialization and is properly placed with other customer-facing UI elements.
767-770: LGTM!The Email Integration Wizard Admin Page is instantiated following the same pattern as other admin pages in the codebase.
936-940: LGTM!The Email_Account_Manager is properly initialized in the managers section, consistent with other manager instantiations.
views/wizards/email-integrations/activation.php (1)
1-112: LGTM!The activation view is well-structured with:
- Proper output escaping throughout
- Clear visual hierarchy with logo display
- Conditional rendering for enabled state
- Good UX with contextual buttons (Activate/Deactivate and Continue)
The implementation follows WordPress best practices and provides clear feedback to users.
inc/admin-pages/class-email-integration-wizard-admin-page.php (3)
87-97: LGTM!The integration loading properly sanitizes the query parameter and gracefully redirects if the integration is not found, preventing potential errors downstream.
321-329: LGTM!The configuration handler properly implements a whitelist approach by:
- Checking the nonce (line 319)
- Getting allowed fields from the integration (line 321)
- Filtering and sanitizing only allowed fields (lines 324-329)
This prevents arbitrary POST data from being processed.
400-407: LGTM!The inline script data is properly escaped using
esc_js()for both the integration ID and the waiting message, preventing XSS vulnerabilities.views/wizards/email-integrations/configuration-results.php (1)
9-9: Input is properly sanitized before JSON decoding and output is correctly escaped.The
$postvariable is sanitized viawu_request()which appliessanitize_text_field()to all scalar values before the template receives it. The view then properly escapes the output withesc_html(). However, verify that nonce verification protects themanualrequest parameter in the display flow, not just the form submission handler.inc/managers/class-email-account-manager.php (9)
62-89: LGTM on initialization and hook registration.The
init()method correctly sets up REST API, WP-CLI, and registers appropriate hooks for async actions, status transitions, and cleanup handlers. The hook priorities and action names are well-structured.
97-112: LGTM on provider loading.The early return when email accounts are disabled prevents unnecessary provider instantiation. The extensibility hook allows third-party providers to register themselves.
131-145: LGTM on enabled providers retrieval.The method correctly filters providers that are both enabled and properly configured.
399-424: LGTM on async deletion handler.The method correctly validates input, retrieves the provider, and handles both success and error cases with appropriate logging.
470-496: LGTM on membership deletion handler.The
instanceofcheck at line 472 properly addresses the type-safety concern from previous reviews. The method correctly queues provider deletion before removing database records.
507-533: LGTM on customer deletion handler.Consistent with the membership handler, the
instanceofcheck properly handles type validation before processing.
541-600: LGTM on AJAX handler security.The handler correctly implements security best practices: nonce verification with
check_ajax_referer(), capability check withcurrent_user_can('manage_network'), and proper input sanitization. Error responses are handled appropriately.
696-705: LGTM on DNS instructions retrieval.Clean delegation pattern with appropriate error handling for invalid providers.
661-674: Missing isset check forpayment_idparameter.Line 671 accesses
$params['payment_id']without verifying it exists, which could trigger a PHP notice when the key is not provided. Although the null coalescing operator handles the undefined case, the array access itself may emit a notice in strict environments.🔎 Proposed fix
$email_account = wu_create_email_account( [ 'customer_id' => $params['customer_id'], 'membership_id' => $params['membership_id'], 'site_id' => $params['site_id'], 'email_address' => $params['email_address'], 'provider' => $params['provider'], 'quota_mb' => $params['quota_mb'], 'purchase_type' => $params['purchase_type'], - 'payment_id' => $params['payment_id'] ?? null, + 'payment_id' => isset($params['payment_id']) ? $params['payment_id'] : null, 'status' => 'pending', ] );Alternatively, add
'payment_id' => nullto the$defaultsarray at line 612.Likely an incorrect or invalid review comment.
inc/integrations/email-providers/class-base-email-provider.php (8)
79-91: LGTM on initialization flow.The
init()method properly registers the provider with the manager via filter, adds to the settings integration list, and conditionally registers hooks only when enabled and setup.
212-232: LGTM on setup validation.The method correctly handles both single constants and grouped alternatives. The truthiness check on
constant($const)ensures empty or zero values don't falsely indicate configuration.
301-308: LGTM on constant setup, but note the sensitive operation.The method correctly filters input to only known constants before writing to wp-config.php. The actual file modification is delegated to the
WP_Confighelper class.Please ensure the
WP_Config::inject_wp_config_constant()method handles file operations safely (atomic writes, proper permissions, backup/rollback on failure).
341-390: LGTM on settings integration.The method correctly handles the early return when disabled, properly escapes the configuration URL with
esc_url(), and builds appropriate status indicators.
421-426: LGTM on feature support check.The method uses strict comparison in
in_array()and gracefully handles subclasses without a$supportsproperty.
445-519: LGTM on abstract method contracts.The abstract methods are well-documented with clear parameter specifications and return type expectations. The contract provides a solid foundation for provider implementations.
577-580: LGTM on logging helper.The provider-specific log naming (
email-provider-{id}) aids in debugging and log filtering.
588-602: LGTM on signup instructions.The method properly escapes the affiliate URL with
esc_url()and gracefully handles missing affiliate URLs.
| public function get_constants_string($constant_values) { | ||
|
|
||
| $content = [ | ||
| sprintf('// Ultimate Multisite - Email Provider - %s', $this->get_title()), | ||
| ]; | ||
|
|
||
| $constant_values = shortcode_atts(array_flip($this->get_all_constants()), $constant_values); | ||
|
|
||
| foreach ($constant_values as $constant => $value) { | ||
| $content[] = sprintf("define( '%s', '%s' );", $constant, $value); | ||
| } | ||
|
|
||
| $content[] = sprintf('// Ultimate Multisite - Email Provider - %s - End', $this->get_title()); | ||
|
|
||
| return implode(PHP_EOL, $content); | ||
| } |
There was a problem hiding this comment.
Escape single quotes in constant values to prevent invalid PHP output.
Line 327 generates define() statements using single quotes, but doesn't escape single quotes within values. If a constant value contains a single quote (e.g., an API key with '), the generated PHP code would be syntactically invalid.
🔎 Proposed fix
foreach ($constant_values as $constant => $value) {
- $content[] = sprintf("define( '%s', '%s' );", $constant, $value);
+ $content[] = sprintf("define( '%s', '%s' );", $constant, addslashes($value));
}🤖 Prompt for AI Agents
In inc/integrations/email-providers/class-base-email-provider.php around lines
318 to 333, the generated define() lines use single-quoted PHP strings but do
not escape single quotes (and backslashes) in the constant values, which can
produce invalid PHP if a value contains a single quote; before creating the
sprintf for each define, escape backslashes and single quotes in $value (e.g.,
replace "\" with "\\\\" and "'" with "\\'") or use a safe PHP string-escaping
helper so the resulting define( '%s', '%s' ); is always syntactically valid.
New wiki pages documenting the email accounts feature: - Email-Accounts-Integration.md: Overview of the feature, supported providers, how it works for admins and customers, database schema, limitations integration, security considerations, troubleshooting - cPanel-Email-Integration.md: Setup instructions for cPanel email provider including API credentials, DNS records, email client settings, and troubleshooting - Purelymail-Email-Integration.md: Setup instructions for Purelymail including API key, DNS records (MX, SPF, DKIM), pricing info, and comparison with alternatives - Google-Workspace-Email-Integration.md: Detailed setup for Google Workspace including service account creation, domain-wide delegation, Admin SDK configuration, and license management - Microsoft-365-Email-Integration.md: Comprehensive guide for Microsoft 365 including Azure AD app registration, Graph API permissions, license SKU configuration, and DNS setup 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🔨 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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.wiki/Purelymail-Email-Integration.md (1)
10-10: Add language specs to code blocks and wrap bare URLs for markdown compliance.The documentation has several markdown linting violations that should be fixed for consistency:
- Line 10: Bare URL needs to be wrapped
- Lines 111, 140, 150, 158, 167: Code blocks need language identifiers for syntax highlighting
🔎 Proposed fixes for markdown linting issues
-Webmail: https://app.purelymail.com/ +Webmail: <https://app.purelymail.com/>For the code blocks, add language identifiers:
-### Authentication - -All API requests include the header: -``` -Purelymail-Token: {your_api_key} -``` +### Authentication + +All API requests include the header: +```http +Purelymail-Token: {your_api_key} +```-Customers can access webmail at: -``` -https://app.purelymail.com/ -``` +Customers can access webmail at: +``` +https://app.purelymail.com/ +```-### Required Records - -``` -Type Host Value Priority -MX @ mailserver.purelymail.com 10 -TXT @ v=spf1 include:_spf.purelymail.com ~all - -``` +### Required Records + +```text +Type Host Value Priority +MX @ mailserver.purelymail.com 10 +TXT @ v=spf1 include:_spf.purelymail.com ~all - +```-### DKIM Records - -``` -Type Host Value -CNAME purelymail1._domainkey key1.dkimroot.purelymail.com +### DKIM Records + +```text +Type Host Value +CNAME purelymail1._domainkey key1.dkimroot.purelymail.com-### DMARC Record (Recommended) - -``` -Type Host Value -TXT _dmarc v=DMARC1; p=quarantine; rua=mailto:dmarc@yourdomain.com -``` +### DMARC Record (Recommended) + +```text +Type Host Value +TXT _dmarc v=DMARC1; p=quarantine; rua=mailto:dmarc@yourdomain.com +```Also applies to: 111-111, 140-140, 150-150, 158-158, 167-167
.wiki/cPanel-Email-Integration.md (1)
82-82: Add language specs to code blocks for markdown compliance.Code blocks at lines 82, 125, 133, and 141 should specify a language identifier for consistent syntax highlighting.
🔎 Proposed fixes for code block language specs
-### API Endpoint Used - -``` -POST https://{host}:{port}/execute/Email/add_pop -``` +### API Endpoint Used + +```http +POST https://{host}:{port}/execute/Email/add_pop +```-Customers can access webmail at: -``` -https://{cpanel_host}:2096/ -``` +Customers can access webmail at: +``` +https://{cpanel_host}:2096/ +```-For each domain using cPanel email, configure these DNS records: - -``` -Type Host Value Priority -MX @ mail.yourdomain.com 10 -A mail {server_ip_address} - -``` +For each domain using cPanel email, configure these DNS records: + +```text +Type Host Value Priority +MX @ mail.yourdomain.com 10 +A mail {server_ip_address} - +```-### SPF Record (Recommended) - -``` -TXT @ v=spf1 +a +mx ~all -``` +### SPF Record (Recommended) + +```text +TXT @ v=spf1 +a +mx ~all +```Also applies to: 125-125, 133-133, 141-141
.wiki/Email-Accounts-Integration.md (1)
126-126: Add language spec to PHP code block for markdown compliance.The PHP code block at line 126 should specify the language identifier for syntax highlighting.
🔎 Proposed fix for code block language spec
-Enable WordPress debug logging to see detailed error messages: - -```php -define('WP_DEBUG', true); -define('WP_DEBUG_LOG', true); -``` +Enable WordPress debug logging to see detailed error messages: + +```php +define('WP_DEBUG', true); +define('WP_DEBUG_LOG', true); +```.wiki/Microsoft-365-Email-Integration.md (1)
10-10: Add language specs to code blocks and wrap bare URL for markdown compliance.Several markdown linting violations should be fixed:
- Line 10: Bare URL needs wrapping
- Lines 202, 207, 217, 226, 233, 249: Code blocks need language identifiers
🔎 Proposed fixes for markdown linting issues
-- **Webmail**: https://outlook.office365.com/ +- **Webmail**: <https://outlook.office365.com/>For code blocks with URLs and DNS records, add language identifiers:
-### Webmail - -``` -https://outlook.office365.com/ -``` +### Webmail + +``` +https://outlook.office365.com/ +```-Or via Microsoft 365 portal: -``` -https://www.office.com/ -``` +Or via Microsoft 365 portal: +``` +https://www.office.com/ +```-### MX Record - -``` -Type Host Value Priority -MX @ {domain-com}.mail.protection.outlook.com 0 -``` +### MX Record + +```text +Type Host Value Priority +MX @ {domain-com}.mail.protection.outlook.com 0 +```-### SPF Record - -``` -Type Host Value -TXT @ v=spf1 include:spf.protection.outlook.com -all -``` +### SPF Record + +```text +Type Host Value +TXT @ v=spf1 include:spf.protection.outlook.com -all +```-### Autodiscover (Required for Outlook) - -``` -Type Host Value -CNAME autodiscover autodiscover.outlook.com -``` +### Autodiscover (Required for Outlook) + +```text +Type Host Value +CNAME autodiscover autodiscover.outlook.com +```-### DMARC (Recommended) - -``` -Type Host Value -TXT _dmarc v=DMARC1; p=quarantine; rua=mailto:dmarc@yourdomain.com -``` +### DMARC (Recommended) + +```text +Type Host Value +TXT _dmarc v=DMARC1; p=quarantine; rua=mailto:dmarc@yourdomain.com +```Also applies to: 202-202, 207-207, 217-217, 226-226, 233-233, 249-249
.wiki/Google-Workspace-Email-Integration.md (1)
10-10: Add language specs to code blocks and wrap bare URL for markdown compliance.Several markdown linting violations should be fixed:
- Line 10: Bare URL needs wrapping
- Lines 84, 198, 203, 213, 224, 238: Code blocks need language identifiers
🔎 Proposed fixes for markdown linting issues
-- **Webmail**: https://mail.google.com/ +- **Webmail**: <https://mail.google.com/>For code blocks with scopes, URLs, and DNS records, add language identifiers:
-6. Add these OAuth scopes: - ``` - https://www.googleapis.com/auth/admin.directory.user - ``` +6. Add these OAuth scopes: + ``` + https://www.googleapis.com/auth/admin.directory.user + ```-Or with account selection: -``` -https://mail.google.com/mail/u/0/ -``` +Or with account selection: +``` +https://mail.google.com/mail/u/0/ +```-### Webmail - -``` -https://mail.google.com/ -``` +### Webmail + +``` +https://mail.google.com/ +```-### MX Records - -``` -Type Host Value Priority -MX @ aspmx.l.google.com 1 +### MX Records + +```text +Type Host Value Priority +MX @ aspmx.l.google.com 1-### SPF Record - -``` -Type Host Value -TXT @ v=spf1 include:_spf.google.com ~all -``` +### SPF Record + +```text +Type Host Value +TXT @ v=spf1 include:_spf.google.com ~all +```-### DMARC (Recommended) - -``` -Type Host Value -TXT _dmarc v=DMARC1; p=quarantine; rua=mailto:dmarc@yourdomain.com -``` +### DMARC (Recommended) + +```text +Type Host Value +TXT _dmarc v=DMARC1; p=quarantine; rua=mailto:dmarc@yourdomain.com +```Also applies to: 84-84, 198-198, 203-203, 213-213, 224-224, 238-238
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.wiki/Email-Accounts-Integration.md.wiki/Google-Workspace-Email-Integration.md.wiki/Microsoft-365-Email-Integration.md.wiki/Purelymail-Email-Integration.md.wiki/cPanel-Email-Integration.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
.wiki/Email-Accounts-Integration.md
66-66: Bare URL used
(MD034, no-bare-urls)
.wiki/Microsoft-365-Email-Integration.md
10-10: Bare URL used
(MD034, no-bare-urls)
202-202: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
207-207: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
217-217: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
226-226: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
233-233: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
249-249: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.wiki/Google-Workspace-Email-Integration.md
10-10: Bare URL used
(MD034, no-bare-urls)
84-84: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
198-198: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
203-203: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
213-213: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
224-224: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
238-238: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.wiki/Purelymail-Email-Integration.md
10-10: Bare URL used
(MD034, no-bare-urls)
111-111: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
140-140: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
150-150: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
158-158: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
167-167: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.wiki/cPanel-Email-Integration.md
82-82: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
125-125: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
133-133: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
141-141: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (4)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: Build Plugin for Testing
- GitHub Check: PHP 7.4
- GitHub Check: cypress (8.2, chrome)
|
Dawmn, insane |
Summary
Implements email account provisioning and management for Ultimate Multisite customers. This allows network admins to configure email providers and enable customers to create/manage email accounts through their wp-admin dashboard.
Key Features
Testing
Test plan
vendor/bin/phpunit tests/WP_Ultimo/Models/Email_Account_Test.php tests/WP_Ultimo/Limitations/Limit_Email_Accounts_Test.php tests/WP_Ultimo/Functions/Email_Account_Functions_Test.phpCloses #254
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.