Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-19004

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            matt.clarkson Matt Clarkson added a comment -

            First patch was broken - attaching a new version.

            Show
            matt.clarkson Matt Clarkson added a comment - First patch was broken - attaching a new version.
            Hide
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            skodak Petr Skoda added a comment -

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

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

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

            Show
            skodak Petr Skoda added a comment - hmm, discovered one more potential problem, going to work more on this today - thanks!
            Hide
            skodak Petr Skoda 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
            skodak Petr Skoda 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:
                  Fix Release Date:
                  21/Oct/09