Create new stage failed ssl to prevent domain without valid ssl certs…#319
Create new stage failed ssl to prevent domain without valid ssl certs…#319superdav42 merged 2 commits intomainfrom
Conversation
… from being active
📝 WalkthroughWalkthroughReplaces hard-coded domain stage strings with Domain_Stage class constants across schemas, models, managers, integrations, and database schema; adds a new stage constant Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DomainManager
participant HostProvider
participant Database
Client->>DomainManager: request domain verification
DomainManager->>HostProvider: check DNS
HostProvider-->>DomainManager: DNS result (ok/fail)
DomainManager->>Database: update stage (CHECKING_SSL / FAILED)
alt DNS ok
DomainManager->>HostProvider: check SSL
HostProvider-->>DomainManager: SSL result (ok/fail)
DomainManager->>Database: update stage (DONE / SSL_FAILED / FAILED)
end
DomainManager-->>Client: final status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
inc/managers/class-domain-manager.php (1)
240-249: Inconsistent: UseDomain_Stage::CHECKING_DNSconstant instead of hardcoded string.This line still uses the hardcoded string
'checking-dns'while the rest of the file has been updated to useDomain_Stageconstants.Proposed fix
$domain = wu_create_domain( [ 'blog_id' => $site->blog_id, 'domain' => $site->domain, 'active' => true, 'primary_domain' => true, 'secure' => false, - 'stage' => 'checking-dns', + 'stage' => Domain_Stage::CHECKING_DNS, ] );inc/apis/schemas/domain-update.php (1)
49-60: Outdated description: Missingssl-failedoption.The description on line 50 lists the valid options but doesn't include the new
ssl-failedstage.Proposed fix
'stage' => [ - 'description' => __('The state of the domain model object. Can be one of this options: checking-dns, checking-ssl-cert, done-without-ssl, done and failed.', 'ultimate-multisite'), + 'description' => __('The state of the domain model object. Can be one of this options: checking-dns, checking-ssl-cert, done-without-ssl, done, failed and ssl-failed.', 'ultimate-multisite'), 'type' => 'string', 'required' => false, 'enum' => [
🤖 Fix all issues with AI agents
In @inc/database/domains/class-domains-table.php:
- Line 74: The CREATE TABLE enum for the "stage" column contains an extra
closing parenthesis before DEFAULT; edit the SQL string in
class-domains-table.php where the "stage" column is defined (the line building
the stage enum using Domain_Stage::CHECKING_DNS, CHECKING_SSL, DONE_WITHOUT_SSL,
DONE, FAILED, SSL_FAILED) and remove the stray ')' so the enum(...) expression
is properly closed once and then followed by DEFAULT
'Domain_Stage::CHECKING_DNS'.
🧹 Nitpick comments (3)
inc/models/class-domain.php (1)
124-136: Consider constructing the validation rule from Domain_Stage constants.The validation rule on line 131 uses hardcoded string literals for stage values, which is inconsistent with the refactoring happening elsewhere in this PR (e.g., domain-create.php uses Domain_Stage constants in enums). This creates a maintenance burden if stage values change.
Consider refactoring to dynamically construct the enum from Domain_Stage constants to maintain consistency and reduce duplication.
♻️ Potential approach
public function validation_rules() { $id = $this->get_id(); + + $stage_values = implode(',', [ + Domain_Stage::CHECKING_DNS, + Domain_Stage::CHECKING_SSL, + Domain_Stage::DONE_WITHOUT_SSL, + Domain_Stage::DONE, + Domain_Stage::FAILED, + Domain_Stage::SSL_FAILED, + ]); return [ 'blog_id' => 'required|integer', 'domain' => "required|domain|unique:\WP_Ultimo\Models\Domain,domain,{$id}", - 'stage' => 'required|in:checking-dns,checking-ssl-cert,done-without-ssl,done,failed,ssl-failed|default:checking-dns', + 'stage' => "required|in:{$stage_values}|default:" . Domain_Stage::CHECKING_DNS, 'active' => 'default:1', 'secure' => 'default:0', 'primary_domain' => 'default:0', ]; }inc/database/domains/class-domains-schema.php (1)
91-97: Inconsistent default value: UseDomain_Stage::CHECKING_DNSconstant.The enum type correctly uses
Domain_Stageconstants, but the default value still uses a hardcoded string. For consistency, use the constant.Proposed fix
[ 'name' => 'stage', 'type' => 'enum(\'' . Domain_Stage::CHECKING_DNS . '\', \'' . Domain_Stage::CHECKING_SSL . '\', \'' . Domain_Stage::DONE_WITHOUT_SSL . '\', \'' . Domain_Stage::DONE . '\', \'' . Domain_Stage::FAILED . '\', \'' . Domain_Stage::SSL_FAILED . '\')', - 'default' => 'checking-dns', + 'default' => Domain_Stage::CHECKING_DNS, 'transition' => true, 'sortable' => true, ],inc/database/domains/class-domains-table.php (1)
107-123: Remove unnecessary phpcs:ignore comment.The
phpcs:ignore PHPCompatibility.FunctionNameRestrictions.ReservedFunctionNames.MethodDoubleUnderscorecomment is unnecessary since the method nameupdate_enumdoesn't contain double underscores. This comment was likely copied from a renamed method.Proposed fix
- protected function update_enum(): bool { // phpcs:ignore PHPCompatibility.FunctionNameRestrictions.ReservedFunctionNames.MethodDoubleUnderscore + protected function update_enum(): bool {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
inc/apis/schemas/domain-create.phpinc/apis/schemas/domain-update.phpinc/class-faker.phpinc/database/domains/class-domain-stage.phpinc/database/domains/class-domains-schema.phpinc/database/domains/class-domains-table.phpinc/integrations/host-providers/class-wpmudev-host-provider.phpinc/managers/class-domain-manager.phpinc/models/class-domain.php
🧰 Additional context used
🧬 Code graph analysis (8)
inc/apis/schemas/domain-create.php (1)
inc/database/domains/class-domain-stage.php (1)
Domain_Stage(22-76)
inc/class-faker.php (1)
inc/database/domains/class-domain-stage.php (1)
Domain_Stage(22-76)
inc/integrations/host-providers/class-wpmudev-host-provider.php (2)
inc/database/domains/class-domain-stage.php (1)
Domain_Stage(22-76)inc/models/class-domain.php (1)
get_stage(335-338)
inc/database/domains/class-domains-table.php (1)
inc/database/domains/class-domain-stage.php (1)
Domain_Stage(22-76)
inc/apis/schemas/domain-update.php (1)
inc/database/domains/class-domain-stage.php (1)
Domain_Stage(22-76)
inc/database/domains/class-domains-schema.php (1)
inc/database/domains/class-domain-stage.php (1)
Domain_Stage(22-76)
inc/managers/class-domain-manager.php (2)
inc/database/domains/class-domain-stage.php (1)
Domain_Stage(22-76)inc/models/class-domain.php (3)
has_correct_dns(418-458)set_stage(348-351)has_valid_ssl_certificate(466-469)
inc/models/class-domain.php (1)
inc/database/domains/class-domain-stage.php (1)
Domain_Stage(22-76)
⏰ 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.1, chrome)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: Build Plugin for Testing
🔇 Additional comments (16)
inc/class-faker.php (3)
14-14: LGTM!The import is correctly added to support Domain_Stage constants used later in the file.
161-189: LGTM!The explicit
return false;at line 188 improves code clarity by ensuring all code paths have an explicit return statement. This is good defensive coding practice.
482-515: LGTM!The migration to Domain_Stage constants is correctly implemented. The faker intentionally uses only active/in-progress stages (CHECKING_DNS, CHECKING_SSL, DONE) for generating test data, which is a reasonable choice that excludes failure states like SSL_FAILED and FAILED.
inc/database/domains/class-domain-stage.php (3)
31-31: LGTM!The new SSL_FAILED constant is properly defined following the existing naming and value conventions.
47-57: LGTM!The CSS class mapping for SSL_FAILED appropriately uses red styling (same as FAILED), which provides consistent visual feedback for error states.
65-75: LGTM!The label for SSL_FAILED is properly internationalized and provides clear user-facing text distinguishing it from DNS failures.
inc/models/class-domain.php (2)
76-76: LGTM!Good refactoring to use the Domain_Stage constant instead of a magic string for the default stage value.
92-97: LGTM!Correctly adds SSL_FAILED to INACTIVE_STAGES, ensuring domains with SSL certificate failures are treated as inactive. This aligns perfectly with the PR objective to prevent domains without valid SSL certificates from being active.
inc/integrations/host-providers/class-wpmudev-host-provider.php (2)
13-13: LGTM!Import correctly added to support the Domain_Stage constant usage.
97-108: LGTM!The stage comparison correctly uses the Domain_Stage::CHECKING_SSL constant instead of a string literal, maintaining functionality while improving maintainability.
inc/apis/schemas/domain-create.php (2)
12-13: LGTM!Import correctly added to support Domain_Stage constants in the schema enum.
49-61: LGTM!The enum correctly uses Domain_Stage constants for all available stage values, including the new SSL_FAILED. This provides proper validation for the API schema.
inc/managers/class-domain-manager.php (2)
16-16: LGTM!Import statement correctly added for Domain_Stage.
668-778: LGTM!The domain stage transition logic correctly uses
Domain_Stageconstants throughout. The newSSL_FAILEDstage appropriately prevents domains with certificate errors from becoming active, which aligns with the PR objective.inc/apis/schemas/domain-update.php (1)
12-13: LGTM!Import statement correctly added for Domain_Stage.
inc/database/domains/class-domains-table.php (1)
51-57: LGTM!Constructor correctly initializes the upgrades array with version-keyed callbacks pointing to the respective upgrade methods.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.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 (1)
inc/database/domains/class-domains-table.php (1)
112-112: Remove unnecessary PHPCS ignore comment.The
phpcs:ignore PHPCompatibility.FunctionNameRestrictions.ReservedFunctionNames.MethodDoubleUnderscorecomment is no longer applicable sinceupdate_enumdoesn't start with double underscores. This appears to be a leftover from when methods were named like__20230601.Suggested fix
- protected function update_enum(): bool { // phpcs:ignore PHPCompatibility.FunctionNameRestrictions.ReservedFunctionNames.MethodDoubleUnderscore + protected function update_enum(): bool {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
inc/database/domains/class-domains-table.php
🧰 Additional context used
🧬 Code graph analysis (1)
inc/database/domains/class-domains-table.php (1)
inc/database/domains/class-domain-stage.php (1)
Domain_Stage(22-76)
⏰ 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 (3)
inc/database/domains/class-domains-table.php (3)
46-57: LGTM!The constructor-based upgrades initialization is a clean approach. The callbacks are correctly mapped and the parent constructor is properly invoked after setting up the upgrades array.
74-74: Good use of constants for enum definition.Using
Domain_Stageconstants instead of hard-coded strings improves maintainability and ensures consistency across the codebase.
87-105: LGTM!Renaming from
__20230601toallow_nullsimproves readability. The error handling is appropriate—returningfalseon first failure allows the upgrade system to track incomplete migrations.
1 participant
… from being active
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.