diff --git a/filter/activitynames/filter.php b/filter/activitynames/filter.php index 03f035e00cf..07123c74a8b 100644 --- a/filter/activitynames/filter.php +++ b/filter/activitynames/filter.php @@ -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, '', false, true); - if ($currentname != $entitisedname) { /// If name has some entity (& " < >) add that filter too. MDL-17545 - self::$activitylist[$cm->id.'-e'] = new filterobject($entitisedname, $href_tag_begin, '', 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, '', false, true); + if ($currentname != $entitisedname) { + // If name has some entity (& " < >) add that filter too. MDL-17545. + self::$activitylist[$cm->id.'-e'] = new filterobject($entitisedname, $href_tag_begin, '', 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; -} - diff --git a/lib/completionlib.php b/lib/completionlib.php index 1b058374442..e22d81b67dc 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -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() { diff --git a/lib/tests/modinfolib_test.php b/lib/tests/modinfolib_test.php index 8bcc5ee0f24..05405305f4c 100644 --- a/lib/tests/modinfolib_test.php +++ b/lib/tests/modinfolib_test.php @@ -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]; - } } diff --git a/mod/assignment/index.php b/mod/assignment/index.php index 5adcbf2b91c..2bc451eef50 100644 --- a/mod/assignment/index.php +++ b/mod/assignment/index.php @@ -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 = "wwwroot}/mod/assignment/type/{$cm->assignmenttype}/". + $type = "wwwroot}/mod/assignment/type/{$assignmenttype}/". "all.php?id={$course->id}\">$type"; } - $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); diff --git a/report/progress/index.php b/report/progress/index.php index 681f4cd8369..69d1058261a 100644 --- a/report/progress/index.php +++ b/report/progress/index.php @@ -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 ''. + $formattedactivityname = format_string($displayname, true, array('context' => $activity->context)); + print ''. ''. ''. @@ -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=datepassedclass.'">'. + print ''. ''.$describe.''; }