Refactoring and fixing

There was a chaos in dimension masterids and localids
This commit is contained in:
David Mudrak 2010-01-04 17:52:04 +00:00
parent fa159f43c1
commit 7a0ae478c3
2 changed files with 56 additions and 81 deletions

View file

@ -30,7 +30,7 @@ require_once(dirname(dirname(__FILE__)) . '/lib.php'); // interface definition
/** /**
* Accumulative grading strategy logic. * Accumulative grading strategy logic.
*/ */
class workshop_accumulative_strategy extends workshop_base_strategy implements workshop_strategy { class workshop_accumulative_strategy implements workshop_strategy {
/** @var workshop the parent workshop instance */ /** @var workshop the parent workshop instance */
protected $workshop; protected $workshop;
@ -50,7 +50,7 @@ class workshop_accumulative_strategy extends workshop_base_strategy implements w
public function __construct($workshop) { public function __construct($workshop) {
$this->workshop = $workshop; $this->workshop = $workshop;
$this->dimensions = $this->load_fields(); $this->dimensions = $this->load_fields();
$this->descriptionopts = array('trusttext' => true, 'subdirs' => true, 'maxfiles' => -1); $this->descriptionopts = array('trusttext' => true, 'subdirs' => false, 'maxfiles' => -1);
} }
/** /**
@ -99,19 +99,19 @@ class workshop_accumulative_strategy extends workshop_base_strategy implements w
} }
/** /**
* Loads the fields of the assessment form * Loads the fields of the assessment form currently used in this workshop
* *
* @return array definition of assessment dimensions * @return array definition of assessment dimensions
*/ */
protected function load_fields() { protected function load_fields() {
global $DB; global $DB;
$sql = 'SELECT master.id,dim.description,dim.descriptionformat,dim.grade,dim.weight $sql = "SELECT master.id,dim.description,dim.descriptionformat,dim.grade,dim.weight
FROM {workshop_forms} master FROM {workshop_forms} master
JOIN {workshop_forms_accumulative} dim ON (dim.id=master.localid) INNER JOIN {workshop_forms_accumulative} dim ON (dim.id=master.localid)
WHERE master.workshopid = ? WHERE master.workshopid = :workshopid AND master.strategy = :strategy
ORDER BY master.sort'; ORDER BY master.sort";
$params[0] = $this->workshop->id; $params = array("workshopid" => $this->workshop->id, "strategy" => $this->workshop->strategy);
return $DB->get_records_sql($sql, $params); return $DB->get_records_sql($sql, $params);
} }
@ -127,7 +127,7 @@ class workshop_accumulative_strategy extends workshop_base_strategy implements w
$formdata = new stdClass(); $formdata = new stdClass();
$key = 0; $key = 0;
foreach ($raw as $dimension) { foreach ($raw as $dimension) {
$formdata->{'dimensionid__idx_' . $key} = $dimension->id; $formdata->{'dimensionid__idx_' . $key} = $dimension->id; // master id, not the local one!
$formdata->{'description__idx_' . $key} = $dimension->description; $formdata->{'description__idx_' . $key} = $dimension->description;
$formdata->{'description__idx_' . $key.'format'} = $dimension->descriptionformat; $formdata->{'description__idx_' . $key.'format'} = $dimension->descriptionformat;
$formdata->{'grade__idx_' . $key} = $dimension->grade; $formdata->{'grade__idx_' . $key} = $dimension->grade;
@ -159,45 +159,61 @@ class workshop_accumulative_strategy extends workshop_base_strategy implements w
$workshopid = $data->workshopid; $workshopid = $data->workshopid;
$norepeats = $data->norepeats; $norepeats = $data->norepeats;
$data = $this->_cook_edit_form_data($data); $data = $this->prepare_database_fields($data);
$masterrecords = $data->forms; $masters = $data->forms; // data to be saved into workshop_forms
$localrecords = $data->forms_accumulative; $locals = $data->forms_accumulative; // data to be saved into workshop_forms_accumulative
$todeletelocal = array(); // local ids to be deleted $todelete = array(); // master ids to be deleted
$todeletemaster = array(); // master ids to be deleted
for ($i=0; $i < $norepeats; $i++) { for ($i=0; $i < $norepeats; $i++) {
$local = $localrecords[$i]; $local = $locals[$i];
$master = $masterrecords[$i]; $master = $masters[$i];
if (empty($local->description_editor['text'])) { if (empty($local->description_editor['text'])) {
if (!empty($local->id)) { if (!empty($master->id)) {
// existing record with empty description - to be deleted // existing record with empty description - to be deleted
$todeletelocal[] = $local->id; $todelete[] = $master->id;
$todeletemaster[] = $this->dimension_master_id($local->id);
} }
continue; continue;
} }
if (empty($local->id)) { if (empty($master->id)) {
// new field // new field
$local->id = $DB->insert_record('workshop_forms_accumulative', $local); $local->id = $DB->insert_record("workshop_forms_accumulative", $local);
$master->localid = $local->id; $master->localid = $local->id;
$master->id = $DB->insert_record('workshop_forms', $master); $master->id = $DB->insert_record("workshop_forms", $master);
} else { } else {
// exiting field // exiting field
$master->id = $this->dimension_master_id($local->id); $DB->update_record("workshop_forms", $master);
$DB->update_record('workshop_forms', $master); $local->id = $DB->get_field("workshop_forms", "localid", array("id" => $master->id), MUST_EXIST);
} }
// $local record now has its id, let us re-save it with correct path to embeded media files // re-save with correct path to embeded media files
$local = file_postupdate_standard_editor($local, 'description', $this->descriptionopts, $local = file_postupdate_standard_editor($local, 'description', $this->descriptionopts,
$PAGE->context, 'workshop_dimension_description', $local->id); $PAGE->context, 'workshop_dimension_description', $master->id);
$DB->update_record('workshop_forms_accumulative', $local); $DB->update_record('workshop_forms_accumulative', $local);
} }
// unlink embedded files and delete emptied dimensions $this->delete_dimensions($todelete);
$fs = get_file_storage(); }
foreach ($todeletelocal as $itemid) {
/**
* Deletes dimensions and removes embedded media from its descriptions
*
* todo we may check that there are no assessments done using these dimensions
*
* @param array $masterids
* @return void
*/
protected function delete_dimensions($masterids) {
global $DB, $PAGE;
$masters = $DB->get_records_list("workshop_forms", "id", $masterids, "", "id,localid");
$masterids = array_keys($masters); // now contains only those really existing
$localids = array();
$fs = get_file_storage();
foreach ($masters as $itemid => $master) {
$fs->delete_area_files($PAGE->context->id, 'workshop_dimension_description', $itemid); $fs->delete_area_files($PAGE->context->id, 'workshop_dimension_description', $itemid);
$localids[] = $master->localid;
} }
$DB->delete_records_list('workshop_forms_accumulative', 'id', $todeletelocal); $DB->delete_records_list("workshop_forms_accumulative", "id", $localids);
$DB->delete_records_list('workshop_forms', 'id', $todeletemaster); $DB->delete_records_list("workshop_forms", "id", $masterids);
} }
/** /**
@ -211,7 +227,7 @@ class workshop_accumulative_strategy extends workshop_base_strategy implements w
* @param object $raw Raw data returned by mform * @param object $raw Raw data returned by mform
* @return array Array of objects to be inserted/updated in DB * @return array Array of objects to be inserted/updated in DB
*/ */
protected function _cook_edit_form_data(stdClass $raw) { protected function prepare_database_fields(stdClass $raw) {
global $PAGE; global $PAGE;
$cook = new stdClass(); // to be returned $cook = new stdClass(); // to be returned
@ -220,8 +236,7 @@ class workshop_accumulative_strategy extends workshop_base_strategy implements w
for ($i = 0; $i < $raw->norepeats; $i++) { for ($i = 0; $i < $raw->norepeats; $i++) {
$cook->forms_accumulative[$i] = new stdClass(); $cook->forms_accumulative[$i] = new stdClass();
$fieldname = 'dimensionid__idx_'.$i;
$cook->forms_accumulative[$i]->id = isset($raw->$fieldname) ? $raw->$fieldname : null;
$fieldname = 'description__idx_'.$i.'_editor'; $fieldname = 'description__idx_'.$i.'_editor';
$cook->forms_accumulative[$i]->description_editor = isset($raw->$fieldname) ? $raw->$fieldname : null; $cook->forms_accumulative[$i]->description_editor = isset($raw->$fieldname) ? $raw->$fieldname : null;
$fieldname = 'grade__idx_'.$i; $fieldname = 'grade__idx_'.$i;
@ -230,11 +245,10 @@ class workshop_accumulative_strategy extends workshop_base_strategy implements w
$cook->forms_accumulative[$i]->weight = isset($raw->$fieldname) ? $raw->$fieldname : null; $cook->forms_accumulative[$i]->weight = isset($raw->$fieldname) ? $raw->$fieldname : null;
$cook->forms[$i] = new stdClass(); $cook->forms[$i] = new stdClass();
$cook->forms[$i]->id = null; // will be checked later, considered unknown at the moment $cook->forms[$i]->id = isset($raw->{'dimensionid__idx_'.$i}) ? $raw->{'dimensionid__idx_'.$i} : null;
$cook->forms[$i]->workshopid = $this->workshop->id; $cook->forms[$i]->workshopid = $this->workshop->id;
$cook->forms[$i]->sort = $i + 1; $cook->forms[$i]->sort = $i + 1;
$cook->forms[$i]->strategy = 'accumulative'; $cook->forms[$i]->strategy = 'accumulative';
$cook->forms[$i]->dimensionid = $cook->forms_accumulative[$i]->id;
} }
return $cook; return $cook;
} }
@ -332,6 +346,10 @@ class workshop_accumulative_strategy extends workshop_base_strategy implements w
* @return float Percentual grade for submission as suggested by the peer * @return float Percentual grade for submission as suggested by the peer
*/ */
protected function update_peer_grade(stdClass $assessment) { protected function update_peer_grade(stdClass $assessment) {
return 60.2; global $DB;
$given = $DB->get_records('workshop_grades', array('assessmentid' => $assessment->id));
// use only grades given within the currently used strategy
} }
} }

