search

emmarichardson / local_autogroup

A local plugin for Moodle 2.7 onwards which handles the dynamic creation, population and cleanup of groups on courses.
6 stars 19 forks source link

New Setting: Preserve manual User assignments. #10

Closed gnat closed 5 years ago

gnat commented 5 years ago

When Users are manually assigned to a group managed by autogroup, preserve these assignments when autogroups are automatically updated (other Users added, removed).

gnat commented 5 years ago

@emmarichardson If this looks good to you, feel free to merge it in. We release this code as public domain.

gnat commented 5 years ago

Thank you for your review @ak4t0sh I've made the changes. This should now be ready for you. @emmarichardson

emmarichardson commented 5 years ago

So I am confused - if you turn off the groups setting, doesn't this accomplish the same thing?

senaiboy commented 5 years ago

So I am confused - if you turn off the groups setting, doesn't this accomplish the same thing?

If I understand it right, this might solve our problem discussed a while ago with users belonging to more than one group. Eg Groups can have 'permanent' members (teachers) and temporary members (students). As students change group, Autogroup plugin will move them into their new group accordingly, while the teachers remain in these groups.

Turning off auto-update setting (presumably this is what you're referring to?) means students will remain in their old group as well as in the new groups. The way to overcome this issue is by deleting all groups every new term/year.

Turning on auto-update (currently) only allow teachers to be in 1 group.

With this new feature, we could have both the convenience of auto-updating members of a group as well as assigning permanent members to it.

gnat commented 5 years ago

With this new feature, we could have both the convenience of auto-updating members of a group as well as assigning permanent members to it.

@senaiboy yup, you've got it. This use case is the exact reason this was developed.

It's intuitive because Users who have been manually assigned, remain manual. Users automatically assigned, remain automatic.

emmarichardson commented 5 years ago

Ok..I will try and get it merged and update moodle.org this weekend ..

On Fri, Nov 30, 2018, 11:22 AM Nathaniel Sabanski <notifications@github.com wrote:

With this new feature, we could have both the convenience of auto-updating members of a group as well as assigning permanent members to it.

@senaiboy https://github.com/senaiboy yup, you've got it. This use case is the exact reason this was developed.

It's intuitive because Users who have been manually assigned, remain manual. Users automatically assigned, remain automatic.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/emmarichardson/local_autogroup/pull/10#issuecomment-443293556, or mute the thread https://github.com/notifications/unsubscribe-auth/AB-5mWJdffMH6Y2ZIVU0cTKCVUD7jg5iks5u0XdfgaJpZM4Y4hpu .

emmarichardson commented 5 years ago

@ak4t0sh Could you review the changes and let me know if it looks good? Then I will merge.

ak4t0sh commented 5 years ago

(Technically) Review OK :+1: Thanks for the changes @gnat

Test OK it works as expected.

But I think that it can produce inconstencies. Example :

  1. Create a course
  2. Create an autogroup based on "Preferred Language"
  3. Enrol 2 users with different languages. (English and French)
  4. 2 groups are created : "En" and "Fr" with one user in each group.
  5. Manually add user with "French" language into the "En" group.

Result : The user is in the 2 groups. This could lead to weird behaviour if the course use group based restrictions.

I think that an user should not be added more than 1 group from the same group set. Example :

  1. Create a course
  2. Create an autogroup based on "Preferred Language"
  3. Enrol 2 users with different languages. (English and French)
  4. 2 groups are created : "En" and "Fr" with one user in each group.
  5. Manually add user with "French" language into the "En" group. Expected : The user is remove from group "Fr" and languages changes in profile are ignored.
emmarichardson commented 5 years ago

I think that the way it works is the intended use case. From what I understand, it is so that they can add teachers to multiple groups, for example. Seeing as it is an optional setting, I see no issue with it as long as we have no concerns about database/code confusion. For example, does the setting automatically uncheck the groups setting that looks for changes in the group and updates if need be...I haven't had time to test it fully.

ak4t0sh commented 5 years ago

@emmarichardson if you are OK with this behaviour you can merge :)