And more options for email verification #196
Conversation
WalkthroughReplace boolean email-verification toggle with a tri-state setting ('never'|'free_only'|'always'), update settings UI and retrieval to supply defaults, and change checkout/membership logic to determine email verification status per the new tri-state semantics while preserving legacy boolean behavior. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User/Admin
participant UI as Settings UI
participant ST as Settings Store
participant C as Checkout
participant M as Membership
participant O as Order
U->>UI: choose "Email verification" (never|free_only|always)
UI->>ST: save `enable_email_verification` (string or legacy bool)
Note over ST: stored value may be 'never'|'free_only'|'always' or legacy boolean
U->>C: place order (O)
C->>ST: get_all_with_defaults() -> effective `enable_email_verification`
C->>O: determine requires_payment?
alt enable == 'never'
C-->>O: verification = none
else enable == 'always'
C-->>O: verification = pending
else enable == 'free_only' and O requires payment
C-->>O: verification = none
else enable == 'free_only' and O does not require payment
C-->>O: verification = pending
else legacy bool (fallback)
Note over C: legacy boolean mapping applied
end
O->>M: evaluate membership save
M->>ST: read effective verification status
alt verification == 'pending'
M-->>O: do not set TRIALING
else
M-->>O: allow TRIALING
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
inc/class-settings.php (1)
728-729: Tighten the description copy.Shorter and a bit clearer: “Sites are created only after the customer’s email is verified.”
- 'desc' => __('Controls when email verification is required during registration. Sites will not be created until the customer email verification status is changed to verified.', 'multisite-ultimate'), + 'desc' => __('Controls when email verification is required during registration. Sites are created only after the customer’s email is verified.', 'multisite-ultimate'),inc/checkout/class-checkout.php (2)
2262-2278: Remove direct reads of enable_email_verification in publish paths; rely on customer->email_verification only.There are still boolean-centric checks earlier (e.g., Lines 815 and 832) mixing wu_get_setting('enable_email_verification', true) with the customer status. That’s redundant and can confuse future readers. Use the derived status solely.
Proposed changes outside this hunk:
@@ - if ($this->order->is_free() && $this->order->get_recurring_total() === 0.0 && (! wu_get_setting('enable_email_verification', true) || $this->customer->get_email_verification() !== 'pending')) { + if ($this->order->is_free() && $this->order->get_recurring_total() === 0.0 && $this->customer->get_email_verification() !== 'pending') { @@ - if (wu_get_setting('allow_trial_without_payment_method') && (! wu_get_setting('enable_email_verification', true) || $this->customer->get_email_verification() !== 'pending')) { + if (wu_get_setting('allow_trial_without_payment_method') && $this->customer->get_email_verification() !== 'pending') {
2262-2278: Add tests for tri‑state + legacy cases.Cover: never/free_only/always and legacy 1/0 across free, paid, and trial carts.
I can scaffold unit tests asserting get_customer_email_verification_status() and publish behavior for each case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
inc/checkout/class-checkout.php(1 hunks)inc/class-settings.php(1 hunks)
⏰ 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). (12)
- GitHub Check: cypress (8.2, firefox)
- GitHub Check: cypress (8.0, chrome)
- GitHub Check: cypress (8.3, chrome)
- GitHub Check: cypress (8.3, firefox)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.4, firefox)
- GitHub Check: cypress (8.4, chrome)
- GitHub Check: cypress (8.0, firefox)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (7.4, firefox)
- GitHub Check: cypress (8.1, firefox)
- GitHub Check: cypress (7.4, chrome)
🔇 Additional comments (1)
inc/checkout/class-checkout.php (1)
2262-2278: Good tri‑state handling with legacy fallback.Switch covers never/free_only/always and defaults to legacy boolean semantics. LGTM.
| 'title' => __('Email verification', 'multisite-ultimate'), | ||
| 'desc' => __('Controls when email verification is required during registration. Sites will not be created until the customer email verification status is changed to verified.', 'multisite-ultimate'), | ||
| 'type' => 'select', | ||
| 'options' => [ | ||
| 'never' => __('Never require email verification', 'multisite-ultimate'), | ||
| 'free_only' => __('Only for free plans', 'multisite-ultimate'), | ||
| 'always' => __('Always require email verification', 'multisite-ultimate'), | ||
| ], | ||
| 'default' => 'free_only', | ||
| ] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Map legacy boolean (1/0) to new tri‑state so the select renders correctly.
Existing installs will have enable_email_verification saved as 1/0. Without normalization, the select will show no option selected and re-save 1/0. Normalize the value for UI/read to avoid admin confusion and to migrate to strings on next save.
Apply this diff to add value/display_value normalizers:
$this->add_field(
'login-and-registration',
'enable_email_verification',
[
'title' => __('Email verification', 'multisite-ultimate'),
'desc' => __('Controls when email verification is required during registration. Sites will not be created until the customer email verification status is changed to verified.', 'multisite-ultimate'),
'type' => 'select',
'options' => [
'never' => __('Never require email verification', 'multisite-ultimate'),
'free_only' => __('Only for free plans', 'multisite-ultimate'),
'always' => __('Always require email verification', 'multisite-ultimate'),
],
- 'default' => 'free_only',
+ 'default' => 'free_only',
+ 'value' => function () {
+ $raw = wu_get_setting('enable_email_verification', 'free_only');
+ if ($raw === 1 || $raw === '1' || $raw === true) {
+ return 'free_only'; // legacy "enabled"
+ }
+ if ($raw === 0 || $raw === '0' || $raw === false) {
+ return 'never'; // legacy "disabled"
+ }
+ return in_array($raw, ['never', 'free_only', 'always'], true) ? $raw : 'free_only';
+ },
+ 'display_value' => function () {
+ $raw = wu_get_setting('enable_email_verification', 'free_only');
+ if ($raw === 1 || $raw === '1' || $raw === true) {
+ return 'free_only';
+ }
+ if ($raw === 0 || $raw === '0' || $raw === false) {
+ return 'never';
+ }
+ return in_array($raw, ['never', 'free_only', 'always'], true) ? $raw : 'free_only';
+ },
]
);📝 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.
| 'title' => __('Email verification', 'multisite-ultimate'), | |
| 'desc' => __('Controls when email verification is required during registration. Sites will not be created until the customer email verification status is changed to verified.', 'multisite-ultimate'), | |
| 'type' => 'select', | |
| 'options' => [ | |
| 'never' => __('Never require email verification', 'multisite-ultimate'), | |
| 'free_only' => __('Only for free plans', 'multisite-ultimate'), | |
| 'always' => __('Always require email verification', 'multisite-ultimate'), | |
| ], | |
| 'default' => 'free_only', | |
| ] | |
| $this->add_field( | |
| 'login-and-registration', | |
| 'enable_email_verification', | |
| [ | |
| 'title' => __('Email verification', 'multisite-ultimate'), | |
| 'desc' => __('Controls when email verification is required during registration. Sites will not be created until the customer email verification status is changed to verified.', 'multisite-ultimate'), | |
| 'type' => 'select', | |
| 'options' => [ | |
| 'never' => __('Never require email verification', 'multisite-ultimate'), | |
| 'free_only' => __('Only for free plans', 'multisite-ultimate'), | |
| 'always' => __('Always require email verification', 'multisite-ultimate'), | |
| ], | |
| 'default' => 'free_only', | |
| 'value' => function () { | |
| $raw = wu_get_setting('enable_email_verification', 'free_only'); | |
| if ($raw === 1 || $raw === '1' || $raw === true) { | |
| return 'free_only'; // legacy "enabled" | |
| } | |
| if ($raw === 0 || $raw === '0' || $raw === false) { | |
| return 'never'; // legacy "disabled" | |
| } | |
| return in_array($raw, ['never', 'free_only', 'always'], true) ? $raw : 'free_only'; | |
| }, | |
| 'display_value' => function () { | |
| $raw = wu_get_setting('enable_email_verification', 'free_only'); | |
| if ($raw === 1 || $raw === '1' || $raw === true) { | |
| return 'free_only'; | |
| } | |
| if ($raw === 0 || $raw === '0' || $raw === false) { | |
| return 'never'; | |
| } | |
| return in_array($raw, ['never', 'free_only', 'always'], true) ? $raw : 'free_only'; | |
| }, | |
| ] | |
| ); |
🤖 Prompt for AI Agents
In inc/class-settings.php around lines 727-736, the select for
'enable_email_verification' doesn't normalize legacy boolean 1/0 values so the
UI shows no selection; add value/display_value normalizers to map legacy numeric
booleans into the new string states (map 1 -> 'always' and 0 -> 'never') for
read/display, and pass-through the new string values on save so the select
renders correctly and future saves persist the string tri-state.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
inc/class-settings.php (1)
742-761: Normalize display_value too; tighten copy; confirm legacy mapping is intentionalOnly
valueis normalized. Some controls rely ondisplay_valuefor initial rendering; normalize it as well to avoid blank/incorrect selections on legacy installs. Also, small copy tweak.'enable_email_verification', [ - 'title' => __('Email verification', 'multisite-ultimate'), - 'desc' => __('Controls if email verification is required during registration. If set, sites will not be created until the customer email verification status is changed to verified.', 'multisite-ultimate'), + 'title' => __('Email verification', 'multisite-ultimate'), + 'desc' => __('Controls when email verification is required during registration. If enabled for a signup, sites will not be created until the customer’s email is verified.', 'multisite-ultimate'), 'type' => 'select', 'options' => [ 'never' => __('Never require email verification', 'multisite-ultimate'), 'free_only' => __('Only for free plans', 'multisite-ultimate'), 'always' => __('Always require email verification', 'multisite-ultimate'), ], 'default' => 'free_only', 'value' => function () { $raw = wu_get_setting('enable_email_verification', 'free_only'); if (1 === $raw || '1' === $raw || true === $raw) { - return 'free_only'; // legacy "enabled" + return 'free_only'; // legacy "enabled" } if (0 === $raw || '0' === $raw || false === $raw) { return 'never'; // legacy "disabled" } return in_array($raw, ['never', 'free_only', 'always'], true) ? $raw : 'free_only'; }, + 'display_value' => function () { + $raw = wu_get_setting('enable_email_verification', 'free_only'); + if (1 === $raw || '1' === $raw || true === $raw) { + return 'free_only'; + } + if (0 === $raw || '0' === $raw || false === $raw) { + return 'never'; + } + return in_array($raw, ['never', 'free_only', 'always'], true) ? $raw : 'free_only'; + }, ]Please confirm: should legacy “enabled” map to ‘free_only’ (current code) or ‘always’? If the old toggle enforced verification for all signups, mapping to ‘always’ may better preserve behavior.
🧹 Nitpick comments (1)
inc/class-settings.php (1)
165-178: Tidy get_all_with_defaults: remove unused var and harden callable check
$section_slugis unused (static analysis warning). Also, guard against missing'value'keys.- public function get_all_with_defaults($check_caps = false) { - $all_settings = $this->get_all($check_caps); - foreach ($this->get_sections() as $section_slug => $section) { - foreach ($section['fields'] as $field_slug => $field_atts) { - if (is_callable($field_atts['value'])) { + public function get_all_with_defaults($check_caps = false) { + $all_settings = $this->get_all($check_caps); + foreach ($this->get_sections() as $_section_slug => $section) { + foreach ($section['fields'] as $field_slug => $field_atts) { + if (isset($field_atts['value']) && is_callable($field_atts['value'])) { $value = $field_atts['value'](); if (isset($all_settings[ $field_slug ]) && $value !== $all_settings[ $field_slug ]) { $all_settings[ $field_slug ] = $value; } } } } return $all_settings; }Optional: consider also seeding
$all_settings[$field_slug] = $valuewhen the key is not set, so Vue gets normalized defaults for fields never saved.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
inc/admin-pages/class-settings-admin-page.php(1 hunks)inc/checkout/class-checkout.php(3 hunks)inc/class-settings.php(2 hunks)inc/models/class-membership.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- inc/checkout/class-checkout.php
🧰 Additional context used
🧬 Code graph analysis (3)
inc/admin-pages/class-settings-admin-page.php (2)
inc/functions/array-helpers.php (1)
wu_array_map_keys(136-143)inc/class-settings.php (1)
get_all_with_defaults(165-178)
inc/models/class-membership.php (1)
inc/models/class-customer.php (1)
get_email_verification(315-318)
inc/class-settings.php (1)
inc/functions/settings.php (1)
wu_get_setting(39-42)
🪛 PHPMD (2.15.0)
inc/class-settings.php
167-167: Avoid unused local variables such as '$section_slug'. (undefined)
(UnusedLocalVariable)
⏰ 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). (12)
- GitHub Check: cypress (8.3, chrome)
- GitHub Check: cypress (8.2, firefox)
- GitHub Check: cypress (8.4, firefox)
- GitHub Check: cypress (8.1, firefox)
- GitHub Check: cypress (8.4, chrome)
- GitHub Check: cypress (8.3, firefox)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (7.4, firefox)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.0, firefox)
- GitHub Check: cypress (7.4, chrome)
- GitHub Check: cypress (8.0, chrome)
🔇 Additional comments (1)
inc/admin-pages/class-settings-admin-page.php (1)
605-606: LGTM: using get_all_with_defaults ensures normalized state reaches VueThis aligns the form state with field-level value callbacks (e.g., legacy → tri‑state mapping). No further changes needed.
Quick check: verify the “Email verification” select shows a valid option on an install with legacy 0/1 values before saving the settings page.
| if ($this->get_status() === Membership_Status::PENDING && $this->is_trialing() && ! $this->get_last_pending_payment()) { | ||
| if ( ! wu_get_setting('enable_email_verification', true) || $this->get_customer()->get_email_verification() !== 'pending') { | ||
| if ($this->get_customer()->get_email_verification() !== 'pending') { | ||
| $this->set_status(Membership_Status::TRIALING); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Null-guard customer before checking email verification; keep gating consistent with tri‑state logic
get_customer() can be falsy; dereferencing without a guard risks a fatal. Also, this check consults the raw email_verification property only. If the tri‑state is set to “never” (or “free_only” for paid plans), a customer might legitimately remain non‑verified yet should not block TRIALING. Consider centralizing the decision via a helper that derives the effective verification requirement/status, or at minimum guard null here.
Apply this minimal guard:
- if ($this->get_customer()->get_email_verification() !== 'pending') {
- $this->set_status(Membership_Status::TRIALING);
- }
+ $customer = $this->get_customer();
+ if ($customer && $customer->get_email_verification() !== 'pending') {
+ $this->set_status(Membership_Status::TRIALING);
+ }Follow‑up:
- Verify that whatever computes the effective email‑verification status (per tri‑state and plan type) is invoked during signup so that
email_verificationis ‘pending’ only when required; otherwise this code may prematurely promote to TRIALING. If there’s a canonical helper (e.g., in Checkout), call it here instead of reading the raw property.
🏁 Script executed:
#!/bin/bash
# Locate get_customer() in the Membership model to inspect its return type and docblock
grep -R -nC3 "function get_customer" inc/models/class-membership.php
# Check where get_email_verification() is defined/used in the Membership class
grep -R -nC3 "get_email_verification" -n inc/models/class-membership.php
# Search for any centralized helper around email-verification logic
grep -R -nC3 "verification" inc/modelsLength of output: 8337
🏁 Script executed:
#!/bin/bash
# Find definition of wu_get_customer to inspect its return behavior
grep -R "function wu_get_customer" -n .
# Find all references and where it's defined
grep -R "wu_get_customer(" -n .Length of output: 3552
🏁 Script executed:
#!/bin/bash
# Print the wu_get_customer implementation to determine return behavior (can it return null/false?)
sed -n '1,80p' inc/functions/customer.phpLength of output: 1520
Guard get_customer() before get_email_verification() and centralize tri-state logic
Wrap get_customer() in a null/false check to prevent fatal errors and, when available, replace the raw get_email_verification() property check with a canonical helper for effective email-verification logic.
inc/models/class-membership.php
@@ -2436,3 +2436,6 @@ if ($this->get_status() === Membership_Status::PENDING && $this->is_trialing() && ! $this->get_last_pending_payment()) {
- if ($this->get_customer()->get_email_verification() !== 'pending') {
- $this->set_status(Membership_Status::TRIALING);
- }
+ $customer = $this->get_customer();
+ if ($customer && $customer->get_email_verification() !== 'pending') {
+ $this->set_status(Membership_Status::TRIALING);
+ }
}Ensure the signup flow or a shared helper sets and evaluates email_verification per plan requirements so only required verifications block trial status.
📝 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.
| if ($this->get_status() === Membership_Status::PENDING && $this->is_trialing() && ! $this->get_last_pending_payment()) { | |
| if ( ! wu_get_setting('enable_email_verification', true) || $this->get_customer()->get_email_verification() !== 'pending') { | |
| if ($this->get_customer()->get_email_verification() !== 'pending') { | |
| $this->set_status(Membership_Status::TRIALING); | |
| } | |
| } | |
| if ( $this->get_status() === Membership_Status::PENDING | |
| && $this->is_trialing() | |
| && ! $this->get_last_pending_payment() | |
| ) { | |
| $customer = $this->get_customer(); | |
| if ( $customer && $customer->get_email_verification() !== 'pending' ) { | |
| $this->set_status( Membership_Status::TRIALING ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In inc/models/class-membership.php around lines 2435 to 2439, guard the
get_customer() call before invoking get_email_verification() and replace the raw
string comparison with a canonical helper that encapsulates the tri-state
(verified/pending/required) logic; specifically, check that
$this->get_customer() returns an object before calling its email verification
API, call the centralized email-verification helper (e.g.,
Customer::is_email_verification_pending() or a Membership-level helper) to
decide whether verification blocks trialing, and ensure the signup flow or
shared helper is the single place that sets and evaluates the email_verification
value so only plans that require verification will prevent moving status from
PENDING to TRIALING.
#194
Summary by CodeRabbit
New Features
Behavior Changes
Documentation
Chores