Moodle
  1. Moodle
  2. MDL-8312

Provide basic Assign Role that only shows roles appropriate to context level

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.8
    • Fix Version/s: 2.0
    • Component/s: Roles / Access
    • Labels:
      None
    • Affected Branches:
      MOODLE_18_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      32964

      Description

      Assign Roles is more complex than it needs to be, because it always shows all roles that the current user is allowed to assign. For example if you do Assign Role for a course it shows 'Administrator' as an option, but that role isn't usually appropriate at course level. Similarly, it shows 'Student' at site level, where that role isn't usually appropriate.

      There might be special cases where that kind of role assignment is useful but they are the exception not the rule. So I think it would be useful to have a 'Basic/Advanced' kind of set up for Assign Roles, where by default it shows only roles that usually apply at a given contextlevel, but if you click Advanced, you can assign others. Alternatively, maybe those special cases are so rare that it doesn't matter, and no Advanced mode is needed.

      In order to configure this system, as part of defining a role, there would be a set of checkboxes for each contextlevel (site, category, course, module, etc). This would control entries in a database table such as mdl_role_contexts (role, contextlevel). That table could then be used to filter the display on the assign roles screen.

      (Note that the OU may have time to implement this. If moodle hq would like us to, please let me know.)

        Issue Links

          Activity

          Hide
          Martin Dougiamas added a comment -

          > In order to configure this system, as part of defining a role, there would be a set of checkboxes for each contextlevel (site, category, course, module, etc). This would control entries in a database table such as mdl_role_contexts (role, contextlevel). That table could then be used to filter the display on the assign roles screen.

          That makes sense to me. I'm seeing these as extra tabs on the define roles page ...

          ...before that happens though, are you sure the cases that prompted this couldn't have been solved by the assignable roles settings? Things like "Admin" are easy to remove from most teachers, say.

          Show
          Martin Dougiamas added a comment - > In order to configure this system, as part of defining a role, there would be a set of checkboxes for each contextlevel (site, category, course, module, etc). This would control entries in a database table such as mdl_role_contexts (role, contextlevel). That table could then be used to filter the display on the assign roles screen. That makes sense to me. I'm seeing these as extra tabs on the define roles page ... ...before that happens though, are you sure the cases that prompted this couldn't have been solved by the assignable roles settings? Things like "Admin" are easy to remove from most teachers, say.
          Hide
          Sam Marshall added a comment -

          Unfortunately it's our admins that are confused We have a team of about ten service administrators who look after the day-to-day usage of the site in conjunction with the course staff (who are Moodle 'teachers'). So our admins aren't all experts in the role system. And I keep getting asked 'why does it have (role x) in (context y)? that doesn't make sense'.

          You're right, the situation is already OK for teacher-equivalents. I do think though that maybe inexperienced Moodle admins would also benefit from this (if it were set reasonably for the default roles).

          Show
          Sam Marshall added a comment - Unfortunately it's our admins that are confused We have a team of about ten service administrators who look after the day-to-day usage of the site in conjunction with the course staff (who are Moodle 'teachers'). So our admins aren't all experts in the role system. And I keep getting asked 'why does it have (role x) in (context y)? that doesn't make sense'. You're right, the situation is already OK for teacher-equivalents. I do think though that maybe inexperienced Moodle admins would also benefit from this (if it were set reasonably for the default roles).
          Hide
          John Isner added a comment -

          Confusion over the Admin role at the course level resulted in the following scenario. I asked the admin of a 1.7.1+ site to make me an admin so I could help out with a problem they were having with one of their courses. Not understanding the difference between site and course, the administrator made me admin in the course. When I logged into the site, I could immediately see that I was not admin, but I proceeded to the problem course. Naturally, Moodle asked me if I wanted to enroll, and stupidly, I said yes. Once inside the course, I was listed as a student (and had only student privileges), yet I could see in my profile->roles that I was also in the administrator role. Does student trump administrator at the course level? Anyway, it was very confusing. Eliminating role x that doesn't make sense in context y would reduce the confusion.

          Show
          John Isner added a comment - Confusion over the Admin role at the course level resulted in the following scenario. I asked the admin of a 1.7.1+ site to make me an admin so I could help out with a problem they were having with one of their courses. Not understanding the difference between site and course, the administrator made me admin in the course. When I logged into the site, I could immediately see that I was not admin, but I proceeded to the problem course. Naturally, Moodle asked me if I wanted to enroll, and stupidly, I said yes. Once inside the course, I was listed as a student (and had only student privileges), yet I could see in my profile->roles that I was also in the administrator role. Does student trump administrator at the course level? Anyway, it was very confusing. Eliminating role x that doesn't make sense in context y would reduce the confusion.
          Hide
          Petr Škoda added a comment -

          I am afraid that this feature would contribute to new types of confusion in admin interface

          My +1 for educating people before assigning them Administrator roles
          My +100 for improving of roles related docs

          Show
          Petr Škoda added a comment - I am afraid that this feature would contribute to new types of confusion in admin interface My +1 for educating people before assigning them Administrator roles My +100 for improving of roles related docs
          Hide
          Petr Škoda added a comment -

          hmm, maybe two checkboxes in the allow assign table - one for system context the other for the rest

          Show
          Petr Škoda added a comment - hmm, maybe two checkboxes in the allow assign table - one for system context the other for the rest
          Hide
          Sam Marshall added a comment -

          Educating people past an awful interface is never a good solution. Here are all the roles that are available on our system at course level to admin users:

          Administrator
          Course creator
          Course staff
          Tutor
          Student
          Guest
          Cannot edit
          Workflow editing
          Search spider
          Newsfeed approver
          Newsfeed poster
          Logged-in users

          Here are the roles which actually apply at course level for admin users:

          Course staff
          Tutor
          Student
          Workflow editing
          Cannot edit

          An interface that offers people 12 options when only 5 are ever valid choices is not a good interface. You're really talking about two orthogonal features:

          • who should be allowed to assign a role
          • where the role makes sense to be assigned

          The first of these is handled OK in current interface, the second isn't.

          Show
          Sam Marshall added a comment - Educating people past an awful interface is never a good solution. Here are all the roles that are available on our system at course level to admin users: Administrator Course creator Course staff Tutor Student Guest Cannot edit Workflow editing Search spider Newsfeed approver Newsfeed poster Logged-in users Here are the roles which actually apply at course level for admin users: Course staff Tutor Student Workflow editing Cannot edit An interface that offers people 12 options when only 5 are ever valid choices is not a good interface. You're really talking about two orthogonal features: who should be allowed to assign a role where the role makes sense to be assigned The first of these is handled OK in current interface, the second isn't.
          Hide
          Petr Škoda added a comment -

          One more menu selector in role definition form?

          Assignable in contexts : Anywhere | System context only | Course category and above | Course and above

          Show
          Petr Škoda added a comment - One more menu selector in role definition form? Assignable in contexts : Anywhere | System context only | Course category and above | Course and above
          Hide
          Petr Škoda added a comment -

          hmm, that would not solve it I guess - maybe a new page then as you proposed

          do you have anything implemented already?

          Show
          Petr Škoda added a comment - hmm, that would not solve it I guess - maybe a new page then as you proposed do you have anything implemented already?
          Hide
          Sam Marshall added a comment -

          No, nothing implemented yet, sorry. But it (or some such solution) has been requested here, so depending on how they prioritise their requests, we may need to do something about it in the next few months.

          I think it would be better if the checkboxes I initially proposed (not wedded to that precise interface, but went on the individual role definition page rather than in a scary table like the who-can-assign-roles one. But it could go in a scary table if that was more appropriate.

          Show
          Sam Marshall added a comment - No, nothing implemented yet, sorry. But it (or some such solution) has been requested here, so depending on how they prioritise their requests, we may need to do something about it in the next few months. I think it would be better if the checkboxes I initially proposed (not wedded to that precise interface, but went on the individual role definition page rather than in a scary table like the who-can-assign-roles one. But it could go in a scary table if that was more appropriate.
          Hide
          Petr Škoda added a comment -

          I was thinking more about this and I agree that it could be useful for many ppl. If we could combine it with MDL-7859 , then the role definition list could show only capabilities that are appropriate for contexts where the role can be assigned and lower - example: Student role would not have a blog:create because student assignable only at course level && blog:create works only in SYSTEM_CONTEXT

          Show
          Petr Škoda added a comment - I was thinking more about this and I agree that it could be useful for many ppl. If we could combine it with MDL-7859 , then the role definition list could show only capabilities that are appropriate for contexts where the role can be assigned and lower - example: Student role would not have a blog:create because student assignable only at course level && blog:create works only in SYSTEM_CONTEXT
          Hide
          Tim Hunt added a comment -

          I am wondering whether we can achieve a partial solution to this in the short term without requiring database changes.

          Useing fetch_context_capabilities, we can get a list of capabilites relevant to a particular context. Then from that, we can get a list of all the roles change whether you can do that capabilit. And that would give a slightly shorter list of roles to go in the dropdown.

          However, on further though (looking at sams list of 13 roles above) this will not be very effective.

          So actually, we do need the solution already talked about: a new table role_relevent_context_levels, with columns id, role and context level.

          And on the role editing form, a multiselect list box (or some check boxes) where you can say which context level this role should be assignable at. Based on what people put there, you create rows in role_relevent_context_levels.

          Then you use that table to determine which roles to display on the role assignment and override screen for a particular context.

          But it involves a database change, so this will have to wait until Moodle 2.0.

          Show
          Tim Hunt added a comment - I am wondering whether we can achieve a partial solution to this in the short term without requiring database changes. Useing fetch_context_capabilities, we can get a list of capabilites relevant to a particular context. Then from that, we can get a list of all the roles change whether you can do that capabilit. And that would give a slightly shorter list of roles to go in the dropdown. However, on further though (looking at sams list of 13 roles above) this will not be very effective. So actually, we do need the solution already talked about: a new table role_relevent_context_levels, with columns id, role and context level. And on the role editing form, a multiselect list box (or some check boxes) where you can say which context level this role should be assignable at. Based on what people put there, you create rows in role_relevent_context_levels. Then you use that table to determine which roles to display on the role assignment and override screen for a particular context. But it involves a database change, so this will have to wait until Moodle 2.0.
          Hide
          Martin Dougiamas added a comment -

          Tim can you look into this again?

          Show
          Martin Dougiamas added a comment - Tim can you look into this again?
          Hide
          Tim Hunt added a comment -

          Yes. I think we should have a set of check-boxes on the role definition form, like:

          This role may be assigned in the following contexts:
          [] System
          [] Course categories
          [] Courses
          [] Activities
          [] Blocks
          [] User

          With a new database table role_context_levels with columns id, roleid, contextlevel. The state of the checkbox is indicated by the presence/absence of a row in this table.

          Show
          Tim Hunt added a comment - Yes. I think we should have a set of check-boxes on the role definition form, like: This role may be assigned in the following contexts: [] System [] Course categories [] Courses [] Activities [] Blocks [] User With a new database table role_context_levels with columns id, roleid, contextlevel. The state of the checkbox is indicated by the presence/absence of a row in this table.
          Hide
          Tim Hunt added a comment -

          Helen, you probably have a good picture of what people actually do with roles. What do you think a sensible set of defaults for which roles can be assigned where would be? My first guess would be:

          set_role_contextlevels($adminrole, array(CONTEXT_SYSTEM));
          set_role_contextlevels($coursecreatorrole, array(CONTEXT_SYSTEM, CONTEXT_COURSECAT));
          set_role_contextlevels($editteacherrole, array(CONTEXT_COURSE, CONTEXT_MODULE));
          set_role_contextlevels($noneditteacherrole, array(CONTEXT_COURSE, CONTEXT_MODULE));
          set_role_contextlevels($studentrole, array(CONTEXT_COURSE, CONTEXT_MODULE));
          set_role_contextlevels($guestrole, array());

          What do you think?

          Show
          Tim Hunt added a comment - Helen, you probably have a good picture of what people actually do with roles. What do you think a sensible set of defaults for which roles can be assigned where would be? My first guess would be: set_role_contextlevels($adminrole, array(CONTEXT_SYSTEM)); set_role_contextlevels($coursecreatorrole, array(CONTEXT_SYSTEM, CONTEXT_COURSECAT)); set_role_contextlevels($editteacherrole, array(CONTEXT_COURSE, CONTEXT_MODULE)); set_role_contextlevels($noneditteacherrole, array(CONTEXT_COURSE, CONTEXT_MODULE)); set_role_contextlevels($studentrole, array(CONTEXT_COURSE, CONTEXT_MODULE)); set_role_contextlevels($guestrole, array()); What do you think?
          Hide
          Tim Hunt added a comment -

          Note to self: Already done

          • Create new table on install
          • populate table on install (subject to above question)
          • Create table on upgrade
          • Populate table on upgrade with the above defaults, plus any additional ones needed to ensure all the role assignments already in the database are allowed.
          • Change get_assignable_roles to respect these settings.

          Todo

          • UI for these settings on the add/edit role form.
          • Save these settings when a role definition is saved.
          • Back up these settings.
          • Restore these settings, including doing something sensible to ensure backup is backwards compatible.
          Show
          Tim Hunt added a comment - Note to self: Already done Create new table on install populate table on install (subject to above question) Create table on upgrade Populate table on upgrade with the above defaults, plus any additional ones needed to ensure all the role assignments already in the database are allowed. Change get_assignable_roles to respect these settings. Todo UI for these settings on the add/edit role form. Save these settings when a role definition is saved. Back up these settings. Restore these settings, including doing something sensible to ensure backup is backwards compatible.
          Hide
          Tim Hunt added a comment -

          Having discussed this with Helen on Jabber, our view is that the above defaults are right, except that both sorts of teacher should be assignable at COURSECAT level.

          When restoring roles, in the situation of restoring a 1.9.x backup, there is no problem in the situations where the role in the backup file is matched to a role in this system. We don't overwrite the existing settings. (That applies to non-legacy restores too. If we are mapping a role in the backup file to a role already in this system, we ignore the data in the file for that role.)

          The question is, when we are creating a new role in the system based on the data in the backup file, and it is a pre Moodle 2.0 file without this sort of data. My proposal is to allow that custom role to be assignable at all context levels, since that is what was allowed for this role in the pre-Moodle-2.0 system.

          Show
          Tim Hunt added a comment - Having discussed this with Helen on Jabber, our view is that the above defaults are right, except that both sorts of teacher should be assignable at COURSECAT level. When restoring roles, in the situation of restoring a 1.9.x backup, there is no problem in the situations where the role in the backup file is matched to a role in this system. We don't overwrite the existing settings. (That applies to non-legacy restores too. If we are mapping a role in the backup file to a role already in this system, we ignore the data in the file for that role.) The question is, when we are creating a new role in the system based on the data in the backup file, and it is a pre Moodle 2.0 file without this sort of data. My proposal is to allow that custom role to be assignable at all context levels, since that is what was allowed for this role in the pre-Moodle-2.0 system.
          Hide
          Penny Leach added a comment -

          I'm very interested in this patch! Tim, any idea when it'll go into HEAD and how backportable it will be for 1.9 ? I have a Catalyst project where it will be very useful!

          Show
          Penny Leach added a comment - I'm very interested in this patch! Tim, any idea when it'll go into HEAD and how backportable it will be for 1.9 ? I have a Catalyst project where it will be very useful!
          Hide
          Tim Hunt added a comment -

          Should go into head very soon. This is next on my list after http://tracker.moodle.org/browse/MDL-16966. It should be quite back-portable, however it does require one new database table.

          Show
          Tim Hunt added a comment - Should go into head very soon. This is next on my list after http://tracker.moodle.org/browse/MDL-16966 . It should be quite back-portable, however it does require one new database table.
          Hide
          Tim Hunt added a comment -

          This patch should do the trick. I just need to test backup and restore, then review the code before committing it.

          Show
          Tim Hunt added a comment - This patch should do the trick. I just need to test backup and restore, then review the code before committing it.
          Hide
          Penny Leach added a comment -

          Awesome! Tim, I'll give this a whirl on 1.9 sometime soon

          Show
          Penny Leach added a comment - Awesome! Tim, I'll give this a whirl on 1.9 sometime soon

            People

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

              Dates

              • Created:
                Updated:
                Resolved: