Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.8
    • Fix Version/s: 1.9
    • Component/s: Groups
    • Labels:
      None
    • Database:
      Any
    • Affected Branches:
      MOODLE_18_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      15658

      Description

      I am working with Juliette White at the Open University to integrate new code - this is a major overhaul. NOTE, for the moment the existing code/tables will be used by default.

      http://moodle.org/mod/forum/discuss.php?d=48373
      http://docs.moodle.org/en/How_groups_work_in_Moodle
      http://docs.moodle.org/en/Groups_documentation_for_module_developers

        Issue Links

          Activity

          Hide
          Petr Škoda added a comment -

          Some comments:

          • the xml header in GUI should be printed after the require_once('../../config.php');
          • you should also set encoding using @header('Content-Type: text/html; charset=utf-8'); - solves problems in IE with default header encoding in apache conf
          • always use proper parameter types in required_param() and optional_param()
          • I would recommend using data_submitted() to check that data is coming from POST requests
          • javascript should not be required for administrative actions - administrators should IMHO disable java/javascript/flash/activeX to protect themselves from XSS attacks; there should be a way to do everything without javascript (accessibility?)
          • ajax should be optional - not all browsers support it
          Show
          Petr Škoda added a comment - Some comments: the xml header in GUI should be printed after the require_once('../../config.php'); you should also set encoding using @header('Content-Type: text/html; charset=utf-8'); - solves problems in IE with default header encoding in apache conf always use proper parameter types in required_param() and optional_param() I would recommend using data_submitted() to check that data is coming from POST requests javascript should not be required for administrative actions - administrators should IMHO disable java/javascript/flash/activeX to protect themselves from XSS attacks; there should be a way to do everything without javascript (accessibility?) ajax should be optional - not all browsers support it
          Hide
          Juliette White added a comment -

          ________________________________________
          From: J.White
          Sent: 14 November 2006 09:57
          To: 'Martin Dougiamas'; 'skodak@moodle.org'; 'stronk7@moodle.org'; S.Marshall
          Cc: N.D.Freear
          Subject: Moodle groups

          Hi everyone,

          Just wanted to comment on various things related to groups that I know were discussed as I'm in a better position to do so than Nick! If I'd had my say it would have gone into a development branch not HEAD, but hey-ho

          On Javascript and Ajax:

          1) The existing groups code requires Javascript so it doesn't make the situation any worse that it currently is.
          2) Once you're using Javascript, using Ajax both makes the code much cleaner and more maintainable (I originally wrote the user interface with Javascript but not Ajax, and believe me it was horrible in comparison) and makes it much easier for someone else to improve the user interface later (which I hope they will!)
          3) It's hard to do a remotely nice user interface for manipulating groups without Javascript.
          4) I totally agree that the Javascript should degrade gracefully and this is in the to do list that was committed too - however I feel there are other priorities at the moment as there is so much other stuff to do. There's no point having a user interface that doesn't use Javascript if groups don't actually work I also think doing this is a better use of my time than Nick's at the moment as it's an easier spare time activity than lots of the other work. From an accessibility point of view, my personal opinion is that there are more urgent accessibility issues with Moodle but I can see that admins might want to turn off Javascript which means that all Javascript in Moodle should degrade gracefully I guess? Groups are really a teacher function not an admin function after all.

          I think it would be useful for me if the Moodle coding guidelines contained clear instructions that none of the Moodle interface should rely on Javascript.

          On roles and permissions:

          The code was written before roles and permissions, and I've never pretended to anyone that it wasn't I have been thinking about the best way for it to integrate with role and permissions recently though - Nick hasn't committed the newer version of modulelib.php yet. Having said that I'm not all that familiar with the intricacies of roles of permissions so once Nick has committed that it might be useful for people who are to take a look and see if there's a better way of doing things. It'd probably be worth removing the remnants of the stuff I did to try and get rid of the separate and visible groups stuff sooner rather than later I think.

          It would theoretically be possible to implement groups using roles, though I think it'd be a fiddly job to do. I think the group APIs and user interfaces could and should stay the same though - users and module developers should still see groups exposed as groups not as roles. A teacher doesn't want to have to create a new instance of a forum say for each group and create a role for people who can see each forum and then assign students to the right roles (is this what you were thinking of, Sam, or am I missing something obvious?). If anybody wants to write new code for the libraries that uses roles instead, I'd personally be very happy for them to do so.

          And generally:

          If there are things that I've done in a non-Moodlish way, again would it be possible to make sure these are in the coding guidelines to stop other people making the same mistakes? I haven't gone through carefully checking the code against the coding guidelines (another thing on the to-do list) but I did read them before I started and this is something that would have helped catch such stuff earlier rather than later. The style of the existing Moodle code is quite inconsistent across the code base in various ways so it's sometimes hard to deduce what the preferred way of doing things actually is. It's also easy to copy the bad code in Moodle rather than the good code! This has been a bit of a baptism by fire for me and if I'm going to continue being involved in the Moodle community then I'd like to feel I've done my bit to make sure other new folk don't have to go through quite the same ordeal

          Juliette

          Show
          Juliette White added a comment - ________________________________________ From: J.White Sent: 14 November 2006 09:57 To: 'Martin Dougiamas'; 'skodak@moodle.org'; 'stronk7@moodle.org'; S.Marshall Cc: N.D.Freear Subject: Moodle groups Hi everyone, Just wanted to comment on various things related to groups that I know were discussed as I'm in a better position to do so than Nick! If I'd had my say it would have gone into a development branch not HEAD, but hey-ho On Javascript and Ajax: 1) The existing groups code requires Javascript so it doesn't make the situation any worse that it currently is. 2) Once you're using Javascript, using Ajax both makes the code much cleaner and more maintainable (I originally wrote the user interface with Javascript but not Ajax, and believe me it was horrible in comparison) and makes it much easier for someone else to improve the user interface later (which I hope they will!) 3) It's hard to do a remotely nice user interface for manipulating groups without Javascript. 4) I totally agree that the Javascript should degrade gracefully and this is in the to do list that was committed too - however I feel there are other priorities at the moment as there is so much other stuff to do. There's no point having a user interface that doesn't use Javascript if groups don't actually work I also think doing this is a better use of my time than Nick's at the moment as it's an easier spare time activity than lots of the other work. From an accessibility point of view, my personal opinion is that there are more urgent accessibility issues with Moodle but I can see that admins might want to turn off Javascript which means that all Javascript in Moodle should degrade gracefully I guess? Groups are really a teacher function not an admin function after all. I think it would be useful for me if the Moodle coding guidelines contained clear instructions that none of the Moodle interface should rely on Javascript. On roles and permissions: The code was written before roles and permissions, and I've never pretended to anyone that it wasn't I have been thinking about the best way for it to integrate with role and permissions recently though - Nick hasn't committed the newer version of modulelib.php yet. Having said that I'm not all that familiar with the intricacies of roles of permissions so once Nick has committed that it might be useful for people who are to take a look and see if there's a better way of doing things. It'd probably be worth removing the remnants of the stuff I did to try and get rid of the separate and visible groups stuff sooner rather than later I think. It would theoretically be possible to implement groups using roles, though I think it'd be a fiddly job to do. I think the group APIs and user interfaces could and should stay the same though - users and module developers should still see groups exposed as groups not as roles. A teacher doesn't want to have to create a new instance of a forum say for each group and create a role for people who can see each forum and then assign students to the right roles (is this what you were thinking of, Sam, or am I missing something obvious?). If anybody wants to write new code for the libraries that uses roles instead, I'd personally be very happy for them to do so. And generally: If there are things that I've done in a non-Moodlish way, again would it be possible to make sure these are in the coding guidelines to stop other people making the same mistakes? I haven't gone through carefully checking the code against the coding guidelines (another thing on the to-do list) but I did read them before I started and this is something that would have helped catch such stuff earlier rather than later. The style of the existing Moodle code is quite inconsistent across the code base in various ways so it's sometimes hard to deduce what the preferred way of doing things actually is. It's also easy to copy the bad code in Moodle rather than the good code! This has been a bit of a baptism by fire for me and if I'm going to continue being involved in the Moodle community then I'd like to feel I've done my bit to make sure other new folk don't have to go through quite the same ordeal Juliette
          Hide
          Juliette White added a comment -

          ----Original Message----
          From: S.Marshall
          Sent: 14 November 2006 13:00
          To: J.White; 'Martin Dougiamas'; 'skodak@moodle.org'; 'stronk7@moodle.org'
          Cc: N.D.Freear
          Subject: RE: Moodle groups

          > exposed as groups not as roles. A teacher doesn't want to
          > have to create a new instance of a forum say for each group
          > and create a role for people who can see each forum and then
          > assign students to the right roles (is this what you were
          > thinking of, Sam, or am I missing something obvious?). If

          No that's not what I was thinking of. My opinion is just that the table mdl_groups_members should be got rid of, in favour of using mdl_role_assignments (which stores the information about who's on what course etc., so why not who's on what group). Groups would then become 'contexts' for the role system.

          In this case group assignment could reuse the same 'role assignment' screen used for courses and other things. So you'd pick a group, pick a role type from those that apply to groups (maybe just 'group member', maybe also 'group leader'), and select people to assign to it. It would be the exact same interface as for assigning students to a course and would benefit from any improvements to that interface. That's the reason for unifying them.

          This wouldn't imply any change to the way groups work in modules.

          It might imply some changes to permissions (that's where the multiple roles in a group come in) but doesn't necessarily have to. You could keep the exact same group system as now, with only a single role ('Group member') applicable at a group level. The only change would be to the interface for assigning people to groups as this could entirely reuse (or at least extend) the standard assigning-role interface which is already written, thus avoiding any concerns about JavaScript.

          There might be some need for enhancements to that interface so it can support groups in a quality manner, but I think it should be possible to fit it within the same code.

          So basically it's just:

          a) I dislike having two different mechanisms (db tables and UI) that do the exact same thing [assign people to thingies]. Any change to the group system also offers an opportunity to fix that.

          b) At some future date it would be possible to grant people different roles within groups. This might potentially be useful in things like role-playing exercises (teacher assigns students to groups of five, one of whom is a dwarven cleric of Thor^W^W^W^W chair of a UN committee, while the others are committee members, they might have different capabilities in some custom module for this activity). However this is a bit complex and there's an issue of how it overlaps with existing roles such as our tutor role, where having a course role automatically grants you a role in the group too. So I think objective (a) is more important in the near term and we should just have a single 'Group member' role (system-created and hidden for the time being, perhaps).

          --sam

          Show
          Juliette White added a comment - ---- Original Message ---- From: S.Marshall Sent: 14 November 2006 13:00 To: J.White; 'Martin Dougiamas'; 'skodak@moodle.org'; 'stronk7@moodle.org' Cc: N.D.Freear Subject: RE: Moodle groups > exposed as groups not as roles. A teacher doesn't want to > have to create a new instance of a forum say for each group > and create a role for people who can see each forum and then > assign students to the right roles (is this what you were > thinking of, Sam, or am I missing something obvious?). If No that's not what I was thinking of. My opinion is just that the table mdl_groups_members should be got rid of, in favour of using mdl_role_assignments (which stores the information about who's on what course etc., so why not who's on what group). Groups would then become 'contexts' for the role system. In this case group assignment could reuse the same 'role assignment' screen used for courses and other things. So you'd pick a group, pick a role type from those that apply to groups (maybe just 'group member', maybe also 'group leader'), and select people to assign to it. It would be the exact same interface as for assigning students to a course and would benefit from any improvements to that interface. That's the reason for unifying them. This wouldn't imply any change to the way groups work in modules. It might imply some changes to permissions (that's where the multiple roles in a group come in) but doesn't necessarily have to. You could keep the exact same group system as now, with only a single role ('Group member') applicable at a group level. The only change would be to the interface for assigning people to groups as this could entirely reuse (or at least extend) the standard assigning-role interface which is already written, thus avoiding any concerns about JavaScript. There might be some need for enhancements to that interface so it can support groups in a quality manner, but I think it should be possible to fit it within the same code. So basically it's just: a) I dislike having two different mechanisms (db tables and UI) that do the exact same thing [assign people to thingies] . Any change to the group system also offers an opportunity to fix that. b) At some future date it would be possible to grant people different roles within groups. This might potentially be useful in things like role-playing exercises (teacher assigns students to groups of five, one of whom is a dwarven cleric of Thor^W^W^W^W chair of a UN committee, while the others are committee members, they might have different capabilities in some custom module for this activity). However this is a bit complex and there's an issue of how it overlaps with existing roles such as our tutor role, where having a course role automatically grants you a role in the group too. So I think objective (a) is more important in the near term and we should just have a single 'Group member' role (system-created and hidden for the time being, perhaps). --sam
          Hide
          Juliette White added a comment -

          ----Original Message----
          From: J.White
          Sent: 14 November 2006 14:23
          To: S.Marshall; 'Martin Dougiamas'; 'skodak@moodle.org'; 'stronk7@moodle.org'
          Cc: N.D.Freear
          Subject: RE: Moodle groups

          Need to get back to my real work so a few extremely quick thoughts!

          • Behind the scenes, the only issue I see with storing group memberships in the role_assignments table (other than that someone have to change the code) is that lots of existing code currently directly queries the group_members table - if we leave it the same then it'll be a less painful transition for various plug-ins etc.
          • I think a fair amount of work would be required to use the role assignment interface, though making sure that code doing essentially the same thing isn't duplicated would certainly be sensible. If you want to quickly knock up a version that does everything the new interface does then I'd be over the moon however
          • I think most teachers conceptually think about groups not roles when it comes to putting their students into groups for activities - I think it's important that we let them do this without having to be exposed to the idea of roles, even if that's how things are done behind the scenes.
          • You'd need an interface for assigning permissions at a grouping level too (e.g. to duplicate the current visible groups functionality).

          I'll try and write up an explanation of how I think groups and permissions should work at some point. I think the most important thing is to make sure we only require changes to all the modules once and that the API they use is basically correct. I'm concerned that we prioritise our time now on the stuff that's vital or that would be hard to change later.

          I don't mind when all this stuff gets released or even to a certain extent whether it does or not, though it'd be a shame if it doesn't as even if it isn't perfect it'd be an improvement on the existing groups in Moodle and would solve lots of problems that are currently coming up on the forums, including from people trying to decide whether to use Moodle or not. But I also know that your team in the OU doesn't have infinite time to work on this

          Juliette

          Show
          Juliette White added a comment - ---- Original Message ---- From: J.White Sent: 14 November 2006 14:23 To: S.Marshall; 'Martin Dougiamas'; 'skodak@moodle.org'; 'stronk7@moodle.org' Cc: N.D.Freear Subject: RE: Moodle groups Need to get back to my real work so a few extremely quick thoughts! Behind the scenes, the only issue I see with storing group memberships in the role_assignments table (other than that someone have to change the code) is that lots of existing code currently directly queries the group_members table - if we leave it the same then it'll be a less painful transition for various plug-ins etc. I think a fair amount of work would be required to use the role assignment interface, though making sure that code doing essentially the same thing isn't duplicated would certainly be sensible. If you want to quickly knock up a version that does everything the new interface does then I'd be over the moon however I think most teachers conceptually think about groups not roles when it comes to putting their students into groups for activities - I think it's important that we let them do this without having to be exposed to the idea of roles, even if that's how things are done behind the scenes. You'd need an interface for assigning permissions at a grouping level too (e.g. to duplicate the current visible groups functionality). I'll try and write up an explanation of how I think groups and permissions should work at some point. I think the most important thing is to make sure we only require changes to all the modules once and that the API they use is basically correct. I'm concerned that we prioritise our time now on the stuff that's vital or that would be hard to change later. I don't mind when all this stuff gets released or even to a certain extent whether it does or not, though it'd be a shame if it doesn't as even if it isn't perfect it'd be an improvement on the existing groups in Moodle and would solve lots of problems that are currently coming up on the forums, including from people trying to decide whether to use Moodle or not. But I also know that your team in the OU doesn't have infinite time to work on this Juliette
          Hide
          Martin Dougiamas added a comment -

          Assigning to me so I can review Nick's patch

          Show
          Martin Dougiamas added a comment - Assigning to me so I can review Nick's patch
          Hide
          Nick Freear added a comment -

          I've been busy on branch MOODLE_18_GROUPS since 4 december. This merge-patch is the result.

          ...it covers all the modules, calendar, blogs, admin, user directory. Lots of get_records('groups'...) to groups_get_group_name etc.

          • There is a new groups management user-interface.
          • API/ groups libraries ...
          Show
          Nick Freear added a comment - I've been busy on branch MOODLE_18_GROUPS since 4 december. This merge-patch is the result. ...it covers all the modules, calendar, blogs, admin, user directory. Lots of get_records('groups'...) to groups_get_group_name etc. There is a new groups management user-interface. API/ groups libraries ...
          Hide
          Martin Dougiamas added a comment -

          This is in HEAD now so we can work on it there

          Show
          Martin Dougiamas added a comment - This is in HEAD now so we can work on it there
          Hide
          Martin Dougiamas added a comment -

          The number of problems is large and time is short, we may have to revert this patch ....

          Show
          Martin Dougiamas added a comment - The number of problems is large and time is short, we may have to revert this patch ....
          Hide
          Nick Freear added a comment -

          OK, I've fixed 4 out of the 9 current issues. Haven't had time to try Vy's fix for MDL-8048, but the JS looks good. We're making progress!

          Show
          Nick Freear added a comment - OK, I've fixed 4 out of the 9 current issues. Haven't had time to try Vy's fix for MDL-8048 , but the JS looks good. We're making progress!
          Hide
          Martin Dougiamas added a comment -

          Petr has put together a document here outlining some remaining issues:

          http://docs.moodle.org/en/Development:Groups

          Nick, will you be able to work on any of this for 1.9?

          Show
          Martin Dougiamas added a comment - Petr has put together a document here outlining some remaining issues: http://docs.moodle.org/en/Development:Groups Nick, will you be able to work on any of this for 1.9?
          Hide
          Petr Škoda added a comment -

          groupings delayed till 1.9 - see http://docs.moodle.org/en/Development:Groups

          Show
          Petr Škoda added a comment - groupings delayed till 1.9 - see http://docs.moodle.org/en/Development:Groups
          Hide
          Dan Poltawski added a comment -

          Are the further Groups improvements still looking likely for 1.9?

          I have a lot of allocated work time to spend on testing for groups functionality (namely global groups)

          Show
          Dan Poltawski added a comment - Are the further Groups improvements still looking likely for 1.9? I have a lot of allocated work time to spend on testing for groups functionality (namely global groups)
          Hide
          Enrique Castro added a comment -

          Groupings need to be put in use for 1.9. Current implementation is only half-way: groupings are created but not used at all.

          At ULPGC we have a working implementation of Grouping in use to get two new features:
          a) activity-specific groupings: making each module instance to declare and use a different set of groups
          b) group-based content delivery
          A diff containing the working code is located in MDL-10383.

          I have re-written the whole Groups/groupings spec to try to get into a common grounds and set a definite pathway to incorporate all new features into new versions. The spec is at http://docs.moodle.org/en/Development:Groupings_and_Groups. Please, edit that as needed to get to a common view of what groups features we want to see in Moodle.

          Show
          Enrique Castro added a comment - Groupings need to be put in use for 1.9. Current implementation is only half-way: groupings are created but not used at all. At ULPGC we have a working implementation of Grouping in use to get two new features: a) activity-specific groupings: making each module instance to declare and use a different set of groups b) group-based content delivery A diff containing the working code is located in MDL-10383 . I have re-written the whole Groups/groupings spec to try to get into a common grounds and set a definite pathway to incorporate all new features into new versions. The spec is at http://docs.moodle.org/en/Development:Groupings_and_Groups . Please, edit that as needed to get to a common view of what groups features we want to see in Moodle.
          Hide
          Martin Dougiamas added a comment -

          Petr's taking care of integrating code into core.

          Show
          Martin Dougiamas added a comment - Petr's taking care of integrating code into core.
          Hide
          Martin Dougiamas added a comment -

          Looks like we can close this!

          Show
          Martin Dougiamas added a comment - Looks like we can close this!

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: