-
Notifications
You must be signed in to change notification settings - Fork 19
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
update_all_cohorts_from_rules includes deleted users #21
Comments
Hi @Lat91 , thank you for this report and sorry for the late reply. We will have a look into this issue and make sure that we find a solution for it. Thanks, |
Also removes incorrectly added users during upgrade
I am still experiencing this issue. For example, for a custom profile checkbox, I create two rules, in order to add everyone on the server to a cohort (I chose to use two rules, because I had the same issue mentioned here: https://moodle.org/mod/forum/discuss.php?d=386022). First rule is defined as: If custom_profile_field Is defined the user will be added to the cohort custom_cohort. Second rule is defined as: If custom_profile_field Is not defined the user will be added to the cohort custom_cohort. It may be a bit of a hacky way to include everyone on the server, but I still observe the issue that the plugin adds already deleted users to the cohort. |
This issue has been finally solved by #27. Cheers, |
The sql in update_all_cohorts_from_rules could include a check that the user is not deleted before adding them to a cohort to avoid adding deleted users to cohorts based on the custom user profile fields.
There is an argument that Moodle should, when a user is deleted, remove those mdl_user_info_data records, however we have seen instances in Moodle databases where this is not the case (i.e. this query returns rows: select * from mdl_user_info_data where userid in (SELECT id FROM mdl_user where deleted=1);)
We recently discovered that this plugin had added deleted users to a cohort which then caused an issue trying to assign a role to members of that cohort.
The text was updated successfully, but these errors were encountered: