fix: improve logic for donor and user profile disconnection

[denisjacobi]

Bug Report

User Story

Sigh, user stories... 🙄... Ok. As a developer, I want code without Bugs 😏

Current Behavior

This does not impact functionality.

However there is a logic bug in includes/admin/donors/donor-actions.php in function give_disconnect_donor_user_id() line 313 of Give 2.4.7 in:

if ( ! $donor->update( $donor_args ) ) {
		update_user_meta( $user_id, '_give_is_donor_disconnected', true );
		update_user_meta( $user_id, '_give_disconnected_donor_id', $donor->id );
		$donor->update_meta( '_give_disconnected_user_id', $user_id );

		$output['success'] = true;

	} else {
		$output['success'] = false;
		give_set_error( 'give-disconnect-user-fail', __( 'Failed to disconnect user from donor.', 'give' ) );
	}

When disconnecting a donor in the admin panel if ( ! $donor->update( $donor_args ) ) does the actual update_meta to set the user_id meta_value to 0. The update will go through successfully and it will always return true. Since you check for false, it will never go into that block and instead incorrectly do the else block which is intended for failure. Wy the error that’s set doesn’t show, I don’t know. Didn’t look into that.

How I realized this is that I was wondering why in includes/admin/donors/donor-functions.php in function give_connect_user_donor_profile() you are checking and deleting metas:

_give_disconnected_user_id
_give_is_donor_disconnected
_give_disconnected_donor_id

Looking at the database these meta keys are never present anywhere for a user. That is because of the incorrect true/false check in give_disconnect_donor_user_id().

Honestly, I can’t even figure out why you wanted to set those 3 metas in the first place. I don’t see any other functions that would use them, so they seem redundant??

Bug Type

• I am not sure whether this functionality ever worked as expected.

Steps to Reproduce

1. Look at the code.

Possible Solution

Though I didn't test it, I'd assume change the if statement to

if ( $donor->update( $donor_args ) ) {

or maybe rethink what those _give_disconnected_user_id, etc were supposed to be used for?

Acceptance Criteria

• Improve logic for user and donor profile disconnection
• Fix the new bug found while investigating the issue - Reconnect user & donor disconnection email is not sent!
• Testing

Environment

Give version 2.4.7