MDL-41192 course: Can not write to the read-only properties of cm_info
authorMarina Glancy <marina@moodle.com>
Wed, 31 Jul 2013 01:37:32 +0000 (11:37 +1000)
committerMarina Glancy <marina@moodle.com>
Tue, 10 Sep 2013 04:11:30 +0000 (14:11 +1000)
filter/activitynames/filter.php
lib/completionlib.php
lib/tests/modinfolib_test.php
mod/assignment/index.php
report/progress/index.php

index 03f035e..07123c7 100644 (file)
@@ -55,27 +55,36 @@ class filter_activitynames extends moodle_text_filter {
 
             $modinfo = get_fast_modinfo($courseid);
             if (!empty($modinfo->cms)) {
-                self::$activitylist = array();      /// We will store all the activities here
+                self::$activitylist = array(); // We will store all the created filters here.
 
-                //Sort modinfo by name length
-                $sortedactivities = fullclone($modinfo->cms);
-                usort($sortedactivities, 'filter_activitynames_comparemodulenamesbylength');
+                // Create array of visible activities sorted by the name length (we are only interested in properties name and url).
+                $sortedactivities = array();
+                foreach ($modinfo->cms as $cm) {
+                    // Exclude labels, hidden activities and activities for group members only.
+                    if ($cm->visible and empty($cm->groupmembersonly) and $cm->has_view()) {
+                        $sortedactivities[] = (object)array(
+                            'name' => $cm->name,
+                            'url' => $cm->url,
+                            'id' => $cm->id,
+                            'namelen' => strlen($cm->name),
+                        );
+                    }
+                }
+                core_collator::asort_objects_by_property($sortedactivities, 'namelen', SORT_NUMERIC);
 
                 foreach ($sortedactivities as $cm) {
-                    //Exclude labels, hidden activities and activities for group members only
-                    if ($cm->visible and empty($cm->groupmembersonly) and $cm->has_view()) {
-                        $title = s(trim(strip_tags($cm->name)));
-                        $currentname = trim($cm->name);
-                        $entitisedname  = s($currentname);
-                        /// Avoid empty or unlinkable activity names
-                        if (!empty($title)) {
-                            $href_tag_begin = html_writer::start_tag('a',
-                                    array('class' => 'autolink', 'title' => $title,
-                                        'href' => $cm->get_url()));
-                            self::$activitylist[$cm->id] = new filterobject($currentname, $href_tag_begin, '</a>', false, true);
-                            if ($currentname != $entitisedname) { /// If name has some entity (&amp; &quot; &lt; &gt;) add that filter too. MDL-17545
-                                self::$activitylist[$cm->id.'-e'] = new filterobject($entitisedname, $href_tag_begin, '</a>', false, true);
-                            }
+                    $title = s(trim(strip_tags($cm->name)));
+                    $currentname = trim($cm->name);
+                    $entitisedname  = s($currentname);
+                    // Avoid empty or unlinkable activity names.
+                    if (!empty($title)) {
+                        $href_tag_begin = html_writer::start_tag('a',
+                                array('class' => 'autolink', 'title' => $title,
+                                    'href' => $cm->url));
+                        self::$activitylist[$cm->id] = new filterobject($currentname, $href_tag_begin, '</a>', false, true);
+                        if ($currentname != $entitisedname) {
+                            // If name has some entity (&amp; &quot; &lt; &gt;) add that filter too. MDL-17545.
+                            self::$activitylist[$cm->id.'-e'] = new filterobject($entitisedname, $href_tag_begin, '</a>', false, true);
                         }
                     }
                 }
@@ -100,14 +109,3 @@ class filter_activitynames extends moodle_text_filter {
         }
     }
 }
-
-
-
-//This function is used to order module names from longer to shorter
-function filter_activitynames_comparemodulenamesbylength($a, $b)  {
-    if (strlen($a->name) == strlen($b->name)) {
-        return 0;
-    }
-    return (strlen($a->name) < strlen($b->name)) ? 1 : -1;
-}
-
index 1b05837..e22d81b 100644 (file)
@@ -1077,7 +1077,7 @@ class completion_info {
      * Obtains a list of activities for which completion is enabled on the
      * course. The list is ordered by the section order of those activities.
      *
-     * @return array Array from $cmid => $cm of all activities with completion enabled,
+     * @return cm_info[] Array from $cmid => $cm of all activities with completion enabled,
      *   empty array if none
      */
     public function get_activities() {
index 8bcc5ee..0540530 100644 (file)
@@ -153,7 +153,7 @@ class core_modinfolib_testcase extends advanced_testcase {
 
         // Switch to a student and reload the context info.
         $this->setUser($student);
-        $cm_info = $this->refresh_cm_info($course, $assign);
+        $cm_info = get_fast_modinfo($course)->instances['assign'][$assign->id];
 
         // Create up a teacher.
         $teacherrole = $DB->get_record('role', array('shortname'=>'editingteacher'), '*', MUST_EXIST);
@@ -174,26 +174,33 @@ class core_modinfolib_testcase extends advanced_testcase {
         $CFG->enablegroupmembersonly = false;
         $this->assertFalse($cm_info->is_user_access_restricted_by_group());
 
-        // If groups are on but "group members only" is off, the activity isn't restricted.
+        // Turn groups setting on.
         $CFG->enablegroupmembersonly = true;
-        $cm_info->groupmembersonly = NOGROUPS;
+        // Create a mod_assign instance with "group members only", the activity should not be restricted.
+        $assignnogroups = $this->getDataGenerator()->create_module('assign', array('course'=>$course->id),
+            array('groupmembersonly' => NOGROUPS));
+        $cm_info = get_fast_modinfo($course->id)->instances['assign'][$assignnogroups->id];
         $this->assertFalse($cm_info->is_user_access_restricted_by_group());
 
         // If "group members only" is on but user is in the wrong group, the activity is restricted.
-        $cm_info->groupmembersonly = SEPARATEGROUPS;
-        $cm_info->groupingid = $grouping1->id;
+        $assignsepgroups = $this->getDataGenerator()->create_module('assign', array('course'=>$course->id),
+            array('groupmembersonly' => SEPARATEGROUPS, 'groupingid' => $grouping1->id));
         $this->assertTrue(groups_add_member($group2, $USER));
+        get_fast_modinfo($course->id, 0, true);
+        $cm_info = get_fast_modinfo($course->id)->instances['assign'][$assignsepgroups->id];
+        $this->assertEquals($grouping1->id, $cm_info->groupingid);
         $this->assertTrue($cm_info->is_user_access_restricted_by_group());
 
         // If the user is in the required group, the activity isn't restricted.
         groups_remove_member($group2, $USER);
         $this->assertTrue(groups_add_member($group1, $USER));
-        $cm_info = $this->refresh_cm_info($course, $assign);
+        get_fast_modinfo($course->id, 0, true);
+        $cm_info = get_fast_modinfo($course->id)->instances['assign'][$assignsepgroups->id];
         $this->assertFalse($cm_info->is_user_access_restricted_by_group());
 
         // Switch to a teacher and reload the context info.
         $this->setUser($teacher);
-        $cm_info = $this->refresh_cm_info($course, $assign);
+        $cm_info = get_fast_modinfo($course->id)->instances['assign'][$assignsepgroups->id];
 
         // If the user isn't in the required group but has 'moodle/site:accessallgroups', the activity isn't restricted.
         $this->assertTrue(has_capability('moodle/site:accessallgroups', $coursecontext));
@@ -210,10 +217,16 @@ class core_modinfolib_testcase extends advanced_testcase {
 
         $this->resetAfterTest();
 
-        // Create a course and a mod_assign instance.
+        // Create a course.
         $course = $this->getDataGenerator()->create_course();
-        $assign = $this->getDataGenerator()->create_module('assign', array('course'=>$course->id));
-        $cm_info = get_fast_modinfo($course)->instances['assign'][$assign->id];
+        // 1. Create an activity that is currently unavailable and hidden entirely (for students).
+        $assign1 = $this->getDataGenerator()->create_module('assign', array('course'=>$course->id),
+                array('availablefrom' => time() + 10000, 'showavailability' => CONDITION_STUDENTVIEW_HIDE));
+        // 2. Create an activity that is currently available.
+        $assign2 = $this->getDataGenerator()->create_module('assign', array('course'=>$course->id));
+        // 3. Create an activity that is currently unavailable and set to be greyed out.
+        $assign3 = $this->getDataGenerator()->create_module('assign', array('course'=>$course->id),
+                array('availablefrom' => time() + 10000, 'showavailability' => CONDITION_STUDENTVIEW_SHOW));
 
         // Set up a teacher.
         $coursecontext = context_course::instance($course->id);
@@ -221,38 +234,38 @@ class core_modinfolib_testcase extends advanced_testcase {
         $teacher = $this->getDataGenerator()->create_user();
         role_assign($teacherrole->id, $teacher->id, $coursecontext);
 
-        // Mark the activity as unavailable (due to unmet conditions).
-        // Testing of the code that normally turns this flag on and off is done in conditionlib_test.php.
-        $cm_info->available = false;
-        // Set the activity to be hidden entirely if it is unavailable to the user.
-        $cm_info->showavailability = CONDITION_STUDENTVIEW_HIDE;
-
         // If conditional availability is disabled the activity will always be unrestricted.
         $CFG->enableavailability = false;
+        $cm_info = get_fast_modinfo($course)->instances['assign'][$assign1->id];
         $this->assertFalse($cm_info->is_user_access_restricted_by_conditional_access());
 
-        // Turn on conditional availability.
+        // Turn on conditional availability and reset the get_fast_modinfo cache.
         $CFG->enableavailability = true;
+        get_fast_modinfo($course, 0, true);
 
         // The unavailable, hidden entirely activity should now be restricted.
+        $cm_info = get_fast_modinfo($course)->instances['assign'][$assign1->id];
+        $this->assertFalse($cm_info->available);
+        $this->assertEquals(CONDITION_STUDENTVIEW_HIDE, $cm_info->showavailability);
         $this->assertTrue($cm_info->is_user_access_restricted_by_conditional_access());
 
         // If the activity is available it should not be restricted.
-        $cm_info->available = true;
+        $cm_info = get_fast_modinfo($course)->instances['assign'][$assign2->id];
+        $this->assertTrue($cm_info->available);
         $this->assertFalse($cm_info->is_user_access_restricted_by_conditional_access());
 
         // If the activity is unavailable and set to be greyed out it should not be restricted.
-        $cm_info->available = false;
-        $cm_info->showavailability = CONDITION_STUDENTVIEW_SHOW;
+        $cm_info = get_fast_modinfo($course)->instances['assign'][$assign3->id];
+        $this->assertFalse($cm_info->available);
+        $this->assertEquals(CONDITION_STUDENTVIEW_SHOW, $cm_info->showavailability);
         $this->assertFalse($cm_info->is_user_access_restricted_by_conditional_access());
 
         // If the activity is unavailable and set to be hidden entirely its restricted unless user has 'moodle/course:viewhiddenactivities'.
-        $cm_info->available = false;
-        $cm_info->showavailability = CONDITION_STUDENTVIEW_HIDE;
-
         // Switch to a teacher and reload the context info.
         $this->setUser($teacher);
-        $cm_info = $this->refresh_cm_info($course, $assign);
+        $cm_info = get_fast_modinfo($course)->instances['assign'][$assign1->id];
+        $this->assertFalse($cm_info->available);
+        $this->assertEquals(CONDITION_STUDENTVIEW_HIDE, $cm_info->showavailability);
 
         $this->assertTrue(has_capability('moodle/course:viewhiddenactivities', $coursecontext));
         $this->assertFalse($cm_info->is_user_access_restricted_by_conditional_access());
@@ -312,9 +325,4 @@ class core_modinfolib_testcase extends advanced_testcase {
         $this->assertFalse($cm->uservisible);
         $this->assertTrue($cm->is_user_access_restricted_by_capability());
     }
-
-    private function refresh_cm_info($course, $assign) {
-        get_fast_modinfo(0, 0, true);
-        return get_fast_modinfo($course)->instances['assign'][$assign->id];
-    }
 }
index 5adcbf2..2bc451e 100644 (file)
@@ -58,9 +58,7 @@ foreach ($modinfo->instances['assignment'] as $cm) {
         continue;
     }
 
-    $cm->timedue        = $cms[$cm->id]->timedue;
-    $cm->assignmenttype = $cms[$cm->id]->assignmenttype;
-    $cm->idnumber       = $cms[$cm->id]->idnumber;
+    $assignmenttype = $cms[$cm->id]->assignmenttype;
 
     //Show dimmed if the mod is hidden
     $class = $cm->visible ? '' : 'class="dimmed"';
@@ -80,12 +78,12 @@ foreach ($modinfo->instances['assignment'] as $cm) {
         }
     }
 
-    if (!file_exists($CFG->dirroot.'/mod/assignment/type/'.$cm->assignmenttype.'/assignment.class.php')) {
+    if (!file_exists($CFG->dirroot.'/mod/assignment/type/'.$assignmenttype.'/assignment.class.php')) {
         continue;
     }
 
-    require_once ($CFG->dirroot.'/mod/assignment/type/'.$cm->assignmenttype.'/assignment.class.php');
-    $assignmentclass = 'assignment_'.$cm->assignmenttype;
+    require_once ($CFG->dirroot.'/mod/assignment/type/'.$assignmenttype.'/assignment.class.php');
+    $assignmentclass = 'assignment_'.$assignmenttype;
     $assignmentinstance = new $assignmentclass($cm->id, NULL, $cm, $course);
 
     $submitted = $assignmentinstance->submittedlink(true);
@@ -98,16 +96,16 @@ foreach ($modinfo->instances['assignment'] as $cm) {
         $grade = '-';
     }
 
-    $type = $types[$cm->assignmenttype];
+    $type = $types[$assignmenttype];
 
     // if type has an 'all.php' defined, make this a link
-    $pathtoall = "{$CFG->dirroot}/mod/assignment/type/{$cm->assignmenttype}/all.php";
+    $pathtoall = "{$CFG->dirroot}/mod/assignment/type/{$assignmenttype}/all.php";
     if (file_exists($pathtoall)) {
-        $type = "<a href=\"{$CFG->wwwroot}/mod/assignment/type/{$cm->assignmenttype}/".
+        $type = "<a href=\"{$CFG->wwwroot}/mod/assignment/type/{$assignmenttype}/".
             "all.php?id={$course->id}\">$type</a>";
     }
 
-    $due = $cm->timedue ? userdate($cm->timedue) : '-';
+    $due = !empty($cms[$cm->id]->timedue) ? userdate($cms[$cm->id]->timedue) : '-';
 
     if ($usesections) {
         $table->data[] = array ($printsection, $link, $type, $due, $submitted, $grade);
index 681f4cd..69d1058 100644 (file)
@@ -305,9 +305,10 @@ if (!$csv) {
 }
 
 // Activities
+$formattedactivities = array();
 foreach($activities as $activity) {
-    $activity->datepassed = $activity->completionexpected && $activity->completionexpected <= time();
-    $activity->datepassedclass=$activity->datepassed ? 'completion-expired' : '';
+    $datepassed = $activity->completionexpected && $activity->completionexpected <= time();
+    $datepassedclass = $datepassed ? 'completion-expired' : '';
 
     if ($activity->completionexpected) {
         $datetext=userdate($activity->completionexpected,get_string('strftimedate','langconfig'));
@@ -316,13 +317,13 @@ foreach($activities as $activity) {
     }
 
     // Some names (labels) come URL-encoded and can be very long, so shorten them
-    $activity->name = shorten_text($activity->name);
+    $displayname = shorten_text($activity->name);
 
     if ($csv) {
-        print $sep.csv_quote(strip_tags($activity->name)).$sep.csv_quote($datetext);
+        print $sep.csv_quote(strip_tags($displayname)).$sep.csv_quote($datetext);
     } else {
-        $formattedactivityname = format_string($activity->name, true, array('context' => $context));
-        print '<th scope="col" class="'.$activity->datepassedclass.'">'.
+        $formattedactivityname = format_string($displayname, true, array('context' => $activity->context));
+        print '<th scope="col" class="'.$datepassedclass.'">'.
             '<a href="'.$CFG->wwwroot.'/mod/'.$activity->modname.
             '/view.php?id='.$activity->id.'" title="' . $formattedactivityname . '">'.
             '<img src="'.$OUTPUT->pix_url('icon', $activity->modname).'" alt="'.
@@ -333,6 +334,10 @@ foreach($activities as $activity) {
         }
         print '</th>';
     }
+    $formattedactivities[$activity->id] = (object)array(
+        'datepassedclass' => $datepassedclass,
+        'displayname' => $displayname,
+    );
 }
 
 if ($csv) {
@@ -382,19 +387,18 @@ foreach($progress as $user) {
             ($activity->completion==COMPLETION_TRACKING_AUTOMATIC ? 'auto' : 'manual').
             '-'.$completiontype;
 
-        $modcontext = context_module::instance($activity->id);
         $describe = get_string('completion-' . $completiontype, 'completion');
         $a=new StdClass;
         $a->state=$describe;
         $a->date=$date;
         $a->user=fullname($user);
-        $a->activity = format_string($activity->name, true, array('context' => $modcontext));
+        $a->activity = format_string($formattedactivities[$activity->id]->displayname, true, array('context' => $activity->context));
         $fulldescribe=get_string('progress-title','completion',$a);
 
         if ($csv) {
             print $sep.csv_quote($describe).$sep.csv_quote($date);
         } else {
-            print '<td class="completion-progresscell '.$activity->datepassedclass.'">'.
+            print '<td class="completion-progresscell '.$formattedactivities[$activity->id]->datepassedclass.'">'.
                 '<img src="'.$OUTPUT->pix_url('i/'.$completionicon).
                 '" alt="'.$describe.'" title="'.$fulldescribe.'" /></td>';
         }