Moodle
  1. Moodle
  2. MDL-38557

Access to the quiz manual grading 'report' should be controlled by mod/quiz:grade, not mod/quiz:viewreports

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.6, 2.4.3, 2.5
    • Fix Version/s: 2.3.7, 2.4.4
    • Component/s: Quiz
    • Labels:
    • Testing Instructions:
      Hide

      Ideally, this needs to be tested in both an upgrade an a newly installed site on each branch.

      1. You need to create users with 4 possible combinations of with/without mod/quiz:viewreports and mod/quiz:grade. (Perhaps easiest using permission overrides?)

      2. Check that mod/quiz:viewreports controls access to the Grades and Responses quiz report

      • In the navigation.
      • Which report the 'Attempts: 123' message links to.
      • Whether you can acutally use the report, once you have gone there in the navigation.
      Show
      Ideally, this needs to be tested in both an upgrade an a newly installed site on each branch. 1. You need to create users with 4 possible combinations of with/without mod/quiz:viewreports and mod/quiz:grade. (Perhaps easiest using permission overrides?) 2. Check that mod/quiz:viewreports controls access to the Grades and Responses quiz report In the navigation. Which report the 'Attempts: 123' message links to. Whether you can acutally use the report, once you have gone there in the navigation.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
    • Pull Master Branch:
    • Rank:
      48575

      Description

      I actually thought it worked like that, but it seems I was wrong.

      I think it makes more sense to use the mod/quiz:grade capability to control access to this bit of UI, rather than mod/quiz:viewreports.

      All we need to do is to change the value in the quiz_reports.capability column.

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          Further explanation and community consultation: https://moodle.org/mod/forum/discuss.php?d=224916

          Show
          Tim Hunt added a comment - Further explanation and community consultation: https://moodle.org/mod/forum/discuss.php?d=224916
          Hide
          Tim Hunt added a comment -

          I think this is the necessary code changes, including some clean-up in master done as separate commits.

          Show
          Tim Hunt added a comment - I think this is the necessary code changes, including some clean-up in master done as separate commits.
          Hide
          Tim Hunt added a comment -

          No objections from the community. I am submitting this for integration.

          Show
          Tim Hunt added a comment - No objections from the community. I am submitting this for integration.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Uhm Tim...

          why this code:

           $DB->set_field('quiz_reports', 'capability', 'mod/quiz:grade'...
          

          in in main quiz upgrade and not in quiz_reports->grading upgrade code?

          I'm pushing this coz seems it should work... but...

          Show
          Eloy Lafuente (stronk7) added a comment - Uhm Tim... why this code: $DB->set_field('quiz_reports', 'capability', 'mod/quiz:grade'... in in main quiz upgrade and not in quiz_reports->grading upgrade code? I'm pushing this coz seems it should work... but...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (23, 24 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
          Hide
          Rajesh Taneja added a comment -

          Thanks for fixing this Tim,

          It works fine, although one minor issue:

          1. Login as user with mod/quiz:viewreports capability and not mod/quiz:grade capability
          2. Expand Quiz administration -> Results
          3. You see "Manual grading" link
          4. Clicking this gives "Sorry, but you do not currently have permissions to do that (Grade quizzes manually)" error

          Not sure if it should be fixed by this bug. But Navigation link should only appear if user have proper capability.

          Let me know if this can be handled by this bug or it's a separate issue.

          Show
          Rajesh Taneja added a comment - Thanks for fixing this Tim, It works fine, although one minor issue: Login as user with mod/quiz:viewreports capability and not mod/quiz:grade capability Expand Quiz administration -> Results You see "Manual grading" link Clicking this gives "Sorry, but you do not currently have permissions to do that (Grade quizzes manually)" error Not sure if it should be fixed by this bug. But Navigation link should only appear if user have proper capability. Let me know if this can be handled by this bug or it's a separate issue.
          Hide
          Tim Hunt added a comment -

          Eloy, I put the code there, because I wanted the same fix for the original bug in master, 2.4 and 2.3.

          Then I moved code to better places in master, but did not want to mess with the upgrade script there. It may not be 100% perfect, but it is better than it was.

          Rajesh, I will investigate. Which branch were you testing when you saw that?

          Show
          Tim Hunt added a comment - Eloy, I put the code there, because I wanted the same fix for the original bug in master, 2.4 and 2.3. Then I moved code to better places in master, but did not want to mess with the upgrade script there. It may not be 100% perfect, but it is better than it was. Rajesh, I will investigate. Which branch were you testing when you saw that?
          Hide
          Rajesh Taneja added a comment -

          Thanks Tim,

          It's master integration. Should I pass this and open another issue for navigation link ?

          Show
          Rajesh Taneja added a comment - Thanks Tim, It's master integration. Should I pass this and open another issue for navigation link ?
          Hide
          Tim Hunt added a comment -

          Raj, can you wait a bit until I have investigated. Once I know for sure what is going on, I will leave a clear comment about whether this should be passed or failed, so the integrators will be able to carry on, even if you have gone home. Thanks.

          Show
          Tim Hunt added a comment - Raj, can you wait a bit until I have investigated. Once I know for sure what is going on, I will leave a clear comment about whether this should be passed or failed, so the integrators will be able to carry on, even if you have gone home. Thanks.
          Hide
          Tim Hunt added a comment -

          Raj, I cannot reproduce what you claim to be seeing. Please can you try again.

          Show
          Tim Hunt added a comment - Raj, I cannot reproduce what you claim to be seeing. Please can you try again.
          Hide
          Rajesh Taneja added a comment -

          Steps to reproduce on master integration:

          1. Log in as admin and go to a course with quiz
          2. For teacher role set mod/quiz:grade => Prohibit and set mod/quiz:viewreports => allow (Course administration -> Users -> Permissions)
          3. Log in as teacher and go to same course
          4. select quiz
          5. You will see Manual grading link in navigation(Quiz administration -> Results -> Manual grading)
          6. Click "Manual grading" and you will see error.
          Show
          Rajesh Taneja added a comment - Steps to reproduce on master integration: Log in as admin and go to a course with quiz For teacher role set mod/quiz:grade => Prohibit and set mod/quiz:viewreports => allow (Course administration -> Users -> Permissions) Log in as teacher and go to same course select quiz You will see Manual grading link in navigation(Quiz administration -> Results -> Manual grading) Click "Manual grading" and you will see error.
          Hide
          Tim Hunt added a comment -

          Yes, that is what I did. I don't see the Manual grading link in the navigation in that case. That was the whole point of this change.

          Please can you check the contents of the quiz_reports table in the database, and paste it here. I assume you have been to admin notifications and run the upgrade?

          Show
          Tim Hunt added a comment - Yes, that is what I did. I don't see the Manual grading link in the navigation in that case. That was the whole point of this change. Please can you check the contents of the quiz_reports table in the database, and paste it here. I assume you have been to admin notifications and run the upgrade?
          Hide
          Rajesh Taneja added a comment -

          Aha...
          Sorry for the noise Tim, I didn't realise quiz was not updated in master copy.

          Last time when I was testing, it was upgraded and gave me error, so I changed by db to set version to new version and it missed db update.

          Passing this now.

          Show
          Rajesh Taneja added a comment - Aha... Sorry for the noise Tim, I didn't realise quiz was not updated in master copy. Last time when I was testing, it was upgraded and gave me error, so I changed by db to set version to new version and it missed db update. Passing this now.
          Hide
          Damyon Wiese added a comment -

          Thanks for your hard work. This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).

          Show
          Damyon Wiese added a comment - Thanks for your hard work. This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).
          Hide
          Dan Poltawski added a comment -

          Just a note that this issue also fixed the version number regression from MDL-31983. There wasn't any comment in this issue mentioning that

          Show
          Dan Poltawski added a comment - Just a note that this issue also fixed the version number regression from MDL-31983 . There wasn't any comment in this issue mentioning that
          Hide
          Mary Cooch added a comment -

          (Just doing some docs_required housekeeping) Removing the docs_required label as I added a short note to http://docs.moodle.org/en/Capabilities/mod/quiz:viewreports

          Show
          Mary Cooch added a comment - (Just doing some docs_required housekeeping) Removing the docs_required label as I added a short note to http://docs.moodle.org/en/Capabilities/mod/quiz:viewreports

            People

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

              Dates

              • Created:
                Updated:
                Resolved: