Moodle
  1. Moodle
  2. MDL-33621

Global scales can give permission error when viewing help

    Details

    • Testing Instructions:
      Hide

      As Admin:

      1. Create a forum that use standard scale rating (eg: scale: separate and connected ways of knowing).
      2. Add a topic or post.

      As a teacher (Role = standard editingteacher and make sure teacher role doesn't has System permission)

      1. Access same rated forum.
      2. Click info/help icon beside scale.

      Make sure the scales help displays without error.

      Show
      As Admin: Create a forum that use standard scale rating (eg: scale: separate and connected ways of knowing). Add a topic or post. As a teacher (Role = standard editingteacher and make sure teacher role doesn't has System permission) Access same rated forum. Click info/help icon beside scale. Make sure the scales help displays without error.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      41566

      Description

      Viewing help for scale displays an error for a user (teacher) with the capability.

      Steps to Reproduce

      1. Click a rated forum that uses a scale as a student.
      2. Add a topic or post.
      3. As a teacher (Role = standard editingteacher) access same rated forum.
      4. Click info/help icon beside scale.
        Expected - Scales help displays.
        Actual - Error displays and reads:

      Sorry, but you do not currently have permissions to do that (View scales)

      More information about this error

      Stack trace:

      • line 691 of /lib/accesslib.php: required_capability_exception thrown
      • line 50 of /course/scales.php: call to require_capability()

      The cause:

      This is caused because the scale is associated with courseid = 0 (a global scale) so the context is set to the site instead of the course that the user is currently in.

        Issue Links

          Activity

          Hide
          Mark Nielsen added a comment -

          Attaching a proposed fix: when scale course ID is empty, it attempts to load the course ID from the rating context.

          Note: I'm not 100% familiar with scales, not sure if the scale courseid could be another course entirely (EG: not global and not from current course). If that's the case, then we would probably want to always try to find the course ID from the rating context.

          Show
          Mark Nielsen added a comment - Attaching a proposed fix: when scale course ID is empty, it attempts to load the course ID from the rating context. Note: I'm not 100% familiar with scales, not sure if the scale courseid could be another course entirely (EG: not global and not from current course). If that's the case, then we would probably want to always try to find the course ID from the rating context.
          Hide
          Rossiani Wijaya added a comment -

          Hi Mark,

          Thanks for reporting and providing solution.

          The changes looks great. I created patch for 2.2, 2.3 and 2.4.

          Submitting for peer review.

          Show
          Rossiani Wijaya added a comment - Hi Mark, Thanks for reporting and providing solution. The changes looks great. I created patch for 2.2, 2.3 and 2.4. Submitting for peer review.
          Hide
          Mark Nelson added a comment -

          [N] Syntax
          [-] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [Y] Testing
          [-] Security
          [Y] Documentation
          [Y] Git
          [Y] Sanity check

          Hi Rosie, logic looks good. However, there are missing fullstops at the end of the comments.

          Show
          Mark Nelson added a comment - [N] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [Y] Documentation [Y] Git [Y] Sanity check Hi Rosie, logic looks good. However, there are missing fullstops at the end of the comments.
          Hide
          Rossiani Wijaya added a comment -

          Hi Mark,

          Thank you for reviewing.

          I'm leaving the patch as it is because it won't produce any regression or error.

          Submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Hi Mark, Thank you for reviewing. I'm leaving the patch as it is because it won't produce any regression or error. Submitting for integration review.
          Hide
          Aparup Banerjee added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Aparup Banerjee added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          Thanks Mark/Rosie, i've integrated this now.

          Show
          Dan Poltawski added a comment - Thanks Mark/Rosie, i've integrated this now.
          Hide
          Michael de Raadt added a comment -

          Test result: Success.

          Tested in 2.2, 2.3 and master.

          Well, it wasn't pretty, but there was no error.

          I've launched MDL-36387 so we can can look at using a more consistent, accessible help bubble, instead of the current pop-up window.

          Show
          Michael de Raadt added a comment - Test result: Success. Tested in 2.2, 2.3 and master. Well, it wasn't pretty, but there was no error. I've launched MDL-36387 so we can can look at using a more consistent, accessible help bubble, instead of the current pop-up window.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

          (not really)

          Closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: