Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Allow half-merged user to be accessed #22105

Merged
merged 1 commit into from Jun 14, 2023
Merged

Conversation

nattsw
Copy link
Contributor

@nattsw nattsw commented Jun 14, 2023

This is somewhat of an uncommon case where if a plugin fails to merge a user, the merge would fail but the user_email of the source_user would have been transferred to the target_user. This leaves the source_user without an email, and this bug would occur when admins try to access /admin/users/<id>/<username>.

This allows the admin to continue dealing with the user.

https://meta.discourse.org/t/error-merging-users-with-duplicate-poll-votes/154711

A consideration here would be to use a transaction (probably very costly), or reorder the order of events in UserMerger.merge!

@nattsw nattsw changed the title FIX: Allow half-deleted user to be accessed for deletion FIX: Allow half-merged user to be accessed for deletion Jun 14, 2023
@nattsw nattsw changed the title FIX: Allow half-merged user to be accessed for deletion FIX: Allow half-merged user to be accessed Jun 14, 2023
Copy link
Contributor

@jjaffeux jjaffeux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, for sure transaction would probably be the most correct.

@nattsw
Copy link
Contributor Author

nattsw commented Jun 14, 2023

Thanks for reviewing @jjaffeux

@discoursebot
Copy link

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/error-merging-users-with-duplicate-poll-votes/154711/10

@nattsw nattsw merged commit 3fe06bb into main Jun 14, 2023
13 checks passed
@nattsw nattsw deleted the user-merger-email-empty branch June 14, 2023 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants