MDL-15498: Fixes to problems Petr identified with completion system checkin

diff --git a/lib/completionlib.php b/lib/completionlib.php
index 9d3c5cc910945e753cf4a2eb5f65b185876e29e5..0e39b09581dd6383a5ff7e7681afbea80828e299 100644 (file)
--- 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 2f21c483f6625cfb8efef13fba6bea212aa92a92..d1b71cacb9bab44f996cf4a78ddbe5d18b7abddc 100644 (file)
--- 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 659bccb782cbf9f6b01cf0cd2cd41094f6384584..c0e050a6783a5930c508c536ca81e166c1563f6d 100644 (file)
--- 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 2c920afa7a6ccf7ea9d08e6efd5e61619553e278..df9157b04fb10507a8d67e609e563a4751271838 100644 (file)
--- 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 e9ce9048d50aae8f99c5cf4113d3bc050a22eb67..292ed5b39750a5ac32bd7e6bcec28f4d05b1dc78 100644 (file)
--- 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 e3fa1424d0649706452ac958b3aa1ee2270578a6..ef67c03b0ff4832adfcbc959328265edac5b2688 100644 (file)
--- 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 fc019f3b9a6d7ec34929ae4d7dcda8f22c4a4b60..7ea1f24ae2cd126c236d4cf6a031faf5a0561cf5 100644 (file)
--- 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;
     }
 }