diff --git a/course/completion.php b/course/completion.php index 2bd3ed9ee2c..3d4e17b1398 100644 --- a/course/completion.php +++ b/course/completion.php @@ -103,44 +103,43 @@ if ($form->is_cancelled()){ // Handle aggregation methods // Overall aggregation - $aggregation = new completion_aggregation(); - $aggregation->course = $data->id; - $aggregation->criteriatype = null; + $aggdata = array( + 'course' => $data->id, + 'criteriatype' => null + ); + $aggregation = new completion_aggregation($aggdata); $aggregation->setMethod($data->overall_aggregation); - $aggregation->insert(); + $aggregation->save(); // Activity aggregation if (empty($data->activity_aggregation)) { $data->activity_aggregation = 0; } - $aggregation = new completion_aggregation(); - $aggregation->course = $data->id; - $aggregation->criteriatype = COMPLETION_CRITERIA_TYPE_ACTIVITY; + $aggdata['criteriatype'] = COMPLETION_CRITERIA_TYPE_ACTIVITY; + $aggregation = new completion_aggregation($aggdata); $aggregation->setMethod($data->activity_aggregation); - $aggregation->insert(); + $aggregation->save(); // Course aggregation if (empty($data->course_aggregation)) { $data->course_aggregation = 0; } - $aggregation = new completion_aggregation(); - $aggregation->course = $data->id; - $aggregation->criteriatype = COMPLETION_CRITERIA_TYPE_COURSE; + $aggdata['criteriatype'] = COMPLETION_CRITERIA_TYPE_COURSE; + $aggregation = new completion_aggregation($aggdata); $aggregation->setMethod($data->course_aggregation); - $aggregation->insert(); + $aggregation->save(); // Role aggregation if (empty($data->role_aggregation)) { $data->role_aggregation = 0; } - $aggregation = new completion_aggregation(); - $aggregation->course = $data->id; - $aggregation->criteriatype = COMPLETION_CRITERIA_TYPE_ROLE; + $aggdata['criteriatype'] = COMPLETION_CRITERIA_TYPE_ROLE; + $aggregation = new completion_aggregation($aggdata); $aggregation->setMethod($data->role_aggregation); - $aggregation->insert(); + $aggregation->save(); // Update course total passing grade if (!empty($data->criteria_grade)) { @@ -152,7 +151,10 @@ if ($form->is_cancelled()){ } } - redirect($CFG->wwwroot."/course/view.php?id=$course->id", get_string('changessaved')); + add_to_log($course->id, 'course', 'completion updated', 'completion.php?id='.$course->id); + + $url = new moodle_url('/course/view.php', array('id' => $course->id)); + redirect($url); } diff --git a/lib/completion/completion_aggregation.php b/lib/completion/completion_aggregation.php index e7a07863781..e6e626a4de9 100644 --- a/lib/completion/completion_aggregation.php +++ b/lib/completion/completion_aggregation.php @@ -49,13 +49,16 @@ class completion_aggregation extends data_object { */ public $required_fields = array('id', 'course', 'criteriatype', 'method', 'value'); + /* @var array Array of unique fields, used in where clauses */ + public $unique_fields = array('course', 'criteriatype'); + /* @var int Course id */ public $course; /* @var int Criteria type this aggregation method applies to, or NULL for overall course aggregation */ public $criteriatype; - /* @var int Aggregation method (COMPLETION_AGGREGATION_* constant)*/ + /* @var int Aggregation method (COMPLETION_AGGREGATION_* constant) */ public $method; /* @var mixed Method value */ @@ -98,4 +101,19 @@ class completion_aggregation extends data_object { $this->method = COMPLETION_AGGREGATION_ALL; } } + + + /** + * Save aggregation method to database + * + * @access public + * @return boolean + */ + public function save() { + if ($this->id) { + return $this->update(); + } else { + return $this->insert(); + } + } } diff --git a/lib/completion/completion_completion.php b/lib/completion/completion_completion.php index 2b5d8bf4bad..e6523eadf26 100644 --- a/lib/completion/completion_completion.php +++ b/lib/completion/completion_completion.php @@ -111,7 +111,7 @@ class completion_completion extends data_object { $this->timeenrolled = $timeenrolled; } - $this->_save(); + return $this->_save(); } /** @@ -137,7 +137,7 @@ class completion_completion extends data_object { $this->timestarted = $timestarted; } - $this->_save(); + return $this->_save(); } /** @@ -165,24 +165,24 @@ class completion_completion extends data_object { $this->timecompleted = $timecomplete; // Save record - $this->_save(); + return $this->_save(); } /** * Save course completion status * * This method creates a course_completions record if none exists + * @access private + * @return bool */ private function _save() { - global $DB; - if ($this->timeenrolled === null) { $this->timeenrolled = 0; } // Save record if ($this->id) { - $this->update(); + return $this->update(); } else { // Make sure reaggregate field is not null if (!$this->reaggregate) { @@ -191,10 +191,10 @@ class completion_completion extends data_object { // Make sure timestarted is not null if (!$this->timestarted) { - $this->timestarted = 0; + $this->timestarted = 0; } - - $this->insert(); + + return $this->insert(); } } } diff --git a/lib/completion/completion_criteria.php b/lib/completion/completion_criteria.php index d93e9a043a2..31f5187edcb 100644 --- a/lib/completion/completion_criteria.php +++ b/lib/completion/completion_criteria.php @@ -167,11 +167,11 @@ abstract class completion_criteria extends data_object { public static function factory($params) { global $CFG, $COMPLETION_CRITERIA_TYPES; - if (!isset($params->criteriatype) || !isset($COMPLETION_CRITERIA_TYPES[$params->criteriatype])) { + if (!isset($params['criteriatype']) || !isset($COMPLETION_CRITERIA_TYPES[$params['criteriatype']])) { error('invalidcriteriatype', 'completion'); } - $class = 'completion_criteria_'.$COMPLETION_CRITERIA_TYPES[$params->criteriatype]; + $class = 'completion_criteria_'.$COMPLETION_CRITERIA_TYPES[$params['criteriatype']]; require_once($CFG->libdir.'/completion/'.$class.'.php'); return new $class($params, false); diff --git a/lib/completion/completion_criteria_activity.php b/lib/completion/completion_criteria_activity.php index 409bf42c08f..ba275468723 100644 --- a/lib/completion/completion_criteria_activity.php +++ b/lib/completion/completion_criteria_activity.php @@ -235,8 +235,7 @@ class completion_criteria_activity extends completion_criteria { // Loop through completions, and mark as complete $rs = $DB->get_recordset_sql($sql); foreach ($rs as $record) { - - $completion = new completion_criteria_completion((array)$record); + $completion = new completion_criteria_completion($record, DATA_OBJECT_FETCH_BY_KEY); $completion->mark_complete($record->timecompleted); } $rs->close(); diff --git a/lib/completion/completion_criteria_completion.php b/lib/completion/completion_criteria_completion.php index f4d700d0772..7f448001d20 100644 --- a/lib/completion/completion_criteria_completion.php +++ b/lib/completion/completion_criteria_completion.php @@ -44,6 +44,9 @@ class completion_criteria_completion extends data_object { /* @var array Array of required table fields, must start with 'id'. */ public $required_fields = array('id', 'userid', 'course', 'criteriaid', 'gradefinal', 'rpl', 'deleted', 'unenroled', 'timecompleted'); + /* @var array Array of unique fields, used in where clauses */ + public $unique_fields = array('userid', 'course', 'criteriaid'); + /* @var int User ID */ public $userid; diff --git a/lib/completion/completion_criteria_course.php b/lib/completion/completion_criteria_course.php index 882ee6365c2..04be448241e 100644 --- a/lib/completion/completion_criteria_course.php +++ b/lib/completion/completion_criteria_course.php @@ -193,8 +193,8 @@ class completion_criteria_course extends completion_criteria { // Loop through completions, and mark as complete $rs = $DB->get_recordset_sql($sql); foreach ($rs as $record) { - $completion = new completion_criteria_completion((array)$record); - $completion->mark_complete($record->timecompleted); + $completion = new completion_criteria_completion($record, DATA_OBJECT_FETCH_BY_KEY); + $completion->mark_complete($record['timecompleted']); } $rs->close(); } diff --git a/lib/completion/completion_criteria_date.php b/lib/completion/completion_criteria_date.php index e6e81c09148..bacc144955d 100644 --- a/lib/completion/completion_criteria_date.php +++ b/lib/completion/completion_criteria_date.php @@ -179,8 +179,8 @@ class completion_criteria_date extends completion_criteria { // Loop through completions, and mark as complete $rs = $DB->get_recordset_sql($sql, array(time())); foreach ($rs as $record) { - $completion = new completion_criteria_completion((array)$record); - $completion->mark_complete($record->timeend); + $completion = new completion_criteria_completion($record, DATA_OBJECT_FETCH_BY_KEY); + $completion->mark_complete($record['timeend']); } $rs->close(); } diff --git a/lib/completion/completion_criteria_duration.php b/lib/completion/completion_criteria_duration.php index 97bcb923b00..962b052fe4e 100644 --- a/lib/completion/completion_criteria_duration.php +++ b/lib/completion/completion_criteria_duration.php @@ -229,8 +229,7 @@ class completion_criteria_duration extends completion_criteria { $now = time(); $rs = $DB->get_recordset_sql($sql, array($now, $now)); foreach ($rs as $record) { - - $completion = new completion_criteria_completion((array)$record); + $completion = new completion_criteria_completion($record, DATA_OBJECT_FETCH_BY_KEY); // Use time start if not 0, otherwise use timeenrolled if ($record->otimestart) { diff --git a/lib/completion/completion_criteria_grade.php b/lib/completion/completion_criteria_grade.php index e452454d080..3e0da11715f 100644 --- a/lib/completion/completion_criteria_grade.php +++ b/lib/completion/completion_criteria_grade.php @@ -215,8 +215,8 @@ class completion_criteria_grade extends completion_criteria { // Loop through completions, and mark as complete $rs = $DB->get_recordset_sql($sql); foreach ($rs as $record) { - $completion = new completion_criteria_completion((array)$record); - $completion->mark_complete($record->timecompleted); + $completion = new completion_criteria_completion($record, DATA_OBJECT_FETCH_BY_KEY); + $completion->mark_complete($record['timecompleted']); } $rs->close(); } diff --git a/lib/completion/cron.php b/lib/completion/cron.php index be480d47a21..3645c0618d6 100644 --- a/lib/completion/cron.php +++ b/lib/completion/cron.php @@ -317,7 +317,7 @@ function completion_cron_completions() { foreach ($completions as $params) { $timecompleted = max($timecompleted, $params->timecompleted); - $completion = new completion_criteria_completion($params, false); + $completion = new completion_criteria_completion((array)$params, false); // Handle aggregation special cases if ($params->criteriatype == COMPLETION_CRITERIA_TYPE_ACTIVITY) { diff --git a/lib/completion/data_object.php b/lib/completion/data_object.php index 01345ee0cc9..9d862ba914a 100644 --- a/lib/completion/data_object.php +++ b/lib/completion/data_object.php @@ -26,6 +26,14 @@ defined('MOODLE_INTERNAL') || die(); + +/** + * Trigger for the new data_object api. + * + * See data_object::__constructor + */ +define('DATA_OBJECT_FETCH_BY_KEY', 2); + /** * A data abstraction object that holds methods and attributes * @@ -50,32 +58,74 @@ abstract class data_object { */ public $optional_fields = array(); + /* @var Array of unique fields, used in where clauses and constructor */ + public $unique_fields = array(); + /* @var int The primary key */ public $id; + /** * Constructor. Optionally (and by default) attempts to fetch corresponding row from DB. * - * @param array $params an array with required parameters for this data object. - * @param bool $fetch Whether to fetch corresponding row from DB or not, - * optional fields might not be defined if false used + * If $fetch is not false, there are a few different things that can happen: + * - true: + * load corresponding row from the database, using $params as the WHERE clause + * + * - DATA_OBJECT_FETCH_BY_KEY: + * load corresponding row from the database, using only the $id in the WHERE clause (if set), + * otherwise using the columns listed in $this->unique_fields. + * + * - array(): + * load corresponding row from the database, using the columns listed in this array + * in the WHERE clause + * + * @param array $params required parameters and their values for this data object + * @param mixed $fetch if false, do not attempt to fetch from the database, otherwise see notes */ public function __construct($params = null, $fetch = true) { - if (!empty($params) and (is_array($params) or is_object($params))) { - if ($fetch) { - if ($data = $this->fetch($params)) { - self::set_properties($this, $data); - } else { - self::set_properties($this, $this->optional_fields);//apply defaults for optional fields - self::set_properties($this, $params); - } - } else { - self::set_properties($this, $params); + if (is_object($params)) { + throw new coding_exception('data_object params should be in the form of an array, not an object'); + } + + // If no params given, apply defaults for optional fields + if (empty($params) || !is_array($params)) { + self::set_properties($this, $this->optional_fields); + return; + } + + // If fetch is false, do not load from database + if ($fetch === false) { + self::set_properties($this, $params); + return; + } + + // Compose where clause only from fields in unique_fields + if ($fetch === DATA_OBJECT_FETCH_BY_KEY && !empty($this->unique_fields)) { + if (empty($params['id'])) { + $where = array_intersect_key($params, array_flip($this->unique_fields)); } - + else { + $where = array('id' => $params['id']); + } + // Compose where clause from given field names + } else if (is_array($fetch) && !empty($fetch)) { + $where = array_intersect_key($params, array_flip($fetch)); + // Use entire params array for where clause } else { - self::set_properties($this, $this->optional_fields);//apply defaults for optional fields + $where = $params; + } + + // Attempt to load from database + if ($data = $this->fetch($where)) { + // Apply data from database, then data sent to constructor + self::set_properties($this, $data); + self::set_properties($this, $params); + } else { + // Apply defaults for optional fields, then data from constructor + self::set_properties($this, $this->optional_fields); + self::set_properties($this, $params); } } @@ -339,4 +389,4 @@ abstract class data_object { */ public function notify_changed($deleted) { } -} \ No newline at end of file +} diff --git a/lib/completionlib.php b/lib/completionlib.php index a47a1f89ee2..2034af148f5 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -322,8 +322,9 @@ class completion_info { */ public function get_user_completion($user_id, $criteria) { $params = array( + 'course' => $this->course_id, + 'userid' => $user_id, 'criteriaid' => $criteria->id, - 'userid' => $user_id ); $completion = new completion_criteria_completion($params); @@ -362,7 +363,7 @@ class completion_info { // Build array of criteria objects $this->criteria = array(); foreach ($records as $record) { - $this->criteria[$record->id] = completion_criteria::factory($record); + $this->criteria[$record->id] = completion_criteria::factory((array)$record); } }