Moodle

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

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 1.9.4
  • Fix Version/s: 1.9.6
  • Component/s: Roles / Access
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

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.

  1. frontpage_role_override_fix3.patch
    02/Oct/09 2:27 AM
    3 kB
    Petr Škoda (skodak)
  2. frontpagerole.patch
    29/Apr/09 8:00 AM
    0.7 kB
    Matt Clarkson
  3. frontpagerole-v2.patch
    29/Apr/09 8:28 AM
    0.6 kB
    Matt Clarkson

Activity

Hide
Matt Clarkson added a comment -

First patch was broken - attaching a new version.

Show
Matt Clarkson added a comment - First patch was broken - attaching a new version.
Hide
Petr Škoda (skodak) added a comment -

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; }
}

Show
Petr Škoda (skodak) added a comment - 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; } }
Hide
Petr Škoda (skodak) added a comment -

Rescheduling, to be fixed right after the 1.9.5 because there is a potential performance problem

Show
Petr Škoda (skodak) added a comment - Rescheduling, to be fixed right after the 1.9.5 because there is a potential performance problem
Hide
Petr Škoda (skodak) added a comment - - 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.

Show
Petr Škoda (skodak) added a comment - - 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.
Hide
Martin Dougiamas added a comment -

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!

Show
Martin Dougiamas added a comment - 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!
Hide
Petr Škoda (skodak) added a comment -

hmm, discovered one more potential problem, going to work more on this today - thanks!

Show
Petr Škoda (skodak) added a comment - hmm, discovered one more potential problem, going to work more on this today - thanks!
Hide
Petr Škoda (skodak) added a comment -

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

Show
Petr Škoda (skodak) added a comment - 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

People

Vote (1)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: