Email and orphans #200
Email and orphans #200
Conversation
WalkthroughAdds an email confirmation signup field and validation to checkout, registers the field type, introduces an Orphaned_Users_Manager with a network modal and AJAX deletion flow, initializes the manager during startup, and removes a void return type from a list-table method. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CheckoutUI as Checkout UI
participant Field as Email Fields
participant Validator as Checkout Validator
User->>CheckoutUI: Open checkout (logged out)
CheckoutUI->>Field: Render email + confirmation inputs
User->>Field: Enter email and confirmation
CheckoutUI->>Validator: Submit data
Validator-->>Validator: Apply rule same:email_address
alt Match
Validator-->>CheckoutUI: Validation passes
CheckoutUI-->>User: Proceed
else Mismatch
Validator-->>CheckoutUI: Return error for confirmation
CheckoutUI-->>User: Show "Email confirmation" error
end
sequenceDiagram
autonumber
actor Admin
participant Settings as Network Settings UI
participant Modal as Orphaned Users Modal
participant AJAX as Admin AJAX
participant Manager as Orphaned_Users_Manager
participant WP as WP Core
Admin->>Settings: Click "Delete Orphaned Users"
Settings->>Modal: Open modal (data-state with users)
Admin->>Modal: Confirm deletion
Modal->>AJAX: POST orphaned_users_delete
AJAX->>Manager: handle_orphaned_users_delete()
Manager->>Manager: find_orphaned_users()
loop For each orphaned user
Manager->>WP: wp_delete_user(user_id) / wpmu_delete_user
WP-->>Manager: deletion result
end
Manager-->>AJAX: JSON { message, redirect_url }
AJAX-->>Modal: Show success and redirect
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/managers/class-signup-fields-manager.php (1)
181-183: Fix fatal: wrong variable used when merging editor fields.
$field_classis undefined; should use$field_type.Apply this diff:
- foreach ($field_types as $field_type) { - $all_editor_fields = array_merge($all_editor_fields, $field_class->get_fields()); - } + foreach ($field_types as $field_type) { + $all_editor_fields = array_merge($all_editor_fields, $field_type->get_fields()); + }
🧹 Nitpick comments (7)
inc/class-orphaned-users-manager.php (2)
232-279: Avoid O(users × sites) meta lookups; compute in bulkCurrent approach loops every user across every site, doing many usermeta reads. For large networks this will be slow.
Option A (single query + set diff):
- Build one DISTINCT list of user_ids having any “{base_prefix}%_capabilities” meta.
- Orphaned users = all users – super admins – users_with_roles.
Sketch:
$cap_like = $wpdb->esc_like($wpdb->base_prefix) . '%_capabilities'; $with_roles = $wpdb->get_col( $wpdb->prepare( "SELECT DISTINCT user_id FROM $wpdb->usermeta WHERE meta_key LIKE %s", $cap_like ) ); $all = get_users(['fields' => ['ID','user_login','user_email','user_registered'], 'number' => 0]); $orphans = array_filter($all, fn($u) => !is_super_admin($u->ID) && !in_array($u->ID, $with_roles, true));Also, use 'number' => 0 (or omit it) instead of empty string for clarity.
246-252: Nit: normalize get_users number paramUse 0 (no limit) or omit the param; an empty string is ambiguous.
- $users = get_users( - [ - 'number' => '', - 'fields' => ['ID', 'user_login', 'user_email', 'user_registered'], - ] - ); + $users = get_users([ + 'number' => 0, + 'fields' => ['ID', 'user_login', 'user_email', 'user_registered'], + ]);inc/managers/class-signup-fields-manager.php (1)
40-45: Docs/filter name mismatch.Docblock mentions
wu_checkout_forms_field_types, code useswu_checkout_field_types. Align or add an alias to avoid confusion.Also applies to: 93-94
inc/checkout/signup-fields/class-signup-field-email-confirmation.php (4)
109-112: Icon class likely invalid.
dashicons-wu-at-signmixes prefixes. Use a core dashicon or a validwu-dashicon-*class.Apply this diff (example using a core icon):
- return 'dashicons-wu-at-sign'; + return 'dashicons-email';
245-248: Avoid undefined index notices for builder attrs.Use safe accessors for optional
name/placeholder/tooltip.Apply this diff:
- 'name' => $attributes['name'], - 'placeholder' => $attributes['placeholder'], - 'tooltip' => $attributes['tooltip'], + 'name' => wu_get_isset($attributes, 'name', __('Email Address', 'multisite-ultimate')), + 'placeholder' => wu_get_isset($attributes, 'placeholder', ''), + 'tooltip' => wu_get_isset($attributes, 'tooltip', ''),
242-255: Improve UX: add autocomplete and keep parity between fields.Add
autocomplete="email"to both inputs; keep existing attrs intact.Apply this diff:
$checkout_fields['email_address'] = [ 'type' => 'email', 'id' => 'email_address', 'name' => wu_get_isset($attributes, 'name', __('Email Address', 'multisite-ultimate')), 'placeholder' => wu_get_isset($attributes, 'placeholder', ''), 'tooltip' => wu_get_isset($attributes, 'tooltip', ''), 'value' => $this->get_value(), 'required' => true, 'wrapper_classes' => wu_get_isset($attributes, 'wrapper_element_classes', ''), 'classes' => wu_get_isset($attributes, 'element_classes', ''), 'wrapper_html_attr' => [ 'style' => $this->calculate_style_attr(), ], + 'html_attr' => [ + 'autocomplete' => 'email', + ], ]; $checkout_fields['email_address_confirmation'] = [ 'type' => 'email', 'id' => 'email_address_confirmation', 'name' => wu_get_isset($attributes, 'confirmation_label', __('Confirm Email Address', 'multisite-ultimate')), 'placeholder' => wu_get_isset($attributes, 'confirmation_placeholder', __('Re-enter your email address', 'multisite-ultimate')), 'tooltip' => __('Please re-enter your email address to confirm it matches.', 'multisite-ultimate'), 'value' => '', 'required' => true, 'wrapper_classes' => wu_get_isset($attributes, 'wrapper_element_classes', ''), 'classes' => wu_get_isset($attributes, 'element_classes', ''), 'wrapper_html_attr' => [ 'style' => $this->calculate_style_attr(), ], 'html_attr' => [ 'data-confirm-email' => 'email_address', + 'autocomplete' => 'email', ], ];Also applies to: 257-273
153-159: Guard against duplicate IDs when both Email and Email Confirmation fields are added.
force_attributes()fixes the ID toemail_address. If admins also add the base Email field, IDs will collide. Consider runtime validation or UI guard to disallow both simultaneously.Also applies to: 242-274
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
inc/checkout/class-checkout.php(2 hunks)inc/checkout/signup-fields/class-signup-field-email-confirmation.php(1 hunks)inc/class-orphaned-users-manager.php(1 hunks)inc/class-wp-ultimo.php(1 hunks)inc/list-tables/class-customer-list-table.php(1 hunks)inc/managers/class-signup-fields-manager.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.0, chrome)
- GitHub Check: cypress (7.4, chrome)
- GitHub Check: cypress (8.1, firefox)
- GitHub Check: cypress (8.4, chrome)
- GitHub Check: cypress (8.3, chrome)
- GitHub Check: cypress (8.4, firefox)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (7.4, firefox)
- GitHub Check: cypress (8.3, firefox)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, firefox)
- GitHub Check: cypress (8.0, firefox)
🔇 Additional comments (7)
inc/checkout/class-checkout.php (1)
1863-1869: Aliases addition looks goodNew aliases (including “Email confirmation”) are correctly localized and merged after base aliases.
inc/list-tables/class-customer-list-table.php (1)
259-268: Ignore return type alignment for single_row_grid
The parent Base_List_Table::single_row_grid() does not declare a return type, so omitting: voidin Customer_List_Table causes no signature mismatch.Likely an incorrect or invalid review comment.
inc/class-orphaned-users-manager.php (3)
43-53: Form registration LGTMForm is properly registered with a network capability gate. Nice.
88-176: Good UX and escaping in modalModal builds a clear, scrollable list; user properties are escaped; copy is localized. Looks solid.
184-224: Remove manual nonce check in handlerForm Manager’s handle_form already enforces
wp_verify_nonce( '_wpnonce', 'wu_form_orphaned_users_delete' )before invoking your callback, so adding an extracheck_ajax_refererhere is redundant.Likely an incorrect or invalid review comment.
inc/class-wp-ultimo.php (1)
897-898: Load order looks correct; confirm scope and cost of Orphaned_Users_Manager.Good to instantiate after Orphaned_Tables_Manager. Please verify the manager only registers network-admin scoped hooks and defers expensive queries until an explicit action (modal open/AJAX) to avoid front-end bloat.
inc/managers/class-signup-fields-manager.php (1)
57-58: LGTM: field type registered.Mapping for 'email_confirmation' is correct and matches the new class namespace.
| 'email_address' => 'required_without:user_id|email|unique:\WP_User,email', | ||
| 'email_address_confirmation' => 'same:email_address', | ||
| 'username' => 'required_without:user_id|alpha_dash|min:4|lowercase|unique:\WP_User,login', | ||
| 'password' => 'required_without:user_id|min:6', | ||
| 'password_conf' => 'same:password', | ||
| 'template_id' => 'integer|site_template', | ||
| 'products' => 'products', | ||
| 'gateway' => '', | ||
| 'valid_password' => 'accepted', | ||
| 'billing_country' => 'country|required_with:billing_country', | ||
| 'billing_zip_code' => 'required_with:billing_zip_code', | ||
| 'billing_state' => 'state', | ||
| 'billing_city' => 'city', | ||
| ]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Require email confirmation when registering new users
Add a presence check so non-logged-in signups must provide the confirmation field, not just match when present.
Apply:
- 'email_address_confirmation' => 'same:email_address',
+ 'email_address_confirmation' => 'required_without:user_id|same:email_address',📝 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.
| 'email_address' => 'required_without:user_id|email|unique:\WP_User,email', | |
| 'email_address_confirmation' => 'same:email_address', | |
| 'username' => 'required_without:user_id|alpha_dash|min:4|lowercase|unique:\WP_User,login', | |
| 'password' => 'required_without:user_id|min:6', | |
| 'password_conf' => 'same:password', | |
| 'template_id' => 'integer|site_template', | |
| 'products' => 'products', | |
| 'gateway' => '', | |
| 'valid_password' => 'accepted', | |
| 'billing_country' => 'country|required_with:billing_country', | |
| 'billing_zip_code' => 'required_with:billing_zip_code', | |
| 'billing_state' => 'state', | |
| 'billing_city' => 'city', | |
| ]; | |
| 'email_address' => 'required_without:user_id|email|unique:\WP_User,email', | |
| 'email_address_confirmation' => 'required_without:user_id|same:email_address', | |
| 'username' => 'required_without:user_id|alpha_dash|min:4|lowercase|unique:\WP_User,login', | |
| 'password' => 'required_without:user_id|min:6', | |
| 'password_conf' => 'same:password', | |
| 'template_id' => 'integer|site_template', | |
| 'products' => 'products', | |
| 'gateway' => '', | |
| 'valid_password' => 'accepted', | |
| 'billing_country' => 'country|required_with:billing_country', | |
| 'billing_zip_code' => 'required_with:billing_zip_code', | |
| 'billing_state' => 'state', | |
| 'billing_city' => 'city', |
🤖 Prompt for AI Agents
In inc/checkout/class-checkout.php around lines 1720 to 1733, the validation
rule for email_address_confirmation only enforces equality when present but does
not require it for new user signups; update the rule to require the confirmation
field when user_id is absent and still validate it matches email_address (i.e.,
add required_without:user_id to the email_address_confirmation rule), leaving
the other rules unchanged.
|
|
||
| <?php | ||
| // translators: %s is the login URL. | ||
| printf(wp_kses_post(__('<a href="%s">Log in</a> to renew or change an existing membership.', 'multisite-ultimate')), esc_attr($login_url)); |
There was a problem hiding this comment.
Escape URLs with esc_url for href attributes.
Use esc_url() instead of esc_attr() to sanitize the login URL.
Apply this diff:
- printf(wp_kses_post(__('<a href="%s">Log in</a> to renew or change an existing membership.', 'multisite-ultimate')), esc_attr($login_url));
+ printf(wp_kses_post(__('<a href="%s">Log in</a> to renew or change an existing membership.', 'multisite-ultimate')), esc_url($login_url));📝 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.
| printf(wp_kses_post(__('<a href="%s">Log in</a> to renew or change an existing membership.', 'multisite-ultimate')), esc_attr($login_url)); | |
| printf( | |
| wp_kses_post( | |
| __( | |
| '<a href="%s">Log in</a> to renew or change an existing membership.', | |
| 'multisite-ultimate' | |
| ) | |
| ), | |
| esc_url( $login_url ) | |
| ); |
🤖 Prompt for AI Agents
In inc/checkout/signup-fields/class-signup-field-email-confirmation.php around
line 295, the href URL is being sanitized with esc_attr() which is intended for
HTML attributes but not optimal for URLs; replace esc_attr($login_url) with
esc_url($login_url) in the printf call so the login link uses esc_url() for the
href attribute, keeping the existing wp_kses_post() and __() wrapper unchanged.
| printf(wp_kses_post(__('Not %1$s? <a href="%2$s">Log in</a> using your account.', 'multisite-ultimate')), esc_html(wp_get_current_user()->display_name), esc_attr($login_url)); | ||
|
|
There was a problem hiding this comment.
Escape URLs with esc_url for href attributes.
Same issue here; switch to esc_url().
Apply this diff:
- printf(wp_kses_post(__('Not %1$s? <a href="%2$s">Log in</a> using your account.', 'multisite-ultimate')), esc_html(wp_get_current_user()->display_name), esc_attr($login_url));
+ printf(wp_kses_post(__('Not %1$s? <a href="%2$s">Log in</a> using your account.', 'multisite-ultimate')), esc_html(wp_get_current_user()->display_name), esc_url($login_url));📝 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.
| printf(wp_kses_post(__('Not %1$s? <a href="%2$s">Log in</a> using your account.', 'multisite-ultimate')), esc_html(wp_get_current_user()->display_name), esc_attr($login_url)); | |
| printf( | |
| wp_kses_post( | |
| __('Not %1$s? <a href="%2$s">Log in</a> using your account.', 'multisite-ultimate') | |
| ), | |
| esc_html( wp_get_current_user()->display_name ), | |
| esc_url( $login_url ) | |
| ); |
🤖 Prompt for AI Agents
In inc/checkout/signup-fields/class-signup-field-email-confirmation.php around
lines 323-324, the printf uses esc_attr() for the URL in the href which should
be escaped with esc_url(); replace esc_attr($login_url) with esc_url($login_url)
so the href attribute is properly escaped (keeping
esc_html(wp_get_current_user()->display_name) as-is).
| public function find_orphaned_users(): array { | ||
|
|
||
| global $wpdb; | ||
|
|
||
| $orphaned_users = []; | ||
|
|
||
| // Get all site IDs in the network | ||
| $site_ids = get_sites( | ||
| [ | ||
| 'fields' => 'ids', | ||
| 'number' => 0, | ||
| ] | ||
| ); | ||
|
|
||
| // Get all users | ||
| $users = get_users( | ||
| [ | ||
| 'number' => '', | ||
| 'fields' => ['ID', 'user_login', 'user_email', 'user_registered'], | ||
| ] | ||
| ); | ||
|
|
||
| foreach ($users as $user) { | ||
| // Skip super administrators - they always have network access | ||
| if (is_super_admin($user->ID)) { | ||
| continue; | ||
| } | ||
|
|
||
| $has_role_on_any_site = false; | ||
|
|
||
| // Check if user has a role on any site in the network | ||
| foreach ($site_ids as $site_id) { | ||
| $user_roles = get_user_meta($user->ID, $wpdb->get_blog_prefix($site_id) . 'capabilities', true); | ||
|
|
||
| if (! empty($user_roles) && is_array($user_roles)) { | ||
| $has_role_on_any_site = true; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // If user has no roles on any site, they are orphaned | ||
| if (! $has_role_on_any_site) { | ||
| $orphaned_users[] = $user; | ||
| } | ||
| } | ||
|
|
||
| return $orphaned_users; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Prevent deleting users tied to WP Ultimo data
Before deletion, consider excluding users that still have WP Ultimo Customer/Membership/Payment records to avoid orphaning plugin data.
Would you like me to scan the codebase for delete hooks (e.g., on delete_user/wpmu_delete_user) that cascade-delete or detach related WP Ultimo entities?
🏁 Script executed:
#!/bin/bash
# Find hooks that react to user deletions and Ultimo models referencing user_id
rg -nP -C2 "add_action\(\s*'(delete_user|wpmu_delete_user)'" --type=php
rg -nP -C2 "\b(get_user_id\(\)|set_user_id\(|user_id\b)" --type=phpLength of output: 91453
Implement user deletion safeguards
No add_action('delete_user'), add_action('deleted_user') or add_action('wpmu_delete_user') hooks found to protect WP Ultimo data; hook into these to prevent deleting users with Customer/Membership/Payment records or to cascade-clean related plugin entities before removal. (developer.wordpress.org)
🤖 Prompt for AI Agents
In inc/class-orphaned-users-manager.php around lines 232-279, add deletion
safeguards by registering handlers for delete_user, deleted_user and
wpmu_delete_user that check for WP Ultimo related data (Customer, Membership,
Payment records) and either prevent deletion or perform safe cascade-cleanup
before removal; implement a pre-delete hook (delete_user) to abort deletion when
related records exist (returning a WP_Error or using wp_die) and a post-delete
hook (deleted_user / wpmu_delete_user) to remove or reassign plugin entities if
deletion is allowed, ensure handlers use capability checks, proper priority and
nonces/context validation, and log actions/errors via existing logger so plugin
data integrity is preserved.
Summary by CodeRabbit