MDL-82126 core_grades: overridden mark with penalty

catalyst · Dec 12, 2024 · a07074b · a07074b
1 parent 75fc440
commit a07074b
Show file tree

Hide file tree

Showing 10 changed files with 127 additions and 34 deletions.
diff --git a/grade/report/grader/lang/en/gradereport_grader.php b/grade/report/grader/lang/en/gradereport_grader.php
@@ -23,8 +23,8 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-$string['applypenaltytext'] = 'Deduct {$a}';
-$string['applypenaltytooltip'] = 'Apply a deduction to the grade. This value is retrieved from the current overridden grade.';
+$string['applypenaltytext'] = 'Penalty exemption';
+$string['applypenaltytooltip'] = 'If the box is checked, penalty will not be applied to the overridden grade.';
 $string['aria:dropdowncolumns'] = 'Collapsed columns found';
 $string['clearsearch'] = 'Clear searched users';
 $string['collapsedcolumns'] = 'Collapsed columns <span class="badge rounded-pill bg-primary text-white ms-1" data-collapse="count">{$a}</span>';

diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php
@@ -241,21 +241,27 @@ public function process_data($data) {
                             continue;
                         }
 
-                        // Detect if there is any mark deduction.
-                        if (!isset($data->deduction[$userid][$itemid])) {
-                            // If the grade item uses a custom scale.
-                            if (!empty($oldvalue->grade_item->scaleid)) {
+                        // Detect changes in exemption checkbox.
+                        if ($oldvalue->can_apply_penalty_to_overridden_mark()) {
+                            $exemptionchanged = isset($data->exemption[$userid][$itemid]) !=
+                                !$oldvalue->is_penalty_applied_to_overridden_mark();
+                        } else {
+                            $exemptionchanged = false;
+                        }
 
-                                if ((int)$oldvalue->finalgrade === (int)$postedvalue) {
-                                    continue;
-                                }
-                            } else {
-                                // The grade item uses a numeric scale.
+                        // If the grade item uses a custom scale
+                        if (!empty($oldvalue->grade_item->scaleid)) {
 
-                                // Format the finalgrade from the DB so that it matches the grade from the client.
-                                if ($postedvalue === format_float($oldvalue->finalgrade, $oldvalue->grade_item->get_decimals())) {
-                                    continue;
-                                }
+                            if ((int)$oldvalue->finalgrade === (int)$postedvalue && !$exemptionchanged) {
+                                continue;
+                            }
+                        } else {
+                            // The grade item uses a numeric scale
+
+                            // Format the finalgrade from the DB so that it matches the grade from the client
+                            if ($postedvalue === format_float($oldvalue->finalgrade, $oldvalue->grade_item->get_decimals())
+                                && !$exemptionchanged) {
+                                continue;
                             }
                         }
 
@@ -336,9 +342,8 @@ public function process_data($data) {
                     $gradeitem->update_overridden_mark($userid, $finalgrade);
 
                     // Apply penalty.
-                    if (isset($data->deduction[$userid][$itemid])) {
-                        $deductedmark = $data->deduction[$userid][$itemid];
-                        $gradeitem->update_final_grade($userid, $finalgrade - $deductedmark, 'gradepenalty', false,
+                    if ($oldvalue->can_apply_penalty_to_overridden_mark() && !isset($data->exemption[$userid][$itemid])) {
+                        $gradeitem->update_final_grade($userid, $finalgrade - $oldvalue->deductedmark, 'gradepenalty', false,
                             FORMAT_MOODLE, null, null, true);
                     }
                 }
@@ -1160,12 +1165,17 @@ public function get_right_rows(bool $displayaverages): array {
                                 $context->extraclasses .= ' statusicons';
                             }
 
-                            // If option to reapply deduction is enabled, add the option to the context.
-                            if (get_config('core', 'gradepenalty_overriddengrade')) {
+                            // Show option for user to apply penalty or not.
+                            if ($grade->can_apply_penalty_to_overridden_mark()) {
+                                $context->canapplypenalty = true;
                                 $context->deductedmark = format_float($grade->deductedmark, $decimalpoints);
-                                $context->reapplydeduction = $grade->deductedmark > 0;
-                                $context->deductionid = 'deduction_' . $userid . '_' . $item->id;
-                                $context->deductionname = 'deduction[' . $userid . '][' . $item->id .']';
+                                $context->penaltyexempted = !$grade->is_penalty_applied_to_overridden_mark();
+                                $context->exemptionid = 'exemption' . $userid . '_' . $item->id;
+                                $context->exemptionname = 'exemption[' . $userid . '][' . $item->id .']';
+                                $context->exemptionlabel = get_string('applypenaltytext', 'gradereport_grader');
+                                $context->exemptionarialabel = $gradelabel . ' ' .
+                                    get_string('applypenaltytext', 'gradereport_grader');
+                                $context->exemptiontooltip = get_string('applypenaltytooltip', 'gradereport_grader');
                             }
                         } else {
                             $context->extraclasses = 'gradevalue' . $hidden . $gradepass;

diff --git a/grade/report/grader/templates/cell.mustache b/grade/report/grader/templates/cell.mustache
@@ -45,14 +45,17 @@
                         <div class="row">
                             {{>core_grades/grades/grader/input}}
                         </div>
-                        {{#reapplydeduction}}
+                        {{#canapplypenalty}}
                             <div class="row deductedmark">
-                                <input type="checkbox" name="{{deductionname}}" value="{{deductedmark}}" id="{{deductionid}}" class="ms-0">
-                                <label for="{{deductionid}}" class="form-check-label ml-1" title="{{#str}}applypenaltytooltip, gradereport_grader{{/str}}">
-                                    {{#str}}applypenaltytext, gradereport_grader, {{deductedmark}}{{/str}}
+                                <input type="checkbox" name="{{exemptionname}}"
+                                       value="{{deductedmark}}" id="{{exemptionid}}" class="ms-0"
+                                       {{#penaltyexempted}}checked{{/penaltyexempted}}>
+                                <label for="{{exemptionid}}" class="form-check-label ml-1 accesshide" title="{{{exemptiontooltip}}}">
+                                    {{{exemptionarialabel}}}
                                 </label>
+                                <span class="form-check-label ml-1 ">{{{exemptionlabel}}}</span>
                             </div>
-                        {{/reapplydeduction}}
+                        {{/canapplypenalty}}
                     </div>
                 {{/scale}}
             {{/iseditable}}

diff --git a/grade/report/grader/tests/behat/grade_override_with_deduction.feature b/grade/report/grader/tests/behat/grade_override_with_deduction.feature
@@ -32,15 +32,19 @@ Feature: As a teacher, I want to override a grade with a deduction and check the
       | -1-                | -2-                  | -3-       | -4-       | -5-       |
       | Student 1          | student1@example.com | 60        | 80        | 140       |
     And I turn editing mode on
-    And I set the field "Student 1 Manual grade 01 grade" to "80"
-    And I click on "Deduct 10.00" "checkbox"
+    And I set the following fields to these values:
+        | Student 1 Manual grade 01 grade             | 80 |
+        | Student 1 Manual grade 01 Penalty exemption | 0  |
+        | Student 1 Manual grade 02 Penalty exemption | 1  |
     And I click on "Save changes" "button"
     And I turn editing mode off
     And the following should exist in the "user-grades" table:
       | -1-                | -2-                  | -3-       | -4-       | -5-       |
       | Student 1          | student1@example.com | 70        | 80        | 150       |
     And I turn editing mode on
-    And I set the field "Student 1 Manual grade 02 grade" to "100"
+    And I set the following fields to these values:
+      | Student 1 Manual grade 02 grade             | 100 |
+      | Student 1 Manual grade 02 Penalty exemption | 1   |
     And I click on "Save changes" "button"
     And I turn editing mode off
     And the following should exist in the "user-grades" table:

diff --git a/lib/db/install.xml b/lib/db/install.xml
@@ -2060,6 +2060,7 @@
         <FIELD NAME="aggregationstatus" TYPE="char" LENGTH="10" NOTNULL="true" DEFAULT="unknown" SEQUENCE="false" COMMENT="One of several values describing how this grade_grade was used when calculating the aggregation. Possible values are &quot;unknown&quot;, &quot;dropped&quot;, &quot;novalue&quot;, &quot;used&quot;"/>
         <FIELD NAME="aggregationweight" TYPE="number" LENGTH="10" NOTNULL="false" SEQUENCE="false" DECIMALS="5" COMMENT="If the aggregationstatus == 'included', then this is the percent this item contributed to the aggregation."/>
         <FIELD NAME="deductedmark" TYPE="number" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" DECIMALS="5" COMMENT="The mark deducted from final grade"/>
+        <FIELD NAME="overriddenmark" TYPE="number" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" DECIMALS="5" COMMENT="The overridden mark before applied penalty"/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id"/>

diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php
@@ -1319,5 +1319,21 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2024112900.03);
     }
 
+    if ($oldversion < 2024121000.00) {
+
+        // Define field overriddenmark to be added to grade_grades.
+        $table = new xmldb_table('grade_grades');
+        $field = new xmldb_field('overriddenmark', XMLDB_TYPE_NUMBER, '10, 5', null,
+            XMLDB_NOTNULL, null, '0', 'deductedmark');
+
+        // Conditionally launch add field penalty.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2024121000.00);
+    }
+
     return true;
 }
diff --git a/lib/grade/grade_grade.php b/lib/grade/grade_grade.php
@@ -50,7 +50,7 @@ class grade_grade extends grade_object {
     public $required_fields = array('id', 'itemid', 'userid', 'rawgrade', 'rawgrademax', 'rawgrademin',
                                  'rawscaleid', 'usermodified', 'finalgrade', 'hidden', 'locked',
                                  'locktime', 'exported', 'overridden', 'excluded', 'timecreated',
-                                 'timemodified', 'aggregationstatus', 'aggregationweight', 'deductedmark');
+                                 'timemodified', 'aggregationstatus', 'aggregationweight', 'deductedmark', 'overriddenmark');
 
     /**
      * Array of optional fields with default values (these should match db defaults)
@@ -221,6 +221,9 @@ class grade_grade extends grade_object {
     /** @var float $deductedmark mark deducted from final grade */
     public $deductedmark = 0;
 
+    /** @var float $overriddenmark mark overridden by teacher */
+    public $overriddenmark = 0;
+
     /**
      * Returns array of grades for given grade_item+users
      *
@@ -1287,4 +1290,45 @@ public function get_context() {
         $this->load_grade_item();
         return $this->grade_item->get_context();
     }
+
+    /**
+     * Determine if penalty is applied to this overridden mark.
+     *
+     * @return bool whether penalty is applied
+     */
+    public function can_apply_penalty_to_overridden_mark(): bool {
+        // Check config.
+        if (!get_config('core', 'gradepenalty_overriddengrade')) {
+            return false;
+        }
+
+        // Check if the raw grade was deducted.
+        if ($this->deductedmark <= 0) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * Whether the penalty is applied to this overridden mark.
+     *
+     * @return bool whether penalty is applied
+     */
+    public function is_penalty_applied_to_overridden_mark(): bool {
+        return $this->overridden > 0 && $this->overriddenmark > $this->finalgrade;
+    }
+
+    /**
+     * Whether the penalty is applied to this final grade.
+     *
+     * @return bool whether penalty is applied
+     */
+    public function is_penalty_applied_to_final_grade(): bool {
+        if ($this->overridden > 0) {
+            return $this->is_penalty_applied_to_overridden_mark();
+        } else {
+            return $this->deductedmark > 0;
+        }
+    }
 }
diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php
@@ -2161,6 +2161,21 @@ public function update_deducted_mark(int $userid, float $deductedmark): void {
         $grade->update();
     }
 
+    /**
+     * Update overridden mark for given user
+     *
+     * @param int $userid The graded user
+     * @param float $overriddenmark The mark deducted from final grade
+     */
+    public function update_overridden_mark(int $userid, float $overriddenmark): void {
+        $grade = new grade_grade([
+            'itemid' => $this->id,
+            'userid' => $userid,
+        ]);
+        $grade->overriddenmark = $overriddenmark;
+        $grade->update();
+    }
+
     /**
      * Calculates final grade values using the formula in the calculation property.
      * The parameters are taken from final grades of grade items in current course only.

diff --git a/lib/gradelib.php b/lib/gradelib.php
@@ -859,7 +859,7 @@ function show_penalty_indicator(grade_grade $grade): string {
     global $PAGE;
 
     // Show penalty indicator if penalty is greater than 0.
-    if ($grade->deductedmark > 0) {
+    if ($grade->is_penalty_applied_to_final_grade()) {
         $indicator = new \core_grades\output\penalty_indicator(2, $grade);
         $renderer = $PAGE->get_renderer('core_grades');
         return $renderer->render_penalty_indicator($indicator);

diff --git a/version.php b/version.php
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2024120500.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2024121200.00;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.
 $release  = '5.0dev (Build: 20241205)';    // Human-friendly version name