Fix two activation bugs in bp_core_activate_signup()

[GaryJones]

Problem

bp_core_activate_signup() in bp-members/bp-members-functions.php has two related bugs that cause activation failures and data loss during user registration.

1. Xprofile data lost for backcompat-created users

When a user is pre-created by BP_Signup::add_backcompat(), the $user_already_created flag is set and the function returns early. The xprofile data persistence block sits after this early return, so profile field data from signup meta is silently discarded. Users complete registration with all their custom profile fields, but the data never makes it into the xprofile tables.

The fix moves the xprofile data block to before the $user_already_created early return, so the data is saved regardless of how the user was created.

2. False activation failure when user_status is already 0

The function treats zero affected rows from UPDATE ... SET user_status = 0 as an activation error and returns WP_Error('invalid_key'). However, wp_update_user() can run during registration and reset user_status to 0 before activation occurs. When this happens, the UPDATE is a no-op (the value is already 0), $wpdb->query() returns 0, and the user receives an "Invalid activation key" error despite having a perfectly valid key.

Since the activation key has already been validated by this point in the function, zero affected rows is not an error — the user is legitimate either way. The fix removes the return-value check from the query.

Test plan

• Register a new user with custom xprofile fields filled in
• Activate the account and verify the xprofile data is present on their profile
• Test activation where user_status is already 0 (e.g. because a plugin called wp_update_user() during registration) — should succeed instead of returning "Invalid activation key"
• Verify normal activation flow (where user_status is non-zero) still works correctly

[welcome]

Thanks for your pull request. It looks like this may be your first contribution to the BuddyBoss Platform open source project. Please note that this project and all contributions to it are public and bounded by the GPL v2.0 license, and that a record of the contribution (including all personal information you submit with it, including your full name and email address) is maintained indefinitely and may be redistributed with this project. If you are not okay with these terms, please close this pull request. Alternatively, you can let us know about your concerns by adding a comment to this pull request.

[chetansatasiya]

@claude Review the PR

[claude]

Claude finished @chetansatasiya's task in 2m 10s —— View job

---

PR Review: Fix two activation bugs in bp_core_activate_signup()

• Gather PR context
• Read the changed file
• Analyze the diff
• Provide review feedback

---

Summary

This PR addresses two legitimate bugs in bp_core_activate_signup() and is well-written overall. The PR description is clear, the code comments are excellent, and there's a bonus cleanup included. One concern is raised below about a potential multisite regression.

---

Bug 1 Fix: Xprofile data lost for backcompat-created users ✅

Correct fix. In the original code, the xprofile persistence block sat after the $user_already_created early return (src/bp-members/bp-members-functions.php:2140). When BP_Signup::add_backcompat() pre-creates a user, the $user_already_created flag causes the function to return before ever saving profile field data from signup meta.

The fix moves the xprofile block to lines 2111–2138, before the early return, ensuring data is always written regardless of creation path. This is the correct approach.

Bonus fix included: The original code at the old location used $key = "field_{$field_id}_visibility" inside the loop, which silently shadowed the function's $key parameter (the activation key). The PR renames this to $visibility_key, which eliminates the variable shadowing entirely. This is a welcome cleanup.

---

Bug 2 Fix: False activation failure when user_status is already 0 ✅

Correct fix. The original check:

// src/bp-members/bp-members-functions.php (original)
if ( ! $wpdb->query( $wpdb->prepare( "UPDATE {$wpdb->users} SET user_status = 0 WHERE ID = %d", $user_id ) ) ) {
    return new WP_Error( 'invalid_key', __( 'Invalid activation key.', 'buddyboss' ) );
}

$wpdb->query() returns the number of affected rows, not a success/failure boolean. When user_status is already 0, the UPDATE affects zero rows and returns 0, causing a false invalid_key error even though the activation key was already validated just above (line 2050).

The fix drops the check entirely. The inline comment clearly explains the rationale. One minor note: $wpdb->query() returns false on a genuine DB failure, which is now silently ignored — but given we're inside a validated user branch, this is acceptable and consistent with other $wpdb->query() calls in the function (e.g. line 2158).

---

Concern: Potential Multisite Regression ⚠️

In the original code, the xprofile data block sat outside the if ( is_multisite() ) { ... } else { ... } structure, meaning it ran for both multisite and non-multisite users after successful activation. The new code places the block inside the else branch, so it now only runs for non-multisite users.

For multisite, wpmu_activate_signup() (a WordPress core function) handles user creation but knows nothing about BuddyBoss xprofile fields. If a multisite signup stored profile_field_ids in signup meta, that data would have been saved on activation in the original code. With this PR, it is not saved.

Please confirm one of the following:

1. Multisite xprofile data is handled elsewhere (e.g. via a hook on wpmu_activate_user), making this change safe.
2. This is an intentional scope reduction (the bug being fixed is non-multisite only, and multisite xprofile saving was already broken/unsupported).
3. This is an unintentional regression that should be addressed before merging.

Investigate multisite xprofile path →

---

Overall Assessment

The two core fixes are correct and well-motivated. Code quality is high — the PR description matches the implementation, comments are clear, and the variable rename is a bonus improvement. The only item requiring clarification before merge is whether moving the xprofile block inside the else branch affects multisite users.