payload: {"action":"opened","issue":{"url":"https://api.github.com/repos/impress-org/give/issues/4113","repository_url":"https://api.github.com/repos/impress-org/give","labels_url":"https://api.github.com/repos/impress-org/give/issues/4113/labels{/name}","comments_url":"https://api.github.com/repos/impress-org/give/issues/4113/comments","events_url":"https://api.github.com/repos/impress-org/give/issues/4113/events","html_url":"https://github.com/impress-org/give/issues/4113","id":446507772,"node_id":"MDU6SXNzdWU0NDY1MDc3NzI=","number":4113,"title":"Bug in give_disconnect_donor_user_id() ","user":{"login":"denisjacobi","id":15694573,"node_id":"MDQ6VXNlcjE1Njk0NTcz","avatar_url":"https://avatars0.githubusercontent.com/u/15694573?v=4","gravatar_id":"","url":"https://api.github.com/users/denisjacobi","html_url":"https://github.com/denisjacobi","followers_url":"https://api.github.com/users/denisjacobi/followers","following_url":"https://api.github.com/users/denisjacobi/following{/other_user}","gists_url":"https://api.github.com/users/denisjacobi/gists{/gist_id}","starred_url":"https://api.github.com/users/denisjacobi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/denisjacobi/subscriptions","organizations_url":"https://api.github.com/users/denisjacobi/orgs","repos_url":"https://api.github.com/users/denisjacobi/repos","events_url":"https://api.github.com/users/denisjacobi/events{/privacy}","received_events_url":"https://api.github.com/users/denisjacobi/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2019-05-21T09:19:02Z","updated_at":"2019-05-21T09:19:02Z","closed_at":null,"author_association":"NONE","body":"# Bug Report\r\n\r\n## User Story\r\n<!-- Required. -->\r\nSigh, user stories... :roll_eyes:... Ok. As a developer, I want code without Bugs :smirk:\r\n\r\n<!-- Expand on the user story if necessary. -->\r\n\r\n## Current Behavior\r\n**This does not impact functionality.**\r\n\r\nHowever 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:\r\n\r\n```\r\nif ( ! $donor->update( $donor_args ) ) {\r\n\t\tupdate_user_meta( $user_id, '_give_is_donor_disconnected', true );\r\n\t\tupdate_user_meta( $user_id, '_give_disconnected_donor_id', $donor->id );\r\n\t\t$donor->update_meta( '_give_disconnected_user_id', $user_id );\r\n\r\n\t\t$output['success'] = true;\r\n\r\n\t} else {\r\n\t\t$output['success'] = false;\r\n\t\tgive_set_error( 'give-disconnect-user-fail', __( 'Failed to disconnect user from donor.', 'give' ) );\r\n\t}\r\n```\r\n\r\nWhen 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.\r\n\r\nHow 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: \r\n```\r\n_give_disconnected_user_id\r\n_give_is_donor_disconnected\r\n_give_disconnected_donor_id\r\n```\r\nLooking 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().\r\n\r\nHonestly, 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??\r\n\r\n## Bug Type\r\n<!-- Required. Include version number if functionality worked as expected in previous version. -->\r\n- [x] I am not sure whether this functionality ever worked as expected.\r\n\r\n## Steps to Reproduce\r\n<!-- Required. -->\r\n1. Look at the code.\r\n\r\n## Possible Solution\r\nThough I didn't test it, I'd assume change the if statement to\r\n\r\n`if ( $donor->update( $donor_args ) ) {`\r\n\r\nor maybe rethink what those _give_disconnected_user_id, etc were supposed to be used for?\r\n\r\n## Acceptance Criteria\r\n<!-- Required. Include a checklist of conditions that must be true in order to close this issue. -->\r\n- [ ] Implement as intended.\r\n\r\n## Environment\r\n<!-- Required. -->\r\n\r\nGive version 2.4.7"}}
other: {"actor":{"display_login":"denisjacobi"}}