Skip to content

Commit

Permalink
MDL-40097 completion: Make restoring grade criteria more robust
Browse files Browse the repository at this point in the history
  • Loading branch information
@abgreeve
Aaron Barnes authored and abgreeve committed Jan 5, 2015
1 parent 440085e commit 39ed46b
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 27 deletions.
32 changes: 26 additions & 6 deletions backup/moodle2/backup_stepslib.php
View file
Expand Up @@ -2368,7 +2368,8 @@ protected function define_structure() {
$cc = new backup_nested_element('course_completion');

$criteria = new backup_nested_element('course_completion_criteria', array('id'), array(
'course','criteriatype', 'module', 'moduleinstance', 'courseinstanceshortname', 'enrolperiod', 'timeend', 'gradepass', 'role'
'course', 'criteriatype', 'module', 'moduleinstance', 'courseinstanceshortname', 'enrolperiod',
'timeend', 'gradepass', 'role', 'roleshortname'
));

$criteriacompletions = new backup_nested_element('course_completion_crit_completions');
Expand All @@ -2391,11 +2392,30 @@ protected function define_structure() {
$cc->add_child($coursecompletions);
$cc->add_child($aggregatemethod);

// We need to get the courseinstances shortname rather than an ID for restore
$criteria->set_source_sql("SELECT ccc.*, c.shortname AS courseinstanceshortname
FROM {course_completion_criteria} ccc
LEFT JOIN {course} c ON c.id = ccc.courseinstance
WHERE ccc.course = ?", array(backup::VAR_COURSEID));
/*
We need some extra data for the restore
- courseinstances shortname rather than an ID
- roleshortname in case restoring on a different site
/
$criteria->set_source_sql(
"
SELECT
ccc.*,
c.shortname AS courseinstanceshortname,
r.shortname AS roleshortname
FROM
{course_completion_criteria} ccc
LEFT JOIN
{course} c
ON c.id = ccc.courseinstance
LEFT JOIN
{role} r
ON r.id = ccc.role
WHERE
ccc.course = ?
",
array(backup::VAR_COURSEID)
);


$aggregatemethod->set_source_table('course_completion_aggr_methd', array('course' => backup::VAR_COURSEID));
Expand Down
60 changes: 39 additions & 21 deletions backup/moodle2/restore_stepslib.php
View file
Expand Up @@ -2504,18 +2504,37 @@ public function process_course_completion_criteria($data) {
$data->timeend = $this->apply_date_offset($data->timeend);

// Map the role from the criteria
if (!empty($data->role)) {
$data->role = $this->get_mappingid('role', $data->role);
}
if (isset($data->role) && $data->role != '') {

// If same site use the same role id, otherwise try calculate it.
if (!$this->task->is_samesite()) {
// Newer backups should include roleshortname, which makes this much easier.
if (!empty($data->roleshortname)) {
$roleinstanceid = $DB->get_field('role', 'id', array('shortname' => $data->roleshortname));
if (!$roleinstanceid) {
debugging('Could not match the role shortname in course_completion_criteria, so skipping');
return;
}
$data->role = $roleinstanceid;
} else {
$data->role = $this->get_mappingid('role', $data->role);
}
}

$skipcriteria = false;
// Check we have an id, otherwise it causes all sorts of bugs.
if (!$data->role) {
debugging('Could not match role in course_completion_criteria, so skipping');
return;
}
}

// If the completion criteria is for a module we need to map the module instance
// to the new module id.
if (!empty($data->moduleinstance) && !empty($data->module)) {
$data->moduleinstance = $this->get_mappingid('course_module', $data->moduleinstance);
if (empty($data->moduleinstance)) {
$skipcriteria = true;
debugging('Could not match the module instance in course_completion_criteria, so skipping');
return;
}
} else {
$data->module = null;
Expand All @@ -2526,28 +2545,27 @@ public function process_course_completion_criteria($data) {
if (!empty($data->courseinstanceshortname)) {
$courseinstanceid = $DB->get_field('course', 'id', array('shortname'=>$data->courseinstanceshortname));
if (!$courseinstanceid) {
$skipcriteria = true;
debugging('Could not match the course instance in course_completion_criteria, so skipping');
return;
}
} else {
$courseinstanceid = null;
}
$data->courseinstance = $courseinstanceid;

if (!$skipcriteria) {
$params = array(
'course' => $data->course,
'criteriatype' => $data->criteriatype,
'enrolperiod' => $data->enrolperiod,
'courseinstance' => $data->courseinstance,
'module' => $data->module,
'moduleinstance' => $data->moduleinstance,
'timeend' => $data->timeend,
'gradepass' => $data->gradepass,
'role' => $data->role
);
$newid = $DB->insert_record('course_completion_criteria', $params);
$this->set_mapping('course_completion_criteria', $data->id, $newid);
}
$params = array(
'course' => $data->course,
'criteriatype' => $data->criteriatype,
'enrolperiod' => $data->enrolperiod,
'courseinstance' => $data->courseinstance,
'module' => $data->module,
'moduleinstance' => $data->moduleinstance,
'timeend' => $data->timeend,
'gradepass' => $data->gradepass,
'role' => $data->role
);
$newid = $DB->insert_record('course_completion_criteria', $params);
$this->set_mapping('course_completion_criteria', $data->id, $newid);
}

/**
Expand Down
3 changes: 3 additions & 0 deletions completion/criteria/completion_criteria_role.php
View file
Expand Up @@ -123,6 +123,9 @@ public function review($completion, $mark = true, $is_complete = false) {
public function get_title() {
global $DB;
$role = $DB->get_record('role', array('id' => $this->role));
if (!$role) {
return '['.get_string('roleidnotfound', 'completion', $this->role).']';
}
return role_get_name($role, context_course::instance($this->course));
}

Expand Down
1 change: 1 addition & 0 deletions lang/en/completion.php
View file
Expand Up @@ -165,6 +165,7 @@
$string['roleaggregation'] = 'Condition requires';
$string['roleaggregation_all'] = 'ALL selected roles to mark when the condition is met';
$string['roleaggregation_any'] = 'ANY selected roles to mark when the condition is met';
$string['roleidnotfound'] = 'Role ID {$a} not found';
$string['saved'] = 'Saved';
$string['seedetails'] = 'See details';
$string['self'] = 'Self';
Expand Down

0 comments on commit 39ed46b

Please sign in to comment.