From 519429b24fbe72678d5cfc381b6a5adabffbf533 Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Thu, 2 Oct 2014 08:50:42 +0800 Subject: [PATCH] MDL-47110 core_grades: Normalisation of grade weights. This includes behat tests for this change. Part of: MDL-46576 --- grade/edit/tree/index.php | 21 ++++ .../behat/grade_natural_normalisation.feature | 117 ++++++++++++++++++ lang/en/grades.php | 1 + lib/grade/grade_category.php | 89 +++++++++++-- 4 files changed, 217 insertions(+), 11 deletions(-) create mode 100644 grade/tests/behat/grade_natural_normalisation.feature diff --git a/grade/edit/tree/index.php b/grade/edit/tree/index.php index 84dac1af3d0..6e4f9145711 100644 --- a/grade/edit/tree/index.php +++ b/grade/edit/tree/index.php @@ -170,6 +170,8 @@ switch ($action) { //Ideally we could do the updates through $grade_edit_tree to avoid recreating it $recreatetree = false; +$normalisationmessage = null; + if ($data = data_submitted() and confirm_sesskey()) { // Perform bulk actions first if (!empty($data->bulkmove)) { @@ -225,6 +227,21 @@ if ($data = data_submitted() and confirm_sesskey()) { } grade_regrade_final_grades($courseid); + // Check to see if any weights were automatically adjusted. + // Run through the data to obtain all of the weight categories. + foreach ($data as $key => $notused) { + // We only want the weight entries. + if (preg_match('/^(weight)_([0-9]+)$/', $key, $matches)) { + // Fetch the weight for the grade item. + $gradeitemweight = grade_item::fetch(array('id' => $matches[2], 'courseid' => $courseid))->aggregationcoef2; + // Compare what was entered from the form with what was actually entered into the database. + if ($data->$matches[0] != ($gradeitemweight * 100)) { + // Send a notification that the weights were automatically adjusted. + $normalisationmessage = get_string('weightsadjusted', 'grades'); + break; + } + } + } } print_grade_page_head($courseid, 'settings', 'setup', get_string('setupgradeslayout', 'grades')); @@ -240,6 +257,10 @@ echo ''; if ($recreatetree) { $grade_edit_tree = new grade_edit_tree($gtree, $movingeid, $gpr); } +// Check to see if we have a normalisation message to send. +if (!empty($normalisationmessage)) { + echo $OUTPUT->notification($normalisationmessage, 'notifymessage'); +} echo html_writer::table($grade_edit_tree->table); diff --git a/grade/tests/behat/grade_natural_normalisation.feature b/grade/tests/behat/grade_natural_normalisation.feature new file mode 100644 index 00000000000..84a9095e0e9 --- /dev/null +++ b/grade/tests/behat/grade_natural_normalisation.feature @@ -0,0 +1,117 @@ +@core @core_grades +Feature: We can use natural aggregation and weights will be normalised to a total of one hundred + In order to override weights + As a teacher + I need to add assessments to the gradebook. + + Background: + Given the following "courses" exist: + | fullname | shortname | category | groupmode | + | Course 1 | C1 | 0 | 1 | + And the following "users" exist: + | username | firstname | lastname | email | idnumber | + | teacher1 | Teacher | 1 | teacher1@asd.com | t1 | + | student1 | Student | 1 | student1@asd.com | s1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + And the following "grade categories" exist: + | fullname | course | + | Sub category 1 | C1 | + And the following "activities" exist: + | activity | course | idnumber | name | intro | grade | + | assign | C1 | a1 | Test assignment one | Submit something! | 300 | + | assign | C1 | a2 | Test assignment two | Submit something! | 100 | + | assign | C1 | a3 | Test assignment three | Submit something! | 150 | + | assign | C1 | a4 | Test assignment four | Submit nothing! | 150 | + And the following "activities" exist: + | activity | course | idnumber | name | intro | gradecategory | grade | + | assign | C1 | a5 | Test assignment five | Submit something! | Sub category 1 | 20 | + | assign | C1 | a6 | Test assignment six | Submit something! | Sub category 1 | 10 | + | assign | C1 | a7 | Test assignment seven | Submit nothing! | Sub category 1 | 15 | + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Grades" + And I set the field "Grade report" to "Set up grades layout" + And I follow "Edit Course 1" + And I set the field "Aggregation" to "Natural" + And I press "Save changes" + And I follow "Edit Sub category 1" + And I set the field "Aggregation" to "Natural" + And I press "Save changes" + + @javascript + Scenario: Setting all weights in a category to less than one hundred is normalised. + + Given I set the field "Override weight of Test assignment five" to "1" + And I set the field "Override weight of Test assignment six" to "1" + And I set the field "Override weight of Test assignment seven" to "1" + And I set the field "Weight of Test assignment five" to "1" + And I set the field "Weight of Test assignment six" to "1" + And I set the field "Weight of Test assignment seven" to "2" + And I press "Save changes" + + Then the field "Weight of Test assignment five" matches value "25.0" + And the field "Weight of Test assignment six" matches value "25.0" + And the field "Weight of Test assignment seven" matches value "50.0" + + @javascript + Scenario: Set one of the grade item weights to a figure over one hundred. + + Given I set the field "Override weight of Test assignment five" to "1" + And I set the field "Weight of Test assignment five" to "120" + And I press "Save changes" + + Then the field "Weight of Test assignment five" matches value "68.355" + And the field "Weight of Test assignment six" matches value "12.658" + And the field "Weight of Test assignment seven" matches value "18.987" + + @javascript + Scenario: Grade items weights are noramlised when all grade item weights are overridden. Extra credit is set to zero. + + Given I follow "Edit assign Test assignment seven" + And I set the field "Extra credit" to "1" + And I press "Save changes" + And I set the field "Override weight of Test assignment five" to "1" + And I set the field "Override weight of Test assignment six" to "1" + And I set the field "Weight of Test assignment five" to "60" + And I set the field "Weight of Test assignment six" to "50" + And I press "Save changes" + + Then the field "Weight of Test assignment five" matches value "54.545" + And the field "Weight of Test assignment six" matches value "45.455" + And the field "Weight of Test assignment seven" matches value "0.0" + + @javascript + Scenario: The extra credit grade item weight is overridden to a figure over one hundred and then + the grade item is set to normal. + + # And I follow "Reset weights of Sub category 1" + Given I follow "Edit assign Test assignment seven" + And I set the field "Extra credit" to "1" + And I press "Save changes" + And I set the field "Override weight of Test assignment seven" to "1" + And I set the field "Weight of Test assignment seven" to "105" + And I press "Save changes" + And I follow "Edit assign Test assignment seven" + And I set the field "Extra credit" to "0" + And I press "Save changes" + + Then the field "Weight of Test assignment five" matches value "32.52" + And the field "Weight of Test assignment six" matches value "16.26" + And the field "Weight of Test assignment seven" matches value "51.22" + + @javascript + Scenario: Two out of three grade items weights are overridden and one is not. + The overridden grade item weights total over one hundred. + + Given I set the field "Override weight of Test assignment six" to "1" + And I set the field "Override weight of Test assignment seven" to "1" + And I set the field "Weight of Test assignment six" to "55" + And I set the field "Weight of Test assignment seven" to "65" + And I press "Save changes" + + Then the field "Weight of Test assignment five" matches value "0.0" + And the field "Weight of Test assignment six" matches value "45.833" + And the field "Weight of Test assignment seven" matches value "54.167" diff --git a/lang/en/grades.php b/lang/en/grades.php index f2853c2a447..81b2fcfbc29 100644 --- a/lang/en/grades.php +++ b/lang/en/grades.php @@ -727,6 +727,7 @@ $string['weightofa'] = 'Weight of {$a}'; $string['weightorextracredit'] = 'Weight or extra credit'; $string['weight'] = 'Weight'; $string['weights'] = 'Weights'; +$string['weightsadjusted'] = 'Your weights have been adjusted to total 100.'; $string['weightsedit'] = 'Edit weights and extra credits'; $string['weightuc'] = 'Calculated weight'; $string['writinggradebookinfo'] = 'Writing gradebook settings'; diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index af0b95171bf..14ca81958ab 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -1237,7 +1237,7 @@ class grade_category extends grade_object { } $children = $this->get_children(); - $grade_item = null; + $gradeitem = null; // Calculate the sum of the grademax's of all the items within this category. $totalgrademax = 0; @@ -1245,13 +1245,36 @@ class grade_category extends grade_object { // Out of 1, how much weight has been manually overriden by a user? $totaloverriddenweight = 0; $totaloverriddengrademax = 0; + + // Has every assessment in this category been overridden? + $alloverriden = true; + // Does the grade item require normalising? + $requiresnormalising = false; + + // This array keeps track of the id and weight of every grade item that has been overridden. + $overridearray = array(); foreach ($children as $sortorder => $child) { - $grade_item = null; + $gradeitem = null; if ($child['type'] == 'item') { - $grade_item = $child['object']; + $gradeitem = $child['object']; } else if ($child['type'] == 'category') { - $grade_item = $child['object']->load_grade_item(); + $gradeitem = $child['object']->load_grade_item(); + } + + // Record the ID and the weight for this grade item. + $overridearray[$gradeitem->id] = array(); + $overridearray[$gradeitem->id]['extracredit'] = $gradeitem->aggregationcoef; + $overridearray[$gradeitem->id]['weight'] = $gradeitem->aggregationcoef2; + $overridearray[$gradeitem->id]['weightoverride'] = $gradeitem->weightoverride; + // If this item has had its weight overridden then set the flag to true, but + // only if all previous items were also overridden. Note that extra credit items + // are counted as overridden grade items. + $alloverriden = (($gradeitem->weightoverride || $gradeitem->aggregationcoef >= 1) && $alloverriden) ? true : false; + + // If the individual weight is higher than 1 then we automatically need to normalise. + if ($gradeitem->aggregationcoef2 > 1) { + $requiresnormalising = true; } if ($grade_item->aggregationcoef > 0) { @@ -1262,24 +1285,68 @@ class grade_category extends grade_object { continue; } - $totalgrademax += $grade_item->grademax; - if ($grade_item->weightoverride) { - $totaloverriddenweight += $grade_item->aggregationcoef2; - $totaloverriddengrademax += $grade_item->grademax; + $totalgrademax += $gradeitem->grademax; + if ($gradeitem->weightoverride) { + $totaloverriddenweight += $gradeitem->aggregationcoef2; + $totaloverriddengrademax += $gradeitem->grademax; } } + // Initialise this variable (used to keep track of the weight override total). + $normalisetotal = 0; + // Keep a record of how much the override total is to see if it is above 100. It it is then we need to set the + // other weights to zero and normalise the others. + $overriddentotal = 0; + // Total up all of the weights. + foreach ($overridearray as $gradeitemdetail) { + // If the grade item has extra credit, then don't add it to the normalisetotal. + if ($gradeitemdetail['extracredit'] < 1) { + $normalisetotal += $gradeitemdetail['weight']; + } + if ($gradeitemdetail['weightoverride']) { + // Add overriden weights up to see if they are greater than 1. + $overriddentotal += $gradeitemdetail['weight']; + } + } + + // If the overridden weight total is higher than 1 then set the other untouched weights to zero. + $setotherweightstozero = false; + if ($overriddentotal > 1 && !$requiresnormalising) { + // Make sure that this catergory of weights gets normalised. + $requiresnormalising = true; + // The normalised weights are only the overridden weights, so we just use the total of those. + $normalisetotal = $overriddentotal; + $setotherweightstozero = true; + } + $totalgrademax -= $totaloverriddengrademax; reset($children); foreach ($children as $sortorder => $child) { - $grade_item = null; + $gradeitem = null; if ($child['type'] == 'item') { - $grade_item = $child['object']; + $gradeitem = $child['object']; } else if ($child['type'] == 'category') { - $grade_item = $child['object']->load_grade_item(); + $gradeitem = $child['object']->load_grade_item(); } + + // If $overridearray is set then the grade items need to be normalised. + // if (isset($overridearray)) { + if (($alloverriden && $normalisetotal != 1) || $requiresnormalising) { + // Set weights that are not overridden to zero. + if ($setotherweightstozero && !$overridearray[$gradeitem->id]['weightoverride']) { + $gradeitem->aggregationcoef2 = 0; + } else { + // Just divide the overriden weight for this item against the total weight override of all items in this category. + $gradeitem->aggregationcoef2 = $overridearray[$gradeitem->id]['weight'] / $normalisetotal; + } + // Update the grade item to reflect these changes. + $gradeitem->update(); + // Don't bother with the next step, move onto the next grade item. + continue; + } + if (!$grade_item->weightoverride) { if ($totaloverriddenweight >= 1) { // There is no more weight to distribute.