Moodle

create_role is too clever - sets allow role assign and override for all admin accounts

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.0
  • Fix Version/s: 2.0
  • Component/s: Roles / Access
  • Labels:
    None
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

The problem is that the roles installation was duplicating the records - install was failing for me because it was creating duplicates in allow role assign table

I believe that hacks like this should not be done at the lowest level, instead they should be moved much closer to UI code

committing patch that comments out the second allow role assign and override from moodle_install_roles()

Activity

Hide
Petr Škoda (skodak) added a comment -

Linking with refactoring required for Moodle External API

Solution could be:

  • create new function allow_admins_override_and_assign($roleid) // TODO: needs some better name
  • call it from roles UI when crating new role
  • call it from restore when adding new role
Show
Petr Škoda (skodak) added a comment - Linking with refactoring required for Moodle External API Solution could be:
  • create new function allow_admins_override_and_assign($roleid) // TODO: needs some better name
  • call it from roles UI when crating new role
  • call it from restore when adding new role
Hide
Tim Hunt added a comment -

Actually, Petr, an alternative approach would be to change get_assignable/overridable roles, so that the fact that people with doanything can assign or override any role does not need to be stored in the DB. If you want me to do that, feel free to assign this bug to me.

Show
Tim Hunt added a comment - Actually, Petr, an alternative approach would be to change get_assignable/overridable roles, so that the fact that people with doanything can assign or override any role does not need to be stored in the DB. If you want me to do that, feel free to assign this bug to me.
Hide
Petr Škoda (skodak) added a comment -

yes Tim, I like that approach
my +2 for head

Show
Petr Škoda (skodak) added a comment - yes Tim, I like that approach my +2 for head
Hide
Petr Škoda (skodak) added a comment -

implemented, please review

Show
Petr Škoda (skodak) added a comment - implemented, please review

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: