Uploaded image for project: '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
    • Status: Closed
    • Priority: 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 Master Branch:

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            timhunt Tim Hunt added a comment -

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

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

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

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

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

            Show
            timhunt Tim Hunt added a comment - No objections from the community. I am submitting this for integration.
            Hide
            stronk7 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
            stronk7 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated (23, 24 & master), thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
            Hide
            rajeshtaneja 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
            rajeshtaneja 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
            timhunt 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
            timhunt 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
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Tim,

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

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Tim, It's master integration. Should I pass this and open another issue for navigation link ?
            Hide
            timhunt 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
            timhunt 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
            timhunt Tim Hunt added a comment -

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

            Show
            timhunt Tim Hunt added a comment - Raj, I cannot reproduce what you claim to be seeing. Please can you try again.
            Hide
            rajeshtaneja 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
            rajeshtaneja 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
            timhunt 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
            timhunt 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
            rajeshtaneja 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
            rajeshtaneja 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 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 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
            poltawski 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
            poltawski 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
            marycooch 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
            marycooch 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:
                  Fix Release Date:
                  13/May/13