MDL-58255 mod_assign: Fix usage of assign object for overrides
authorJun Pataleta <jun@moodle.com>
Tue, 14 Mar 2017 08:25:14 +0000 (16:25 +0800)
committerJun Pataleta <jun@moodle.com>
Fri, 17 Mar 2017 01:33:00 +0000 (09:33 +0800)
* Fix the parameters being passed for the assign constructor.
* Use assign::get_instance() instead of assign::get_context() to fetch
the assignment instance's properties.

mod/assign/lib.php
mod/assign/locallib.php
mod/assign/override_form.php
mod/assign/overridedelete.php
mod/assign/overrideedit.php
mod/assign/tests/events_test.php

index f8eaf08..e648f00 100644 (file)
@@ -204,8 +204,7 @@ function assign_update_instance(stdClass $data, $form) {
  * If $override is non-zero, then it updates only the events
  * associated with the specified override.
  *
- * @uses ASSIGN_MAX_EVENT_LENGTH
- * @param object $assign the assign object.
+ * @param assign $assign the assign object.
  * @param object $override (optional) limit to a specific override
  */
 function assign_update_events($assign, $override = null) {
@@ -213,9 +212,10 @@ function assign_update_events($assign, $override = null) {
 
     require_once($CFG->dirroot . '/calendar/lib.php');
 
+    $assigninstance = $assign->get_instance();
+
     // Load the old events relating to this assign.
-    $conds = array('modulename' => 'assign',
-        'instance' => $assign->get_context()->id);
+    $conds = array('modulename' => 'assign', 'instance' => $assigninstance->id);
     if (!empty($override)) {
         // Only load events for this override.
         if (isset($override->userid)) {
@@ -229,7 +229,7 @@ function assign_update_events($assign, $override = null) {
     // Now make a to-do list of all that needs to be updated.
     if (empty($override)) {
         // We are updating the primary settings for the assign, so we need to add all the overrides.
-        $overrides = $DB->get_records('assign_overrides', array('assignid' => $assign->id));
+        $overrides = $DB->get_records('assign_overrides', array('assignid' => $assigninstance->id));
         // As well as the original assign (empty override).
         $overrides[] = new stdClass();
     } else {
@@ -237,38 +237,38 @@ function assign_update_events($assign, $override = null) {
         $overrides = array($override);
     }
 
+    if (!empty($assign->get_course_module())) {
+        $cmid = $assign->get_course_module()->id;
+    } else {
+        $cmid = get_coursemodule_from_instance('assign', $assigninstance->id, $assigninstance->course)->id;
+    }
+
     foreach ($overrides as $current) {
         $groupid   = isset($current->groupid) ? $current->groupid : 0;
         $userid    = isset($current->userid) ? $current->userid : 0;
-        $duedate = isset($current->duedate) ? $current->duedate : $assign->get_context()->duedate;
+        $duedate = isset($current->duedate) ? $current->duedate : $assigninstance->duedate;
 
         // Only add 'due' events for an override if they differ from the assign default.
         $addclose = empty($current->id) || !empty($current->duedate);
 
-        if (!empty($assign->coursemodule)) {
-            $cmid = $assign->coursemodule;
-        } else {
-            $cmid = get_coursemodule_from_instance('assign', $assign->get_context()->id, $assign->get_context()->course)->id;
-        }
-
         $event = new stdClass();
-        $event->description = format_module_intro('assign', $assign->get_context(), $cmid);
+        $event->description = format_module_intro('assign', $assigninstance, $cmid);
         // Events module won't show user events when the courseid is nonzero.
-        $event->courseid    = ($userid) ? 0 : $assign->get_context()->course;
+        $event->courseid    = ($userid) ? 0 : $assigninstance->course;
         $event->groupid     = $groupid;
         $event->userid      = $userid;
         $event->modulename  = 'assign';
-        $event->instance    = $assign->get_context()->id;
+        $event->instance    = $assigninstance->id;
         $event->timestart   = $duedate;
         $event->timeduration = 0;
-        $event->visible     = instance_is_visible('assign', $assign);
+        $event->visible     = instance_is_visible('assign', $assigninstance);
         $event->eventtype   = 'due';
 
         // Determine the event name and priority.
         if ($groupid) {
             // Group override event.
             $params = new stdClass();
-            $params->assign = $assign->get_context()->name;
+            $params->assign = $assigninstance->name;
             $params->group = groups_get_group_name($groupid);
             if ($params->group === false) {
                 // Group doesn't exist, just skip it.
@@ -282,13 +282,13 @@ function assign_update_events($assign, $override = null) {
         } else if ($userid) {
             // User override event.
             $params = new stdClass();
-            $params->assign = $assign->get_context()->name;
+            $params->assign = $assigninstance->name;
             $eventname = get_string('overrideusereventname', 'assign', $params);
             // Set user override priority.
             $event->priority = CALENDAR_EVENT_USER_OVERRIDE_PRIORITY;
         } else {
             // The parent event.
-            $eventname = $assign->name;
+            $eventname = $assigninstance->name;
         }
 
         if ($duedate && $addclose) {
index 8ad4244..1ab6631 100644 (file)
@@ -775,13 +775,16 @@ class assign {
 
         require_once($CFG->dirroot . '/calendar/lib.php');
 
-        $cm = get_coursemodule_from_instance('assign', $this->get_context()->id, $this->get_context()->course);
+        $cm = $this->get_course_module();
+        if (empty($cm)) {
+            $instance = $this->get_instance();
+            $cm = get_coursemodule_from_instance('assign', $instance->id, $instance->course);
+        }
 
         $override = $DB->get_record('assign_overrides', array('id' => $overrideid), '*', MUST_EXIST);
 
         // Delete the events.
-        $conds = array('modulename' => 'assign',
-            'instance' => $this->get_context()->id);
+        $conds = array('modulename' => 'assign', 'instance' => $this->get_instance()->id);
         if (isset($override->userid)) {
             $conds['userid'] = $override->userid;
         } else {
@@ -825,7 +828,7 @@ class assign {
     public function delete_all_overrides() {
         global $DB;
 
-        $overrides = $DB->get_records('assign_overrides', array('assignid' => $this->get_context()->id), 'id');
+        $overrides = $DB->get_records('assign_overrides', array('assignid' => $this->get_instance()->id), 'id');
         foreach ($overrides as $override) {
             $this->delete_override($override->id);
         }
index ef0944d..b3cf8d4 100644 (file)
@@ -172,7 +172,7 @@ class assign_override_form extends moodleform {
         array_push($users, $this->userid);
         $extensionmax = 0;
         foreach ($users as $value) {
-            $extension = $DB->get_record('assign_user_flags', array('assignment' => $this->assign->get_context()->id,
+            $extension = $DB->get_record('assign_user_flags', array('assignment' => $this->assign->get_instance()->id,
                 'userid' => $value));
             if ($extension) {
                 if ($extensionmax < $extension->extensionduedate) {
@@ -182,23 +182,23 @@ class assign_override_form extends moodleform {
         }
 
         if ($extensionmax) {
-            $this->assign->get_context()->extensionduedate = $extensionmax;
+            $this->assign->get_instance()->extensionduedate = $extensionmax;
         }
 
         // Open and close dates.
         $mform->addElement('date_time_selector', 'allowsubmissionsfromdate',
             get_string('allowsubmissionsfromdate', 'assign'), array('optional' => true));
-        $mform->setDefault('allowsubmissionsfromdate', $this->assign->get_context()->allowsubmissionsfromdate);
+        $mform->setDefault('allowsubmissionsfromdate', $this->assign->get_instance()->allowsubmissionsfromdate);
 
         $mform->addElement('date_time_selector', 'duedate', get_string('duedate', 'assign'), array('optional' => true));
-        $mform->setDefault('duedate', $this->assign->get_context()->duedate);
+        $mform->setDefault('duedate', $this->assign->get_instance()->duedate);
 
         $mform->addElement('date_time_selector', 'cutoffdate', get_string('cutoffdate', 'assign'), array('optional' => true));
-        $mform->setDefault('cutoffdate', $this->assign->get_context()->cutoffdate);
+        $mform->setDefault('cutoffdate', $this->assign->get_instance()->cutoffdate);
 
-        if (isset($this->assign->get_context()->extensionduedate)) {
+        if (isset($this->assign->get_instance()->extensionduedate)) {
             $mform->addElement('static', 'extensionduedate', get_string('extensionduedate', 'assign'),
-                userdate($this->assign->get_context()->extensionduedate));
+                userdate($this->assign->get_instance()->extensionduedate));
         }
 
         // Submit buttons.
@@ -263,13 +263,13 @@ class assign_override_form extends moodleform {
         }
 
         // Ensure that override duedate/allowsubmissionsfromdate are before extension date if exist.
-        if (!empty($assign->get_context()->extensionduedate) && !empty($data['duedate'])) {
-            if ($assign->get_context()->extensionduedate < $data['duedate']) {
+        if (!empty($assign->get_instance()->extensionduedate) && !empty($data['duedate'])) {
+            if ($assign->get_instance()->extensionduedate < $data['duedate']) {
                 $errors['duedate'] = get_string('extensionnotafterduedate', 'assign');
             }
         }
-        if (!empty($assign->get_context()->extensionduedate) && !empty($data['allowsubmissionsfromdate'])) {
-            if ($assign->get_context()->extensionduedate < $data['allowsubmissionsfromdate']) {
+        if (!empty($assign->get_instance()->extensionduedate) && !empty($data['allowsubmissionsfromdate'])) {
+            if ($assign->get_instance()->extensionduedate < $data['allowsubmissionsfromdate']) {
                 $errors['allowsubmissionsfromdate'] = get_string('extensionnotafterfromdate', 'assign');
             }
         }
@@ -278,7 +278,7 @@ class assign_override_form extends moodleform {
         $changed = false;
         $keys = array('duedate', 'cutoffdate', 'allowsubmissionsfromdate');
         foreach ($keys as $key) {
-            if ($data[$key] != $assign->get_context()->{$key}) {
+            if ($data[$key] != $assign->get_instance()->{$key}) {
                 $changed = true;
                 break;
             }
index ba37c60..69d2800 100644 (file)
@@ -35,14 +35,9 @@ if (! $override = $DB->get_record('assign_overrides', array('id' => $overrideid)
     print_error('invalidoverrideid', 'assign');
 }
 
-$assign = new assign($DB->get_record('assign', array('id' => $override->assignid), '*', MUST_EXIST), null, null);
-
-if (! $cm = get_coursemodule_from_instance("assign", $assign->get_context()->id, $assign->get_context()->course)) {
-    print_error('invalidcoursemodule');
-}
-$course = $DB->get_record('course', array('id' => $cm->course), '*', MUST_EXIST);
-
+list($course, $cm) = get_course_and_cm_from_instance($override->assignid, 'assign');
 $context = context_module::instance($cm->id);
+$assign = new assign($context, null, null);
 
 require_login($course, false, $cm);
 
@@ -63,7 +58,7 @@ if ($confirm) {
 
     $assign->delete_override($override->id);
 
-    reorder_group_overrides($assign->get_context()->id);
+    reorder_group_overrides($assign->get_instance()->id);
 
     redirect($cancelurl);
 }
@@ -79,7 +74,7 @@ $PAGE->set_title($title);
 $PAGE->set_heading($course->fullname);
 
 echo $OUTPUT->header();
-echo $OUTPUT->heading(format_string($assign->get_context()->name, true, array('context' => $context)));
+echo $OUTPUT->heading(format_string($assign->get_instance()->name, true, array('context' => $context)));
 
 if ($override->groupid) {
     $group = $DB->get_record('groups', array('id' => $override->groupid), 'id, name');
index 227b363..24af320 100644 (file)
@@ -43,18 +43,14 @@ if ($overrideid) {
         print_error('invalidoverrideid', 'assign');
     }
 
-    $assign = new assign($DB->get_record('assign', array('id' => $override->assignid), '*',  MUST_EXIST), null, null);
-
-    list($course, $cm) = get_course_and_cm_from_instance($assign->get_context(), 'assign');
+    list($course, $cm) = get_course_and_cm_from_instance($override->assignid, 'assign');
 
 } else if ($cmid) {
     list($course, $cm) = get_course_and_cm_from_cmid($cmid, 'assign');
-    $assign = new assign($DB->get_record('assign', array('id' => $cm->instance), '*', MUST_EXIST), null, null);
 
 } else {
     print_error('invalidcoursemodule');
 }
-$course = $DB->get_record('course', array('id' => $cm->course), '*', MUST_EXIST);
 
 $url = new moodle_url('/mod/assign/overrideedit.php');
 if ($action) {
@@ -71,6 +67,8 @@ $PAGE->set_url($url);
 require_login($course, false, $cm);
 
 $context = context_module::instance($cm->id);
+$assign = new assign($context, $cm, $course);
+$assigninstance = $assign->get_instance();
 
 // Add or edit an override.
 require_capability('mod/assign:manageoverrides', $context);
@@ -87,7 +85,7 @@ if ($overrideid) {
 $keys = array('duedate', 'cutoffdate', 'allowsubmissionsfromdate');
 foreach ($keys as $key) {
     if (!isset($data->{$key}) || $reset) {
-        $data->{$key} = $assign->get_context()->{$key};
+        $data->{$key} = $assigninstance->{$key};
     }
 }
 
@@ -121,11 +119,11 @@ if ($mform->is_cancelled()) {
 
 } else if ($fromform = $mform->get_data()) {
     // Process the data.
-    $fromform->assignid = $assign->get_context()->id;
+    $fromform->assignid = $assigninstance->id;
 
     // Replace unchanged values with null.
     foreach ($keys as $key) {
-        if (($fromform->{$key} == $assign->get_context()->{$key})) {
+        if (($fromform->{$key} == $assigninstance->{$key})) {
             $fromform->{$key} = null;
         }
     }
@@ -142,7 +140,7 @@ if ($mform->is_cancelled()) {
 
     if ($userorgroupchanged) {
         $conditions = array(
-                'assignid' => $assign->get_context()->id,
+                'assignid' => $assigninstance->id,
                 'userid' => empty($fromform->userid) ? null : $fromform->userid,
                 'groupid' => empty($fromform->groupid) ? null : $fromform->groupid);
         if ($oldoverride = $DB->get_record('assign_overrides', $conditions)) {
@@ -162,7 +160,7 @@ if ($mform->is_cancelled()) {
     $params = array(
         'context' => $context,
         'other' => array(
-            'assignid' => $assign->get_context()->id
+            'assignid' => $assigninstance->id
         )
     );
     if (!empty($override->id)) {
@@ -188,8 +186,8 @@ if ($mform->is_cancelled()) {
             $fromform->sortorder = $fromform->id;
 
             $overridecountgroup = $DB->count_records('assign_overrides',
-                array('userid' => null, 'assignid' => $assign->get_context()->id));
-            $overridecountall = $DB->count_records('assign_overrides', array('assignid' => $assign->get_context()->id));
+                array('userid' => null, 'assignid' => $assigninstance->id));
+            $overridecountall = $DB->count_records('assign_overrides', array('assignid' => $assigninstance->id));
             if ((!$overridecountgroup) && ($overridecountall)) { // No group overrides and there are user overrides.
                 $fromform->sortorder = 1;
             } else {
@@ -235,7 +233,7 @@ $PAGE->set_pagelayout('admin');
 $PAGE->set_title($pagetitle);
 $PAGE->set_heading($course->fullname);
 echo $OUTPUT->header();
-echo $OUTPUT->heading(format_string($assign->get_context()->name, true, array('context' => $context)));
+echo $OUTPUT->heading(format_string($assigninstance->name, true, array('context' => $context)));
 
 $mform->display();
 
index 6715cb4..5d14028 100644 (file)
@@ -1164,24 +1164,26 @@ class assign_events_testcase extends mod_assign_base_testcase {
         global $DB;
 
         $course = $this->getDataGenerator()->create_course();
-        $assign = $this->getDataGenerator()->create_module('assign', array('course' => $course->id));
-        $this->assign = new assign($assign, null, null);
+        $assigninstance = $this->getDataGenerator()->create_module('assign', array('course' => $course->id));
+        $cm = get_coursemodule_from_instance('assign', $assigninstance->id, $course->id);
+        $context = context_module::instance($cm->id);
+        $assign = new assign($context, $cm, $course);
 
         // Create an override.
         $override = new stdClass();
-        $override->assign = $this->assign->get_context()->id;
+        $override->assign = $assigninstance->id;
         $override->userid = 2;
         $override->id = $DB->insert_record('assign_overrides', $override);
 
         // Trigger and capture the event.
         $sink = $this->redirectEvents();
-        $this->assign->delete_override($override->id);
+        $assign->delete_override($override->id);
         $events = $sink->get_events();
         $event = reset($events);
 
         // Check that the event data is valid.
         $this->assertInstanceOf('\mod_assign\event\user_override_deleted', $event);
-        $this->assertEquals(context_module::instance($assign->cmid), $event->get_context());
+        $this->assertEquals(context_module::instance($cm->id), $event->get_context());
         $this->assertEventContextNotUsed($event);
     }
 
@@ -1192,24 +1194,26 @@ class assign_events_testcase extends mod_assign_base_testcase {
         global $DB;
 
         $course = $this->getDataGenerator()->create_course();
-        $assign = $this->getDataGenerator()->create_module('assign', array('course' => $course->id));
-        $this->assign = new assign($assign, null, null);
+        $assigninstance = $this->getDataGenerator()->create_module('assign', array('course' => $course->id));
+        $cm = get_coursemodule_from_instance('assign', $assigninstance->id, $course->id);
+        $context = context_module::instance($cm->id);
+        $assign = new assign($context, $cm, $course);
 
         // Create an override.
         $override = new stdClass();
-        $override->assign = $this->assign->get_context()->id;
+        $override->assign = $assigninstance->id;
         $override->groupid = 2;
         $override->id = $DB->insert_record('assign_overrides', $override);
 
         // Trigger and capture the event.
         $sink = $this->redirectEvents();
-        $this->assign->delete_override($override->id);
+        $assign->delete_override($override->id);
         $events = $sink->get_events();
         $event = reset($events);
 
         // Check that the event data is valid.
         $this->assertInstanceOf('\mod_assign\event\group_override_deleted', $event);
-        $this->assertEquals(context_module::instance($assign->cmid), $event->get_context());
+        $this->assertEquals(context_module::instance($cm->id), $event->get_context());
         $this->assertEventContextNotUsed($event);
     }
 }