From 8f48fbf6f600ff1c5a4cd9753fcac14d890407cf Mon Sep 17 00:00:00 2001
From: Jun Pataleta <jun@moodle.com>
Date: Tue, 14 Mar 2017 16:25:14 +0800
Subject: [PATCH] MDL-58255 mod_assign: Fix usage of assign object for
 overrides

* 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               | 38 ++++++++++++++++----------------
 mod/assign/locallib.php          | 11 +++++----
 mod/assign/override_form.php     | 24 ++++++++++----------
 mod/assign/overridedelete.php    | 13 ++++-------
 mod/assign/overrideedit.php      | 24 +++++++++-----------
 mod/assign/tests/events_test.php | 24 +++++++++++---------
 6 files changed, 67 insertions(+), 67 deletions(-)

diff --git a/mod/assign/lib.php b/mod/assign/lib.php
index 81f13c9a834..b3b0123ec71 100644
--- a/mod/assign/lib.php
+++ b/mod/assign/lib.php
@@ -205,7 +205,7 @@ function assign_update_instance(stdClass $data, $form) {
  * 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 +213,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)) {
@@ -228,9 +229,8 @@ function assign_update_events($assign, $override = null) {
 
     // Now make a todo 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));
+        // 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' => $assigninstance->id));
         // As well as the original assign (empty override).
         $overrides[] = new stdClass();
     } else {
@@ -238,31 +238,31 @@ 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;
         $allowsubmissionsfromdate  = isset($current->allowsubmissionsfromdate
         ) ? $current->allowsubmissionsfromdate : $assign->get_context()->allowsubmissionsfromdate;
-        $duedate = isset($current->duedate) ? $current->duedate : $assign->get_context()->duedate;
+        $duedate = isset($current->duedate) ? $current->duedate : $assigninstance->duedate;
 
         // Only add open/close events for an override if they differ from the assign default.
         $addopen  = empty($current->id) || !empty($current->allowsubmissionsfromdate);
         $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   = $allowsubmissionsfromdate;
         $event->timeduration = max($duedate - $allowsubmissionsfromdate, 0);
         $event->visible     = instance_is_visible('assign', $assign);
@@ -271,7 +271,7 @@ function assign_update_events($assign, $override = null) {
         // Determine the event name.
         if ($groupid) {
             $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.
@@ -280,10 +280,10 @@ function assign_update_events($assign, $override = null) {
             $eventname = get_string('overridegroupeventname', 'assign', $params);
         } else if ($userid) {
             $params = new stdClass();
-            $params->assign = $assign->get_context()->name;
+            $params->assign = $assigninstance->name;
             $eventname = get_string('overrideusereventname', 'assign', $params);
         } else {
-            $eventname = $assign->name;
+            $eventname = $assigninstance->name;
         }
         if ($addopen or $addclose) {
             if ($duedate and $allowsubmissionsfromdate and $event->timeduration <= ASSIGN_MAX_EVENT_LENGTH) {
diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php
index d1070a02019..67d804c4835 100644
--- a/mod/assign/locallib.php
+++ b/mod/assign/locallib.php
@@ -774,13 +774,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 {
@@ -824,7 +827,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);
         }
diff --git a/mod/assign/override_form.php b/mod/assign/override_form.php
index ef0944d6310..b3cf8d4bb83 100644
--- a/mod/assign/override_form.php
+++ b/mod/assign/override_form.php
@@ -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;
             }
diff --git a/mod/assign/overridedelete.php b/mod/assign/overridedelete.php
index ba37c60dd69..69d280010ed 100644
--- a/mod/assign/overridedelete.php
+++ b/mod/assign/overridedelete.php
@@ -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');
diff --git a/mod/assign/overrideedit.php b/mod/assign/overrideedit.php
index 227b363915f..24af320ee7b 100644
--- a/mod/assign/overrideedit.php
+++ b/mod/assign/overrideedit.php
@@ -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();
 
diff --git a/mod/assign/tests/events_test.php b/mod/assign/tests/events_test.php
index 6715cb46783..5d1402861c1 100644
--- a/mod/assign/tests/events_test.php
+++ b/mod/assign/tests/events_test.php
@@ -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);
     }
 }