View file

@ -81,46 +81,3 @@ interface workshop_strategy {
*/ */
public function save_assessment(stdClass $assessment, stdClass $data); public function save_assessment(stdClass $assessment, stdClass $data);
} }
/**
* Provides common methods for all grading strategies
*
* @copyright 2009 David Mudrak <david.mudrak@gmail.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class workshop_base_strategy {
/**
* Returns the dimension's master id from table {workshop_forms}
*
* Every grading strategy defines its assessment dimensions in its own tables (eg workshop_forms_accumulative).
* But we need a unique identifier across all strategies to be able to embed media into dimension descriptions.
* So, every dimension has to have a record in the master table workshop_forms. This is a similar pattern
* to what Moodle already uses with course_modules and instances.
*
* @param int $localid The id if the dimension in the subplugin table (eg workshop_forms_accumulative)
* @return int|false The master id from workshop_forms_accumulative or false, if not found
*/
public function dimension_master_id($localid) {
global $DB;
/** @var array Cache database query results so we can call from a loop */
static $cache=null;
if (is_null($cache)) {
$strategy = $this->name();
$dimids = $DB->get_records('workshop_forms',
array('workshopid' => $this->workshop->id, 'strategy' => 'accumulative'), '', 'id,localid');
$cache = array();
foreach ($dimids as $masterid => $master) {
$cache[$masterid] = $master->localid;
}
$cache = array_flip($cache);
// now the $cache contains records [$localid] => [$masterid]
}
if (isset($cache[$localid])) {
return $localid;
}
return false;
}
}