Issue Details (XML | Word | Printable)

Key: MDL-19004
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Critical Critical
Assignee: Petr Skoda
Reporter: Matt Clarkson
Votes: 1
Watchers: 3
Operations

Add/Edit UI Mockup to this issue
If you were logged in you would be able to see more operations.
Moodle

get_users_by_capability() does not apply $CFG->defaultfrontpagerole at contexts below CONTEXT_COURSE

Created: 29/Apr/09 08:00 AM   Updated: 06/Oct/09 01:17 AM
Return to search
Component/s: Roles
Affects Version/s: 1.9.4
Fix Version/s: 1.9.6

File Attachments: 1. Text File frontpage_role_override_fix3.patch (3 kB)
2. Text File frontpagerole-v2.patch (0.6 kB)
3. Text File frontpagerole.patch (0.7 kB)


Participants: Martin Dougiamas, Matt Clarkson and Petr Skoda
Security Level: None
Resolved date: 06/Oct/09
Affected Branches: MOODLE_19_STABLE
Fixed Branches: MOODLE_19_STABLE


 Description  « Hide
get_users_by_capability() doesn't apply the default front page role when the context level being requested is below CONTEXT_COURSE.

Once noticeable side effect of this is that front page forum posts are not mailed out because the context of the forum is below the course context.

A patch that should fix this issue is attached.

 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
Matt Clarkson added a comment - 29/Apr/09 08:28 AM
First patch was broken - attaching a new version.

Petr Skoda added a comment - 05/May/09 05:50 AM
I suppose this might work better:

$isfrontpage = false; // Context is part of frontpage
$iscoursepage = false; // coursepage other than fp

if (is_inside_frontpage($context)) { $isfrontpage = true; } else if ($context->contextlevel == CONTEXT_COURSE) {
if ($context->instanceid == SITEID) { $isfrontpage = true; } else { $iscoursepage = true; }
}


Petr Skoda added a comment - 06/May/09 11:45 PM
Rescheduling, to be fixed right after the 1.9.5 because there is a potential performance problem

Petr Skoda added a comment - 02/Oct/09 02:25 AM - edited
hmm, I spent a few hours trying to fix this and also trying to find out all potential problems.

The attached patches are incorrect - they seem to fix only one particular capability setup, it does not deal with negative capability overrides.

Major problems are:
1/ it is going to run out of memory and course major performance problems if there are a few thousand site users with the given capability
2/ if we implement support for the CAP_PREVENT override of the default frontpage role the normal role assignments on the frontpage will be eliminated by the frontpage role (thanks to the over-engineered capability evaluation in 1.9.x)

Sending new patch, I am not really sure if it is a good idea to make this change, so I have added on new config.php switch that enables the patch.

Expect big problems especially on large sites where Student role is selected as the default frontpage role.


Martin Dougiamas added a comment - 02/Oct/09 04:44 PM
Makes good sense to me. +1 for this as it is.

Just also add a line in config-dist.php and perhaps include a MDL-19004 reference in the comments near the code.

Thanks!


Petr Skoda added a comment - 02/Oct/09 04:54 PM
hmm, discovered one more potential problem, going to work more on this today - thanks!

Petr Skoda added a comment - 06/Oct/09 01:17 AM
grrr, I messed up the number in commit - used MDL-19039 instead, patch in cvs, please note you need to enable one option in config.php, see config-dist.php

petr