Moodle
  1. Moodle
  2. MDL-36757

Editing an activity reveals hidden grades

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3, 2.4
    • Fix Version/s: 2.3.5, 2.4.2
    • Component/s: Gradebook
    • Labels:
    • Testing Instructions:
      Hide

      You'll need a course.

      Create a new visible activity. (Its visible by default, just don't set any conditions or anything that could cause it to become hidden).

      Go to the Categories and Items page within the grade book. Hide the activity's grade item.

      Check that the activity is still visible on the course page.

      Edit the activity settings, change its description and save.

      Go back to the Categories and Items page and check that the activity grade item is still hidden.

      Unhide it.

      Edit the activity settings and set "Visible" to "Hide".

      Check that the activity is now hidden on both the course page and the Categories and Items page.

      Show
      You'll need a course. Create a new visible activity. (Its visible by default, just don't set any conditions or anything that could cause it to become hidden). Go to the Categories and Items page within the grade book. Hide the activity's grade item. Check that the activity is still visible on the course page. Edit the activity settings, change its description and save. Go back to the Categories and Items page and check that the activity grade item is still hidden. Unhide it. Edit the activity settings and set "Visible" to "Hide". Check that the activity is now hidden on both the course page and the Categories and Items page.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-36757_hidden_24
    • Pull Master Branch:
      MDL-36757_hidden
    • Rank:
      46269

      Description

      1. Create a graded activity such as an Assignment or Quiz
      2. In Grade administration > Categories and items > Full view toggle the 'eye' icon to hide the grades
      3. Edit any aspect of the activity, such as when it becomes available and save the change
      4. Go back to Grade administration > Categories and items > Full view
      5. Grade should still be hidden, but is now revealed

      I see this in 2.3 and 2.4 but not in 2.2

        Issue Links

          Activity

          Hide
          Andrew Davis added a comment -

          I think I've figured this out. On the categories and items screen you're hiding the grade item associated with the activity. When you edit the activity you're inadvertently setting the visible flag on the activity and that is propagated down to the grade item.

          I've added a check so that an event isn't generated and the flag isn't propagated unless the visible flag has actually changed. I'm putting Fred down to peer review this as he seems to be the most likely candidate to have introduced this bug as part of a previous bug fix.

          Show
          Andrew Davis added a comment - I think I've figured this out. On the categories and items screen you're hiding the grade item associated with the activity. When you edit the activity you're inadvertently setting the visible flag on the activity and that is propagated down to the grade item. I've added a check so that an event isn't generated and the flag isn't propagated unless the visible flag has actually changed. I'm putting Fred down to peer review this as he seems to be the most likely candidate to have introduced this bug as part of a previous bug fix.
          Hide
          Andrew Davis added a comment -

          Putting this up for peer review.

          Show
          Andrew Davis added a comment - Putting this up for peer review.
          Hide
          Frédéric Massart added a comment -

          Hi Andrew, the patch looks good and I think this fixes an issue which as been there for a long time, I could replicate it on 2.2. IMO this should be backported down to 2.3.

          A minor comment about your patch, I would only use the if statement around the grade items logic, this to prevent possible regressions by skipping the rest of the logic. This does not look like it's required, but I think that is safer.

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

          About the commit message, I don't think it's necessary to describe the file and function you're altering in there. And I know that some integrators would be charmed to see a 70 characters commit message .

          Cheers,

          Fred

          Show
          Frédéric Massart added a comment - Hi Andrew, the patch looks good and I think this fixes an issue which as been there for a long time, I could replicate it on 2.2. IMO this should be backported down to 2.3. A minor comment about your patch, I would only use the if statement around the grade items logic, this to prevent possible regressions by skipping the rest of the logic. This does not look like it's required, but I think that is safer. [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [Y] Documentation [?] Git [?] Sanity check About the commit message, I don't think it's necessary to describe the file and function you're altering in there. And I know that some integrators would be charmed to see a 70 characters commit message . Cheers, Fred
          Hide
          Andrew Davis added a comment -

          I've shortened the commit message to avoid the wrath of the integrators

          Re shifting the code so that only grade related code is skipped, the only non-grade code there is raising a show/hide event and calling rebuild_course_cache(). There is some possibility of a regression from not rebuilding the cache in particular. That said, if we're relying on rebuilding the cache in a class that hasn't actually changed we have problems elsewhere. At the moment I'm inclined to leave it the way it is with you have every right to tell me "I told you so" if this causes problems.

          Show
          Andrew Davis added a comment - I've shortened the commit message to avoid the wrath of the integrators Re shifting the code so that only grade related code is skipped, the only non-grade code there is raising a show/hide event and calling rebuild_course_cache(). There is some possibility of a regression from not rebuilding the cache in particular. That said, if we're relying on rebuilding the cache in a class that hasn't actually changed we have problems elsewhere. At the moment I'm inclined to leave it the way it is with you have every right to tell me "I told you so" if this causes problems.
          Hide
          Andrew Davis added a comment -

          Adding branches and sending this to the integrators for their consideration.

          Show
          Andrew Davis added a comment - Adding branches and sending this to the integrators for their consideration.
          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
          Sam Hemelryk added a comment -

          Thanks guys, this has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks guys, this has been integrated now.
          Hide
          Rajesh Taneja added a comment -

          Thanks Andrew,

          Works as described.

          FYI:
          If grade is hidden, changing visibility on activity from hide => show make grade visible. After talking to Fred, it seems fine.

          Show
          Rajesh Taneja added a comment - Thanks Andrew, Works as described. FYI: If grade is hidden, changing visibility on activity from hide => show make grade visible. After talking to Fred, it seems fine.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: