diff --git a/mod/workshop/assessment.php b/mod/workshop/assessment.php index 528f911a0bb..cc7072b3fe7 100644 --- a/mod/workshop/assessment.php +++ b/mod/workshop/assessment.php @@ -106,10 +106,10 @@ if ($mform->is_cancelled()) { redirect($workshop->editform_url()); } $rawgrade = $strategy->save_assessment($assessment, $data); - if (isset($data->saveandclose)) { + if (!is_null($rawgrade) and isset($data->saveandclose)) { echo $OUTPUT->header($navigation); echo $OUTPUT->heading(get_string('assessmentresult', 'workshop'), 2); - echo $OUTPUT->box('Given grade: ' . $rawgrade); // todo more detailed info using own renderer + echo $OUTPUT->box('Given grade: ' . sprintf("%01.2f", $rawgrade * 100) . ' %'); // todo more detailed info using own renderer echo $OUTPUT->continue_button($workshop->view_url()); echo $OUTPUT->footer(); die(); // bye-bye diff --git a/mod/workshop/grading/accumulative/simpletest/teststrategy.php b/mod/workshop/grading/accumulative/simpletest/teststrategy.php index d02be4bfc60..e913115b258 100644 --- a/mod/workshop/grading/accumulative/simpletest/teststrategy.php +++ b/mod/workshop/grading/accumulative/simpletest/teststrategy.php @@ -28,15 +28,30 @@ defined('MOODLE_INTERNAL') || die(); // Include the code to test require_once($CFG->dirroot . '/mod/workshop/grading/accumulative/strategy.php'); +global $DB; +Mock::generate(get_class($DB), 'mockDB'); + /** * Test subclass that makes all the protected methods we want to test public */ class testable_workshop_accumulative_strategy extends workshop_accumulative_strategy { + /** allows to set dimensions manually */ + public $dimensions = array(); + + /** + * This is where the calculation of suggested grade for submission is done + */ + public function calculate_peer_grade(array $grades) { + return parent::calculate_peer_grade($grades); + } } class workshop_accumulative_strategy_test extends UnitTestCase { + /** real database */ + protected $realDB; + /** workshop instance emulation */ protected $workshop; @@ -47,6 +62,10 @@ class workshop_accumulative_strategy_test extends UnitTestCase { * Setup testing environment */ public function setUp() { + global $DB; + $this->realDB = $DB; + $DB = new mockDB(); + $cm = (object)array('id' => 3); $course = (object)array('id' => 11); $workshop = (object)array('id' => 42, 'strategy' => 'accumulative'); @@ -55,8 +74,155 @@ class workshop_accumulative_strategy_test extends UnitTestCase { } public function tearDown() { + global $DB; + $DB = $this->realDB; + $this->workshop = null; $this->strategy = null; } + public function test_calculate_peer_grade_null_grade() { + // fixture set-up + $this->dimensions = array(); + $grades = array(); + // excercise SUT + $suggested = $this->strategy->calculate_peer_grade($grades); + // validate + $this->assertNull($suggested); + } + + public function test_calculate_peer_grade_one_numerical() { + // fixture set-up + $this->strategy->dimensions[1003] = (object)array('grade' => 20, 'weight' => 1); + $grades[] = (object)array('dimensionid' => 1003, 'grade' => 5); + // excercise SUT + $suggested = $this->strategy->calculate_peer_grade($grades); + // validate + $this->assertEqual(5/20, $suggested); + } + + public function test_calculate_peer_grade_negative_weight() { + // fixture set-up + $this->strategy->dimensions[1003] = (object)array('grade' => 20, 'weight' => -1); + $grades[] = (object)array('dimensionid' => 1003, 'grade' => 20); + $this->expectException('coding_exception'); + // excercise SUT + $suggested = $this->strategy->calculate_peer_grade($grades); + } + + public function test_calculate_peer_grade_one_numerical_weighted() { + // fixture set-up + $this->strategy->dimensions[1003] = (object)array('grade' => 20, 'weight' => 3); + $grades[] = (object)array('dimensionid' => 1003, 'grade' => 5); + // excercise SUT + $suggested = $this->strategy->calculate_peer_grade($grades); + // validate + $this->assertEqual(5/20, $suggested); + } + + public function test_calculate_peer_grade_three_numericals_same_weight() { + // fixture set-up + $this->strategy->dimensions[1003] = (object)array('grade' =>20, 'weight' => 2); + $this->strategy->dimensions[1004] = (object)array('grade' =>100, 'weight' => 2); + $this->strategy->dimensions[1005] = (object)array('grade' =>10, 'weight' => 2); + + $grades[] = (object)array('dimensionid' => 1003, 'grade' => 11); + $grades[] = (object)array('dimensionid' => 1004, 'grade' => 87); + $grades[] = (object)array('dimensionid' => 1005, 'grade' => 10); + + // excercise SUT + $suggested = $this->strategy->calculate_peer_grade($grades); + + // validate + $this->assertEqual((11/20 + 87/100 + 10/10)/3, $suggested); + } + + public function test_calculate_peer_grade_three_numericals_different_weights() { + // fixture set-up + $this->strategy->dimensions[1003] = (object)array('grade' =>15, 'weight' => 3); + $this->strategy->dimensions[1004] = (object)array('grade' =>80, 'weight' => 1); + $this->strategy->dimensions[1005] = (object)array('grade' =>5, 'weight' => 2); + + $grades[] = (object)array('dimensionid' => 1003, 'grade' => 7); + $grades[] = (object)array('dimensionid' => 1004, 'grade' => 66); + $grades[] = (object)array('dimensionid' => 1005, 'grade' => 4); + + // excercise SUT + $suggested = $this->strategy->calculate_peer_grade($grades); + + // validate + $this->assertEqual((7/15*3 + 66/80*1 + 4/5*2)/6, $suggested); + } + + public function test_calculate_peer_grade_one_scale_max() { + global $DB; + + // fixture set-up + $mockscale = 'E,D,C,B,A'; + $this->strategy->dimensions[1008] = (object)array('grade' => -10, 'weight' => 1); + $grades[] = (object)array('dimensionid' => 1008, 'grade' => 5); + $DB->expectOnce('get_field', array("scales", "scale", array("id" => 10), MUST_EXIST)); + $DB->setReturnValue('get_field', $mockscale); + + // excercise SUT + $suggested = $this->strategy->calculate_peer_grade($grades); + + // validate + $this->assertEqual(1, $suggested); + } + + public function test_calculate_peer_grade_one_scale_min_with_scale_caching() { + global $DB; + + // fixture set-up + $this->strategy->dimensions[1008] = (object)array('grade' => -10, 'weight' => 1); + $grades[] = (object)array('dimensionid' => 1008, 'grade' => 1); + $DB->expectNever('get_field', array("scales", "scale", array("id" => 10), MUST_EXIST)); // cached + + // excercise SUT + $suggested = $this->strategy->calculate_peer_grade($grades); + + // validate + $this->assertEqual(0, $suggested); + } + + public function test_calculate_peer_grade_two_scales_weighted() { + global $DB; + + // fixture set-up + $mockscale13 = 'Poor,Good,Excellent'; + $mockscale17 = '-,*,**,***,****,*****,******'; + + $this->strategy->dimensions[1012] = (object)array('grade' => -13, 'weight' => 2); + $this->strategy->dimensions[1019] = (object)array('grade' => -17, 'weight' => 3); + + $grades[] = (object)array('dimensionid' => 1012, 'grade' => 2); // "Good" + $grades[] = (object)array('dimensionid' => 1019, 'grade' => 5); // "****" + + $DB->expectAt(0, 'get_field', array("scales", "scale", array("id" => 13), MUST_EXIST)); + $DB->setReturnValueAt(0, 'get_field', $mockscale13); + + $DB->expectAt(1, 'get_field', array("scales", "scale", array("id" => 17), MUST_EXIST)); + $DB->setReturnValueAt(1, 'get_field', $mockscale17); + + // excercise SUT + $suggested = $this->strategy->calculate_peer_grade($grades); + + // validate + $this->assertEqual((1/2*2 + 4/6*3)/5, $suggested); + } + + public function test_calculate_peer_grade_scale_exception() { + global $DB; + + // fixture set-up + $mockscale13 = 'Poor,Good,Excellent'; + $this->strategy->dimensions[1012] = (object)array('grade' => -13, 'weight' => 1); + $DB->expectNever('get_field', array("scales", "scale", array("id" => 13), MUST_EXIST)); // cached + $grades[] = (object)array('dimensionid' => 1012, 'grade' => 4); // exceeds the number of scale items + $this->expectException('coding_exception'); + + // excercise SUT + $suggested = $this->strategy->calculate_peer_grade($grades); + } } diff --git a/mod/workshop/grading/accumulative/strategy.php b/mod/workshop/grading/accumulative/strategy.php index a6587ac63d0..82d240edffc 100644 --- a/mod/workshop/grading/accumulative/strategy.php +++ b/mod/workshop/grading/accumulative/strategy.php @@ -195,7 +195,7 @@ class workshop_accumulative_strategy implements workshop_strategy { /** * Deletes dimensions and removes embedded media from its descriptions * - * todo we may check that there are no assessments done using these dimensions + * todo we may check that there are no assessments done using these dimensions and probably remove them * * @param array $masterids * @return void @@ -276,7 +276,7 @@ class workshop_accumulative_strategy implements workshop_strategy { if ('assessment' === $mode and !empty($assessment)) { // load the previously saved assessment data - $grades = $DB->get_records('workshop_grades', array('assessmentid' => $assessment->id), '', 'dimensionid,*'); + $grades = $this->reindex_grades_by_dimension($this->get_current_assessment_data($assessment)); $current = new stdClass(); for ($i = 0; $i < $nodimensions; $i++) { $dimid = $fields->{'dimensionid__idx_'.$i}; @@ -308,7 +308,7 @@ class workshop_accumulative_strategy implements workshop_strategy { * * @param object $assessment Assessment being filled * @param object $data Raw data as returned by the assessment form - * @return float Percentual grade for submission as suggested by the peer + * @return float|null Percentual grade for submission as suggested by the peer */ public function save_assessment(stdClass $assessment, stdClass $data) { global $DB; @@ -340,16 +340,128 @@ class workshop_accumulative_strategy implements workshop_strategy { } /** - * Aggregate the assessment form data and set the grade for the submission given by the peer + * Returns the list of current grades filled by the reviewer * - * @param stdClass $assessment Assessment record - * @return float Percentual grade for submission as suggested by the peer + * @param object $assessment Assessment record + * @return array of filtered records from the table workshop_grades */ - protected function update_peer_grade(stdClass $assessment) { + protected function get_current_assessment_data(stdClass $assessment) { global $DB; - $given = $DB->get_records('workshop_grades', array('assessmentid' => $assessment->id)); - // use only grades given within the currently used strategy - + // fetch all grades accociated with this assessment + $grades = $DB->get_records("workshop_grades", array("assessmentid" => $assessment->id)); + + // filter grades given under an other strategy or assessment form + foreach ($grades as $grade) { + if (!isset($this->dimensions[$grade->dimensionid])) { + unset ($grades[$grade->id]); + } + } + return $grades; + } + + /** + * Reindexes the records returned by {@link get_current_assessment_data} by dimensionid + * + * @param mixed $grades + * @return array + */ + protected function reindex_grades_by_dimension($grades) { + $reindexed = array(); + foreach ($grades as $grade) { + $reindexed[$grade->dimensionid] = $grade; + } + return $reindexed; + } + + /** + * Aggregates the assessment form data and sets the grade for the submission given by the peer + * + * @param stdClass $assessment Assessment record + * @return float|null Percentual grade for submission as suggested by the peer + */ + protected function update_peer_grade(stdClass $assessment) { + $grades = $this->get_current_assessment_data($assessment); + $suggested = $this->calculate_peer_grade($grades); + if (!is_null($suggested)) { + // todo save into workshop_assessments + } + return $suggested; + } + + /** + * Calculates the aggregated grade given by the reviewer + * + * @param array $grades Grade records as returned by {@link get_current_assessment_data} + * @uses $this->dimensions + * @return float|null Percentual grade for submission as suggested by the peer + */ + protected function calculate_peer_grade(array $grades) { + + if (empty($grades)) { + return null; + } + $sumgrades = 0; + $sumweights = 0; + foreach ($grades as $grade) { + $dimension = $this->dimensions[$grade->dimensionid]; + if ($dimension->weight < 0) { + throw new coding_exception('Negative weights are not supported any more. Something is wrong with your data'); + } + if ($dimension->weight == 0 or $dimension->grade == 0) { + // does not influence the final grade + continue; + } + if ($dimension->grade < 0) { + // this is a scale + $scaleid = -$dimension->grade; + $sumgrades += $this->scale_to_grade($scaleid, $grade->grade) * $dimension->weight; + $sumweights += $dimension->weight; + } else { + // regular grade + $sumgrades += ($grade->grade / $dimension->grade) * $dimension->weight; + $sumweights += $dimension->weight; + } + } + + if ($sumweights === 0) { + return 0; + } + return $sumgrades / $sumweights; + } + + /** + * Convert scale grade to numerical grades + * + * In accumulative grading strategy, scales are considered as grades from 0 to M-1, where M is the number of scale items. + * + * @throws coding_exception + * @param string $scaleid Scale identifier + * @param int $item Selected scale item number, numbered 1, 2, 3, ... M + * @return float + */ + protected function scale_to_grade($scaleid, $item) { + global $DB; + + /** @var cache of numbers of scale items */ + static $numofscaleitems = array(); + + if (!isset($numofscaleitems[$scaleid])) { + $scale = $DB->get_field("scales", "scale", array("id" => $scaleid), MUST_EXIST); + $items = explode(',', $scale); + $numofscaleitems[$scaleid] = count($items); + unset($scale); + unset($items); + } + + if ($numofscaleitems[$scaleid] <= 1) { + throw new coding_exception('Invalid scale definition, no scale items found'); + } + + if ($item <= 0 or $numofscaleitems[$scaleid] < $item) { + throw new coding_exception('Invalid scale item number'); + } + + return ($item - 1) / ($numofscaleitems[$scaleid] - 1); } } diff --git a/mod/workshop/grading/lib.php b/mod/workshop/grading/lib.php index 00125cd8834..97b9da04f01 100644 --- a/mod/workshop/grading/lib.php +++ b/mod/workshop/grading/lib.php @@ -77,7 +77,7 @@ interface workshop_strategy { * * @param object $assessment Assessment being filled * @param object $data Raw data as returned by the assessment form - * @return float Percentual grade for submission as suggested by the peer + * @return float|float Percentual grade for submission as suggested by the peer or null if impossible to count */ public function save_assessment(stdClass $assessment, stdClass $data); }