[#MDL-16701] Bulk user enrol hard-codes the student role id as 5 and incorrectly shows teachers as enrolled

Issue Details (XML | Word | Printable)

Key:	MDL-16701
Type:	Bug
Status:	Open
Priority:	Minor
Assignee:	Eloy Lafuente (stronk7)
Reporter:	Tim Hunt
Votes:	3
Watchers:	7

Operations

Add/Edit UI Mockup to this issue

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

Moodle

Bulk user enrol hard-codes the student role id as 5 and incorrectly shows teachers as enrolled

Created: 26/Sep/08 04:11 PM Updated: 03/Mar/09 08:10 AM

Return to search

Component/s:

Administration

Affects Version/s:

2.0

Fix Version/s:

2.0

Issue Links:

Dependency

This issue will be resolved by:

MDL-16793 Bulk user actions improvement

Relates

This issue has a non-specific relationship to:

~~MDL-15449~~ enrol multiple users to multiple courses

Participants:	Anthony Borrow, Eloy Lafuente (stronk7) and Tim Hunt
Security Level:	None
Affected Branches:	MOODLE_20_STABLE
Fixed Branches:	MOODLE_20_STABLE

Description

« Hide

This was checked in as ~~MDL-15449~~.

This is BAD. The code should look for the role in the database according to some condition, for example shortname = 'student' or having capability legacy/student, with appropriate error handling.

All

Comments

Change History

Version Control

Sort Order:

[ Permalink | « Hide ]

Tim Hunt added a comment - 26/Sep/08 04:20 PM

It also shows teachers as if they were already enrolled, however this is misleading as they are enrolled as teachers, and this page only lets you enrol people as students.

[ Permalink | « Hide ]

Anthony Borrow added a comment - 28/Feb/09 02:49 AM

Yep, hard coding the role id ~ line 93:

if( role_unassign(5, $ids[0], 0, $context->id) ) {

is not pretty although I confess doing something similar on one of my servers just because it was a quick/easy way to do it without having to look up the id.

[ Permalink | « Hide ]

Eloy Lafuente (stronk7) added a comment - 03/Mar/09 08:10 AM

Looks really horrible, yup. Perhaps we could use $CFG->defaultcourseroleid as the role to be used when using that bulk operation? Both when detecting current enrolments and when assigning them.

Though it's possible that this will be changed in 2.0 new enrolments... I think it's better to have it working properly (at least "old behaviour") since now, yup?

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