Moodle
  1. Moodle
  2. MDL-31487

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

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Critical 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 2.4 Branch:
    • Pull 2.5 Branch:
    • Pull Master Branch:
    • Rank:
      38019

      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

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Tim Hunt added a comment -

          OK, here is the patch for peer review.

          Show
          Tim Hunt added a comment - OK, here is the patch for peer review.
          Hide
          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
          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 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 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
          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
          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
          Sam Hemelryk added a comment -

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

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

          It passes, tested in 24, 25 and master

          Show
          David Monllaó added a comment - It passes, tested in 24, 25 and master
          Hide
          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
          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
          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
          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?

            People

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

              Dates

              • Created:
                Updated:
                Resolved: