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

Ensure grade items remain hidden if explicitly hidden via gradebook (regardless of activity state)

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.2, 2.1, 2.2, 2.4.5, 2.5.1
    • Fix Version/s: 2.4.6, 2.5.2
    • Component/s: Gradebook, Quiz
    • Labels:
    • Testing Instructions:
      Hide
      1. In a new test course, create a graded activity like a mod_assign, where visibility of the grade item is controlled by the gradebook.
      2. Create a quiz where the reviwe options say that marks are visible all the time. (This should be the default.)
      3. In the gradebook, go to the Categries and items simple view.
      4. In another tab, open your User grades report.
      5. Verify that both grade items are there, as is the course total item, and the category for all the grades in the course.
      6. Verify that, on the categories and items page, all the grade items, except the quiz, have an 'eye-con', and that they work to show or hide the item.
      7. Click the 'eye-con' for the grade category. Verify that all the grades items are hidden except the quiz.
      8. Then click the eyecon again to make them all visible.
      9. On the course page, click the eyecon to hide both the quiz and the assignment.
      10. Verify that both grade items are now hidden in the grade book.
      11. Repeat the last two steps, but by using the Edit settings form for each activity, rather than the eyecon on the course page.
      12. If you get through all that, have a well-earned coffee/beer/mohito.
      Show
      In a new test course, create a graded activity like a mod_assign, where visibility of the grade item is controlled by the gradebook. Create a quiz where the reviwe options say that marks are visible all the time. (This should be the default.) In the gradebook, go to the Categries and items simple view. In another tab, open your User grades report. Verify that both grade items are there, as is the course total item, and the category for all the grades in the course. Verify that, on the categories and items page, all the grade items, except the quiz, have an 'eye-con', and that they work to show or hide the item. Click the 'eye-con' for the grade category. Verify that all the grades items are hidden except the quiz. Then click the eyecon again to make them all visible. On the course page, click the eyecon to hide both the quiz and the assignment. Verify that both grade items are now hidden in the grade book. Repeat the last two steps, but by using the Edit settings form for each activity, rather than the eyecon on the course page. If you get through all that, have a well-earned coffee/beer/mohito.
    • Workaround:
      Hide

      Remember to Show/Hide each grade item within the gradebook whenever the state changes on the main course page.

      Show
      Remember to Show/Hide each grade item within the gradebook whenever the state changes on the main course page.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Add additional data to grade item for the purposes of show/hide checking.

      It should be possible for a grade item to remain hidden if explicitly hidden (via the show/hide icon or the "hidden until" setting) regardless of the activity's state in the course.

      As per discussion http://moodle.org/mod/forum/discuss.php?d=195014

      Action Course Page Gradebook Notes
      Create a quiz + show it to students Quiz activity displayed Quiz grade item is displayed This is good!
      Hide the quiz on the main course page Quiz activity hidden Quiz grade item is hidden this is good!
      Show the quiz again Quiz activity displayed Quiz grade item is displayed good again!
      Hide the quiz grade item in the gradebook (without altering the quiz activity) Quiz activity still displayed Quiz grade item is hidden good...
      Hide the quiz activity on the main course page (without altering the quiz grade item) Quiz is hidden Quiz grade item is still hidden this is good too!
      Show the quiz activity on the main course page (without altering the quiz grade item) Quiz activity now displayed Quiz grade item now displayed NOT GOOD! I expect the grade item to remain hidden as set within the gradebook

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for proposing that.

            I've put that on the backlog.

            In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.

            Show
            salvetore Michael de Raadt added a comment - Thanks for proposing that. I've put that on the backlog. In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.
            Hide
            mhughes2k Michael Hughes added a comment -

            I think this is good idea and it's one that our users have requested.

            I was thinking that it might make sense to have grade items hide when an activity is hidden, but not to change visibility if an activity is made visible.

            The change then required in set_coursemodule_visible($id,$visible) goes from

                
                $grade_items = grade_item::fetch_all(array('itemtype'=>'mod', 'itemmodule'=>$modulename, 'iteminstance'=>$cm->instance, 'courseid'=>$cm->course));
                if ($grade_items) {
                    foreach ($grade_items as $grade_item) {
                        $grade_item->set_hidden(!$visible);
                    }
                }
             

            to

                
            if (!$visible) { //this only hides grade book items, if an activity is made visible it doesn't change the state.
            	    $grade_items = grade_item::fetch_all(array('itemtype'=>'mod', 'itemmodule'=>$modulename, 'iteminstance'=>$cm->instance, 'courseid'=>$cm->course));
            	    if ($grade_items) {
            	        foreach ($grade_items as $grade_item) {
            	            $grade_item->set_hidden(!$visible);
            	        }
            	    }
                }

            Show
            mhughes2k Michael Hughes added a comment - I think this is good idea and it's one that our users have requested. I was thinking that it might make sense to have grade items hide when an activity is hidden, but not to change visibility if an activity is made visible. The change then required in set_coursemodule_visible($id,$visible) goes from $grade_items = grade_item::fetch_all(array('itemtype'=>'mod', 'itemmodule'=>$modulename, 'iteminstance'=>$cm->instance, 'courseid'=>$cm->course)); if ($grade_items) { foreach ($grade_items as $grade_item) { $grade_item->set_hidden(!$visible); } }   to if (!$visible) { //this only hides grade book items, if an activity is made visible it doesn't change the state. $grade_items = grade_item::fetch_all(array('itemtype'=>'mod', 'itemmodule'=>$modulename, 'iteminstance'=>$cm->instance, 'courseid'=>$cm->course)); if ($grade_items) { foreach ($grade_items as $grade_item) { $grade_item->set_hidden(!$visible); } } }
            Hide
            mhughes2k Michael Hughes added a comment - - edited

            Git hub patch (equivalent to above):https://github.com/mhughes2k/moodle/tree/MDL-31487-Grade-Item-Visibility but can't add patch label.

            Show
            mhughes2k Michael Hughes added a comment - - edited Git hub patch (equivalent to above): https://github.com/mhughes2k/moodle/tree/MDL-31487-Grade-Item-Visibility but can't add patch label.
            Hide
            andyjdavis Andrew Davis added a comment -

            There are an interconnected set of issues that probably need to be considered as a unit.

            MDL-38383
            MDL-31487
            MDL-38445
            MDL-18301 (this is actually closed but was recently fixed and is relevant)

            Show
            andyjdavis Andrew Davis added a comment - There are an interconnected set of issues that probably need to be considered as a unit. MDL-38383 MDL-31487 MDL-38445 MDL-18301 (this is actually closed but was recently fixed and is relevant)
            Hide
            andyjdavis Andrew Davis added a comment -

            This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.

            For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

            Show
            andyjdavis Andrew Davis added a comment - This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment
            Hide
            timhunt Tim Hunt added a comment -

            Right, I am about to try to fix this.

            The policy, established in MDL-18301, is that for activities like quiz, the quiz controls whetehr the grade item is visible or not (based on the quiz settings). The user cannot control it in the gradebook UI.

            There were serveral problems with what was implmeneted in MDL-18301:

            1. On the edit categories and items screens, all items had an eye-con to control the visibility, even if the user was not supposed to be able to do that. (MDL-18301 only changed the edit grade-item form.)
            2. If you changed the visibility of a category in the grade-book, then the visibiliities of all quiz grade items in that category were incorrectly changed.
            3. If the quiz was hidden using the Visiable to students: No setting that is in every type of activity, then the grade item would still be visible. This was reported as MDL-40880. That is, the code where the quiz was controlling visibility was not checking $cm->visible.
            Show
            timhunt Tim Hunt added a comment - Right, I am about to try to fix this. The policy, established in MDL-18301 , is that for activities like quiz, the quiz controls whetehr the grade item is visible or not (based on the quiz settings). The user cannot control it in the gradebook UI. There were serveral problems with what was implmeneted in MDL-18301 : On the edit categories and items screens, all items had an eye-con to control the visibility, even if the user was not supposed to be able to do that. ( MDL-18301 only changed the edit grade-item form.) If you changed the visibility of a category in the grade-book, then the visibiliities of all quiz grade items in that category were incorrectly changed. If the quiz was hidden using the Visiable to students: No setting that is in every type of activity, then the grade item would still be visible. This was reported as MDL-40880 . That is, the code where the quiz was controlling visibility was not checking $cm->visible.
            Hide
            timhunt Tim Hunt added a comment -

            OK, here is the patch for peer review.

            Show
            timhunt Tim Hunt added a comment - OK, here is the patch for peer review.
            Hide
            timhunt Tim Hunt added a comment -

            Just adding Frédéric Massart as a watcher, since he did MDL-18301, and hence might be a good person to peer review thi.

            Show
            timhunt Tim Hunt added a comment - Just adding Frédéric Massart as a watcher, since he did MDL-18301 , and hence might be a good person to peer review thi.
            Hide
            damyon Damyon Wiese added a comment - - edited

            Thanks Tim,

            This fix looks right to me and worked when I tested it. I only found the two points listed below - other than that it looks good for integration.

            Minor points:

            I think the error message in grade/edit/tree/action.php is not correct. It should probably be "grades/componentcontrolsvisibility" (Even though it was not meant as an error I think it would make more sense than "nopermissiontohide").

            Also - no unit tests? (This actually sounds like a very good candidate for unit testing)

            [Y] Syntax
            [Y] Whitespace
            [-] Output
            [?] Language - see comment about error message.
            [-] Databases
            [?] Testing - no unit tests
            [-] Security
            [-] Documentation
            [Y] Git
            [Y] Sanity check

            Show
            damyon Damyon Wiese added a comment - - edited Thanks Tim, This fix looks right to me and worked when I tested it. I only found the two points listed below - other than that it looks good for integration. Minor points: I think the error message in grade/edit/tree/action.php is not correct. It should probably be "grades/componentcontrolsvisibility" (Even though it was not meant as an error I think it would make more sense than "nopermissiontohide"). Also - no unit tests? (This actually sounds like a very good candidate for unit testing) [Y] Syntax [Y] Whitespace [-] Output [?] Language - see comment about error message. [-] Databases [?] Testing - no unit tests [-] Security [-] Documentation [Y] Git [Y] Sanity check
            Hide
            timhunt Tim Hunt added a comment -

            Thanks for the lang string suggestion. Good idea.

            A lot of this code does not currently have unit tests, or has unit tests written in an insane style (https://github.com/moodle/moodle/blob/master/lib/grade/tests/grade_item_test.php#L29). I have added tests for can_control_visibility, but I think that is all that is really possible at this stage.

            Just waiting for unit tests to run, then I will back-port and submit for integration.

            Show
            timhunt Tim Hunt added a comment - Thanks for the lang string suggestion. Good idea. A lot of this code does not currently have unit tests, or has unit tests written in an insane style ( https://github.com/moodle/moodle/blob/master/lib/grade/tests/grade_item_test.php#L29 ). I have added tests for can_control_visibility, but I think that is all that is really possible at this stage. Just waiting for unit tests to run, then I will back-port and submit for integration.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Tim - nice clean up - has been integrated now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Tim - nice clean up - has been integrated now.
            Hide
            dmonllao David Monllaó added a comment -

            It passes, tested in 24, 25 and master

            Show
            dmonllao David Monllaó added a comment - It passes, tested in 24, 25 and master
            Hide
            poltawski Dan Poltawski added a comment -

            Cảm ơn!

            Your changes have now been merged upstream in git and will be available on the Moodle download sites shortly!

            Một hai ba, yo

            Show
            poltawski Dan Poltawski added a comment - Cảm ơn! Your changes have now been merged upstream in git and will be available on the Moodle download sites shortly! Một hai ba, yo
            Hide
            elenaivanova Elena Ivanova added a comment -

            Hi

            I think there was a regression introduced after this patch.
            I can: hide the quiz activity on the homepage, but allow students to see the quiz grades (via Review Options).
            Students now see the greyed out quiz column in the gradebook, but they do not see the grade itself.
            https://moodle.org/mod/forum/discuss.php?d=241474

            Do others see the same?

            Show
            elenaivanova Elena Ivanova added a comment - Hi I think there was a regression introduced after this patch. I can: hide the quiz activity on the homepage, but allow students to see the quiz grades (via Review Options). Students now see the greyed out quiz column in the gradebook, but they do not see the grade itself. https://moodle.org/mod/forum/discuss.php?d=241474 Do others see the same?
            Hide
            mhughes2k Michael Hughes added a comment -

            I don't think that there is any code here that actually solves the issue that was being raised, mainly the fact that people would like the grade item of an activity to remain set regardless of the activity's visibility state.

            I've reviewed the stable branches and all I can see is the conditional calling of *_grade_item_update if the mod reports it supports it (which mod assign doesn't) otherwise the grade item simply gets set the the inverted $visible state.

            So how is this actually resolved?!?

            Show
            mhughes2k Michael Hughes added a comment - I don't think that there is any code here that actually solves the issue that was being raised, mainly the fact that people would like the grade item of an activity to remain set regardless of the activity's visibility state. I've reviewed the stable branches and all I can see is the conditional calling of *_grade_item_update if the mod reports it supports it (which mod assign doesn't) otherwise the grade item simply gets set the the inverted $visible state. So how is this actually resolved?!?

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  9/Sep/13