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

Deleted users appear in search results #273

Open
leonstr opened this issue Jul 23, 2024 · 3 comments
Open

Deleted users appear in search results #273

leonstr opened this issue Jul 23, 2024 · 3 comments

Comments

@leonstr
Copy link
Contributor

leonstr commented Jul 23, 2024

When searching users to select which accounts to merge deleted users appear:

usersearch

In this screenshot we see user 99, a normal user, user 100 which has been deleted, and user 101 which has been suspended.

If there is a use-case for merging with deleted accounts the search results should indicate this, similar to how suspended accounts are greyed. The current appearance is confusing unless the site admin is familiar with the changes Moodle makes to mdl_user.username and mdl_user.email when deleting a user.

If there is no use-case for merging with deleted accounts then deleted users should be excluded from the search results.

@jpahullo
Copy link
Owner

Thanks for the reporting @leonstr .

This is unitentional. Deleted users should not be listed. Any of the other users must be listed. Imagine you merge 2 users in the inverse order. So, proceeding with the inverse merging operation should leave things as they should initially. So, for this reason, only suspended and non suspended users must be listed.

If you believe you can report a patch to solve this, it would be easier and quicker to get solved. Otherwise, we get it recorded to address it when we have availability.

Thanks a lot in advance,

Jordi

@leonstr
Copy link
Contributor Author

leonstr commented Jul 24, 2024

@jpahullo The commit for the PR to fix #275 is built upon the commit to fix this issue as 1) they're directly related, and 2) they both touch the same code. I hope these two changes are not confusing!

@jpahullo
Copy link
Owner

Ok about your above comment. Thanks a lot for working on it!

Please, close the PR that finally is not useful by now by your own, so I finally do not get confused in the future.

Let me know analyse the PR #276 then.

Thanks again.

Jordi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants