Moodle
  1. Moodle
  2. MDL-19004

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

    Details

    • Type: Bug Bug
    • Status: 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
    • Rank:
      31747

      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
        3 kB
        Petr Škoda
      2. frontpagerole.patch
        0.7 kB
        Matt Clarkson
      3. frontpagerole-v2.patch
        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 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 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 added a comment -

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

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

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

        Show
        Petr Škoda added a comment - hmm, discovered one more potential problem, going to work more on this today - thanks!
        Hide
        Petr Škoda 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 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

          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: