From 8a1ba8ac042e43eada5cd20aba1cd3a1d23fe843 Mon Sep 17 00:00:00 2001 From: David Mudrak Date: Mon, 4 Jan 2010 18:12:12 +0000 Subject: [PATCH] MDL-20264 work in progress on grading evaluation support Also fixes small bugs in workshop api constructor and in using "scales" instead of "scale" for the core table name. --- mod/workshop/aggregate.php | 9 +- mod/workshop/eval/best/lib.php | 91 +++++++++++++++++++ mod/workshop/form/accumulative/lib.php | 49 +++++++++- .../accumulative/simpletest/teststrategy.php | 10 +- mod/workshop/form/lib.php | 2 +- mod/workshop/form/numerrors/lib.php | 2 +- mod/workshop/lib.php | 24 +++-- mod/workshop/locallib.php | 6 +- mod/workshop/view.php | 2 +- 9 files changed, 170 insertions(+), 25 deletions(-) diff --git a/mod/workshop/aggregate.php b/mod/workshop/aggregate.php index 459c89f122..e1e42d0e51 100644 --- a/mod/workshop/aggregate.php +++ b/mod/workshop/aggregate.php @@ -39,12 +39,17 @@ $PAGE->set_url(new moodle_url($workshop->aggregate_url(), array('cmid' => $cmid) require_login($course, false, $cm); require_capability('mod/workshop:overridegrades', $PAGE->context); +// load the grading evaluator +$evaluator = $workshop->grading_evaluation_instance(); + if ($confirm) { if (!confirm_sesskey()) { throw new moodle_exception('confirmsesskeybad'); } - $workshop->update_submission_grades(); - $workshop->update_grading_grades(); + $workshop->aggregate_submission_grades(); + //$evaluator->update_grading_grades(); + //$workshop->aggregate_grading_grades(); + //$workshop->aggregate_total_grades(); redirect($workshop->view_url()); } diff --git a/mod/workshop/eval/best/lib.php b/mod/workshop/eval/best/lib.php index c1b60f0c39..b868623414 100644 --- a/mod/workshop/eval/best/lib.php +++ b/mod/workshop/eval/best/lib.php @@ -27,10 +27,101 @@ defined('MOODLE_INTERNAL') || die(); require_once(dirname(dirname(__FILE__)) . '/lib.php'); // interface definition +require_once($CFG->libdir . '/gradelib.php'); /** * Defines the computation login of the grading evaluation subplugin */ class workshop_best_evaluation implements workshop_evaluation { + /** @var workshop the parent workshop instance */ + protected $workshop; + + /** + * Constructor + * + * @param workshop $workshop The workshop api instance + * @return void + */ + public function __construct(workshop $workshop) { + $this->workshop = $workshop; + } + + /** + * TODO + * + * @param null|int|array $restrict If null, update all reviewers, otherwise update just grades for the given reviewers(s) + * + * @return void + */ + public function update_grading_grades($restrict=null) { + global $DB; + + $grader = $this->workshop->grading_strategy_instance(); + if (! $grader->supports_evaluation($this)) { + throw new coding_exception('The currently selected grading strategy plugin does not + support this method of grading evaluation.'); + } + + // fetch a recordset with all assessments to process + $rs = $grader->get_assessments_recordset($restrict); + $batch = array(); // will contain a set of all assessments of a single submission + $previous = null; // a previous record in the recordset + foreach ($rs as $current) { + if (is_null($previous)) { + // we are processing the very first record in the recordset + $previous = $current; + } + if ($current->submissionid == $previous->submissionid) { + $batch[] = $current; + } else { + // process all the assessments of a sigle submission + $this->process_assessments($batch); + // start with a new batch to be processed + $batch = array($current); + $previous = $current; + } + } + // do not forget to process the last batch! + $this->process_assessments($batch); + $rs->close(); + } + +//////////////////////////////////////////////////////////////////////////////// +// Internal methods // +//////////////////////////////////////////////////////////////////////////////// + + /** + * Given a list of all assessments of a single submission, updates the grading grades in database + * + * @param array $assessments of stdClass Object(->assessmentid ->assessmentweight ->reviewerid ->submissionid + * ->dimensionid ->grade ->dimensionweight) + * @return void + */ + protected function process_assessments(array $assessments) { + global $DB; + + $grades = $this->evaluate_assessments($assessments); + foreach ($grades as $assessmentid => $grade) { + $record = new stdClass(); + $record->id = $assessmentid; + $record->gradinggrade = grade_floatval($grade); + $DB->update_record('workshop_assessments', $record, true); + } + } + + /** + * Given a list of all assessments of a single submission, calculates the grading grades for them + * + * @param array $assessments same structure as for {@link self::process_assessments()} + * @return array [(int)assessmentid => (float)gradinggrade] to be saved into {workshop_assessments} + */ + protected function evaluate_assessments(array $assessments) { + $gradinggrades = array(); + foreach ($assessments as $assessment) { + $gradinggrades[$assessment->assessmentid] = grade_floatval(rand(0, 100)); // todo + } + return $gradinggrades; + } + } diff --git a/mod/workshop/form/accumulative/lib.php b/mod/workshop/form/accumulative/lib.php index bd19b46011..ffe072fa4e 100644 --- a/mod/workshop/form/accumulative/lib.php +++ b/mod/workshop/form/accumulative/lib.php @@ -252,8 +252,51 @@ class workshop_accumulative_strategy implements workshop_strategy { * @param workshop_evaluation $evaluation the instance of grading evaluation class * @return bool true if the evaluation method is supported, false otherwise */ - public function evaluation_supported(workshop_evaluation $evaluation) { - return false; // todo does not support any evaluation yet + public function supports_evaluation(workshop_evaluation $evaluation) { + if (is_a($evaluation, 'workshop_best_evaluation')) { + return true; + } + // all other evaluation methods are not supported yet + return false; + } + +//////////////////////////////////////////////////////////////////////////////// +// Methods needed by 'best' evaluation plugin // +//////////////////////////////////////////////////////////////////////////////// + + /** + * TODO: short description. + * + * @param resource $restrict + * @return TODO + */ + public function get_assessments_recordset($restrict) { + global $DB; + + $sql = 'SELECT a.id AS assessmentid,a.weight AS assessmentweight,a.reviewerid, + s.id AS submissionid, + g.dimensionid,g.grade, + d.weight AS dimensionweight + 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) + JOIN {workshopform_accumulative} d ON (d.id = g.dimensionid) + WHERE s.example=0 AND s.workshopid=:workshopid'; // to be cont. + $params = array('workshopid' => $this->workshop->id, 'strategy' => $this->workshop->strategy); + + if (is_null($restrict)) { + // update all users - no more conditions + } elseif (!empty($restrict)) { + list($usql, $uparams) = $DB->get_in_or_equal($restrict, SQL_PARAMS_NAMED); + $sql .= " AND a.reviewerid $usql"; + $params = array_merge($params, $uparams); + } else { + throw new coding_exception('Empty value is not a valid parameter here'); + } + + $sql .= ' ORDER BY s.id'; // this is important for bulk processing + + return $DB->get_recordset_sql($sql, $params); } //////////////////////////////////////////////////////////////////////////////// @@ -443,7 +486,7 @@ class workshop_accumulative_strategy implements workshop_strategy { static $numofscaleitems = array(); if (!isset($numofscaleitems[$scaleid])) { - $scale = $DB->get_field("scales", "scale", array("id" => $scaleid), MUST_EXIST); + $scale = $DB->get_field('scale', 'scale', array('id' => $scaleid), MUST_EXIST); $items = explode(',', $scale); $numofscaleitems[$scaleid] = count($items); unset($scale); diff --git a/mod/workshop/form/accumulative/simpletest/teststrategy.php b/mod/workshop/form/accumulative/simpletest/teststrategy.php index 17a4dccaaa..4dcf21ea2d 100644 --- a/mod/workshop/form/accumulative/simpletest/teststrategy.php +++ b/mod/workshop/form/accumulative/simpletest/teststrategy.php @@ -162,7 +162,7 @@ class workshop_accumulative_strategy_test extends UnitTestCase { $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('scales', 'scale', array('id' => 10), MUST_EXIST)); + $DB->expectOnce('get_field', array('scale', 'scale', array('id' => 10), MUST_EXIST)); $DB->setReturnValue('get_field', $mockscale); // excercise SUT @@ -178,7 +178,7 @@ class workshop_accumulative_strategy_test extends UnitTestCase { // 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('scales', 'scale', array('id' => 10), MUST_EXIST)); // cached + $DB->expectNever('get_field', array('scale', 'scale', array('id' => 10), MUST_EXIST)); // cached // excercise SUT $suggested = $this->strategy->calculate_peer_grade($grades); @@ -200,10 +200,10 @@ class workshop_accumulative_strategy_test extends UnitTestCase { $grades[] = (object)array('dimensionid' => 1012, 'grade' => '2.00000'); // "Good" $grades[] = (object)array('dimensionid' => 1019, 'grade' => '5.00000'); // "****" - $DB->expectAt(0, 'get_field', array('scales', 'scale', array('id' => 13), MUST_EXIST)); + $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('scales', 'scale', array('id' => 17), MUST_EXIST)); + $DB->expectAt(1, 'get_field', array('scale', 'scale', array('id' => 17), MUST_EXIST)); $DB->setReturnValueAt(1, 'get_field', $mockscale17); // excercise SUT @@ -219,7 +219,7 @@ class workshop_accumulative_strategy_test extends UnitTestCase { // 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 + $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'); diff --git a/mod/workshop/form/lib.php b/mod/workshop/form/lib.php index adf7a879e9..d482978b37 100644 --- a/mod/workshop/form/lib.php +++ b/mod/workshop/form/lib.php @@ -94,6 +94,6 @@ interface workshop_strategy { * @param workshop_evaluation $evaluation the instance of grading evaluation class * @return bool true if the evaluation method is supported, false otherwise */ - public function evaluation_supported(workshop_evaluation $evaluation); + public function supports_evaluation(workshop_evaluation $evaluation); } diff --git a/mod/workshop/form/numerrors/lib.php b/mod/workshop/form/numerrors/lib.php index 617d47e3c1..8eceb3c975 100644 --- a/mod/workshop/form/numerrors/lib.php +++ b/mod/workshop/form/numerrors/lib.php @@ -287,7 +287,7 @@ class workshop_numerrors_strategy implements workshop_strategy { * @param workshop_evaluation $evaluation the instance of grading evaluation class * @return bool true if the evaluation method is supported, false otherwise */ - public function evaluation_supported(workshop_evaluation $evaluation) { + public function supports_evaluation(workshop_evaluation $evaluation) { if (is_a($evaluation, 'workshop_best_evaluation')) { return true; } diff --git a/mod/workshop/lib.php b/mod/workshop/lib.php index 2ba5bddb6f..2fd3b83d6d 100644 --- a/mod/workshop/lib.php +++ b/mod/workshop/lib.php @@ -167,16 +167,22 @@ function workshop_delete_instance($id) { if (! $workshop = $DB->get_record('workshop', array('id' => $id))) { return false; } + // delete all associated aggregations + $DB->delete_records('workshop_aggregations', array('workshopid' => $workshop->id)); + // get the list of ids of all submissions + $submissions = $DB->get_records('workshop_submissions', array('workshopid' => $workshop->id), '', 'id'); + // get the list of all allocated assessments + $assessments = $DB->get_records_list('workshop_assessments', 'submissionid', array_keys($submissions), '', 'id'); + // delete the associated records from the workshop core tables + $DB->delete_records_list('workshop_grades', 'assessmentid', array_keys($assessments)); + $DB->delete_records_list('workshop_assessments', 'id', array_keys($assessments)); + $DB->delete_records_list('workshop_submissions', 'id', array_keys($submissions)); + // todo call the static clean-up methods of all available subplugins + // ... + // finally remove the workshop record itself + $DB->delete_records('workshop', array('id' => $workshop->id)); - $result = true; - - # Delete any dependent records here # - - if (! $DB->delete_records('workshop', array('id' => $workshop->id))) { - $result = false; - } - - return $result; + return true; } /** diff --git a/mod/workshop/locallib.php b/mod/workshop/locallib.php index 64899a6177..d514467989 100644 --- a/mod/workshop/locallib.php +++ b/mod/workshop/locallib.php @@ -91,7 +91,7 @@ class workshop { // this is intentional - IMO there should be no such field as it violates // 3rd normal form with no real performance gain. This way I try to // demonstrate how backwards compatibility could be achieved --mudrd8mz - $this->context = get_context_instance(CONTEXT_MODULE, $this->id); + $this->context = get_context_instance(CONTEXT_MODULE, $this->cm->id); } //////////////////////////////////////////////////////////////////////////////// @@ -1136,7 +1136,7 @@ class workshop { * @param null|int|array $restrict If null, update all authors, otherwise update just grades for the given author(s) * @return void */ - public function update_submission_grades($restrict=null) { + public function aggregate_submission_grades($restrict=null) { global $DB; // fetch a recordset with all assessments to process @@ -1222,7 +1222,7 @@ class workshop { * @param null|int|array $restrict If null, update all reviewers, otherwise update just grades for the given reviewer(s) * @return void */ - public function update_grading_grades($restrict=null) { + public function aggregate_grading_grades($restrict=null) { global $DB; // todo diff --git a/mod/workshop/view.php b/mod/workshop/view.php index 035a0ea1c6..183f3bf181 100644 --- a/mod/workshop/view.php +++ b/mod/workshop/view.php @@ -160,7 +160,7 @@ case workshop::PHASE_EVALUATION: $page = optional_param($pagingvar, 0, PARAM_INT); $perpage = 10; // todo let the user modify this $groups = ''; // todo let the user choose the group - $sortby = 'submissiongrade'; // todo let the user choose the column to sort by + $sortby = 'assessmentgrade'; // todo let the user choose the column to sort by $sorthow = 'DESC'; // todo detto $data = $workshop->prepare_grading_report($USER->id, $groups, $page, $perpage, $sortby, $sorthow); -- 2.39.5