From: David Mudrak Date: Mon, 4 Jan 2010 18:21:42 +0000 (+0000) Subject: MDL-19930 workshop: comments grading strategy X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=d84d06ceff82cd7475ddb32ce765c82a07e9c367;p=moodle.git MDL-19930 workshop: comments grading strategy --- diff --git a/mod/workshop/assessment.php b/mod/workshop/assessment.php index c6339ca19e..9c87490585 100644 --- a/mod/workshop/assessment.php +++ b/mod/workshop/assessment.php @@ -132,5 +132,5 @@ if ($isreviewer) { $mform->display(); if ($canoverridegrades) { $feedbackform->display(); - echo $OUTPUT->footer(); } +echo $OUTPUT->footer(); diff --git a/mod/workshop/eval/best/lib.php b/mod/workshop/eval/best/lib.php index b4aa88dcae..c9c1004658 100644 --- a/mod/workshop/eval/best/lib.php +++ b/mod/workshop/eval/best/lib.php @@ -241,7 +241,11 @@ class workshop_best_evaluation implements workshop_evaluation { foreach ($assessment->dimgrades as $dimid => $dimgrade) { $dimmin = $diminfo[$dimid]->min; $dimmax = $diminfo[$dimid]->max; - $assessment->dimgrades[$dimid] = grade_floatval(($dimgrade - $dimmin) / ($dimmax - $dimmin) * 100); + if ($dimmin == $dimmax) { + $assessment->dimgrades[$dimid] = grade_floatval($dimmax); + } else { + $assessment->dimgrades[$dimid] = grade_floatval(($dimgrade - $dimmin) / ($dimmax - $dimmin) * 100); + } } } return $assessments; diff --git a/mod/workshop/eval/best/simpletest/testlib.php b/mod/workshop/eval/best/simpletest/testlib.php index 82109a02ee..16d9caa187 100644 --- a/mod/workshop/eval/best/simpletest/testlib.php +++ b/mod/workshop/eval/best/simpletest/testlib.php @@ -115,6 +115,22 @@ class workshop_best_evaluation_test extends UnitTestCase { $this->assertEqual($norm[7]->dimgrades[4], 0); } + public function test_normalize_grades_max_equals_min() { + // fixture set-up + $assessments = array(); + $assessments[1] = (object)array( + 'dimgrades' => array(3 => 100.0000), + ); + $diminfo = array( + 3 => (object)array('min' => 100, 'max' => 100), + ); + // excersise SUT + $norm = $this->evaluator->normalize_grades($assessments, $diminfo); + // validate + $this->assertIsA($norm, 'array'); + $this->assertEqual($norm[1]->dimgrades[3], 100); + } + public function test_average_assessment() { // fixture set-up $assessments = array(); diff --git a/mod/workshop/form/assessment_form.php b/mod/workshop/form/assessment_form.php index 16dd069178..af2c616c45 100644 --- a/mod/workshop/form/assessment_form.php +++ b/mod/workshop/form/assessment_form.php @@ -64,13 +64,13 @@ class workshop_assessment_form extends moodleform { $buttonarray = array(); if ($this->mode == 'preview') { - $buttonarray[] = $mform->createElement('submit', 'backtoeditform', get_string('backtoeditform', 'workshop')); + $buttonarray[] = $mform->createElement('cancel', 'backtoeditform', get_string('backtoeditform', 'workshop')); } if ($this->mode == 'assessment') { $buttonarray[] = $mform->createElement('submit', 'saveandcontinue', get_string('saveandcontinue', 'workshop')); $buttonarray[] = $mform->createElement('submit', 'saveandclose', get_string('saveandclose', 'workshop')); + $buttonarray[] = $mform->createElement('cancel'); } - $buttonarray[] = $mform->createElement('cancel'); $mform->addGroup($buttonarray, 'buttonar', '', array(' '), false); $mform->closeHeaderBefore('buttonar'); } diff --git a/mod/workshop/form/comments/assessment_form.php b/mod/workshop/form/comments/assessment_form.php index 3ac6bf9d4d..dd9d5e4ac8 100644 --- a/mod/workshop/form/comments/assessment_form.php +++ b/mod/workshop/form/comments/assessment_form.php @@ -65,15 +65,11 @@ class workshop_comments_assessment_form extends workshop_assessment_form { $desc .= "\n"; $mform->addElement('html', $desc); - // grade for this aspect - $label = get_string('dimensiongrade', 'workshopform_comments'); - $options = make_grades_menu($fields->{'grade__idx_' . $i}); - $mform->addElement('select', 'grade__idx_' . $i, $label, $options); - // comment $label = get_string('dimensioncomment', 'workshopform_comments'); //$mform->addElement('editor', 'peercomment__idx_' . $i, $label, null, array('maxfiles' => 0)); - $mform->addElement('textarea', 'peercomment__idx_' . $i, $label, array('cols' => 60, 'rows' => 5)); + $mform->addElement('textarea', 'peercomment__idx_' . $i, $label, array('cols' => 60, 'rows' => 10)); + $mform->addRule('peercomment__idx_' . $i, null, 'required', null, 'client'); } $this->set_data($current); } diff --git a/mod/workshop/form/comments/edit_form.php b/mod/workshop/form/comments/edit_form.php index ffc3778e12..a8c7d0b18a 100644 --- a/mod/workshop/form/comments/edit_form.php +++ b/mod/workshop/form/comments/edit_form.php @@ -59,13 +59,6 @@ class workshop_edit_comments_strategy_form extends workshop_edit_strategy_form { $mform->addElement('hidden', 'dimensionid__idx_'.$i); $mform->addElement('editor', 'description__idx_'.$i.'_editor', get_string('dimensiondescription', 'workshopform_comments'), '', $descriptionopts); - // todo replace modgrade with an advanced element (usability issue discussed with Olli) - $mform->addElement('modgrade', 'grade__idx_'.$i, - get_string('dimensionmaxgrade','workshopform_comments'), null, true); - $mform->setDefault('grade__idx_'.$i, 10); - $mform->addElement('select', 'weight__idx_'.$i, - get_string('dimensionweight', 'workshopform_comments'), $weights); - $mform->setDefault('weight__idx_'.$i, 1); } $mform->registerNoSubmitButton('noadddims'); diff --git a/mod/workshop/form/comments/lib.php b/mod/workshop/form/comments/lib.php index 88eb40937a..881161c0de 100644 --- a/mod/workshop/form/comments/lib.php +++ b/mod/workshop/form/comments/lib.php @@ -175,7 +175,6 @@ class workshop_comments_strategy implements workshop_strategy { $dimid = $fields->{'dimensionid__idx_'.$i}; if (isset($grades[$dimid])) { $current->{'gradeid__idx_'.$i} = $grades[$dimid]->id; - $current->{'grade__idx_'.$i} = $grades[$dimid]->grade; $current->{'peercomment__idx_'.$i} = $grades[$dimid]->peercomment; } } @@ -202,7 +201,7 @@ class workshop_comments_strategy implements workshop_strategy { * * @param stdClass $assessment Assessment being filled * @param stdClass $data Raw data as returned by the assessment form - * @return float|null Raw grade (0.00000 to 100.00000) for submission as suggested by the peer + * @return float|null Constant raw grade 100.00000 for submission as suggested by the peer */ public function save_assessment(stdClass $assessment, stdClass $data) { global $DB; @@ -216,7 +215,7 @@ class workshop_comments_strategy implements workshop_strategy { $grade->assessmentid = $assessment->id; $grade->strategy = 'comments'; $grade->dimensionid = $data->{'dimensionid__idx_' . $i}; - $grade->grade = $data->{'grade__idx_' . $i}; + $grade->grade = 100.00000; $grade->peercomment = $data->{'peercomment__idx_' . $i}; $grade->peercommentformat = FORMAT_MOODLE; if (empty($grade->id)) { @@ -227,7 +226,8 @@ class workshop_comments_strategy implements workshop_strategy { $DB->update_record('workshop_grades', $grade); } } - return $this->update_peer_grade($assessment); + $this->workshop->set_peer_grade($assessment->id, 100.00000); + return 100.0000; } /** @@ -250,7 +250,7 @@ class workshop_comments_strategy implements workshop_strategy { $sql = 'SELECT s.id AS submissionid, a.id AS assessmentid, a.weight AS assessmentweight, a.reviewerid, a.gradinggrade, - g.dimensionid, g.grade + g.dimensionid, 100.00000 AS grade FROM {workshop_submissions} s JOIN {workshop_assessments} a ON (a.submissionid = s.id) JOIN {workshop_grades} g ON (g.assessmentid = a.id AND g.strategy = :strategy) @@ -278,26 +278,15 @@ class workshop_comments_strategy implements workshop_strategy { public function get_dimensions_info() { global $DB; - $sql = 'SELECT d.id, d.grade, d.weight, s.scale - FROM {workshopform_comments} d - LEFT JOIN {scale} s ON (d.grade < 0 AND -d.grade = s.id) - WHERE d.workshopid = :workshopid'; $params = array('workshopid' => $this->workshop->id); - $dimrecords = $DB->get_records_sql($sql, $params); + $dimrecords = $DB->get_records('workshopform_comments', array('workshopid' => $this->workshop->id), 'sort', 'id'); $diminfo = array(); foreach ($dimrecords as $dimid => $dimrecord) { $diminfo[$dimid] = new stdClass(); $diminfo[$dimid]->id = $dimid; - $diminfo[$dimid]->weight = $dimrecord->weight; - if ($dimrecord->grade < 0) { - // the dimension uses a scale - $diminfo[$dimid]->min = 1; - $diminfo[$dimid]->max = count(explode(',', $dimrecord->scale)); - } else { - // the dimension uses points - $diminfo[$dimid]->min = 0; - $diminfo[$dimid]->max = grade_floatval($dimrecord->grade); - } + $diminfo[$dimid]->weight = 1; + $diminfo[$dimid]->min = 100; + $diminfo[$dimid]->max = 100; } return $diminfo; } @@ -313,14 +302,7 @@ class workshop_comments_strategy implements workshop_strategy { */ protected function load_fields() { global $DB; - - $sql = 'SELECT * - FROM {workshopform_comments} - WHERE workshopid = :workshopid - ORDER BY sort'; - $params = array('workshopid' => $this->workshop->id); - - return $DB->get_records_sql($sql, $params); + return $DB->get_records('workshopform_comments', array('workshopid' => $this->workshop->id), 'sort'); } /** @@ -337,8 +319,6 @@ class workshop_comments_strategy implements workshop_strategy { $formdata->{'dimensionid__idx_' . $key} = $dimension->id; $formdata->{'description__idx_' . $key} = $dimension->description; $formdata->{'description__idx_' . $key.'format'} = $dimension->descriptionformat; - $formdata->{'grade__idx_' . $key} = $dimension->grade; - $formdata->{'weight__idx_' . $key} = $dimension->weight; $key++; } return $formdata; @@ -387,8 +367,6 @@ class workshop_comments_strategy implements workshop_strategy { $cook->comments[$i]->workshopid = $this->workshop->id; $cook->comments[$i]->sort = $i + 1; $cook->comments[$i]->description_editor = $raw->{'description__idx_'.$i.'_editor'}; - $cook->comments[$i]->grade = $raw->{'grade__idx_'.$i}; - $cook->comments[$i]->weight = $raw->{'weight__idx_'.$i}; } return $cook; } @@ -415,95 +393,4 @@ class workshop_comments_strategy implements workshop_strategy { return $DB->get_records_sql($sql, $params); } - - /** - * Aggregates the assessment form data and sets the grade for the submission given by the peer - * - * @param stdClass $assessment Assessment record - * @return float|null Raw grade (from 0.00000 to 100.00000) 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)) { - $this->workshop->set_peer_grade($assessment->id, $suggested); - } - 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 Raw grade (from 0.00000 to 100.00000) 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 (grade_floats_equal($dimension->weight, 0) or grade_floats_equal($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 * 100; - $sumweights += $dimension->weight; - } else { - // regular grade - $sumgrades += ($grade->grade / $dimension->grade) * $dimension->weight * 100; - $sumweights += $dimension->weight; - } - } - - if ($sumweights === 0) { - return 0; - } - return grade_floatval($sumgrades / $sumweights); - } - - /** - * Convert scale grade to numerical grades - * - * In comments 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('scale', '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/form/comments/simpletest/testlib.php b/mod/workshop/form/comments/simpletest/testlib.php deleted file mode 100644 index 4156b7904f..0000000000 --- a/mod/workshop/form/comments/simpletest/testlib.php +++ /dev/null @@ -1,230 +0,0 @@ -. - -/** - * Unit tests for Accumulative grading strategy logic - * - * @package mod-workshopform-comments - * @copyright 2009 David Mudrak - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -defined('MOODLE_INTERNAL') || die(); - -// Include the code to test -require_once($CFG->dirroot . '/mod/workshop/locallib.php'); -require_once($CFG->dirroot . '/mod/workshop/form/comments/lib.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_comments_strategy extends workshop_comments_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_comments_strategy_test extends UnitTestCase { - - /** real database */ - protected $realDB; - - /** workshop instance emulation */ - protected $workshop; - - /** instance of the strategy logic class being tested */ - protected $strategy; - - /** - * Setup testing environment - */ - public function setUp() { - global $DB; - $this->realDB = $DB; - $DB = new mockDB(); - - $cm = new stdClass(); - $course = new stdClass(); - $context = new stdClass(); - $workshop = (object)array('id' => 42, 'strategy' => 'comments'); - $this->workshop = new workshop($workshop, $cm, $course, $context); - $this->strategy = new testable_workshop_comments_strategy($this->workshop); - } - - 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->strategy->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.00000'); - // excercise SUT - $suggested = $this->strategy->calculate_peer_grade($grades); - // validate - $this->assertEqual(grade_floatval(5/20 * 100), $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(grade_floatval(5/20 * 100), $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.00000'); - $grades[] = (object)array('dimensionid' => 1004, 'grade' => '87.00000'); - $grades[] = (object)array('dimensionid' => 1005, 'grade' => '10.00000'); - - // excercise SUT - $suggested = $this->strategy->calculate_peer_grade($grades); - - // validate - $this->assertEqual(grade_floatval((11/20 + 87/100 + 10/10)/3 * 100), $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.00000'); - $grades[] = (object)array('dimensionid' => 1004, 'grade' => '66.00000'); - $grades[] = (object)array('dimensionid' => 1005, 'grade' => '4.00000'); - - // excercise SUT - $suggested = $this->strategy->calculate_peer_grade($grades); - - // validate - $this->assertEqual(grade_floatval((7/15*3 + 66/80*1 + 4/5*2)/6 * 100), $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.00000'); - $DB->expectOnce('get_field', array('scale', 'scale', array('id' => 10), MUST_EXIST)); - $DB->setReturnValue('get_field', $mockscale); - - // excercise SUT - $suggested = $this->strategy->calculate_peer_grade($grades); - - // validate - $this->assertEqual(100.00000, $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.00000'); - $DB->expectNever('get_field', array('scale', 'scale', array('id' => 10), MUST_EXIST)); // cached - - // excercise SUT - $suggested = $this->strategy->calculate_peer_grade($grades); - - // validate - $this->assertEqual(0.00000, $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.00000'); // "Good" - $grades[] = (object)array('dimensionid' => 1019, 'grade' => '5.00000'); // "****" - - $DB->expectAt(0, 'get_field', array('scale', 'scale', array('id' => 13), MUST_EXIST)); - $DB->setReturnValueAt(0, 'get_field', $mockscale13); - - $DB->expectAt(1, 'get_field', array('scale', 'scale', array('id' => 17), MUST_EXIST)); - $DB->setReturnValueAt(1, 'get_field', $mockscale17); - - // excercise SUT - $suggested = $this->strategy->calculate_peer_grade($grades); - - // validate - $this->assertEqual(grade_floatval((1/2*2 + 4/6*3)/5 * 100), $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('scale', 'scale', array('id' => 13), MUST_EXIST)); // cached - $grades[] = (object)array('dimensionid' => 1012, 'grade' => '4.00000'); // exceeds the number of scale items - $this->expectException('coding_exception'); - - // excercise SUT - $suggested = $this->strategy->calculate_peer_grade($grades); - } -} diff --git a/mod/workshop/renderer.php b/mod/workshop/renderer.php index a92d90d8b9..bebcbc3a0f 100644 --- a/mod/workshop/renderer.php +++ b/mod/workshop/renderer.php @@ -622,7 +622,7 @@ class moodle_mod_workshop_renderer extends moodle_renderer_base { * @param string $separator between the grade and the reviewer/author * @return string */ - protected function grading_report_assessment(stdClass $assessment, $shownames, array $userinfo, $separator) { + protected function grading_report_assessment($assessment, $shownames, array $userinfo, $separator) { global $CFG; if (is_null($assessment)) {