[#MDL-17267] create_role is too clever - sets allow role assign and override for all admin accounts

Issue Details (XML | Word | Printable)

Key:	MDL-17267
Type:	Sub-task
Status:	Resolved
Resolution:	Fixed
Priority:	Major
Assignee:	Petr Skoda
Reporter:	Petr Skoda
Votes:	0
Watchers:	1

Operations

Add/Edit UI Mockup to this issue

If you were logged in you would be able to see more operations.

Moodle

MDL-17268

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

Created: 18/Nov/08 06:55 PM Updated: 18/Jan/09 08:24 AM

Return to search

Component/s:

Roles

Affects Version/s:

2.0

Fix Version/s:

2.0

Participants:	Petr Skoda and Tim Hunt
Security Level:	None
Resolved date:	18/Jan/09
Affected Branches:	MOODLE_20_STABLE
Fixed Branches:	MOODLE_20_STABLE

Description

« Hide

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()

All

Comments

Change History

Version Control

Sort Order:

Petr Skoda committed 1 file to 'Moodle CVS' - 18/Nov/08 06:56 PM

~~MDL-17267~~ roles, install: fixed allow role assign table duplicates problem during install - see tracker for more details

MODIFY

lib/accesslib.php

Rev. 1.541

(+5 -3 lines)

Petr Skoda made changes - 18/Nov/08 07:12 PM

Field	Original Value	New Value
Link		This issue has been marked as being related by MDL-17268 [ MDL-17268 ]

[ Permalink | « Hide ]

Petr Skoda added a comment - 18/Nov/08 07:14 PM

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

Petr Skoda made changes - 18/Nov/08 07:15 PM

Parent		MDL-17268 [ 29438 ]
Issue Type	Bug [ 1 ]	Sub-task [ 5 ]

Petr Skoda made changes - 18/Nov/08 07:15 PM

Link

This issue has been marked as being related by MDL-17268 [ MDL-17268 ]

[ Permalink | « Hide ]

Tim Hunt added a comment - 19/Nov/08 09:40 PM

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.

[ Permalink | « Hide ]

Petr Skoda added a comment - 19/Nov/08 10:26 PM

yes Tim, I like that approach

my +2 for head

Petr Skoda committed 2 files to 'Moodle CVS' - 19/Nov/08 11:43 PM

~~MDL-17267~~ fixed regression caused by my last commit

MODIFY	lib/setup.php		Rev. 1.251		(+2 -6 lines)
MODIFY	lib/accesslib.php		Rev. 1.542		(+19 -19 lines)

Petr Skoda made changes - 18/Jan/09 07:36 AM

Assignee

moodle.com [ moodle.com ]

Petr ?koda [ skodak ]

Petr Skoda made changes - 18/Jan/09 08:06 AM

Status

Open [ 1 ]

In Progress [ 3 ]

Petr Skoda committed 3 files to 'Moodle CVS' - 18/Jan/09 08:16 AM

~~MDL-17267~~ simplified create_role - not adding assign and override perm to admin accounts, they have it now always

MODIFY	lib/db/install.php	Rev. 1.5	(+4 -1 lines)
MODIFY	admin/index.php	Rev. 1.358	(+1 -4 lines)
MODIFY	lib/accesslib.php	Rev. 1.571	(+51 -64 lines)

[ Permalink | « Hide ]

Petr Skoda added a comment - 18/Jan/09 08:24 AM

implemented, please review

Petr Skoda made changes - 18/Jan/09 08:24 AM

Resolution		Fixed [ 1 ]
Status	In Progress [ 3 ]	Resolved [ 5 ]

All content on this web site is made available under the GNU General Public License, unless otherwise stated.