Moodle

Role unassign doesn't remove users from groups if contextlevel != course

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.9.5, 1.9.6
  • Fix Version/s: None
  • Component/s: Roles / Access
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE

Description

As posted on development forum (http://moodle.org/mod/forum/discuss.php?d=134417) , the function role_unassign only check for group membership if the contextlevel is the course.
I am posting a patch that do this job for every contextlevel < COURSE , and finds all the child contexts, iterates over them and in the course ones checks the capability and do the group membership removal.
Hope you like.

Activity

Hide
Daniel Neis added a comment -

This second patch (ra_groups_remove_v2.diff) should be applied instead of the first one, cause it tests if context_childs is really an array, avoiding php warnings

Show
Daniel Neis added a comment - This second patch (ra_groups_remove_v2.diff) should be applied instead of the first one, cause it tests if context_childs is really an array, avoiding php warnings
Hide
Anthony Borrow added a comment -

Nicolas - Here is another patch from Daniel which I thought might be helpful to look at before 1.9.6. It looked like you had looked at one of his other patches regarding something similar; however, I understand if you do not have time. Daniel asked if I would help ensure that it at least received a quick glance before 1.9.6. I looked at the patch and it did seem to have a typo in the foreach line.

+ if (!$child_contexts = get_child_contexts($context)) {
+ foreach ($childe_contexts as $cc) {

I suspect it should be

+ foreach ($child_contexts as $cc) {

Peace - Anthony

Show
Anthony Borrow added a comment - Nicolas - Here is another patch from Daniel which I thought might be helpful to look at before 1.9.6. It looked like you had looked at one of his other patches regarding something similar; however, I understand if you do not have time. Daniel asked if I would help ensure that it at least received a quick glance before 1.9.6. I looked at the patch and it did seem to have a typo in the foreach line. + if (!$child_contexts = get_child_contexts($context)) { + foreach ($childe_contexts as $cc) { I suspect it should be + foreach ($child_contexts as $cc) { Peace - Anthony
Hide
Daniel Neis added a comment -

Hello, Anthony

thanks for the attention and for schedule it for 1.9.6 , and sorry for the typo.

=O)

Show
Daniel Neis added a comment - Hello, Anthony thanks for the attention and for schedule it for 1.9.6 , and sorry for the typo. =O)
Hide
Anthony Borrow added a comment -

Daniel - Actually I will let Nicolas make the determination whether to change the Fix version to 1.9.6 but I am happy to give this a little bump as the more little things we can clean up for 1.9.6 the better. Peace - Anthony

Show
Anthony Borrow added a comment - Daniel - Actually I will let Nicolas make the determination whether to change the Fix version to 1.9.6 but I am happy to give this a little bump as the more little things we can clean up for 1.9.6 the better. Peace - Anthony
Hide
Daniel Neis added a comment -

Hello,

is someone looking at this yet?
I did some more tests and that condition before the foreach mentioned by Anthony has no sense.
So, we must remove the " IF ! " condition and just take the child contexts.

Show
Daniel Neis added a comment - Hello, is someone looking at this yet? I did some more tests and that condition before the foreach mentioned by Anthony has no sense. So, we must remove the " IF ! " condition and just take the child contexts.
Hide
Daniel Neis added a comment -

Hello, Eloy and Petr

i am adding you as watchers, because i don't have find a leader of "course module", so if you can take a look at this, i will be really happy.
Thanks.

Show
Daniel Neis added a comment - Hello, Eloy and Petr i am adding you as watchers, because i don't have find a leader of "course module", so if you can take a look at this, i will be really happy. Thanks.
Hide
Daniel Neis added a comment -

sorry, the maintainer should be the one for "roles" instead of "course", but i think there is no one too.

Show
Daniel Neis added a comment - sorry, the maintainer should be the one for "roles" instead of "course", but i think there is no one too.
Hide
Michael de Raadt added a comment -

Thanks for reporting this issue.

We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported.

If you believe that this issue is still relevant to current versions (2.1 and beyond), please comment on the issue. Issues left inactive for a further month will be closed.

Michael d;

lqjjLKA0p6

Show
Michael de Raadt added a comment - Thanks for reporting this issue. We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported. If you believe that this issue is still relevant to current versions (2.1 and beyond), please comment on the issue. Issues left inactive for a further month will be closed. Michael d; lqjjLKA0p6
Hide
Anthony Borrow added a comment -

Daniel - Would you mind providing testing instructions so that we can check and verify if this is still an issue in 2.x? If it is, then I think it would be good to keep it open and work on it; however, if it has already been addressed then I think it would be OK to just move on or fix it in 1.9. Peace - Anthony

Show
Anthony Borrow added a comment - Daniel - Would you mind providing testing instructions so that we can check and verify if this is still an issue in 2.x? If it is, then I think it would be good to keep it open and work on it; however, if it has already been addressed then I think it would be OK to just move on or fix it in 1.9. Peace - Anthony
Hide
Daniel Neis added a comment -

Hello, Anthony

here is what is needed to test:

  • Enrol a user in a category (or at system level)
  • Add the user as member of a group
  • Unenrol the user from category (or system level)
  • Enrol the user back to the category (or system level)

In Moodle 1.9.x, the users would still be member of the group. In Moodle 2.x things are slightly different: users with role assigned at system level cannot be added to a group unless it is enrolled to the course; the enrolment in category context is done as a cohort sync and in my tests, a manual enrol in a category creates a manual enrol in the course that is not removed when i remove from category. The user is removed from all groups it is member when the last enrol is removed (line 1360 of lib/enrollib.php).
So, i think this is not a bug anymore in 2.2.1. The only thing that seems strange is that the enrol from course is not deleted when i delete from category, but it may be a misconfiguration issue.

HTH,
Daniel

Show
Daniel Neis added a comment - Hello, Anthony here is what is needed to test:
  • Enrol a user in a category (or at system level)
  • Add the user as member of a group
  • Unenrol the user from category (or system level)
  • Enrol the user back to the category (or system level)
In Moodle 1.9.x, the users would still be member of the group. In Moodle 2.x things are slightly different: users with role assigned at system level cannot be added to a group unless it is enrolled to the course; the enrolment in category context is done as a cohort sync and in my tests, a manual enrol in a category creates a manual enrol in the course that is not removed when i remove from category. The user is removed from all groups it is member when the last enrol is removed (line 1360 of lib/enrollib.php). So, i think this is not a bug anymore in 2.2.1. The only thing that seems strange is that the enrol from course is not deleted when i delete from category, but it may be a misconfiguration issue. HTH, Daniel

People

Vote (0)
Watch (6)

Dates

  • Created:
    Updated: