From 78835c1a6c0ea1912aec2b63f7d46ea39b09f9bc Mon Sep 17 00:00:00 2001 From: Rajesh Taneja Date: Thu, 12 Dec 2013 09:53:48 +0800 Subject: [PATCH] MDL-21180 Gradebook: Consistent handling of change in aggregation Change from/to GRADE_AGGREGATE_WEIGHTED_MEAN and GRADE_AGGREGATE_EXTRACREDIT_MEAN was only handled prior to this patch. GRADE_AGGREGATE_SUM and GRADE_AGGREGATE_WEIGHTED_MEAN2 also behave similar and should be handled same way --- lib/grade/grade_category.php | 43 +++++++++---- lib/grade/tests/grade_category_test.php | 80 ++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 14 deletions(-) diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index 190ab09b12c..d5a9e6d72fa 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -967,10 +967,20 @@ class grade_category extends grade_object { * * @return bool True if extra credit used */ - function is_extracredit_used() { - return ($this->aggregation == GRADE_AGGREGATE_WEIGHTED_MEAN2 - or $this->aggregation == GRADE_AGGREGATE_EXTRACREDIT_MEAN - or $this->aggregation == GRADE_AGGREGATE_SUM); + public function is_extracredit_used() { + return self::aggregation_uses_extracredit($this->aggregation); + } + + /** + * Returns true if aggregation passed is using extracredit. + * + * @param int $aggregation Aggregation const. + * @return bool True if extra credit used + */ + public static function aggregation_uses_extracredit($aggregation) { + return ($aggregation == GRADE_AGGREGATE_WEIGHTED_MEAN2 + or $aggregation == GRADE_AGGREGATE_EXTRACREDIT_MEAN + or $aggregation == GRADE_AGGREGATE_SUM); } /** @@ -979,10 +989,21 @@ class grade_category extends grade_object { * @return bool True if an aggregation coefficient is being used */ public function is_aggregationcoef_used() { - return ($this->aggregation == GRADE_AGGREGATE_WEIGHTED_MEAN - or $this->aggregation == GRADE_AGGREGATE_WEIGHTED_MEAN2 - or $this->aggregation == GRADE_AGGREGATE_EXTRACREDIT_MEAN - or $this->aggregation == GRADE_AGGREGATE_SUM); + return self::aggregation_uses_aggregationcoef($this->aggregation); + + } + + /** + * Returns true if aggregation uses aggregationcoef + * + * @param int $aggregation Aggregation const. + * @return bool True if an aggregation coefficient is being used + */ + public static function aggregation_uses_aggregationcoef($aggregation) { + return ($aggregation == GRADE_AGGREGATE_WEIGHTED_MEAN + or $aggregation == GRADE_AGGREGATE_WEIGHTED_MEAN2 + or $aggregation == GRADE_AGGREGATE_EXTRACREDIT_MEAN + or $aggregation == GRADE_AGGREGATE_SUM); } @@ -1573,14 +1594,14 @@ class grade_category extends grade_object { //weight and extra credit share a column :( Would like a default of 1 for weight and 0 for extra credit //Flip from the default of 0 to 1 (or vice versa) if ALL items in the category are still set to the old default. - if ($params->aggregation==GRADE_AGGREGATE_WEIGHTED_MEAN || $params->aggregation==GRADE_AGGREGATE_EXTRACREDIT_MEAN) { + if (self::aggregation_uses_aggregationcoef($params->aggregation)) { $sql = $defaultaggregationcoef = null; - if ($params->aggregation==GRADE_AGGREGATE_WEIGHTED_MEAN) { + if (!self::aggregation_uses_extracredit($params->aggregation)) { //if all items in this category have aggregation coefficient of 0 we can change it to 1 ie evenly weighted $sql = "select count(id) from {grade_items} where categoryid=:categoryid and aggregationcoef!=0"; $defaultaggregationcoef = 1; - } else if ($params->aggregation==GRADE_AGGREGATE_EXTRACREDIT_MEAN) { + } else { //if all items in this category have aggregation coefficient of 1 we can change it to 0 ie no extra credit $sql = "select count(id) from {grade_items} where categoryid=:categoryid and aggregationcoef!=1"; $defaultaggregationcoef = 0; diff --git a/lib/grade/tests/grade_category_test.php b/lib/grade/tests/grade_category_test.php index 44277609b90..b78dd869b81 100644 --- a/lib/grade/tests/grade_category_test.php +++ b/lib/grade/tests/grade_category_test.php @@ -41,6 +41,7 @@ class grade_category_testcase extends grade_base_testcase { $this->sub_test_grade_category_aggregate_grades(); $this->sub_test_grade_category_apply_limit_rules(); $this->sub_test_grade_category_is_aggregationcoef_used(); + $this->sub_test_grade_category_aggregation_uses_aggregationcoef(); $this->sub_test_grade_category_fetch_course_tree(); $this->sub_test_grade_category_get_children(); $this->sub_test_grade_category_load_grade_item(); @@ -67,6 +68,8 @@ class grade_category_testcase extends grade_base_testcase { //do this last as adding a second course category messes up the data $this->sub_test_grade_category_insert_course_category(); + $this->sub_test_grade_category_is_extracredit_used(); + $this->sub_test_grade_category_aggregation_uses_extracredit(); } //adds 3 new grade categories at various depths @@ -534,11 +537,43 @@ class grade_category_testcase extends grade_base_testcase { } - /** - * TODO implement - */ protected function sub_test_grade_category_is_aggregationcoef_used() { + $category = new grade_category(); + // Following use aggregationcoef. + $category->aggregation = GRADE_AGGREGATE_WEIGHTED_MEAN; + $this->assertTrue($category->is_aggregationcoef_used()); + $category->aggregation = GRADE_AGGREGATE_WEIGHTED_MEAN2; + $this->assertTrue($category->is_aggregationcoef_used()); + $category->aggregation = GRADE_AGGREGATE_EXTRACREDIT_MEAN; + $this->assertTrue($category->is_aggregationcoef_used()); + $category->aggregation = GRADE_AGGREGATE_SUM; + $this->assertTrue($category->is_aggregationcoef_used()); + // Following don't use aggregationcoef. + $category->aggregation = GRADE_AGGREGATE_MAX; + $this->assertFalse($category->is_aggregationcoef_used()); + $category->aggregation = GRADE_AGGREGATE_MEAN; + $this->assertFalse($category->is_aggregationcoef_used()); + $category->aggregation = GRADE_AGGREGATE_MEDIAN; + $this->assertFalse($category->is_aggregationcoef_used()); + $category->aggregation = GRADE_AGGREGATE_MIN; + $this->assertFalse($category->is_aggregationcoef_used()); + $category->aggregation = GRADE_AGGREGATE_MODE; + $this->assertFalse($category->is_aggregationcoef_used()); + } + + protected function sub_test_grade_category_aggregation_uses_aggregationcoef() { + + $this->assertTrue(grade_category::aggregation_uses_aggregationcoef(GRADE_AGGREGATE_WEIGHTED_MEAN)); + $this->assertTrue(grade_category::aggregation_uses_aggregationcoef(GRADE_AGGREGATE_WEIGHTED_MEAN2)); + $this->assertTrue(grade_category::aggregation_uses_aggregationcoef(GRADE_AGGREGATE_EXTRACREDIT_MEAN)); + $this->assertTrue(grade_category::aggregation_uses_aggregationcoef(GRADE_AGGREGATE_SUM)); + + $this->assertFalse(grade_category::aggregation_uses_aggregationcoef(GRADE_AGGREGATE_MAX)); + $this->assertFalse(grade_category::aggregation_uses_aggregationcoef(GRADE_AGGREGATE_MEAN)); + $this->assertFalse(grade_category::aggregation_uses_aggregationcoef(GRADE_AGGREGATE_MEDIAN)); + $this->assertFalse(grade_category::aggregation_uses_aggregationcoef(GRADE_AGGREGATE_MIN)); + $this->assertFalse(grade_category::aggregation_uses_aggregationcoef(GRADE_AGGREGATE_MODE)); } protected function sub_test_grade_category_fetch_course_tree() { @@ -724,4 +759,43 @@ class grade_category_testcase extends grade_base_testcase { $grade->insert(); return $grade->rawgrade; } + + protected function sub_test_grade_category_is_extracredit_used() { + $category = new grade_category(); + // Following use aggregationcoef. + $category->aggregation = GRADE_AGGREGATE_WEIGHTED_MEAN2; + $this->assertTrue($category->is_extracredit_used()); + $category->aggregation = GRADE_AGGREGATE_EXTRACREDIT_MEAN; + $this->assertTrue($category->is_extracredit_used()); + $category->aggregation = GRADE_AGGREGATE_SUM; + $this->assertTrue($category->is_extracredit_used()); + + // Following don't use aggregationcoef. + $category->aggregation = GRADE_AGGREGATE_WEIGHTED_MEAN; + $this->assertFalse($category->is_extracredit_used()); + $category->aggregation = GRADE_AGGREGATE_MAX; + $this->assertFalse($category->is_extracredit_used()); + $category->aggregation = GRADE_AGGREGATE_MEAN; + $this->assertFalse($category->is_extracredit_used()); + $category->aggregation = GRADE_AGGREGATE_MEDIAN; + $this->assertFalse($category->is_extracredit_used()); + $category->aggregation = GRADE_AGGREGATE_MIN; + $this->assertFalse($category->is_extracredit_used()); + $category->aggregation = GRADE_AGGREGATE_MODE; + $this->assertFalse($category->is_extracredit_used()); + } + + protected function sub_test_grade_category_aggregation_uses_extracredit() { + + $this->assertTrue(grade_category::aggregation_uses_extracredit(GRADE_AGGREGATE_WEIGHTED_MEAN2)); + $this->assertTrue(grade_category::aggregation_uses_extracredit(GRADE_AGGREGATE_EXTRACREDIT_MEAN)); + $this->assertTrue(grade_category::aggregation_uses_extracredit(GRADE_AGGREGATE_SUM)); + + $this->assertFalse(grade_category::aggregation_uses_extracredit(GRADE_AGGREGATE_WEIGHTED_MEAN)); + $this->assertFalse(grade_category::aggregation_uses_extracredit(GRADE_AGGREGATE_MAX)); + $this->assertFalse(grade_category::aggregation_uses_extracredit(GRADE_AGGREGATE_MEAN)); + $this->assertFalse(grade_category::aggregation_uses_extracredit(GRADE_AGGREGATE_MEDIAN)); + $this->assertFalse(grade_category::aggregation_uses_extracredit(GRADE_AGGREGATE_MIN)); + $this->assertFalse(grade_category::aggregation_uses_extracredit(GRADE_AGGREGATE_MODE)); + } }