From: sam_marshall Date: Mon, 28 Jul 2008 15:58:50 +0000 (+0000) Subject: MDL-15498: Fixes to problems Petr identified with completion system checkin X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=49f6e5f4762cf2dc35bda47aa6e47e256a040a91;p=moodle.git MDL-15498: Fixes to problems Petr identified with completion system checkin --- diff --git a/lib/completionlib.php b/lib/completionlib.php index 9d3c5cc910..0e39b09581 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -81,7 +81,7 @@ class completion_info { * @param object $course Moodle course object. Must have at least ->id, ->enablecompletion * @return completion_info */ - public function completion_info($course) { + public function __construct($course) { $this->course=$course; } @@ -314,7 +314,7 @@ class completion_info { SELECT COUNT(1) FROM - {$CFG->prefix}course_modules_completion + {course_modules_completion} WHERE coursemoduleid=? AND completionstate<>0",array($cm->id)); } @@ -402,9 +402,11 @@ class completion_info { $currentuser=$userid==$USER->id; if($currentuser) { - // Make sure cache is present - if(!isset($SESSION->completioncache)) { + // Make sure cache is present and is for current user (loginas + // changes this) + if(!isset($SESSION->completioncache) || $SESSION->completioncacheuserid!=$USER->id) { $SESSION->completioncache=array(); + $SESSION->completioncacheuserid=$USER->id; } // Expire any old data from cache foreach($SESSION->completioncache as $courseid=>$activities) { @@ -427,8 +429,8 @@ class completion_info { SELECT cmc.* FROM - {$CFG->prefix}course_modules cm - INNER JOIN {$CFG->prefix}course_modules_completion cmc ON cmc.coursemoduleid=cm.id + {course_modules} cm + INNER JOIN {course_modules_completion} cmc ON cmc.coursemoduleid=cm.id WHERE cm.course=? AND cmc.userid=?",array($this->course->id,$userid)); @@ -614,8 +616,8 @@ class completion_info { SELECT cmc.* FROM - {$CFG->prefix}course_modules cm - INNER JOIN {$CFG->prefix}course_modules_completion cmc ON cm.id=cmc.coursemoduleid + {course_modules} cm + INNER JOIN {course_modules_completion} cmc ON cm.id=cmc.coursemoduleid WHERE cm.course=? AND cmc.userid $insql ",$params); diff --git a/lib/grade/grade_grade.php b/lib/grade/grade_grade.php index 2f21c483f6..d1b71cacb9 100644 --- a/lib/grade/grade_grade.php +++ b/lib/grade/grade_grade.php @@ -743,42 +743,45 @@ class grade_grade extends grade_object { */ function notify_changed($deleted) { // Ignore during restore + // TODO There should be a proper way to determine when we are in restore + // so that this hack looking for a $restore global is not needed. global $restore; if(!empty($restore->backup_unique_code)) { return; } global $CFG,$COURSE,$DB; require_once($CFG->libdir.'/completionlib.php'); - + + // Bail out immediately if completion is not enabled for site (saves loading + // grade item below) + if(!completion_info::is_enabled_for_site()) { + return; + } + + // Load information about grade item + $this->load_grade_item(); + + // Only course-modules have completion data + if($this->grade_item->itemtype!='mod') { + return; + } + // Use $COURSE if available otherwise get it via item fields - if(!empty($COURSE)) { + if(!empty($COURSE) && $COURSE->id==$this->grade_item->courseid) { $course=$COURSE; } else { - $this->load_grade_item(); - $course=get_record('course','id',$grade_item->courseid); + $course=$DB->get_record('course',array('id',$this->grade_item->courseid)); } - // Bail out immediately if completion is not enabled for course + // Bail out if completion is not enabled for course $completion=new completion_info($course); if(!$completion->is_enabled()) { return; } - // Get the grade item and course-module which we will need - $this->load_grade_item(); - if($this->grade_item->itemtype!='mod') { - return; - } - $cm=$DB->get_record_sql(" -SELECT - cm.*,m.name AS modname -FROM - {$CFG->prefix}modules m - INNER JOIN {$CFG->prefix}course_modules cm ON m.id=cm.module -WHERE - m.name=? AND cm.instance=? AND cm.course=?", - array($this->grade_item->itemmodule,$this->grade_item->iteminstance, - $this->grade_item->courseid)); + // Get course-module + $cm=get_coursemodule_from_instance($this->grade_item->itemmodule, + $this->grade_item->iteminstance,$this->grade_item->courseid); if(!$cm) { debugging("Couldn't find course-module for module '{$this->grade_item->itemmodule}', instance '{$this->grade_item->iteminstance}', diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 659bccb782..c0e050a678 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -270,6 +270,18 @@ define ('PASSWORD_UPPER', 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'); define ('PASSWORD_DIGITS', '0123456789'); define ('PASSWORD_NONALPHANUM', '.,;:!?_-+/*@#&$'); +// Feature constants. Used for plugin_supports() to report features that are, +// or are not, supported by a module. + +/** True if module can provide a grade */ +define('FEATURE_GRADE_HAS_GRADE','grade_has_grade'); +/** True if module has code to track whether somebody viewed it */ +define('FEATURE_COMPLETION_TRACKS_VIEWS','completion_tracks_views'); +/** True if module has custom completion rules */ +define('FEATURE_COMPLETION_HAS_RULES','completion_has_rules'); + + + /// PARAMETER HANDLING //////////////////////////////////////////////////// /** @@ -5938,22 +5950,15 @@ function get_list_of_plugins($plugin='mod', $exclude='', $basedir='') { return $plugins; } -// Feature constants -/** True if module can provide a grade */ -define('FEATURE_GRADE_HAS_GRADE','grade_has_grade'); -/** True if module has code to track whether somebody viewed it */ -define('FEATURE_COMPLETION_TRACKS_VIEWS','completion_tracks_views'); -/** True if module has custom completion rules */ -define('FEATURE_COMPLETION_HAS_RULES','completion_has_rules'); - /** * Checks whether a plugin supports a specified feature. * * @param string $type Plugin type e.g. 'mod' * @param string $name Plugin name * @param string $feature Feature code (FEATURE_xx constant) - * @return Feature result (false if not supported, usually true but may have - * other feature-specific value otherwise) + * @return Feature result (false if not supported, null if feature is unknown + * [=not supported, usually]; otherwise usually true but may have + * other feature-specific value) */ function plugin_supports($type,$name,$feature) { global $CFG; @@ -5975,7 +5980,7 @@ function plugin_supports($type,$name,$feature) { switch($feature) { // If some features can also be checked in other ways // for legacy support, this could be added here - default: return false; + default: return null; } } } diff --git a/lib/simpletest/testcompletionlib.php b/lib/simpletest/testcompletionlib.php index 2c920afa7a..df9157b04f 100644 --- a/lib/simpletest/testcompletionlib.php +++ b/lib/simpletest/testcompletionlib.php @@ -147,7 +147,7 @@ class completionlib_test extends UnitTestCase { function test_update_state() { $c=new completion_cutdown2(); - $c->completion_info((object)array('id'=>42)); + $c->__construct((object)array('id'=>42)); $cm=(object)array('id'=>13,'course'=>42); // Not enabled, should do nothing @@ -210,7 +210,7 @@ class completionlib_test extends UnitTestCase { global $DB; $c=new completion_cutdown3(); - $c->completion_info((object)array('id'=>42)); + $c->__construct((object)array('id'=>42)); $cm=(object)array('id'=>13,'course'=>42,'completiongradeitemnumber'=>null); // If view is required, but they haven't viewed it yet @@ -243,7 +243,7 @@ class completionlib_test extends UnitTestCase { function test_set_module_viewed() { $c=new completion_cutdown(); - $c->completion_info((object)array('id'=>42)); + $c->__construct((object)array('id'=>42)); $cm=(object)array('id'=>13,'course'=>42); // Not tracking completion, should do nothing @@ -286,7 +286,7 @@ class completionlib_test extends UnitTestCase { $DB->expectOnce('get_field_sql',array(new IgnoreWhitespaceExpectation("SELECT COUNT(1) FROM - test_course_modules_completion + {course_modules_completion} WHERE coursemoduleid=? AND completionstate<>0"),array(42))); $c=new completion_info(null); @@ -325,7 +325,7 @@ WHERE function test_reset_all_state() { global $DB; $c=new completion_cutdown(); - $c->completion_info((object)array('id'=>42)); + $c->__construct((object)array('id'=>42)); $cm=(object)array('id'=>13,'course'=>42); @@ -421,8 +421,8 @@ WHERE SELECT cmc.* FROM - test_course_modules cm - INNER JOIN test_course_modules_completion cmc ON cmc.coursemoduleid=cm.id + {course_modules} cm + INNER JOIN {course_modules_completion} cmc ON cmc.coursemoduleid=cm.id WHERE cm.course=? AND cmc.userid=?"),array(42,314159))); @@ -501,7 +501,7 @@ WHERE global $DB; $c=new completion_cutdown(); - $c->completion_info((object)array('id'=>42)); + $c->__construct((object)array('id'=>42)); // 1) Basic usage $c->expectAt(0,'internal_get_tracked_users',array(false,0)); @@ -515,8 +515,8 @@ WHERE SELECT cmc.* FROM - test_course_modules cm - INNER JOIN test_course_modules_completion cmc ON cm.id=cmc.coursemoduleid + {course_modules} cm + INNER JOIN {course_modules_completion} cmc ON cm.id=cmc.coursemoduleid WHERE cm.course=? AND cmc.userid IN (100,201)"),array(42))); $progress1=(object)array('userid'=>100,'coursemoduleid'=>13); @@ -552,8 +552,8 @@ WHERE SELECT cmc.* FROM - test_course_modules cm - INNER JOIN test_course_modules_completion cmc ON cm.id=cmc.coursemoduleid + {course_modules} cm + INNER JOIN {course_modules_completion} cmc ON cm.id=cmc.coursemoduleid WHERE cm.course=? AND cmc.userid IN whatever"),array(42))); $DB->setReturnValueAt(1,'get_recordset_sql',new fake_recordset(array_slice($progress,0,1000))); @@ -563,8 +563,8 @@ WHERE SELECT cmc.* FROM - test_course_modules cm - INNER JOIN test_course_modules_completion cmc ON cm.id=cmc.coursemoduleid + {course_modules} cm + INNER JOIN {course_modules_completion} cmc ON cm.id=cmc.coursemoduleid WHERE cm.course=? AND cmc.userid IN whatever2"),array(42))); $DB->setReturnValueAt(2,'get_recordset_sql',new fake_recordset(array_slice($progress,1000))); @@ -590,7 +590,7 @@ WHERE function test_inform_grade_changed() { $c=new completion_cutdown(); - $c->completion_info((object)array('id'=>42)); + $c->__construct((object)array('id'=>42)); $cm=(object)array('course'=>42,'id'=>13,'completiongradeitemnumber'=>null); $item=(object)array('itemnumber'=>3); diff --git a/mod/forum/lib.php b/mod/forum/lib.php index e9ce9048d5..292ed5b397 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -219,7 +219,7 @@ function forum_supports($feature) { switch($feature) { case FEATURE_COMPLETION_TRACKS_VIEWS: return true; case FEATURE_COMPLETION_HAS_RULES: return true; - default: return false; + default: return null; } } @@ -250,8 +250,8 @@ function forum_get_completion_state($course,$cm,$userid,$type) { SELECT COUNT(1) FROM - {$CFG->prefix}forum_posts fp - INNER JOIN {$CFG->prefix}forum_discussions fd ON fp.discussion=fd.id + {forum_posts} fp + INNER JOIN {forum_discussions} fd ON fp.discussion=fd.id WHERE fp.userid=:userid AND fd.forum=:forumid"; diff --git a/mod/forum/mod_form.php b/mod/forum/mod_form.php index e3fa1424d0..ef67c03b0f 100644 --- a/mod/forum/mod_form.php +++ b/mod/forum/mod_form.php @@ -235,8 +235,8 @@ class mod_forum_mod_form extends moodleform_mod { (!empty($data['completionpostsenabled']) && $data['completionposts']!=0); } - function get_data($slashed=true) { - $data=parent::get_data($slashed); + function get_data() { + $data=parent::get_data(); if(!$data) { return false; } diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index fc019f3b9a..7ea1f24ae2 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -1218,7 +1218,7 @@ function quiz_supports($feature) { switch($feature) { case FEATURE_GRADE_HAS_GRADE: return true; case FEATURE_COMPLETION_TRACKS_VIEWS: return true; - default: return false; + default: return null; } }