Moodle
  1. Moodle
  2. MDL-32759

Assignment Submission Grade Doesn't Scale After Assignment Max Grade is Changed

    Details

    • Testing Instructions:
      Hide

      TEST 1:

      1. Create an assignment 2.2 and set grade to "no grade"
      2. As a student submit assignment.
      3. As a teacher "edit assignment" and change grade to 100
      4. Make sure you don't see any warning pop-up
      5. As a teacher again "edit assignment" and change grade to 30
      6. Make sure you don't see any warning pop-up
      7. Now grade student submission
      8. As a teacher again "edit assignment" and change grade to 50
      9. Make sure you see warning message, showing grades will not be scaled.
      10. Repeat above for New assignment (on 23 and master)

      TEST 2: (only on Master and 2.3)

      1. Create an assignment (New assignment) and set grade to "100"
      2. As a student submit assignment
      3. Grade assignment submission (give 45)
      4. Now edit assignment and change grade to 50, you should see pop-up warning.
      5. Save and view submission grades and make sure grade is 45/50 and final grade is 45/50.
      Show
      TEST 1: Create an assignment 2.2 and set grade to "no grade" As a student submit assignment. As a teacher "edit assignment" and change grade to 100 Make sure you don't see any warning pop-up As a teacher again "edit assignment" and change grade to 30 Make sure you don't see any warning pop-up Now grade student submission As a teacher again "edit assignment" and change grade to 50 Make sure you see warning message, showing grades will not be scaled. Repeat above for New assignment (on 23 and master) TEST 2: (only on Master and 2.3) Create an assignment (New assignment) and set grade to "100" As a student submit assignment Grade assignment submission (give 45) Now edit assignment and change grade to 50, you should see pop-up warning. Save and view submission grades and make sure grade is 45/50 and final grade is 45/50.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:
      wip-mdl-32759-warning
    • Rank:
      39751

      Description

      When an assignment's max grade is changed after assignment submissions have been graded, the existing grades aren't scaled to fit the new max grade.

      1. Create an assignment worth 100 points.
      2. As a student, submit the assignment.
      3. As a teacher/admin, grade the submission with 90 points (90%).
      4. Edit the assignment activity and change the grade value to 50 (half the original value).
      5. View the course gradebook and note that the student has 50 points for the assignment. Expected would be 45 (half the original grade and still 90% of the new max grade).
      6. View the assignment submissions (mod/assignment/submissions.php?id=XXXX). Note the Grade column shows 90/50 points and Final Grade column shows 50 points. Expected would be 45 for both.

      Similar behavior has also been observed when using rubrics to grade the assignment.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that, Chris.

          Feel free to contribute to the solution for this issue.

          Show
          Michael de Raadt added a comment - Thanks for reporting that, Chris. Feel free to contribute to the solution for this issue.
          Hide
          Mark Nielsen added a comment -

          Adding a patch for a workaround, not a fix. The patch just warns the user about changing the grade.

          Show
          Mark Nielsen added a comment - Adding a patch for a workaround, not a fix. The patch just warns the user about changing the grade.
          Hide
          Rajesh Taneja added a comment -

          Hello Chris and Mark,

          Not sure, but it seems to be intentional behaviour. As, assignments are marked manually, changing them automatically might create regression.

          It might be nice to notify user about the behaviour and not change manual grades.

          FYI: Change causing this.
          https://github.com/rajeshtaneja/moodle/blob/master/lib/grade/grade_item.php#L743

          It seems we have missed to add check for new assignment module as well.

          Adding Petr to get more feedback.

          Show
          Rajesh Taneja added a comment - Hello Chris and Mark, Not sure, but it seems to be intentional behaviour. As, assignments are marked manually, changing them automatically might create regression. It might be nice to notify user about the behaviour and not change manual grades. FYI: Change causing this. https://github.com/rajeshtaneja/moodle/blob/master/lib/grade/grade_item.php#L743 It seems we have missed to add check for new assignment module as well. Adding Petr to get more feedback.
          Hide
          Michael de Raadt added a comment -

          Hi, guys.

          I've been discussing this with Raj and I want to be sure we head down the right road.

          Did this come to you from concerned users or did you just see this as a flaw from a development perspective?

          I can think if a scenario where scaling would be undesirable: A teacher creates an assignment and marks their students out of 20. Later they realise that the grade value is set to 100, which is the default, and changes this to 20. In this scenario, the teacher would be unhappy if all their grades were scaled. I think this would be more common than a teacher marking students then deciding they want to scale all their students by changing the grade value. The current mechanism (which is exceptional for assignment where there is more manual marking) seems safer to me. Of course I am considering the "Simple grading" route; scaling might still be needed for other grading methods.

          So I'm just wanting to suggest that discussion may be useful before we make any changes here.

          I do like the idea of informing the user about the consequences of changing the grade value after assignments have been marked. Even if we did implement scaling of final results after changing a grade, a warning to a user before doing this would be good.

          This also has ramifications on the new assignment module, which I believe follows the more regular behaviour of scaling.

          Show
          Michael de Raadt added a comment - Hi, guys. I've been discussing this with Raj and I want to be sure we head down the right road. Did this come to you from concerned users or did you just see this as a flaw from a development perspective? I can think if a scenario where scaling would be undesirable: A teacher creates an assignment and marks their students out of 20. Later they realise that the grade value is set to 100, which is the default, and changes this to 20. In this scenario, the teacher would be unhappy if all their grades were scaled. I think this would be more common than a teacher marking students then deciding they want to scale all their students by changing the grade value. The current mechanism (which is exceptional for assignment where there is more manual marking) seems safer to me. Of course I am considering the "Simple grading" route; scaling might still be needed for other grading methods. So I'm just wanting to suggest that discussion may be useful before we make any changes here. I do like the idea of informing the user about the consequences of changing the grade value after assignments have been marked. Even if we did implement scaling of final results after changing a grade, a warning to a user before doing this would be good. This also has ramifications on the new assignment module, which I believe follows the more regular behaviour of scaling.
          Hide
          Andrew Davis added a comment - - edited

          Unfortunately the best option for the user is probably going to be the most complex to develop. It's easy to imagine the following scenarios occurring:

          1) No scaling when its desirable - A teacher grades a bunch of assignments then needs to alter the max grade. Either they forgot an assessment when setting up their gradebook or someone just mandated that they include an additional assessment. They reduce the max grade of an existing assignment to make room. Without any sort of automatic scaling they have to get out their calculator and manually scale their already entered grades.

          2) Scaling when its not desirable - A teacher grades a bunch of assignments. Assignment is meant to be out of 20 and they grade it accordingly. Its actually set to 100. They realize its set up incorrectly and turn down max grade to 20. In this scenario they won't want their grades scaled. 15/20 (but actually out of 100) would get scaled down to 3/20. Again, the teacher would have to go through and manually fix this.

          Either of these scenarios would make someone very unhappy with even a handful of students let alone a university class with a few hundred students.

          This may be something where we need to ask the user. If max grade changes and there are already entered grades ask the user whether they would like the existing grades to be scaled.

          Show
          Andrew Davis added a comment - - edited Unfortunately the best option for the user is probably going to be the most complex to develop. It's easy to imagine the following scenarios occurring: 1) No scaling when its desirable - A teacher grades a bunch of assignments then needs to alter the max grade. Either they forgot an assessment when setting up their gradebook or someone just mandated that they include an additional assessment. They reduce the max grade of an existing assignment to make room. Without any sort of automatic scaling they have to get out their calculator and manually scale their already entered grades. 2) Scaling when its not desirable - A teacher grades a bunch of assignments. Assignment is meant to be out of 20 and they grade it accordingly. Its actually set to 100. They realize its set up incorrectly and turn down max grade to 20. In this scenario they won't want their grades scaled. 15/20 (but actually out of 100) would get scaled down to 3/20. Again, the teacher would have to go through and manually fix this. Either of these scenarios would make someone very unhappy with even a handful of students let alone a university class with a few hundred students. This may be something where we need to ask the user. If max grade changes and there are already entered grades ask the user whether they would like the existing grades to be scaled.
          Hide
          Rajesh Taneja added a comment -

          Thanks Andrew,

          I agree, we should get this option from user and scale grades accordingly.

          Show
          Rajesh Taneja added a comment - Thanks Andrew, I agree, we should get this option from user and scale grades accordingly.
          Hide
          Chris Follin added a comment -

          >> Did this come to you from concerned users or did you just see this as a flaw from a development perspective?

          Michael, it was reported to us by clients who feel the current behavior is wrong. Of the two scenarios that Andrew identified, they fall squarely into #1.

          Show
          Chris Follin added a comment - >> Did this come to you from concerned users or did you just see this as a flaw from a development perspective? Michael, it was reported to us by clients who feel the current behavior is wrong. Of the two scenarios that Andrew identified, they fall squarely into #1.
          Hide
          Michael de Raadt added a comment -

          I think we should avoid scaling as there is a workaround for Andrew's scenario 1. Just scale the item in the gradebook.

          If we pursue a remedy that involves automatic scaling, I think this will create a change in behaviour that the user cannot understand.

          Show
          Michael de Raadt added a comment - I think we should avoid scaling as there is a workaround for Andrew's scenario 1. Just scale the item in the gradebook. If we pursue a remedy that involves automatic scaling, I think this will create a change in behaviour that the user cannot understand.
          Hide
          Rajesh Taneja added a comment -

          As discussed in Scrum today, I will be just using patch attached by Mark as resolution for this issue.

          I started working on providing scaling option to user, but it seems like a dev work and needs lot of thinking before we can satisfy all use-case scenarios.

          FYI:
          Patch for adding scaling option for assignment and assign
          https://github.com/rajeshtaneja/moodle/compare/master...wip-mdl-32759

          Show
          Rajesh Taneja added a comment - As discussed in Scrum today, I will be just using patch attached by Mark as resolution for this issue. I started working on providing scaling option to user, but it seems like a dev work and needs lot of thinking before we can satisfy all use-case scenarios. FYI: Patch for adding scaling option for assignment and assign https://github.com/rajeshtaneja/moodle/compare/master...wip-mdl-32759
          Hide
          Rajesh Taneja added a comment -

          Added Damyon, as it affects new assignment module as well.

          Show
          Rajesh Taneja added a comment - Added Damyon, as it affects new assignment module as well.
          Hide
          Frédéric Massart added a comment -

          Hi Mark, Raj,

          The patch looks good, but I'm concerned about the users who don't have Javascript enabled. Shouldn't they get a warning too?

          I also noticed two minor white space issues (link). Cutting hair, me?!

          If the non-javascript people don't need an alert, then feel free to push for integration.

          Thanks!

          Show
          Frédéric Massart added a comment - Hi Mark, Raj, The patch looks good, but I'm concerned about the users who don't have Javascript enabled. Shouldn't they get a warning too? I also noticed two minor white space issues (link) . Cutting hair, me?! If the non-javascript people don't need an alert, then feel free to push for integration. Thanks!
          Hide
          Rajesh Taneja added a comment -

          Good point Fred,

          I thought so, but not sure how well we can alert users without JS. As per current framework, grades option comes from course form and not module. Will have a look at it again and see what else can be done.

          Show
          Rajesh Taneja added a comment - Good point Fred, I thought so, but not sure how well we can alert users without JS. As per current framework, grades option comes from course form and not module. Will have a look at it again and see what else can be done.
          Hide
          Rajesh Taneja added a comment - - edited

          Thanks Fred, I have adding warning msg in noscript as well, for non-js.

          Will open another bug to take care of hard-coding https://github.com/rajeshtaneja/moodle/compare/MOODLE_23_STABLE...wip-mdl-32759-m23#L0R745.

          Show
          Rajesh Taneja added a comment - - edited Thanks Fred, I have adding warning msg in noscript as well, for non-js. Will open another bug to take care of hard-coding https://github.com/rajeshtaneja/moodle/compare/MOODLE_23_STABLE...wip-mdl-32759-m23#L0R745 .
          Hide
          Frédéric Massart added a comment -

          Hi Raj,

          patch looks good, although I am not convinced that this is the best way of alerting teachers, I can't think of a better way.

          Two minor remarks before you push for integration.

          • Instead of the <noscript> tag, we could use html_writer::tag('noscript'), it will not change a thing, but that keeps consistency across Moodle.
          • The wording of the sentence in the noscript says 'If you wish to continue' which sounds a bit strange as there is no validation whatsoever.

          Feel free to push for integration whenever you like. Thanks!

          Show
          Frédéric Massart added a comment - Hi Raj, patch looks good, although I am not convinced that this is the best way of alerting teachers, I can't think of a better way. Two minor remarks before you push for integration. Instead of the <noscript> tag, we could use html_writer::tag('noscript'), it will not change a thing, but that keeps consistency across Moodle. The wording of the sentence in the noscript says 'If you wish to continue' which sounds a bit strange as there is no validation whatsoever. Feel free to push for integration whenever you like. Thanks!
          Hide
          Rajesh Taneja added a comment -

          Thanks for the review Fred,

          I have done necessary changes. I agree this is not the best solution, but this is what we discussed in scrum to go with.

          Anyway pushing for integration.

          Show
          Rajesh Taneja added a comment - Thanks for the review Fred, I have done necessary changes. I agree this is not the best solution, but this is what we discussed in scrum to go with. Anyway pushing for integration.
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          I was going to formally integrate this and it could be enough for current stable branches... but for sure we need something able to specify if one module supports "re-scaling" of the grades or no, instead of harcoding the modules in gradelib (xxxxx_supports() perhaps?).

          Also, for those not allowing the recalculations to happen once certain condition is achieved (there are already graded submissions, for assignments) the solution shouldn't be to show one custom message but really locking the corresponding maxgrade setting (in the module), avoiding any change to it and then inform about the cause. At least that will provide consistent calculations for everybody. If the teacher wants to "re-scale" the grades, that can be done in the gradebook without modifying the activity.

          I think there are other places where we already lock some settings (perhaps conditional activities/completion) under similar circumstances. And they only get unlocked if the condition is cleaned (aka, all submission grades are cleaned; in this case, not sure if we want to offer the unlock possibility at all, so simple lock should be enough).

          For your consideration, as said, perhaps current code is ok for stables... but we need a better and supported solution / lock handling across all modules. For sure only assignments are affected right now, but we should support it globally.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited I was going to formally integrate this and it could be enough for current stable branches... but for sure we need something able to specify if one module supports "re-scaling" of the grades or no, instead of harcoding the modules in gradelib (xxxxx_supports() perhaps?). Also, for those not allowing the recalculations to happen once certain condition is achieved (there are already graded submissions, for assignments) the solution shouldn't be to show one custom message but really locking the corresponding maxgrade setting (in the module), avoiding any change to it and then inform about the cause. At least that will provide consistent calculations for everybody. If the teacher wants to "re-scale" the grades, that can be done in the gradebook without modifying the activity. I think there are other places where we already lock some settings (perhaps conditional activities/completion) under similar circumstances. And they only get unlocked if the condition is cleaned (aka, all submission grades are cleaned; in this case, not sure if we want to offer the unlock possibility at all, so simple lock should be enough). For your consideration, as said, perhaps current code is ok for stables... but we need a better and supported solution / lock handling across all modules. For sure only assignments are affected right now, but we should support it globally. Ciao
          Hide
          Rajesh Taneja added a comment -

          I agree with you Eloy,

          Fred and I was discussing about this and we were thinking about having a checkbox which will enable grades selection (If submission has been graded, then grades will be disabled by default and will get enabled by deselecting checkbox).

          @ELOY:
          Can you please suggest why we don't scale grades automatically or request for user input, if existing scale should be scaled? Point 2 is just an alternative, but not solution. Also, do we provide different patches for stable and master in same tracker issue (if they are so different) ?

          I will create two separate bugs to take care of these issue:

          1. Remove hard-coding
          2. Look at alternatives to alert user of grade scaling behaviour (waiting for Eloy to make sure this needs to be a separate bug)
          Show
          Rajesh Taneja added a comment - I agree with you Eloy, Fred and I was discussing about this and we were thinking about having a checkbox which will enable grades selection (If submission has been graded, then grades will be disabled by default and will get enabled by deselecting checkbox). @ELOY: Can you please suggest why we don't scale grades automatically or request for user input, if existing scale should be scaled? Point 2 is just an alternative, but not solution. Also, do we provide different patches for stable and master in same tracker issue (if they are so different) ? I will create two separate bugs to take care of these issue: Remove hard-coding Look at alternatives to alert user of grade scaling behaviour (waiting for Eloy to make sure this needs to be a separate bug)
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

          This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

          Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
          Hide
          Rajesh Taneja added a comment -

          Branches re-based.

          Show
          Rajesh Taneja added a comment - Branches re-based.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

          This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

          Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (22, 23 & master), thanks!

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

          What I'm looking for (ideally) is to implement some way to handle grades in core in a more organised way. So each module "declares" what it's using (ratings, grades, re-escale yes/no....) and we provide one standard way to control the interdependences with max grade and friends. But perhaps that's way too much ambitious and won't cover all "creative" possibilities out there.

          So, perhaps, for now, it's enough with removing harcoded and rely on each module "supports" to know from the gradebook stuff. Not really sure, I just consider that now each module is doing its own "war" (but ratings that are normalised) and we should be able to provide better support for other common grading schemes.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - What I'm looking for (ideally) is to implement some way to handle grades in core in a more organised way. So each module "declares" what it's using (ratings, grades, re-escale yes/no....) and we provide one standard way to control the interdependences with max grade and friends. But perhaps that's way too much ambitious and won't cover all "creative" possibilities out there. So, perhaps, for now, it's enough with removing harcoded and rely on each module "supports" to know from the gradebook stuff. Not really sure, I just consider that now each module is doing its own "war" (but ratings that are normalised) and we should be able to provide better support for other common grading schemes. Ciao
          Hide
          Damyon Wiese added a comment -

          FYI - we will start work on advanced marking features for the assignment module soon (Still writing the spec): http://docs.moodle.org/dev/Assignment_Advanced_Marking

          Show
          Damyon Wiese added a comment - FYI - we will start work on advanced marking features for the assignment module soon (Still writing the spec): http://docs.moodle.org/dev/Assignment_Advanced_Marking
          Hide
          Rossiani Wijaya added a comment -

          This works great.

          Test passed.

          Show
          Rossiani Wijaya added a comment - This works great. Test passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          YEAR!*

          CAF*, TOT!*

          • Your effort amazingly resulted. (unbelievable :-P)
          • Closing as fixed.
          • Tons of thanks.
          Show
          Eloy Lafuente (stronk7) added a comment - YEAR!* CAF*, TOT!* Your effort amazingly resulted. (unbelievable :-P) Closing as fixed. Tons of thanks.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: