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

      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

        1. frontpage_role_override_fix3.patch
          3 kB
          Petr Skoda
        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 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
          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
          Petr Skoda added a comment -

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

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

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

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