From 9aac9f70748c016419cc037d5213d643a1e8d2e2 Mon Sep 17 00:00:00 2001 From: Ryan Wyllie Date: Thu, 5 Oct 2017 08:07:07 +0000 Subject: [PATCH 1/3] MDL-60062 quiz: add support for drag drop of calendar events --- mod/quiz/lang/en/quiz.php | 1 + mod/quiz/lib.php | 172 +++++ mod/quiz/locallib.php | 39 ++ .../tests/calendar_event_modified_test.php | 638 ++++++++++++++++++ mod/quiz/tests/locallib_test.php | 144 ++++ 5 files changed, 994 insertions(+) create mode 100644 mod/quiz/tests/calendar_event_modified_test.php diff --git a/mod/quiz/lang/en/quiz.php b/mod/quiz/lang/en/quiz.php index 7ef3a14683a..4b61fe42eac 100644 --- a/mod/quiz/lang/en/quiz.php +++ b/mod/quiz/lang/en/quiz.php @@ -565,6 +565,7 @@ $string['onlyteachersexport'] = 'Only teachers can export questions'; $string['onlyteachersimport'] = 'Only teachers with editing rights can import questions'; $string['onthispage'] = 'This page'; $string['open'] = 'Not answered'; +$string['openafterclose'] = 'Could not update the quiz. You have specified an open date after the close date.'; $string['openclosedatesupdated'] = 'Quiz open and close dates updated'; $string['optional'] = 'optional'; $string['orderandpaging'] = 'Order and paging'; diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index 499f0386c29..4ee34de8e63 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -2253,3 +2253,175 @@ function mod_quiz_get_completion_active_rule_descriptions($cm) { } return $descriptions; } + +/** + * Returns the min and max values for the timestart property of a quiz + * activity event. + * + * The min and max values will be the timeopen and timeclose properties + * of the quiz, respectively, if they are set. + * + * If either value isn't set then null will be returned instead to + * indicate that there is no cutoff for that value. + * + * A minimum and maximum cutoff return value will look like: + * [ + * [1505704373, 'The date must be after this date'], + * [1506741172, 'The date must be before this date'] + * ] + * + * @param \calendar_event $event The calendar event to get the time range for + * @param stdClass|null $quiz The module instance to get the range from + * @return array + */ +function mod_quiz_core_calendar_get_valid_event_timestart_range(\calendar_event $event, \stdClass $quiz = null) { + global $CFG, $DB; + require_once($CFG->dirroot . '/mod/quiz/locallib.php'); + + // No restrictions on override events. + if (quiz_is_overriden_calendar_event($event)) { + return [null, null]; + } + + if (!$quiz) { + $quiz = $DB->get_record('quiz', ['id' => $event->instance]); + } + + $mindate = null; + $maxdate = null; + + if ($event->eventtype == QUIZ_EVENT_TYPE_OPEN) { + if (!empty($quiz->timeclose)) { + $maxdate = [ + $quiz->timeclose, + get_string('openafterclose', 'quiz') + ]; + } + } else if ($event->eventtype == QUIZ_EVENT_TYPE_CLOSE) { + if (!empty($quiz->timeopen)) { + $mindate = [ + $quiz->timeopen, + get_string('closebeforeopen', 'quiz') + ]; + } + } + + return [$mindate, $maxdate]; +} + +/** + * This function will check that the given event is valid for it's + * corresponding quiz module. + * + * An exception is thrown if the event fails validation. + * + * @throws \moodle_exception + * @param \calendar_event $event + * @return bool + */ +function mod_quiz_core_calendar_validate_event_timestart(\calendar_event $event) { + global $DB; + + if (!isset($event->instance)) { + return; + } + + // Something weird going on. The event is for a different module so + // we should ignore it. + if ($event->modulename != 'quiz') { + return; + } + + // We need to read from the DB directly because course module may + // currently be getting created so it won't be in mod info yet. + $quiz = $DB->get_record('quiz', ['id' => $event->instance], '*', MUST_EXIST); + $timestart = $event->timestart; + list($min, $max) = mod_quiz_core_calendar_get_valid_event_timestart_range($event, $quiz); + + if ($min && $timestart < $min[0]) { + throw new \moodle_exception($min[1]); + } + + if ($max && $timestart > $max[0]) { + throw new \moodle_exception($max[1]); + } +} + +/** + * This function will update the quiz module according to the + * event that has been modified. + * + * It will set the timeopen or timeclose value of the quiz instance + * according to the type of event provided. + * + * @throws \moodle_exception + * @param \calendar_event $event + */ +function mod_quiz_core_calendar_event_timestart_updated(\calendar_event $event) { + global $CFG, $DB; + require_once($CFG->dirroot . '/mod/quiz/locallib.php'); + + // We don't update the activity if it's an override event that has + // been modified. + if (quiz_is_overriden_calendar_event($event)) { + return; + } + + $courseid = $event->courseid; + $modulename = $event->modulename; + $instanceid = $event->instance; + $modified = false; + $closedatechanged = false; + + // Something weird going on. The event is for a different module so + // we should ignore it. + if ($modulename != 'quiz') { + return; + } + + $coursemodule = get_fast_modinfo($courseid)->instances[$modulename][$instanceid]; + $context = context_module::instance($coursemodule->id); + + // The user does not have the capability to modify this activity. + if (!has_capability('moodle/course:manageactivities', $context)) { + return; + } + + if ($event->eventtype == QUIZ_EVENT_TYPE_OPEN) { + // If the event is for the quiz activity opening then we should + // set the start time of the quiz activity to be the new start + // time of the event. + $quiz = $DB->get_record('quiz', ['id' => $instanceid], '*', MUST_EXIST); + + if ($quiz->timeopen != $event->timestart) { + $quiz->timeopen = $event->timestart; + $modified = true; + } + } else if ($event->eventtype == QUIZ_EVENT_TYPE_CLOSE) { + // If the event is for the quiz activity closing then we should + // set the end time of the quiz activity to be the new start + // time of the event. + $quiz = $DB->get_record('quiz', ['id' => $instanceid], '*', MUST_EXIST); + + if ($quiz->timeclose != $event->timestart) { + $quiz->timeclose = $event->timestart; + $modified = true; + $closedatechanged = true; + } + } + + if ($modified) { + $quiz->timemodified = time(); + $DB->update_record('quiz', $quiz); + + if ($closedatechanged) { + quiz_update_open_attempts(array('quizid' => $quiz->id)); + } + + // Delete any previous preview attempts. + quiz_delete_previews($quiz); + quiz_update_events($quiz); + $event = \core\event\course_module_updated::create_from_cm($coursemodule, $context); + $event->trigger(); + } +} diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index c1164f60ad9..36bca77dd7a 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -2316,3 +2316,42 @@ function quiz_prepare_and_start_new_attempt(quiz $quizobj, $attemptnumber, $last return $attempt; } + +/** + * Check if the given calendar_event is either a user or group override + * event for quiz. + * + * @param calendar_event $event The calendar event to check + * @return bool + */ +function quiz_is_overriden_calendar_event(\calendar_event $event) { + global $DB; + + if (!isset($event->modulename)) { + return false; + } + + if ($event->modulename != 'quiz') { + return false; + } + + if (!isset($event->instance)) { + return false; + } + + if (!isset($event->userid) && !isset($event->groupid)) { + return false; + } + + $overrideparams = [ + 'quiz' => $event->instance + ]; + + if (isset($event->groupid)) { + $overrideparams['groupid'] = $event->groupid; + } else if (isset($event->userid)) { + $overrideparams['userid'] = $event->userid; + } + + return $DB->record_exists('quiz_overrides', $overrideparams); +} diff --git a/mod/quiz/tests/calendar_event_modified_test.php b/mod/quiz/tests/calendar_event_modified_test.php new file mode 100644 index 00000000000..ac36dd5b203 --- /dev/null +++ b/mod/quiz/tests/calendar_event_modified_test.php @@ -0,0 +1,638 @@ +. + +/** + * Unit tests for the calendar event modification callbacks used + * for dragging and dropping quiz calendar events in the calendar + * UI. + * + * @package mod_quiz + * @category test + * @copyright 2017 Ryan Wyllie + * @license http://www.gnu.org/copyleft/gpl.html GNU Public License + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/mod/quiz/lib.php'); + +/** + * @copyright 2017 Ryan Wyllie + * @license http://www.gnu.org/copyleft/gpl.html GNU Public License + */ +class mod_quiz_calendar_event_modified_testcase extends advanced_testcase { + + /** + * Create an instance of the quiz activity. + * + * @param array $properties Properties to set on the activity + * @return stdClass Quiz activity instance + */ + protected function create_quiz_instance(array $properties) { + global $DB; + + $generator = $this->getDataGenerator(); + + if (empty($properties['course'])) { + $course = $generator->create_course(); + $courseid = $course->id; + } else { + $courseid = $properties['course']; + } + + $quizgenerator = $generator->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(array_merge(['course' => $courseid], $properties)); + + if (isset($properties['timemodified'])) { + // The generator overrides the timemodified value to set it as + // the current time even if a value is provided so we need to + // make sure it's set back to the requested value. + $quiz->timemodified = $properties['timemodified']; + $DB->update_record('quiz', $quiz); + } + + return $quiz; + } + + /** + * Create a calendar event for a quiz activity instance. + * + * @param stdClass $quiz The activity instance + * @param array $eventproperties Properties to set on the calendar event + * @return calendar_event + */ + protected function create_quiz_calendar_event(\stdClass $quiz, array $eventproperties) { + $defaultproperties = [ + 'name' => 'Test event', + 'description' => '', + 'format' => 1, + 'courseid' => $quiz->course, + 'groupid' => 0, + 'userid' => 2, + 'modulename' => 'quiz', + 'instance' => $quiz->id, + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => time(), + 'timeduration' => 86400, + 'visible' => 1 + ]; + + return new \calendar_event(array_merge($defaultproperties, $eventproperties)); + } + + /** + * You can't create a quiz module event when the module doesn't exist. + */ + public function test_mod_quiz_core_calendar_validate_event_timestart_no_activity() { + global $CFG; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + + $event = new \calendar_event([ + 'name' => 'Test event', + 'description' => '', + 'format' => 1, + 'courseid' => $course->id, + 'groupid' => 0, + 'userid' => 2, + 'modulename' => 'quiz', + 'instance' => 1234, + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => time(), + 'timeduration' => 86400, + 'visible' => 1 + ]); + + $this->expectException('moodle_exception'); + mod_quiz_core_calendar_validate_event_timestart($event); + } + + /** + * A QUIZ_EVENT_TYPE_OPEN must be before the close time of the quiz activity. + */ + public function test_mod_quiz_core_calendar_validate_event_timestart_valid_open_event() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $quiz = $this->create_quiz_instance(['timeopen' => $timeopen, 'timeclose' => $timeclose]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => $timeopen + ]); + + mod_quiz_core_calendar_validate_event_timestart($event); + // The function above will throw an exception if the event is + // invalid. + $this->assertTrue(true); + } + + /** + * A QUIZ_EVENT_TYPE_OPEN can not have a start time set after the close time + * of the quiz activity. + */ + public function test_mod_quiz_core_calendar_validate_event_timestart_invalid_open_event() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $quiz = $this->create_quiz_instance(['timeopen' => $timeopen, 'timeclose' => $timeclose]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => $timeclose + 1 + ]); + + $this->expectException('moodle_exception'); + mod_quiz_core_calendar_validate_event_timestart($event); + } + + /** + * A QUIZ_EVENT_TYPE_CLOSE must be after the open time of the quiz activity. + */ + public function test_mod_quiz_core_calendar_validate_event_timestart_valid_close_event() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $quiz = $this->create_quiz_instance(['timeopen' => $timeopen, 'timeclose' => $timeclose]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => $timeclose + ]); + + mod_quiz_core_calendar_validate_event_timestart($event); + // The function above will throw an exception if the event isn't + // valid. + $this->assertTrue(true); + } + + /** + * A QUIZ_EVENT_TYPE_CLOSE can not have a start time set before the open time + * of the quiz activity. + */ + public function test_mod_quiz_core_calendar_validate_event_timestart_invalid_close_event() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $quiz = $this->create_quiz_instance(['timeopen' => $timeopen, 'timeclose' => $timeclose]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_CLOSE, + 'timestart' => $timeopen - 1 + ]); + + $this->expectException('moodle_exception'); + mod_quiz_core_calendar_validate_event_timestart($event); + } + + /** + * An unkown event type should not change the quiz instance. + */ + public function test_mod_quiz_core_calendar_event_timestart_updated_unknown_event() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $quiz = $this->create_quiz_instance(['timeopen' => $timeopen, 'timeclose' => $timeclose]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_OPEN . "SOMETHING ELSE", + 'timestart' => 1 + ]); + + mod_quiz_core_calendar_event_timestart_updated($event); + + $quiz = $DB->get_record('quiz', ['id' => $quiz->id]); + $this->assertEquals($timeopen, $quiz->timeopen); + $this->assertEquals($timeclose, $quiz->timeclose); + } + + /** + * A QUIZ_EVENT_TYPE_OPEN event should update the timeopen property of + * the quiz activity. + */ + public function test_mod_quiz_core_calendar_event_timestart_updated_open_event() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $timemodified = 1; + $newtimeopen = $timeopen - DAYSECS; + $quiz = $this->create_quiz_instance([ + 'timeopen' => $timeopen, + 'timeclose' => $timeclose, + 'timemodified' => $timemodified + ]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => $newtimeopen + ]); + + mod_quiz_core_calendar_event_timestart_updated($event); + + $quiz = $DB->get_record('quiz', ['id' => $quiz->id]); + // Ensure the timeopen property matches the event timestart. + $this->assertEquals($newtimeopen, $quiz->timeopen); + // Ensure the timeclose isn't changed. + $this->assertEquals($timeclose, $quiz->timeclose); + // Ensure the timemodified property has been changed. + $this->assertNotEquals($timemodified, $quiz->timemodified); + } + + /** + * A QUIZ_EVENT_TYPE_CLOSE event should update the timeclose property of + * the quiz activity. + */ + public function test_mod_quiz_core_calendar_event_timestart_updated_close_event() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $timemodified = 1; + $newtimeclose = $timeclose + DAYSECS; + $quiz = $this->create_quiz_instance([ + 'timeopen' => $timeopen, + 'timeclose' => $timeclose, + 'timemodified' => $timemodified + ]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_CLOSE, + 'timestart' => $newtimeclose + ]); + + mod_quiz_core_calendar_event_timestart_updated($event); + + $quiz = $DB->get_record('quiz', ['id' => $quiz->id]); + // Ensure the timeclose property matches the event timestart. + $this->assertEquals($newtimeclose, $quiz->timeclose); + // Ensure the timeopen isn't changed. + $this->assertEquals($timeopen, $quiz->timeopen); + // Ensure the timemodified property has been changed. + $this->assertNotEquals($timemodified, $quiz->timemodified); + } + + /** + * A QUIZ_EVENT_TYPE_OPEN event should not update the timeopen property of + * the quiz activity if it's an override. + */ + public function test_mod_quiz_core_calendar_event_timestart_updated_open_event_override() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $user = $this->getDataGenerator()->create_user(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $timemodified = 1; + $newtimeopen = $timeopen - DAYSECS; + $quiz = $this->create_quiz_instance([ + 'timeopen' => $timeopen, + 'timeclose' => $timeclose, + 'timemodified' => $timemodified + ]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'userid' => $user->id, + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => $newtimeopen + ]); + $record = (object) [ + 'quiz' => $quiz->id, + 'userid' => $user->id + ]; + + $DB->insert_record('quiz_overrides', $record); + + mod_quiz_core_calendar_event_timestart_updated($event); + + $quiz = $DB->get_record('quiz', ['id' => $quiz->id]); + // Ensure the timeopen property doesn't change. + $this->assertEquals($timeopen, $quiz->timeopen); + // Ensure the timeclose isn't changed. + $this->assertEquals($timeclose, $quiz->timeclose); + // Ensure the timemodified property has not been changed. + $this->assertEquals($timemodified, $quiz->timemodified); + } + + /** + * If a student somehow finds a way to update the quiz calendar event + * then the callback should not update the quiz activity otherwise that + * would be a security issue. + */ + public function test_student_role_cant_update_quiz_activity() { + global $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user = $generator->create_user(); + $course = $generator->create_course(); + $context = context_course::instance($course->id); + $roleid = $generator->create_role(); + $now = time(); + $timeopen = (new DateTime())->setTimestamp($now); + $newtimeopen = (new DateTime())->setTimestamp($now)->modify('+1 day'); + $quiz = $this->create_quiz_instance([ + 'course' => $course->id, + 'timeopen' => $timeopen->getTimestamp() + ]); + + $generator->enrol_user($user->id, $course->id, 'student'); + $generator->role_assign($roleid, $user->id, $context->id); + + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => $timeopen->getTimestamp() + ]); + + assign_capability('moodle/course:manageactivities', CAP_PROHIBIT, $roleid, $context, true); + + $this->setUser($user); + + mod_quiz_core_calendar_event_timestart_updated($event); + + $newquiz = $DB->get_record('quiz', ['id' => $quiz->id]); + // The time open shouldn't have changed even though we updated the calendar + // event. + $this->assertEquals($timeopen->getTimestamp(), $newquiz->timeopen); + } + + /** + * A teacher with the capability to modify a quiz module should be + * able to update the quiz activity dates by changing the calendar + * event. + */ + public function test_teacher_role_can_update_quiz_activity() { + global $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user = $generator->create_user(); + $course = $generator->create_course(); + $context = context_course::instance($course->id); + $roleid = $generator->create_role(); + $now = time(); + $timeopen = (new DateTime())->setTimestamp($now); + $newtimeopen = (new DateTime())->setTimestamp($now)->modify('+1 day'); + $quiz = $this->create_quiz_instance([ + 'course' => $course->id, + 'timeopen' => $timeopen->getTimestamp() + ]); + + $generator->enrol_user($user->id, $course->id, 'teacher'); + $generator->role_assign($roleid, $user->id, $context->id); + + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => $newtimeopen->getTimestamp() + ]); + + assign_capability('moodle/course:manageactivities', CAP_ALLOW, $roleid, $context, true); + + $this->setUser($user); + + // Trigger and capture the event. + $sink = $this->redirectEvents(); + + mod_quiz_core_calendar_event_timestart_updated($event); + + $triggeredevents = $sink->get_events(); + $moduleupdatedevents = array_filter($triggeredevents, function($e) { + return is_a($e, 'core\event\course_module_updated'); + }); + + $newquiz = $DB->get_record('quiz', ['id' => $quiz->id]); + // The should be updated along with the event because the user has sufficient + // capabilities. + $this->assertEquals($newtimeopen->getTimestamp(), $newquiz->timeopen); + // Confirm that a module updated event is fired when the module + // is changed. + $this->assertNotEmpty($moduleupdatedevents); + } + + + /** + * An unkown event type should not have any limits + */ + public function test_mod_quiz_core_calendar_get_valid_event_timestart_range_unknown_event() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $quiz = $this->create_quiz_instance([ + 'timeopen' => $timeopen, + 'timeclose' => $timeclose + ]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_OPEN . "SOMETHING ELSE", + 'timestart' => 1 + ]); + + list ($min, $max) = mod_quiz_core_calendar_get_valid_event_timestart_range($event, $quiz); + $this->assertNull($min); + $this->assertNull($max); + } + + /** + * The open event should be limited by the quiz's timeclose property, if it's set. + */ + public function test_mod_quiz_core_calendar_get_valid_event_timestart_range_open_event() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $quiz = $this->create_quiz_instance([ + 'timeopen' => $timeopen, + 'timeclose' => $timeclose + ]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => 1 + ]); + + // The max limit should be bounded by the timeclose value. + list ($min, $max) = mod_quiz_core_calendar_get_valid_event_timestart_range($event, $quiz); + + $this->assertNull($min); + $this->assertEquals($timeclose, $max[0]); + + // No timeclose value should result in no upper limit. + $quiz->timeclose = 0; + list ($min, $max) = mod_quiz_core_calendar_get_valid_event_timestart_range($event, $quiz); + + $this->assertNull($min); + $this->assertNull($max); + } + + /** + * An override event should not have any limits. + */ + public function test_mod_quiz_core_calendar_get_valid_event_timestart_range_override_event() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $generator = $this->getDataGenerator(); + $user = $generator->create_user(); + $course = $generator->create_course(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $quiz = $this->create_quiz_instance([ + 'course' => $course->id, + 'timeopen' => $timeopen, + 'timeclose' => $timeclose + ]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'userid' => $user->id, + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => 1 + ]); + $record = (object) [ + 'quiz' => $quiz->id, + 'userid' => $user->id + ]; + + $DB->insert_record('quiz_overrides', $record); + + list ($min, $max) = mod_quiz_core_calendar_get_valid_event_timestart_range($event, $quiz); + + $this->assertNull($min); + $this->assertNull($max); + } + + /** + * The close event should be limited by the quiz's timeopen property, if it's set. + */ + public function test_mod_quiz_core_calendar_get_valid_event_timestart_range_close_event() { + global $DB; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = time(); + $timeclose = $timeopen + DAYSECS; + $quiz = $this->create_quiz_instance([ + 'timeopen' => $timeopen, + 'timeclose' => $timeclose + ]); + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_CLOSE, + 'timestart' => 1, + ]); + + // The max limit should be bounded by the timeclose value. + list ($min, $max) = mod_quiz_core_calendar_get_valid_event_timestart_range($event, $quiz); + + $this->assertEquals($timeopen, $min[0]); + $this->assertNull($max); + + // No timeclose value should result in no upper limit. + $quiz->timeopen = 0; + list ($min, $max) = mod_quiz_core_calendar_get_valid_event_timestart_range($event, $quiz); + + $this->assertNull($min); + $this->assertNull($max); + } + + /** + * When the close date event is changed and it results in the time close value of + * the quiz being updated then the open quiz attempts should also be updated. + */ + public function test_core_calendar_event_timestart_updated_update_quiz_attempt() { + global $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $teacher = $generator->create_user(); + $student = $generator->create_user(); + $course = $generator->create_course(); + $context = context_course::instance($course->id); + $roleid = $generator->create_role(); + $now = time(); + $timelimit = 600; + $timeopen = (new DateTime())->setTimestamp($now); + $timeclose = (new DateTime())->setTimestamp($now)->modify('+1 day'); + // The new close time being earlier than the time open + time limit should + // result in an update to the quiz attempts. + $newtimeclose = $timeopen->getTimestamp() + $timelimit - 10; + $quiz = $this->create_quiz_instance([ + 'course' => $course->id, + 'timeopen' => $timeopen->getTimestamp(), + 'timeclose' => $timeclose->getTimestamp(), + 'timelimit' => $timelimit + ]); + + $generator->enrol_user($student->id, $course->id, 'student'); + $generator->enrol_user($teacher->id, $course->id, 'teacher'); + $generator->role_assign($roleid, $teacher->id, $context->id); + + $event = $this->create_quiz_calendar_event($quiz, [ + 'eventtype' => QUIZ_EVENT_TYPE_CLOSE, + 'timestart' => $newtimeclose + ]); + + assign_capability('moodle/course:manageactivities', CAP_ALLOW, $roleid, $context, true); + + $attemptid = $DB->insert_record( + 'quiz_attempts', + [ + 'quiz' => $quiz->id, + 'userid' => $student->id, + 'state' => 'inprogress', + 'timestart' => $timeopen->getTimestamp(), + 'timecheckstate' => 0, + 'layout' => '', + 'uniqueid' => 1 + ] + ); + + $this->setUser($teacher); + + mod_quiz_core_calendar_event_timestart_updated($event); + + $quiz = $DB->get_record('quiz', ['id' => $quiz->id]); + $attempt = $DB->get_record('quiz_attempts', ['id' => $attemptid]); + // When the close date is changed so that it's earlier than the time open + // plus the time limit of the quiz then the attempt's timecheckstate should + // be updated to the new time close date of the quiz. + $this->assertEquals($newtimeclose, $attempt->timecheckstate); + $this->assertEquals($newtimeclose, $quiz->timeclose); + } +} diff --git a/mod/quiz/tests/locallib_test.php b/mod/quiz/tests/locallib_test.php index 8fa40e6c516..081c7577867 100644 --- a/mod/quiz/tests/locallib_test.php +++ b/mod/quiz/tests/locallib_test.php @@ -152,4 +152,148 @@ class mod_quiz_locallib_testcase extends advanced_testcase { $completiondata = $completion->get_data($cm); $this->assertEquals(1, $completiondata->completionstate); } + + /** + * Return false when there are not overrides for this quiz instance. + */ + public function test_quiz_is_overriden_calendar_event_no_override() { + global $CFG, $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user = $generator->create_user(); + $course = $generator->create_course(); + $quizgenerator = $generator->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(['course' => $course->id]); + + $event = new \calendar_event((object)[ + 'modulename' => 'quiz', + 'instance' => $quiz->id, + 'userid' => $user->id + ]); + + $this->assertFalse(quiz_is_overriden_calendar_event($event)); + } + + /** + * Return false if the given event isn't an quiz module event. + */ + public function test_quiz_is_overriden_calendar_event_no_module_event() { + global $CFG, $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user = $generator->create_user(); + $course = $generator->create_course(); + $quizgenerator = $generator->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(['course' => $course->id]); + + $event = new \calendar_event((object)[ + 'userid' => $user->id + ]); + + $this->assertFalse(quiz_is_overriden_calendar_event($event)); + } + + /** + * Return false if there is overrides for this use but they belong to another quiz + * instance. + */ + public function test_quiz_is_overriden_calendar_event_different_quiz_instance() { + global $CFG, $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user = $generator->create_user(); + $course = $generator->create_course(); + $quizgenerator = $generator->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(['course' => $course->id]); + $quiz2 = $quizgenerator->create_instance(['course' => $course->id]); + + $event = new \calendar_event((object) [ + 'modulename' => 'quiz', + 'instance' => $quiz->id, + 'userid' => $user->id + ]); + + $record = (object) [ + 'quiz' => $quiz2->id, + 'userid' => $user->id + ]; + + $DB->insert_record('quiz_overrides', $record); + + $this->assertFalse(quiz_is_overriden_calendar_event($event)); + } + + /** + * Return true if there is a user override for this event and quiz instance. + */ + public function test_quiz_is_overriden_calendar_event_user_override() { + global $CFG, $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user = $generator->create_user(); + $course = $generator->create_course(); + $quizgenerator = $generator->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(['course' => $course->id]); + + $event = new \calendar_event((object) [ + 'modulename' => 'quiz', + 'instance' => $quiz->id, + 'userid' => $user->id + ]); + + $record = (object) [ + 'quiz' => $quiz->id, + 'userid' => $user->id + ]; + + $DB->insert_record('quiz_overrides', $record); + + $this->assertTrue(quiz_is_overriden_calendar_event($event)); + } + + /** + * Return true if there is a group override for the event and quiz instance. + */ + public function test_quiz_is_overriden_calendar_event_group_override() { + global $CFG, $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $generator = $this->getDataGenerator(); + $user = $generator->create_user(); + $course = $generator->create_course(); + $quizgenerator = $generator->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(['course' => $course->id]); + $group = $this->getDataGenerator()->create_group(array('courseid' => $quiz->course)); + $groupid = $group->id; + $userid = $user->id; + + $event = new \calendar_event((object) [ + 'modulename' => 'quiz', + 'instance' => $quiz->id, + 'groupid' => $groupid + ]); + + $record = (object) [ + 'quiz' => $quiz->id, + 'groupid' => $groupid + ]; + + $DB->insert_record('quiz_overrides', $record); + + $this->assertTrue(quiz_is_overriden_calendar_event($event)); + } } From 478b1d194fda4a3a9061475b2db3bc3b739639d8 Mon Sep 17 00:00:00 2001 From: Ryan Wyllie Date: Mon, 30 Oct 2017 08:15:10 +0000 Subject: [PATCH 2/3] MDL-60062 calendar: remove validate_event_timestart callback --- .../external/calendar_event_exporter.php | 9 +- calendar/classes/local/api.php | 40 ++- calendar/tests/local_api_test.php | 311 +++++++++++++++++ calendar/upgrade.txt | 2 +- mod/assign/lib.php | 89 ++--- mod/assign/tests/lib_test.php | 313 +----------------- mod/choice/lib.php | 70 +--- mod/choice/tests/lib_test.php | 196 +---------- mod/feedback/lib.php | 80 ++--- mod/feedback/tests/lib_test.php | 208 +----------- mod/quiz/lib.php | 73 ++-- .../tests/calendar_event_modified_test.php | 130 +------- 12 files changed, 459 insertions(+), 1062 deletions(-) diff --git a/calendar/classes/external/calendar_event_exporter.php b/calendar/classes/external/calendar_event_exporter.php index f7d2555dd7e..f3dd9ffd2ca 100644 --- a/calendar/classes/external/calendar_event_exporter.php +++ b/calendar/classes/external/calendar_event_exporter.php @@ -213,14 +213,19 @@ class calendar_event_exporter extends event_exporter_base { * @return array */ protected function get_module_timestamp_limits($event) { + global $DB; + $values = []; $mapper = container::get_event_mapper(); $starttime = $event->get_times()->get_start_time(); + $modname = $event->get_course_module()->get('modname'); + $modid = $event->get_course_module()->get('instance'); + $moduleinstance = $DB->get_record($modname, ['id' => $modid]); list($min, $max) = component_callback( - 'mod_' . $event->get_course_module()->get('modname'), + 'mod_' . $modname, 'core_calendar_get_valid_event_timestart_range', - [$mapper->from_event_to_legacy_event($event)], + [$mapper->from_event_to_legacy_event($event), $moduleinstance], [null, null] ); diff --git a/calendar/classes/local/api.php b/calendar/classes/local/api.php index 00713f43f84..1b81231c991 100644 --- a/calendar/classes/local/api.php +++ b/calendar/classes/local/api.php @@ -231,9 +231,12 @@ class api { event_interface $event, \DateTimeInterface $startdate ) { + global $DB; + $mapper = container::get_event_mapper(); $legacyevent = $mapper->from_event_to_legacy_event($event); $hascoursemodule = !empty($event->get_course_module()); + $moduleinstance = null; $starttime = $event->get_times()->get_start_time()->setDate( $startdate->format('Y'), $startdate->format('n'), @@ -241,15 +244,32 @@ class api { ); if ($hascoursemodule) { - $legacyevent->timestart = $starttime->getTimestamp(); - // If this event is from an activity then we need to call - // the activity callback to let it validate that the changes - // to the event are correct. - component_callback( - 'mod_' . $event->get_course_module()->get('modname'), - 'core_calendar_validate_event_timestart', - [$legacyevent] + $moduleinstance = $DB->get_record( + $event->get_course_module()->get('modname'), + ['id' => $event->get_course_module()->get('instance')], + '*', + MUST_EXIST ); + $legacyevent->timestart = $starttime->getTimestamp(); + + // If there is a timestart range callback implemented then we can + // use the values returned from the valid timestart range to apply + // some default validation on the event's timestart value to ensure + // that it falls within the specified range. + list($min, $max) = component_callback( + 'mod_' . $event->get_course_module()->get('modname'), + 'core_calendar_get_valid_event_timestart_range', + [$legacyevent, $moduleinstance], + [null, null] + ); + + if ($min && $legacyevent->timestart < $min[0]) { + throw new \moodle_exception($min[1]); + } + + if ($max && $legacyevent->timestart > $max[0]) { + throw new \moodle_exception($max[1]); + } } // This function does our capability checks. @@ -262,7 +282,7 @@ class api { // We don't want to call the event update callback if the user isn't allowed // to modify course modules because depending on the callback it can make // some changes that would be considered security issues, such as updating the - // due date for and assignment. + // due date for an assignment. if ($hascoursemodule && calendar_edit_event_allowed($legacyevent, true)) { // If this event is from an activity then we need to call // the activity callback to let it know that the event it @@ -270,7 +290,7 @@ class api { component_callback( 'mod_' . $event->get_course_module()->get('modname'), 'core_calendar_event_timestart_updated', - [$legacyevent] + [$legacyevent, $moduleinstance] ); } diff --git a/calendar/tests/local_api_test.php b/calendar/tests/local_api_test.php index 7f17a288efc..d97c08cac51 100644 --- a/calendar/tests/local_api_test.php +++ b/calendar/tests/local_api_test.php @@ -44,6 +44,37 @@ class core_calendar_local_api_testcase extends advanced_testcase { $this->resetAfterTest(); } + /** + * Create a feedback activity instance and a calendar event for + * that instance. + * + * @param array $feedbackproperties Properties to set on the feedback activity + * @param array $eventproperties Properties to set on the calendar event + * @return array The feedback activity and the calendar event + */ + protected function create_feedback_activity_and_event(array $feedbackproperties = [], array $eventproperties = []) { + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + $mapper = container::get_event_mapper(); + $feedbackgenerator = $generator->get_plugin_generator('mod_feedback'); + $feedback = $feedbackgenerator->create_instance(array_merge( + ['course' => $course->id], + $feedbackproperties + )); + + $event = create_event(array_merge( + [ + 'courseid' => $course->id, + 'modulename' => 'feedback', + 'instance' => $feedback->id + ], + $eventproperties + )); + $event = $mapper->from_legacy_event_to_event($event); + + return [$feedback, $event]; + } + /** * Requesting calendar events from a given time should return all events with a sort * time at or after the requested time. All events prior to that time should not @@ -928,4 +959,284 @@ class core_calendar_local_api_testcase extends advanced_testcase { $this->expectException('moodle_exception'); $newEvent = \core_calendar\local\api::update_event_start_day($event, $newstartdate); } + + /** + * Updating the start day of an event with no maximum cutoff should + * update the corresponding activity property. + * + * Note: This test uses the feedback activity because it requires + * module callbacks to be in place to test. + */ + public function test_update_event_start_day_activity_event_no_max() { + global $CFG, $DB; + require_once($CFG->dirroot . '/mod/feedback/lib.php'); + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = new DateTimeImmutable('2017-01-1T15:00:00+08:00'); + $newstartdate = new DateTimeImmutable('2018-02-2T10:00:00+08:00'); + $expected = new DateTimeImmutable('2018-02-2T15:00:00+08:00'); + list($feedback, $event) = $this->create_feedback_activity_and_event( + [ + 'timeopen' => $timeopen->getTimestamp(), + 'timeclose' => 0 + ], + [ + 'eventtype' => FEEDBACK_EVENT_TYPE_OPEN, + 'timestart' => $timeopen->getTimestamp() + ] + ); + $newevent = \core_calendar\local\api::update_event_start_day($event, $newstartdate); + $actual = $newevent->get_times()->get_start_time(); + $feedback = $DB->get_record('feedback', ['id' => $feedback->id]); + + $this->assertEquals($expected->getTimestamp(), $actual->getTimestamp()); + $this->assertEquals($expected->getTimestamp(), $feedback->timeopen); + } + + /** + * Updating the start day of an event belonging to an activity to a value + * less than the maximum cutoff should update the corresponding activity + * property. + * + * Note: This test uses the feedback activity because it requires + * module callbacks to be in place to test. + */ + public function test_update_event_start_day_activity_event_less_than_max() { + global $CFG, $DB; + require_once($CFG->dirroot . '/mod/feedback/lib.php'); + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = new DateTimeImmutable('2017-01-1T15:00:00+08:00'); + $timeclose = new DateTimeImmutable('2019-01-1T15:00:00+08:00'); + $newstartdate = new DateTimeImmutable('2018-02-2T10:00:00+08:00'); + $expected = new DateTimeImmutable('2018-02-2T15:00:00+08:00'); + list($feedback, $event) = $this->create_feedback_activity_and_event( + [ + 'timeopen' => $timeopen->getTimestamp(), + 'timeclose' => $timeclose->getTimestamp() + ], + [ + 'eventtype' => FEEDBACK_EVENT_TYPE_OPEN, + 'timestart' => $timeopen->getTimestamp() + ] + ); + + $newevent = \core_calendar\local\api::update_event_start_day($event, $newstartdate); + $actual = $newevent->get_times()->get_start_time(); + $feedback = $DB->get_record('feedback', ['id' => $feedback->id]); + + $this->assertEquals($expected->getTimestamp(), $actual->getTimestamp()); + $this->assertEquals($expected->getTimestamp(), $feedback->timeopen); + } + + /** + * Updating the start day of an event belonging to an activity to a value + * equal to the maximum cutoff should update the corresponding activity + * property. + * + * Note: This test uses the feedback activity because it requires + * module callbacks to be in place to test. + */ + public function test_update_event_start_day_activity_event_equal_to_max() { + global $CFG, $DB; + require_once($CFG->dirroot . '/mod/feedback/lib.php'); + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = new DateTimeImmutable('2017-01-1T15:00:00+08:00'); + $timeclose = new DateTimeImmutable('2018-02-2T15:00:00+08:00'); + $newstartdate = new DateTimeImmutable('2018-02-2T10:00:00+08:00'); + list($feedback, $event) = $this->create_feedback_activity_and_event( + [ + 'timeopen' => $timeopen->getTimestamp(), + 'timeclose' => $timeclose->getTimestamp(), + ], + [ + 'eventtype' => FEEDBACK_EVENT_TYPE_OPEN, + 'timestart' => $timeopen->getTimestamp() + ] + ); + + $newevent = \core_calendar\local\api::update_event_start_day($event, $newstartdate); + $actual = $newevent->get_times()->get_start_time(); + $feedback = $DB->get_record('feedback', ['id' => $feedback->id]); + + $this->assertEquals($timeclose->getTimestamp(), $actual->getTimestamp()); + $this->assertEquals($timeclose->getTimestamp(), $feedback->timeopen); + } + + /** + * Updating the start day of an event belonging to an activity to a value + * after the maximum cutoff should not update the corresponding activity + * property. Instead it should throw an exception. + * + * Note: This test uses the feedback activity because it requires + * module callbacks to be in place to test. + */ + public function test_update_event_start_day_activity_event_after_max() { + global $CFG, $DB; + require_once($CFG->dirroot . '/mod/feedback/lib.php'); + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = new DateTimeImmutable('2017-01-1T15:00:00+08:00'); + $timeclose = new DateTimeImmutable('2017-02-2T15:00:00+08:00'); + $newstartdate = new DateTimeImmutable('2018-02-2T10:00:00+08:00'); + list($feedback, $event) = $this->create_feedback_activity_and_event( + [ + 'timeopen' => $timeopen->getTimestamp(), + 'timeclose' => $timeclose->getTimestamp(), + ], + [ + 'eventtype' => FEEDBACK_EVENT_TYPE_OPEN, + 'timestart' => $timeopen->getTimestamp() + ] + ); + + $this->expectException('moodle_exception'); + $newevent = \core_calendar\local\api::update_event_start_day($event, $newstartdate); + } + + /** + * Updating the start day of an event with no minimum cutoff should + * update the corresponding activity property. + * + * Note: This test uses the feedback activity because it requires + * module callbacks to be in place to test. + */ + public function test_update_event_start_day_activity_event_no_min() { + global $CFG, $DB; + require_once($CFG->dirroot . '/mod/feedback/lib.php'); + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeclose = new DateTimeImmutable('2017-01-1T15:00:00+08:00'); + $newstartdate = new DateTimeImmutable('2016-02-2T10:00:00+08:00'); + $expected = new DateTimeImmutable('2016-02-2T15:00:00+08:00'); + list($feedback, $event) = $this->create_feedback_activity_and_event( + [ + 'timeopen' => 0, + 'timeclose' => $timeclose->getTimestamp() + ], + [ + 'eventtype' => FEEDBACK_EVENT_TYPE_OPEN, + 'timestart' => $timeclose->getTimestamp() + ] + ); + + $newevent = \core_calendar\local\api::update_event_start_day($event, $newstartdate); + $actual = $newevent->get_times()->get_start_time(); + $feedback = $DB->get_record('feedback', ['id' => $feedback->id]); + + $this->assertEquals($expected->getTimestamp(), $actual->getTimestamp()); + $this->assertEquals($expected->getTimestamp(), $feedback->timeopen); + } + + /** + * Updating the start day of an event belonging to an activity to a value + * greater than the minimum cutoff should update the corresponding activity + * property. + * + * Note: This test uses the feedback activity because it requires + * module callbacks to be in place to test. + */ + public function test_update_event_start_day_activity_event_greater_than_min() { + global $CFG, $DB; + require_once($CFG->dirroot . '/mod/feedback/lib.php'); + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = new DateTimeImmutable('2016-01-1T15:00:00+08:00'); + $timeclose = new DateTimeImmutable('2019-01-1T15:00:00+08:00'); + $newstartdate = new DateTimeImmutable('2018-02-2T10:00:00+08:00'); + $expected = new DateTimeImmutable('2018-02-2T15:00:00+08:00'); + list($feedback, $event) = $this->create_feedback_activity_and_event( + [ + 'timeopen' => $timeopen->getTimestamp(), + 'timeclose' => $timeclose->getTimestamp() + ], + [ + 'eventtype' => FEEDBACK_EVENT_TYPE_CLOSE, + 'timestart' => $timeclose->getTimestamp() + ] + ); + + $newevent = \core_calendar\local\api::update_event_start_day($event, $newstartdate); + $actual = $newevent->get_times()->get_start_time(); + $feedback = $DB->get_record('feedback', ['id' => $feedback->id]); + + $this->assertEquals($expected->getTimestamp(), $actual->getTimestamp()); + $this->assertEquals($expected->getTimestamp(), $feedback->timeclose); + } + + /** + * Updating the start day of an event belonging to an activity to a value + * equal to the minimum cutoff should update the corresponding activity + * property. + * + * Note: This test uses the feedback activity because it requires + * module callbacks to be in place to test. + */ + public function test_update_event_start_day_activity_event_equal_to_min() { + global $CFG, $DB; + require_once($CFG->dirroot . '/mod/feedback/lib.php'); + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = new DateTimeImmutable('2017-01-1T15:00:00+08:00'); + $timeclose = new DateTimeImmutable('2018-02-2T15:00:00+08:00'); + $newstartdate = new DateTimeImmutable('2017-01-1T10:00:00+08:00'); + $expected = new DateTimeImmutable('2017-01-1T15:00:00+08:00'); + list($feedback, $event) = $this->create_feedback_activity_and_event( + [ + 'timeopen' => $timeopen->getTimestamp(), + 'timeclose' => $timeclose->getTimestamp(), + ], + [ + 'eventtype' => FEEDBACK_EVENT_TYPE_CLOSE, + 'timestart' => $timeclose->getTimestamp() + ] + ); + + $newevent = \core_calendar\local\api::update_event_start_day($event, $newstartdate); + $actual = $newevent->get_times()->get_start_time(); + $feedback = $DB->get_record('feedback', ['id' => $feedback->id]); + + $this->assertEquals($expected->getTimestamp(), $actual->getTimestamp()); + $this->assertEquals($expected->getTimestamp(), $feedback->timeclose); + } + + /** + * Updating the start day of an event belonging to an activity to a value + * before the minimum cutoff should not update the corresponding activity + * property. Instead it should throw an exception. + * + * Note: This test uses the feedback activity because it requires + * module callbacks to be in place to test. + */ + public function test_update_event_start_day_activity_event_before_min() { + global $CFG, $DB; + require_once($CFG->dirroot . '/mod/feedback/lib.php'); + + $this->resetAfterTest(true); + $this->setAdminUser(); + $timeopen = new DateTimeImmutable('2017-01-1T15:00:00+08:00'); + $timeclose = new DateTimeImmutable('2017-02-2T15:00:00+08:00'); + $newstartdate = new DateTimeImmutable('2016-02-2T10:00:00+08:00'); + list($feedback, $event) = $this->create_feedback_activity_and_event( + [ + 'timeopen' => $timeopen->getTimestamp(), + 'timeclose' => $timeclose->getTimestamp(), + ], + [ + 'eventtype' => FEEDBACK_EVENT_TYPE_CLOSE, + 'timestart' => $timeclose->getTimestamp() + ] + ); + + $this->expectException('moodle_exception'); + $newevent = \core_calendar\local\api::update_event_start_day($event, $newstartdate); + } } diff --git a/calendar/upgrade.txt b/calendar/upgrade.txt index 0007c3c65a8..528cc8d9268 100644 --- a/calendar/upgrade.txt +++ b/calendar/upgrade.txt @@ -4,7 +4,7 @@ information provided here is intended especially for developers. === 3.4 === * calendar_get_mini has been deprecated. Please update to use the new exporters and renderers. -* added core_calendar_validate_event_timestart and core_calendar_event_timestart_updated callbacks for module events +* added core_calendar_get_valid_event_timestart_range and core_calendar_event_timestart_updated callbacks for module events when the update_event_start_day function is used in the local api. === 3.3 === diff --git a/mod/assign/lib.php b/mod/assign/lib.php index c6b0121f105..214362b5133 100644 --- a/mod/assign/lib.php +++ b/mod/assign/lib.php @@ -1947,30 +1947,17 @@ function mod_assign_core_calendar_event_action_shows_item_count(calendar_event $ * ] * * @param calendar_event $event The calendar event to get the time range for - * @param stdClass|null $instance The module instance to get the range from + * @param stdClass $instance The module instance to get the range from */ -function mod_assign_core_calendar_get_valid_event_timestart_range(\calendar_event $event, \stdClass $instance = null) { - global $CFG, $DB; +function mod_assign_core_calendar_get_valid_event_timestart_range(\calendar_event $event, \stdClass $instance) { + global $CFG; require_once($CFG->dirroot . '/mod/assign/locallib.php'); - if (!$instance) { - $instance = $DB->get_record('assign', ['id' => $event->instance]); - } - - $coursemodule = get_coursemodule_from_instance('assign', - $event->instance, - $event->courseid, - false, - MUST_EXIST); - - if (empty($coursemodule)) { - // If we don't have a course module yet then it likely means - // the activity is still being set up. In this case there is - // nothing for us to do anyway. - return; - } - + $courseid = $event->courseid; + $modulename = $event->modulename; + $instanceid = $event->instance; + $coursemodule = get_fast_modinfo($courseid)->instances[$modulename][$instanceid]; $context = context_module::instance($coursemodule->id); $assign = new assign($context, null, null); $assign->set_instance($instance); @@ -1978,46 +1965,15 @@ function mod_assign_core_calendar_get_valid_event_timestart_range(\calendar_even return $assign->get_valid_calendar_event_timestart_range($event); } -/** - * This function will check that the given event is valid for it's - * corresponding assign module instance. - * - * An exception is thrown if the event fails validation. - * - * @throws \moodle_exception - * @param \calendar_event $event - * @return bool - */ -function mod_assign_core_calendar_validate_event_timestart(\calendar_event $event) { - global $DB; - - if (!isset($event->instance)) { - return; - } - - // We need to read from the DB directly because course module may - // currently be getting created so it won't be in mod info yet. - $instance = $DB->get_record('assign', ['id' => $event->instance], '*', MUST_EXIST); - $timestart = $event->timestart; - list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event, $instance); - - if ($min && $timestart < $min[0]) { - throw new \moodle_exception($min[1]); - } - - if ($max && $timestart > $max[0]) { - throw new \moodle_exception($max[1]); - } -} - /** * This function will update the assign module according to the * event that has been modified. * * @throws \moodle_exception * @param \calendar_event $event + * @param stdClass $instance The module instance to get the range from */ -function mod_assign_core_calendar_event_timestart_updated(\calendar_event $event) { +function mod_assign_core_calendar_event_timestart_updated(\calendar_event $event, \stdClass $instance) { global $CFG, $DB; require_once($CFG->dirroot . '/mod/assign/locallib.php'); @@ -2026,19 +1982,19 @@ function mod_assign_core_calendar_event_timestart_updated(\calendar_event $event return; } - $coursemodule = get_coursemodule_from_instance('assign', - $event->instance, - $event->courseid, - false, - MUST_EXIST); - - if (empty($coursemodule)) { - // If we don't have a course module yet then it likely means - // the activity is still being set up. In this case there is - // nothing for us to do anyway. + if ($instance->id != $event->instance) { return; } + if (!in_array($event->eventtype, [ASSIGN_EVENT_TYPE_DUE, ASSIGN_EVENT_TYPE_GRADINGDUE])) { + return; + } + + $courseid = $event->courseid; + $modulename = $event->modulename; + $instanceid = $event->instance; + $modified = false; + $coursemodule = get_fast_modinfo($courseid)->instances[$modulename][$instanceid]; $context = context_module::instance($coursemodule->id); // The user does not have the capability to modify this activity. @@ -2047,7 +2003,7 @@ function mod_assign_core_calendar_event_timestart_updated(\calendar_event $event } $assign = new assign($context, $coursemodule, null); - $modified = false; + $assign->set_instance($instance); if ($event->eventtype == ASSIGN_EVENT_TYPE_DUE) { // This check is in here because due date events are currently @@ -2058,26 +2014,23 @@ function mod_assign_core_calendar_event_timestart_updated(\calendar_event $event return; } - $instance = $assign->get_instance(); $newduedate = $event->timestart; if ($newduedate != $instance->duedate) { $instance->duedate = $newduedate; - $instance->timemodified = time(); $modified = true; } } else if ($event->eventtype == ASSIGN_EVENT_TYPE_GRADINGDUE) { - $instance = $assign->get_instance(); $newduedate = $event->timestart; if ($newduedate != $instance->gradingduedate) { $instance->gradingduedate = $newduedate; - $instance->timemodified = time(); $modified = true; } } if ($modified) { + $instance->timemodified = time(); // Persist the assign instance changes. $DB->update_record('assign', $instance); $assign->update_calendar($coursemodule->id); diff --git a/mod/assign/tests/lib_test.php b/mod/assign/tests/lib_test.php index 79c924c15ee..5e057eea08f 100644 --- a/mod/assign/tests/lib_test.php +++ b/mod/assign/tests/lib_test.php @@ -847,7 +847,7 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { 'eventtype' => 'SOME RANDOM EVENT' ]); - list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event); + list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event, $instance); $this->assertNull($min); $this->assertNull($max); } @@ -882,7 +882,7 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { $DB->insert_record('assign_overrides', $record); - list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event); + list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event, $instance); $this->assertNull($min); $this->assertNull($max); } @@ -914,7 +914,7 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { 'eventtype' => ASSIGN_EVENT_TYPE_DUE ]); - list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event); + list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event, $instance); $this->assertNull($min); $this->assertNull($max); } @@ -948,7 +948,7 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { 'eventtype' => ASSIGN_EVENT_TYPE_DUE ]); - list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event); + list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event, $instance); $this->assertEquals($submissionsfromdate, $min[0]); $this->assertNotEmpty($min[1]); $this->assertEquals($cutoffdate, $max[0]); @@ -979,7 +979,7 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { 'eventtype' => ASSIGN_EVENT_TYPE_GRADINGDUE ]); - list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event); + list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event, $instance); $this->assertNull($min); $this->assertNull($max); } @@ -1008,307 +1008,12 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { 'eventtype' => ASSIGN_EVENT_TYPE_GRADINGDUE ]); - list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event); + list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event, $instance); $this->assertEquals($duedate, $min[0]); $this->assertNotEmpty($min[1]); $this->assertNull($max); } - /** - * Calendar events without and instance id should be ignored by the validate - * event function. - */ - public function test_mod_assign_core_calendar_validate_event_timestart_no_instance_id() { - global $CFG; - require_once($CFG->dirroot . '/calendar/lib.php'); - - $this->resetAfterTest(); - $this->setAdminUser(); - - $event = new \calendar_event((object) [ - 'modulename' => 'assign', - 'eventtype' => ASSIGN_EVENT_TYPE_DUE - ]); - - mod_assign_core_calendar_validate_event_timestart($event); - // The function above throws an exception so all we need to do is make sure - // it gets here and that is considered success. - $this->assertTrue(true); - } - - /** - * Calendar events for an unknown instance should throw an exception. - */ - public function test_mod_assign_core_calendar_validate_event_timestart_no_instance_found() { - global $CFG; - require_once($CFG->dirroot . '/calendar/lib.php'); - - $this->resetAfterTest(); - $this->setAdminUser(); - - $event = new \calendar_event((object) [ - 'modulename' => 'assign', - 'instance' => 1234, - 'eventtype' => ASSIGN_EVENT_TYPE_DUE - ]); - - $this->expectException('moodle_exception'); - mod_assign_core_calendar_validate_event_timestart($event); - } - - /** - * Assignments configured without any limits on the due date should not - * throw an exception. - */ - public function test_mod_assign_core_calendar_validate_event_timestart_no_limit() { - global $CFG, $DB; - require_once($CFG->dirroot . '/calendar/lib.php'); - - $this->resetAfterTest(); - $this->setAdminUser(); - - $assign = $this->create_instance([ - 'duedate' => 0, - 'allowsubmissionsfromdate' => 0, - 'cutoffdate' => 0, - ]); - $instance = $assign->get_instance(); - - $event = new \calendar_event((object) [ - 'courseid' => $instance->course, - 'modulename' => 'assign', - 'instance' => $instance->id, - 'eventtype' => ASSIGN_EVENT_TYPE_DUE, - 'timestart' => time() - ]); - - mod_assign_core_calendar_validate_event_timestart($event); - // The function above throws an exception so all we need to do is make sure - // it gets here and that is considered success. - $this->assertTrue(true); - } - - /** - * Due date events with a timestart equal to or greater than the minimum limit - * should not throw an exception. Timestart values below the minimum limit should - * throw an exception. - */ - public function test_mod_assign_core_calendar_validate_due_event_min_limit() { - global $CFG, $DB; - require_once($CFG->dirroot . '/calendar/lib.php'); - - $this->resetAfterTest(); - $this->setAdminUser(); - - $duedate = time(); - $submissionsfromdate = $duedate - DAYSECS; - $cutoffdate = $duedate + DAYSECS; - $assign = $this->create_instance([ - 'duedate' => $duedate, - 'allowsubmissionsfromdate' => $submissionsfromdate, - 'cutoffdate' => $cutoffdate, - ]); - $instance = $assign->get_instance(); - - $event = new \calendar_event((object) [ - 'courseid' => $instance->course, - 'modulename' => 'assign', - 'instance' => $instance->id, - 'eventtype' => ASSIGN_EVENT_TYPE_DUE, - 'timestart' => $submissionsfromdate + 1, - ]); - - // No exception when new time is above minimum cutoff. - mod_assign_core_calendar_validate_event_timestart($event); - $this->assertTrue(true); - - // No exception when new time is equal to minimum cutoff. - $event->timestart = $submissionsfromdate; - mod_assign_core_calendar_validate_event_timestart($event); - $this->assertTrue(true); - - // Exception when new time is earlier than minimum cutoff. - $event->timestart = $submissionsfromdate - 1; - $this->expectException('moodle_exception'); - mod_assign_core_calendar_validate_event_timestart($event); - } - - /** - * A due date event with a timestart less than or equal to the max limit should - * not throw an exception. A timestart greater than the max limit should throw - * an exception. - */ - public function test_mod_assign_core_calendar_validate_due_event_max_limit() { - global $CFG, $DB; - require_once($CFG->dirroot . '/calendar/lib.php'); - - $this->resetAfterTest(); - $this->setAdminUser(); - - $duedate = time(); - $submissionsfromdate = $duedate - DAYSECS; - $cutoffdate = $duedate + DAYSECS; - $assign = $this->create_instance([ - 'duedate' => $duedate, - 'allowsubmissionsfromdate' => $submissionsfromdate, - 'cutoffdate' => $cutoffdate, - ]); - $instance = $assign->get_instance(); - - $event = new \calendar_event((object) [ - 'courseid' => $instance->course, - 'modulename' => 'assign', - 'instance' => $instance->id, - 'eventtype' => ASSIGN_EVENT_TYPE_DUE, - 'timestart' => $cutoffdate - 1, - ]); - - // No exception when new time is below maximum cutoff. - mod_assign_core_calendar_validate_event_timestart($event); - $this->assertTrue(true); - - // No exception when new time is equal to maximum cutoff. - $event->timestart = $cutoffdate; - mod_assign_core_calendar_validate_event_timestart($event); - $this->assertTrue(true); - - // Exception when new time is later than maximum cutoff. - $event->timestart = $submissionsfromdate - 1; - $this->expectException('moodle_exception'); - mod_assign_core_calendar_validate_event_timestart($event); - } - - /** - * Due date override events should not throw an exception. - */ - public function test_mod_assign_core_calendar_validate_due_event_override() { - global $CFG, $DB; - require_once($CFG->dirroot . '/calendar/lib.php'); - - $this->resetAfterTest(); - $this->setAdminUser(); - - $duedate = time(); - $submissionsfromdate = $duedate - DAYSECS; - $cutoffdate = $duedate + DAYSECS; - $assign = $this->create_instance([ - 'duedate' => $duedate, - 'allowsubmissionsfromdate' => $submissionsfromdate, - 'cutoffdate' => $cutoffdate, - ]); - $instance = $assign->get_instance(); - $userid = $this->students[0]->id; - - $event = new \calendar_event((object) [ - 'courseid' => $instance->course, - 'modulename' => 'assign', - 'instance' => $instance->id, - 'userid' => $userid, - 'eventtype' => ASSIGN_EVENT_TYPE_DUE, - 'timestart' => $duedate + (2 * DAYSECS) - ]); - - $record = (object) [ - 'assignid' => $instance->id, - 'userid' => $userid, - 'duedate' => $duedate + (2 * DAYSECS) - ]; - - $DB->insert_record('assign_overrides', $record); - - // No exception when dealing with an override. - mod_assign_core_calendar_validate_event_timestart($event); - $this->assertTrue(true); - } - - /** - * Grading due date event should throw an exception if it's timestart is less than the - * assignment due date. - */ - public function test_mod_assign_core_calendar_validate_gradingdue_event_min_limit_duedate() { - global $CFG, $DB; - require_once($CFG->dirroot . '/calendar/lib.php'); - - $this->resetAfterTest(); - $this->setAdminUser(); - - $duedate = time(); - $submissionsfromdate = $duedate - DAYSECS; - $cutoffdate = $duedate + DAYSECS; - $assign = $this->create_instance([ - 'duedate' => $duedate, - 'allowsubmissionsfromdate' => $submissionsfromdate, - 'cutoffdate' => $cutoffdate, - ]); - $instance = $assign->get_instance(); - - $event = new \calendar_event((object) [ - 'courseid' => $instance->course, - 'modulename' => 'assign', - 'instance' => $instance->id, - 'eventtype' => ASSIGN_EVENT_TYPE_GRADINGDUE, - 'timestart' => $duedate + 1, - ]); - - // No exception when new time is above minimum cutoff. - mod_assign_core_calendar_validate_event_timestart($event); - $this->assertTrue(true); - - // No exception when new time is equal to minimum cutoff. - $event->timestart = $duedate; - mod_assign_core_calendar_validate_event_timestart($event); - $this->assertTrue(true); - - // Exception when new time is earlier than minimum cutoff. - $event->timestart = $duedate - 1; - $this->expectException('moodle_exception'); - mod_assign_core_calendar_validate_event_timestart($event); - } - - /** - * Grading due date event should throw an exception if it's timestart is less than the - * submissions allowed from date if there is no due date set. - */ - public function test_mod_assign_core_calendar_validate_gradingdue_event_min_limit_submissionsfromdate() { - global $CFG, $DB; - require_once($CFG->dirroot . '/calendar/lib.php'); - - $this->resetAfterTest(); - $this->setAdminUser(); - - $duedate = 0; - $submissionsfromdate = time() - DAYSECS; - $cutoffdate = time() + DAYSECS; - $assign = $this->create_instance([ - 'duedate' => $duedate, - 'allowsubmissionsfromdate' => $submissionsfromdate, - 'cutoffdate' => $cutoffdate, - ]); - $instance = $assign->get_instance(); - - $event = new \calendar_event((object) [ - 'courseid' => $instance->course, - 'modulename' => 'assign', - 'instance' => $instance->id, - 'eventtype' => ASSIGN_EVENT_TYPE_GRADINGDUE, - 'timestart' => $submissionsfromdate + 1, - ]); - - // No exception when new time is above minimum cutoff. - mod_assign_core_calendar_validate_event_timestart($event); - $this->assertTrue(true); - - // No exception when new time is equal to minimum cutoff. - $event->timestart = $submissionsfromdate; - mod_assign_core_calendar_validate_event_timestart($event); - $this->assertTrue(true); - - // Exception when new time is earlier than minimum cutoff. - $event->timestart = $submissionsfromdate - 1; - $this->expectException('moodle_exception'); - mod_assign_core_calendar_validate_event_timestart($event); - } - /** * Non due date events should not update the assignment due date. */ @@ -1337,7 +1042,7 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { 'timestart' => $duedate + 1 ]); - mod_assign_core_calendar_event_timestart_updated($event); + mod_assign_core_calendar_event_timestart_updated($event, $instance); $newinstance = $DB->get_record('assign', ['id' => $instance->id]); $this->assertEquals($duedate, $newinstance->duedate); @@ -1381,7 +1086,7 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { $DB->insert_record('assign_overrides', $record); - mod_assign_core_calendar_event_timestart_updated($event); + mod_assign_core_calendar_event_timestart_updated($event, $instance); $newinstance = $DB->get_record('assign', ['id' => $instance->id]); $this->assertEquals($duedate, $newinstance->duedate); @@ -1416,7 +1121,7 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { 'timestart' => $newduedate ]); - mod_assign_core_calendar_event_timestart_updated($event); + mod_assign_core_calendar_event_timestart_updated($event, $instance); $newinstance = $DB->get_record('assign', ['id' => $instance->id]); $this->assertEquals($newduedate, $newinstance->duedate); diff --git a/mod/choice/lib.php b/mod/choice/lib.php index 8f8513231a8..cebacfa79aa 100644 --- a/mod/choice/lib.php +++ b/mod/choice/lib.php @@ -1258,15 +1258,9 @@ function mod_choice_core_calendar_provide_event_action(calendar_event $event, * ] * * @param calendar_event $event The calendar event to get the time range for - * @param stdClass|null $instance The module instance to get the range from + * @param stdClass $choice The module instance to get the range from */ -function mod_choice_core_calendar_get_valid_event_timestart_range(\calendar_event $event, \stdClass $choice = null) { - global $DB; - - if (!$choice) { - $choice = $DB->get_record('choice', ['id' => $event->instance]); - } - +function mod_choice_core_calendar_get_valid_event_timestart_range(\calendar_event $event, \stdClass $choice) { $mindate = null; $maxdate = null; @@ -1289,38 +1283,6 @@ function mod_choice_core_calendar_get_valid_event_timestart_range(\calendar_even return [$mindate, $maxdate]; } -/** - * This function will check that the given event is valid for it's - * corresponding choice module. - * - * An exception is thrown if the event fails validation. - * - * @throws \moodle_exception - * @param \calendar_event $event - * @return bool - */ -function mod_choice_core_calendar_validate_event_timestart(\calendar_event $event) { - global $DB; - - if (!isset($event->instance)) { - return; - } - - // We need to read from the DB directly because course module may - // currently be getting created so it won't be in mod info yet. - $instance = $DB->get_record('choice', ['id' => $event->instance], '*', MUST_EXIST); - $timestart = $event->timestart; - list($min, $max) = mod_choice_core_calendar_get_valid_event_timestart_range($event, $instance); - - if ($min && $timestart < $min[0]) { - throw new \moodle_exception($min[1]); - } - - if ($max && $timestart > $max[0]) { - throw new \moodle_exception($max[1]); - } -} - /** * This function will update the choice module according to the * event that has been modified. @@ -1330,10 +1292,15 @@ function mod_choice_core_calendar_validate_event_timestart(\calendar_event $even * * @throws \moodle_exception * @param \calendar_event $event + * @param stdClass $choice The module instance to get the range from */ -function mod_choice_core_calendar_event_timestart_updated(\calendar_event $event) { +function mod_choice_core_calendar_event_timestart_updated(\calendar_event $event, \stdClass $choice) { global $DB; + if (!in_array($event->eventtype, [CHOICE_EVENT_TYPE_OPEN, CHOICE_EVENT_TYPE_CLOSE])) { + return; + } + $courseid = $event->courseid; $modulename = $event->modulename; $instanceid = $event->instance; @@ -1345,6 +1312,10 @@ function mod_choice_core_calendar_event_timestart_updated(\calendar_event $event return; } + if ($choice->id != $instanceid) { + return; + } + $coursemodule = get_fast_modinfo($courseid)->instances[$modulename][$instanceid]; $context = context_module::instance($coursemodule->id); @@ -1357,29 +1328,24 @@ function mod_choice_core_calendar_event_timestart_updated(\calendar_event $event // If the event is for the choice activity opening then we should // set the start time of the choice activity to be the new start // time of the event. - $record = $DB->get_record('choice', ['id' => $instanceid], '*', MUST_EXIST); - - if ($record->timeopen != $event->timestart) { - $record->timeopen = $event->timestart; - $record->timemodified = time(); + if ($choice->timeopen != $event->timestart) { + $choice->timeopen = $event->timestart; $modified = true; } } else if ($event->eventtype == CHOICE_EVENT_TYPE_CLOSE) { // If the event is for the choice activity closing then we should // set the end time of the choice activity to be the new start // time of the event. - $record = $DB->get_record('choice', ['id' => $instanceid], '*', MUST_EXIST); - - if ($record->timeclose != $event->timestart) { - $record->timeclose = $event->timestart; - $record->timemodified = time(); + if ($choice->timeclose != $event->timestart) { + $choice->timeclose = $event->timestart; $modified = true; } } if ($modified) { + $choice->timemodified = time(); // Persist the instance changes. - $DB->update_record('choice', $record); + $DB->update_record('choice', $choice); $event = \core\event\course_module_updated::create_from_cm($coursemodule, $context); $event->trigger(); } diff --git a/mod/choice/tests/lib_test.php b/mod/choice/tests/lib_test.php index 92ee1bf316e..9f10a7436cc 100644 --- a/mod/choice/tests/lib_test.php +++ b/mod/choice/tests/lib_test.php @@ -489,196 +489,6 @@ class mod_choice_lib_testcase extends externallib_advanced_testcase { $this->assertEquals(mod_choice_get_completion_active_rule_descriptions(new stdClass()), []); } - /** - * You can't create a choice module event when the module doesn't exist. - */ - public function test_mod_choice_core_calendar_validate_event_timestart_no_activity() { - global $CFG; - require_once($CFG->dirroot . "/calendar/lib.php"); - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - - $event = new \calendar_event([ - 'name' => 'Test event', - 'description' => '', - 'format' => 1, - 'courseid' => $course->id, - 'groupid' => 0, - 'userid' => 2, - 'modulename' => 'choice', - 'instance' => 1234, - 'eventtype' => CHOICE_EVENT_TYPE_OPEN, - 'timestart' => time(), - 'timeduration' => 86400, - 'visible' => 1 - ]); - - $this->expectException('moodle_exception'); - mod_choice_core_calendar_validate_event_timestart($event); - } - - /** - * A CHOICE_EVENT_TYPE_OPEN must be before the close time of the choice activity. - */ - public function test_mod_choice_core_calendar_validate_event_timestart_valid_open_event() { - global $CFG, $DB; - require_once($CFG->dirroot . "/calendar/lib.php"); - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - $choicegenerator = $generator->get_plugin_generator('mod_choice'); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $choice = $choicegenerator->create_instance(['course' => $course->id]); - $choice->timeopen = $timeopen; - $choice->timeclose = $timeclose; - $DB->update_record('choice', $choice); - - $event = new \calendar_event([ - 'name' => 'Test event', - 'description' => '', - 'format' => 1, - 'courseid' => $course->id, - 'groupid' => 0, - 'userid' => 2, - 'modulename' => 'choice', - 'instance' => $choice->id, - 'eventtype' => CHOICE_EVENT_TYPE_OPEN, - 'timestart' => $timeopen, - 'timeduration' => 86400, - 'visible' => 1 - ]); - - mod_choice_core_calendar_validate_event_timestart($event); - // The function above will throw an exception if the event is - // invalid. - $this->assertTrue(true); - } - - /** - * A CHOICE_EVENT_TYPE_OPEN can not have a start time set after the close time - * of the choice activity. - */ - public function test_mod_choice_core_calendar_validate_event_timestart_invalid_open_event() { - global $CFG, $DB; - require_once($CFG->dirroot . "/calendar/lib.php"); - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - $choicegenerator = $generator->get_plugin_generator('mod_choice'); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $choice = $choicegenerator->create_instance(['course' => $course->id]); - $choice->timeopen = $timeopen; - $choice->timeclose = $timeclose; - $DB->update_record('choice', $choice); - - $event = new \calendar_event([ - 'name' => 'Test event', - 'description' => '', - 'format' => 1, - 'courseid' => $course->id, - 'groupid' => 0, - 'userid' => 2, - 'modulename' => 'choice', - 'instance' => $choice->id, - 'eventtype' => CHOICE_EVENT_TYPE_OPEN, - 'timestart' => $timeclose + 1, - 'timeduration' => 86400, - 'visible' => 1 - ]); - - $this->expectException('moodle_exception'); - mod_choice_core_calendar_validate_event_timestart($event); - } - - /** - * A CHOICE_EVENT_TYPE_CLOSE must be after the open time of the choice activity. - */ - public function test_mod_choice_core_calendar_validate_event_timestart_valid_close_event() { - global $CFG, $DB; - require_once($CFG->dirroot . "/calendar/lib.php"); - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - $choicegenerator = $generator->get_plugin_generator('mod_choice'); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $choice = $choicegenerator->create_instance(['course' => $course->id]); - $choice->timeopen = $timeopen; - $choice->timeclose = $timeclose; - $DB->update_record('choice', $choice); - - $event = new \calendar_event([ - 'name' => 'Test event', - 'description' => '', - 'format' => 1, - 'courseid' => $course->id, - 'groupid' => 0, - 'userid' => 2, - 'modulename' => 'choice', - 'instance' => $choice->id, - 'eventtype' => CHOICE_EVENT_TYPE_CLOSE, - 'timestart' => $timeclose, - 'timeduration' => 86400, - 'visible' => 1 - ]); - - mod_choice_core_calendar_validate_event_timestart($event); - // The function above will throw an exception if the event isn't - // valid. - $this->assertTrue(true); - } - - /** - * A CHOICE_EVENT_TYPE_CLOSE can not have a start time set before the open time - * of the choice activity. - */ - public function test_mod_choice_core_calendar_validate_event_timestart_invalid_close_event() { - global $CFG, $DB; - require_once($CFG->dirroot . "/calendar/lib.php"); - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - $choicegenerator = $generator->get_plugin_generator('mod_choice'); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $choice = $choicegenerator->create_instance(['course' => $course->id]); - $choice->timeopen = $timeopen; - $choice->timeclose = $timeclose; - $DB->update_record('choice', $choice); - - // Create a valid event. - $event = new \calendar_event([ - 'name' => 'Test event', - 'description' => '', - 'format' => 1, - 'courseid' => $course->id, - 'groupid' => 0, - 'userid' => 2, - 'modulename' => 'choice', - 'instance' => $choice->id, - 'eventtype' => CHOICE_EVENT_TYPE_CLOSE, - 'timestart' => $timeopen - 1, - 'timeduration' => 86400, - 'visible' => 1 - ]); - - $this->expectException('moodle_exception'); - mod_choice_core_calendar_validate_event_timestart($event); - } - /** * An unkown event type should not change the choice instance. */ @@ -714,7 +524,7 @@ class mod_choice_lib_testcase extends externallib_advanced_testcase { 'visible' => 1 ]); - mod_choice_core_calendar_event_timestart_updated($event); + mod_choice_core_calendar_event_timestart_updated($event, $choice); $choice = $DB->get_record('choice', ['id' => $choice->id]); $this->assertEquals($timeopen, $choice->timeopen); @@ -763,7 +573,7 @@ class mod_choice_lib_testcase extends externallib_advanced_testcase { // Trigger and capture the event when adding a contact. $sink = $this->redirectEvents(); - mod_choice_core_calendar_event_timestart_updated($event); + mod_choice_core_calendar_event_timestart_updated($event, $choice); $triggeredevents = $sink->get_events(); $moduleupdatedevents = array_filter($triggeredevents, function($e) { @@ -824,7 +634,7 @@ class mod_choice_lib_testcase extends externallib_advanced_testcase { // Trigger and capture the event when adding a contact. $sink = $this->redirectEvents(); - mod_choice_core_calendar_event_timestart_updated($event); + mod_choice_core_calendar_event_timestart_updated($event, $choice); $triggeredevents = $sink->get_events(); $moduleupdatedevents = array_filter($triggeredevents, function($e) { diff --git a/mod/feedback/lib.php b/mod/feedback/lib.php index c87bbca0fb9..724647a51ec 100644 --- a/mod/feedback/lib.php +++ b/mod/feedback/lib.php @@ -3575,16 +3575,10 @@ function mod_feedback_get_completion_active_rule_descriptions($cm) { * ] * * @param calendar_event $event The calendar event to get the time range for - * @param stdClass|null $instance The module instance to get the range from + * @param stdClass $instance The module instance to get the range from * @return array */ -function mod_feedback_core_calendar_get_valid_event_timestart_range(\calendar_event $event, \stdClass $instance = null) { - global $DB; - - if (!$instance) { - $instance = $DB->get_record('feedback', ['id' => $event->instance], '*', MUST_EXIST); - } - +function mod_feedback_core_calendar_get_valid_event_timestart_range(\calendar_event $event, \stdClass $instance) { $mindate = null; $maxdate = null; @@ -3611,32 +3605,6 @@ function mod_feedback_core_calendar_get_valid_event_timestart_range(\calendar_ev return [$mindate, $maxdate]; } -/** - * This function will check that the given event is valid for it's - * corresponding feedback module. - * - * An exception is thrown if the event fails validation. - * - * @throws \moodle_exception - * @param \calendar_event $event - */ -function mod_feedback_core_calendar_validate_event_timestart(\calendar_event $event) { - global $DB; - - $record = $DB->get_record('feedback', ['id' => $event->instance], '*', MUST_EXIST); - $timestart = $event->timestart; - - list($min, $max) = mod_feedback_core_calendar_get_valid_event_timestart_range($event, $record); - - if ($min && $timestart < $min[0]) { - throw new \moodle_exception($min[1]); - } - - if ($max && $timestart > $max[0]) { - throw new \moodle_exception($max[1]); - } -} - /** * This function will update the feedback module according to the * event that has been modified. @@ -3646,27 +3614,29 @@ function mod_feedback_core_calendar_validate_event_timestart(\calendar_event $ev * * @throws \moodle_exception * @param \calendar_event $event + * @param stdClass $feedback The module instance to get the range from */ -function mod_feedback_core_calendar_event_timestart_updated(\calendar_event $event) { +function mod_feedback_core_calendar_event_timestart_updated(\calendar_event $event, \stdClass $feedback) { global $CFG, $DB; if (empty($event->instance) || $event->modulename != 'feedback') { return; } - $coursemodule = get_coursemodule_from_instance('feedback', - $event->instance, - $event->courseid, - false, - MUST_EXIST); - - if (empty($coursemodule)) { - // If we don't have a course module yet then it likely means - // the activity is still being set up. In this case there is - // nothing for us to do anyway. + if ($event->instance != $feedback->id) { return; } + if (!in_array($event->eventtype, [FEEDBACK_EVENT_TYPE_OPEN, FEEDBACK_EVENT_TYPE_CLOSE])) { + return; + } + + $courseid = $event->courseid; + $modulename = $event->modulename; + $instanceid = $event->instance; + $modified = false; + + $coursemodule = get_fast_modinfo($courseid)->instances[$modulename][$instanceid]; $context = context_module::instance($coursemodule->id); // The user does not have the capability to modify this activity. @@ -3674,34 +3644,28 @@ function mod_feedback_core_calendar_event_timestart_updated(\calendar_event $eve return; } - $modified = false; - if ($event->eventtype == FEEDBACK_EVENT_TYPE_OPEN) { // If the event is for the feedback activity opening then we should // set the start time of the feedback activity to be the new start // time of the event. - $record = $DB->get_record('feedback', ['id' => $event->instance], '*', MUST_EXIST); - - if ($record->timeopen != $event->timestart) { - $record->timeopen = $event->timestart; - $record->timemodified = time(); + if ($feedback->timeopen != $event->timestart) { + $feedback->timeopen = $event->timestart; + $feedback->timemodified = time(); $modified = true; } } else if ($event->eventtype == FEEDBACK_EVENT_TYPE_CLOSE) { // If the event is for the feedback activity closing then we should // set the end time of the feedback activity to be the new start // time of the event. - $record = $DB->get_record('feedback', ['id' => $event->instance], '*', MUST_EXIST); - - if ($record->timeclose != $event->timestart) { - $record->timeclose = $event->timestart; - $record->timemodified = time(); + if ($feedback->timeclose != $event->timestart) { + $feedback->timeclose = $event->timestart; $modified = true; } } if ($modified) { - $DB->update_record('feedback', $record); + $feedback->timemodified = time(); + $DB->update_record('feedback', $feedback); $event = \core\event\course_module_updated::create_from_cm($coursemodule, $context); $event->trigger(); } diff --git a/mod/feedback/tests/lib_test.php b/mod/feedback/tests/lib_test.php index b0b7c84a75c..37cbf30e7b2 100644 --- a/mod/feedback/tests/lib_test.php +++ b/mod/feedback/tests/lib_test.php @@ -415,7 +415,7 @@ class mod_feedback_lib_testcase extends advanced_testcase { 'visible' => 1 ]); - list($min, $max) = mod_feedback_core_calendar_get_valid_event_timestart_range($event); + list($min, $max) = mod_feedback_core_calendar_get_valid_event_timestart_range($event, $feedback); $this->assertNull($min); $this->assertNull($max); } @@ -455,7 +455,7 @@ class mod_feedback_lib_testcase extends advanced_testcase { 'visible' => 1 ]); - list($min, $max) = mod_feedback_core_calendar_get_valid_event_timestart_range($event); + list($min, $max) = mod_feedback_core_calendar_get_valid_event_timestart_range($event, $feedback); $this->assertNull($min); $this->assertEquals($timeclose, $max[0]); $this->assertNotEmpty($max[1]); @@ -496,7 +496,7 @@ class mod_feedback_lib_testcase extends advanced_testcase { 'visible' => 1 ]); - list($min, $max) = mod_feedback_core_calendar_get_valid_event_timestart_range($event); + list($min, $max) = mod_feedback_core_calendar_get_valid_event_timestart_range($event, $feedback); $this->assertNull($min); $this->assertNull($max); } @@ -536,7 +536,7 @@ class mod_feedback_lib_testcase extends advanced_testcase { 'visible' => 1 ]); - list($min, $max) = mod_feedback_core_calendar_get_valid_event_timestart_range($event); + list($min, $max) = mod_feedback_core_calendar_get_valid_event_timestart_range($event, $feedback); $this->assertEquals($timeopen, $min[0]); $this->assertNotEmpty($min[1]); $this->assertNull($max); @@ -577,199 +577,11 @@ class mod_feedback_lib_testcase extends advanced_testcase { 'visible' => 1 ]); - list($min, $max) = mod_feedback_core_calendar_get_valid_event_timestart_range($event); + list($min, $max) = mod_feedback_core_calendar_get_valid_event_timestart_range($event, $feedback); $this->assertNull($min); $this->assertNull($max); } - /** - * You can't create a feedback module event when the module doesn't exist. - */ - public function test_mod_feedback_core_calendar_validate_event_timestart_no_activity() { - global $CFG; - require_once($CFG->dirroot . "/calendar/lib.php"); - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - - $event = new \calendar_event([ - 'name' => 'Test event', - 'description' => '', - 'format' => 1, - 'courseid' => $course->id, - 'groupid' => 0, - 'userid' => 2, - 'modulename' => 'feedback', - 'instance' => 1234, - 'eventtype' => FEEDBACK_EVENT_TYPE_OPEN, - 'timestart' => time(), - 'timeduration' => 86400, - 'visible' => 1 - ]); - - $this->expectException('moodle_exception'); - mod_feedback_core_calendar_validate_event_timestart($event); - } - - /** - * A FEEDBACK_EVENT_TYPE_OPEN must be before the close time of the feedback activity. - */ - public function test_mod_feedback_core_calendar_validate_event_timestart_valid_open_event() { - global $CFG, $DB; - require_once($CFG->dirroot . "/calendar/lib.php"); - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - $feedbackgenerator = $generator->get_plugin_generator('mod_feedback'); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $feedback = $feedbackgenerator->create_instance(['course' => $course->id]); - $feedback->timeopen = $timeopen; - $feedback->timeclose = $timeclose; - $DB->update_record('feedback', $feedback); - - $event = new \calendar_event([ - 'name' => 'Test event', - 'description' => '', - 'format' => 1, - 'courseid' => $course->id, - 'groupid' => 0, - 'userid' => 2, - 'modulename' => 'feedback', - 'instance' => $feedback->id, - 'eventtype' => FEEDBACK_EVENT_TYPE_OPEN, - 'timestart' => $timeopen, - 'timeduration' => 86400, - 'visible' => 1 - ]); - - // This will throw an exception if the event is invald. - mod_feedback_core_calendar_validate_event_timestart($event); - $this->assertTrue(true); - } - - /** - * A FEEDBACK_EVENT_TYPE_OPEN can not have a start time set after the close time - * of the feedback activity. - */ - public function test_mod_feedback_core_calendar_validate_event_timestart_invalid_open_event() { - global $CFG, $DB; - require_once($CFG->dirroot . "/calendar/lib.php"); - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - $feedbackgenerator = $generator->get_plugin_generator('mod_feedback'); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $feedback = $feedbackgenerator->create_instance(['course' => $course->id]); - $feedback->timeopen = $timeopen; - $feedback->timeclose = $timeclose; - $DB->update_record('feedback', $feedback); - - $event = new \calendar_event([ - 'name' => 'Test event', - 'description' => '', - 'format' => 1, - 'courseid' => $course->id, - 'groupid' => 0, - 'userid' => 2, - 'modulename' => 'feedback', - 'instance' => $feedback->id, - 'eventtype' => FEEDBACK_EVENT_TYPE_OPEN, - 'timestart' => $timeclose + 1, - 'timeduration' => 86400, - 'visible' => 1 - ]); - - $this->expectException('moodle_exception'); - mod_feedback_core_calendar_validate_event_timestart($event); - } - - /** - * A FEEDBACK_EVENT_TYPE_CLOSE must be after the open time of the feedback activity. - */ - public function test_mod_feedback_core_calendar_validate_event_timestart_valid_close_event() { - global $CFG, $DB; - require_once($CFG->dirroot . "/calendar/lib.php"); - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - $feedbackgenerator = $generator->get_plugin_generator('mod_feedback'); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $feedback = $feedbackgenerator->create_instance(['course' => $course->id]); - $feedback->timeopen = $timeopen; - $feedback->timeclose = $timeclose; - $DB->update_record('feedback', $feedback); - - $event = new \calendar_event([ - 'name' => 'Test event', - 'description' => '', - 'format' => 1, - 'courseid' => $course->id, - 'groupid' => 0, - 'userid' => 2, - 'modulename' => 'feedback', - 'instance' => $feedback->id, - 'eventtype' => FEEDBACK_EVENT_TYPE_CLOSE, - 'timestart' => $timeclose, - 'timeduration' => 86400, - 'visible' => 1 - ]); - - // This will throw an exception if the event is invald. - mod_feedback_core_calendar_validate_event_timestart($event); - $this->assertTrue(true); - } - - /** - * A FEEDBACK_EVENT_TYPE_CLOSE can not have a start time set before the open time - * of the feedback activity. - */ - public function test_mod_feedback_core_calendar_validate_event_timestart_invalid_close_event() { - global $CFG, $DB; - require_once($CFG->dirroot . "/calendar/lib.php"); - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - $feedbackgenerator = $generator->get_plugin_generator('mod_feedback'); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $feedback = $feedbackgenerator->create_instance(['course' => $course->id]); - $feedback->timeopen = $timeopen; - $feedback->timeclose = $timeclose; - $DB->update_record('feedback', $feedback); - - // Create a valid event. - $event = new \calendar_event([ - 'name' => 'Test event', - 'description' => '', - 'format' => 1, - 'courseid' => $course->id, - 'groupid' => 0, - 'userid' => 2, - 'modulename' => 'feedback', - 'instance' => $feedback->id, - 'eventtype' => FEEDBACK_EVENT_TYPE_CLOSE, - 'timestart' => $timeopen - 1, - 'timeduration' => 86400, - 'visible' => 1 - ]); - - $this->expectException('moodle_exception'); - mod_feedback_core_calendar_validate_event_timestart($event); - } - /** * An unkown event type should not change the feedback instance. */ @@ -805,7 +617,7 @@ class mod_feedback_lib_testcase extends advanced_testcase { 'visible' => 1 ]); - mod_feedback_core_calendar_event_timestart_updated($event); + mod_feedback_core_calendar_event_timestart_updated($event, $feedback); $feedback = $DB->get_record('feedback', ['id' => $feedback->id]); $this->assertEquals($timeopen, $feedback->timeopen); @@ -851,7 +663,7 @@ class mod_feedback_lib_testcase extends advanced_testcase { 'visible' => 1 ]); - mod_feedback_core_calendar_event_timestart_updated($event); + mod_feedback_core_calendar_event_timestart_updated($event, $feedback); $feedback = $DB->get_record('feedback', ['id' => $feedback->id]); // Ensure the timeopen property matches the event timestart. @@ -901,7 +713,7 @@ class mod_feedback_lib_testcase extends advanced_testcase { 'visible' => 1 ]); - mod_feedback_core_calendar_event_timestart_updated($event); + mod_feedback_core_calendar_event_timestart_updated($event, $feedback); $feedback = $DB->get_record('feedback', ['id' => $feedback->id]); // Ensure the timeclose property matches the event timestart. @@ -964,7 +776,7 @@ class mod_feedback_lib_testcase extends advanced_testcase { $this->setUser($user); - mod_feedback_core_calendar_event_timestart_updated($event); + mod_feedback_core_calendar_event_timestart_updated($event, $feedback); $newfeedback = $DB->get_record('feedback', ['id' => $feedback->id]); // The activity shouldn't have been updated because the user @@ -1025,7 +837,7 @@ class mod_feedback_lib_testcase extends advanced_testcase { $sink = $this->redirectEvents(); - mod_feedback_core_calendar_event_timestart_updated($event); + mod_feedback_core_calendar_event_timestart_updated($event, $feedback); $triggeredevents = $sink->get_events(); $moduleupdatedevents = array_filter($triggeredevents, function($e) { diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index 4ee34de8e63..49aef8d3a34 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -2270,11 +2270,12 @@ function mod_quiz_get_completion_active_rule_descriptions($cm) { * [1506741172, 'The date must be before this date'] * ] * + * @throws \moodle_exception * @param \calendar_event $event The calendar event to get the time range for - * @param stdClass|null $quiz The module instance to get the range from + * @param stdClass $quiz The module instance to get the range from * @return array */ -function mod_quiz_core_calendar_get_valid_event_timestart_range(\calendar_event $event, \stdClass $quiz = null) { +function mod_quiz_core_calendar_get_valid_event_timestart_range(\calendar_event $event, \stdClass $quiz) { global $CFG, $DB; require_once($CFG->dirroot . '/mod/quiz/locallib.php'); @@ -2283,10 +2284,6 @@ function mod_quiz_core_calendar_get_valid_event_timestart_range(\calendar_event return [null, null]; } - if (!$quiz) { - $quiz = $DB->get_record('quiz', ['id' => $event->instance]); - } - $mindate = null; $maxdate = null; @@ -2309,44 +2306,6 @@ function mod_quiz_core_calendar_get_valid_event_timestart_range(\calendar_event return [$mindate, $maxdate]; } -/** - * This function will check that the given event is valid for it's - * corresponding quiz module. - * - * An exception is thrown if the event fails validation. - * - * @throws \moodle_exception - * @param \calendar_event $event - * @return bool - */ -function mod_quiz_core_calendar_validate_event_timestart(\calendar_event $event) { - global $DB; - - if (!isset($event->instance)) { - return; - } - - // Something weird going on. The event is for a different module so - // we should ignore it. - if ($event->modulename != 'quiz') { - return; - } - - // We need to read from the DB directly because course module may - // currently be getting created so it won't be in mod info yet. - $quiz = $DB->get_record('quiz', ['id' => $event->instance], '*', MUST_EXIST); - $timestart = $event->timestart; - list($min, $max) = mod_quiz_core_calendar_get_valid_event_timestart_range($event, $quiz); - - if ($min && $timestart < $min[0]) { - throw new \moodle_exception($min[1]); - } - - if ($max && $timestart > $max[0]) { - throw new \moodle_exception($max[1]); - } -} - /** * This function will update the quiz module according to the * event that has been modified. @@ -2355,15 +2314,15 @@ function mod_quiz_core_calendar_validate_event_timestart(\calendar_event $event) * according to the type of event provided. * * @throws \moodle_exception - * @param \calendar_event $event + * @param \calendar_event $event A quiz activity calendar event + * @param \stdClass $quiz A quiz activity instance */ -function mod_quiz_core_calendar_event_timestart_updated(\calendar_event $event) { +function mod_quiz_core_calendar_event_timestart_updated(\calendar_event $event, \stdClass $quiz) { global $CFG, $DB; require_once($CFG->dirroot . '/mod/quiz/locallib.php'); - // We don't update the activity if it's an override event that has - // been modified. - if (quiz_is_overriden_calendar_event($event)) { + if (!in_array($event->eventtype, [QUIZ_EVENT_TYPE_OPEN, QUIZ_EVENT_TYPE_CLOSE])) { + // This isn't an event that we care about so we can ignore it. return; } @@ -2379,6 +2338,18 @@ function mod_quiz_core_calendar_event_timestart_updated(\calendar_event $event) return; } + if ($quiz->id != $instanceid) { + // The provided quiz instance doesn't match the event so + // there is nothing to do here. + return; + } + + // We don't update the activity if it's an override event that has + // been modified. + if (quiz_is_overriden_calendar_event($event)) { + return; + } + $coursemodule = get_fast_modinfo($courseid)->instances[$modulename][$instanceid]; $context = context_module::instance($coursemodule->id); @@ -2391,8 +2362,6 @@ function mod_quiz_core_calendar_event_timestart_updated(\calendar_event $event) // If the event is for the quiz activity opening then we should // set the start time of the quiz activity to be the new start // time of the event. - $quiz = $DB->get_record('quiz', ['id' => $instanceid], '*', MUST_EXIST); - if ($quiz->timeopen != $event->timestart) { $quiz->timeopen = $event->timestart; $modified = true; @@ -2401,8 +2370,6 @@ function mod_quiz_core_calendar_event_timestart_updated(\calendar_event $event) // If the event is for the quiz activity closing then we should // set the end time of the quiz activity to be the new start // time of the event. - $quiz = $DB->get_record('quiz', ['id' => $instanceid], '*', MUST_EXIST); - if ($quiz->timeclose != $event->timestart) { $quiz->timeclose = $event->timestart; $modified = true; diff --git a/mod/quiz/tests/calendar_event_modified_test.php b/mod/quiz/tests/calendar_event_modified_test.php index ac36dd5b203..e514e047a60 100644 --- a/mod/quiz/tests/calendar_event_modified_test.php +++ b/mod/quiz/tests/calendar_event_modified_test.php @@ -94,122 +94,6 @@ class mod_quiz_calendar_event_modified_testcase extends advanced_testcase { return new \calendar_event(array_merge($defaultproperties, $eventproperties)); } - /** - * You can't create a quiz module event when the module doesn't exist. - */ - public function test_mod_quiz_core_calendar_validate_event_timestart_no_activity() { - global $CFG; - - $this->resetAfterTest(true); - $this->setAdminUser(); - $generator = $this->getDataGenerator(); - $course = $generator->create_course(); - - $event = new \calendar_event([ - 'name' => 'Test event', - 'description' => '', - 'format' => 1, - 'courseid' => $course->id, - 'groupid' => 0, - 'userid' => 2, - 'modulename' => 'quiz', - 'instance' => 1234, - 'eventtype' => QUIZ_EVENT_TYPE_OPEN, - 'timestart' => time(), - 'timeduration' => 86400, - 'visible' => 1 - ]); - - $this->expectException('moodle_exception'); - mod_quiz_core_calendar_validate_event_timestart($event); - } - - /** - * A QUIZ_EVENT_TYPE_OPEN must be before the close time of the quiz activity. - */ - public function test_mod_quiz_core_calendar_validate_event_timestart_valid_open_event() { - global $DB; - - $this->resetAfterTest(true); - $this->setAdminUser(); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $quiz = $this->create_quiz_instance(['timeopen' => $timeopen, 'timeclose' => $timeclose]); - $event = $this->create_quiz_calendar_event($quiz, [ - 'eventtype' => QUIZ_EVENT_TYPE_OPEN, - 'timestart' => $timeopen - ]); - - mod_quiz_core_calendar_validate_event_timestart($event); - // The function above will throw an exception if the event is - // invalid. - $this->assertTrue(true); - } - - /** - * A QUIZ_EVENT_TYPE_OPEN can not have a start time set after the close time - * of the quiz activity. - */ - public function test_mod_quiz_core_calendar_validate_event_timestart_invalid_open_event() { - global $DB; - - $this->resetAfterTest(true); - $this->setAdminUser(); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $quiz = $this->create_quiz_instance(['timeopen' => $timeopen, 'timeclose' => $timeclose]); - $event = $this->create_quiz_calendar_event($quiz, [ - 'eventtype' => QUIZ_EVENT_TYPE_OPEN, - 'timestart' => $timeclose + 1 - ]); - - $this->expectException('moodle_exception'); - mod_quiz_core_calendar_validate_event_timestart($event); - } - - /** - * A QUIZ_EVENT_TYPE_CLOSE must be after the open time of the quiz activity. - */ - public function test_mod_quiz_core_calendar_validate_event_timestart_valid_close_event() { - global $DB; - - $this->resetAfterTest(true); - $this->setAdminUser(); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $quiz = $this->create_quiz_instance(['timeopen' => $timeopen, 'timeclose' => $timeclose]); - $event = $this->create_quiz_calendar_event($quiz, [ - 'eventtype' => QUIZ_EVENT_TYPE_OPEN, - 'timestart' => $timeclose - ]); - - mod_quiz_core_calendar_validate_event_timestart($event); - // The function above will throw an exception if the event isn't - // valid. - $this->assertTrue(true); - } - - /** - * A QUIZ_EVENT_TYPE_CLOSE can not have a start time set before the open time - * of the quiz activity. - */ - public function test_mod_quiz_core_calendar_validate_event_timestart_invalid_close_event() { - global $DB; - - $this->resetAfterTest(true); - $this->setAdminUser(); - $timeopen = time(); - $timeclose = $timeopen + DAYSECS; - $quiz = $this->create_quiz_instance(['timeopen' => $timeopen, 'timeclose' => $timeclose]); - $event = $this->create_quiz_calendar_event($quiz, [ - 'eventtype' => QUIZ_EVENT_TYPE_CLOSE, - 'timestart' => $timeopen - 1 - ]); - - $this->expectException('moodle_exception'); - mod_quiz_core_calendar_validate_event_timestart($event); - } - /** * An unkown event type should not change the quiz instance. */ @@ -226,7 +110,7 @@ class mod_quiz_calendar_event_modified_testcase extends advanced_testcase { 'timestart' => 1 ]); - mod_quiz_core_calendar_event_timestart_updated($event); + mod_quiz_core_calendar_event_timestart_updated($event, $quiz); $quiz = $DB->get_record('quiz', ['id' => $quiz->id]); $this->assertEquals($timeopen, $quiz->timeopen); @@ -256,7 +140,7 @@ class mod_quiz_calendar_event_modified_testcase extends advanced_testcase { 'timestart' => $newtimeopen ]); - mod_quiz_core_calendar_event_timestart_updated($event); + mod_quiz_core_calendar_event_timestart_updated($event, $quiz); $quiz = $DB->get_record('quiz', ['id' => $quiz->id]); // Ensure the timeopen property matches the event timestart. @@ -290,7 +174,7 @@ class mod_quiz_calendar_event_modified_testcase extends advanced_testcase { 'timestart' => $newtimeclose ]); - mod_quiz_core_calendar_event_timestart_updated($event); + mod_quiz_core_calendar_event_timestart_updated($event, $quiz); $quiz = $DB->get_record('quiz', ['id' => $quiz->id]); // Ensure the timeclose property matches the event timestart. @@ -332,7 +216,7 @@ class mod_quiz_calendar_event_modified_testcase extends advanced_testcase { $DB->insert_record('quiz_overrides', $record); - mod_quiz_core_calendar_event_timestart_updated($event); + mod_quiz_core_calendar_event_timestart_updated($event, $quiz); $quiz = $DB->get_record('quiz', ['id' => $quiz->id]); // Ensure the timeopen property doesn't change. @@ -379,7 +263,7 @@ class mod_quiz_calendar_event_modified_testcase extends advanced_testcase { $this->setUser($user); - mod_quiz_core_calendar_event_timestart_updated($event); + mod_quiz_core_calendar_event_timestart_updated($event, $quiz); $newquiz = $DB->get_record('quiz', ['id' => $quiz->id]); // The time open shouldn't have changed even though we updated the calendar @@ -426,7 +310,7 @@ class mod_quiz_calendar_event_modified_testcase extends advanced_testcase { // Trigger and capture the event. $sink = $this->redirectEvents(); - mod_quiz_core_calendar_event_timestart_updated($event); + mod_quiz_core_calendar_event_timestart_updated($event, $quiz); $triggeredevents = $sink->get_events(); $moduleupdatedevents = array_filter($triggeredevents, function($e) { @@ -625,7 +509,7 @@ class mod_quiz_calendar_event_modified_testcase extends advanced_testcase { $this->setUser($teacher); - mod_quiz_core_calendar_event_timestart_updated($event); + mod_quiz_core_calendar_event_timestart_updated($event, $quiz); $quiz = $DB->get_record('quiz', ['id' => $quiz->id]); $attempt = $DB->get_record('quiz_attempts', ['id' => $attemptid]); From 6688ae2ba20e93bed4b781650758b9630de215b1 Mon Sep 17 00:00:00 2001 From: Ryan Wyllie Date: Tue, 7 Nov 2017 05:47:18 +0000 Subject: [PATCH 3/3] MDL-60062 calendar: prevent drag and drop of override events --- .../external/calendar_event_exporter.php | 18 +++++++- calendar/classes/local/api.php | 8 +++- calendar/templates/month_detailed.mustache | 4 +- calendar/tests/local_api_test.php | 46 +++++++++++++++++++ mod/assign/lib.php | 4 ++ mod/assign/locallib.php | 9 +++- mod/assign/tests/lib_test.php | 4 +- mod/quiz/lib.php | 7 ++- .../tests/calendar_event_modified_test.php | 4 +- 9 files changed, 92 insertions(+), 12 deletions(-) diff --git a/calendar/classes/external/calendar_event_exporter.php b/calendar/classes/external/calendar_event_exporter.php index f3dd9ffd2ca..ad97c962745 100644 --- a/calendar/classes/external/calendar_event_exporter.php +++ b/calendar/classes/external/calendar_event_exporter.php @@ -74,6 +74,10 @@ class calendar_event_exporter extends event_exporter_base { 'type' => PARAM_TEXT, 'optional' => true ]; + $values['draggable'] = [ + 'type' => PARAM_BOOL, + 'default' => false + ]; return $values; } @@ -90,6 +94,10 @@ class calendar_event_exporter extends event_exporter_base { $values = parent::get_other_values($output); $event = $this->event; + // By default all events that can be edited are + // draggable. + $values['draggable'] = $values['canedit']; + if ($moduleproxy = $event->get_course_module()) { $modulename = $moduleproxy->get('modname'); $moduleid = $moduleproxy->get('id'); @@ -226,9 +234,17 @@ class calendar_event_exporter extends event_exporter_base { 'mod_' . $modname, 'core_calendar_get_valid_event_timestart_range', [$mapper->from_event_to_legacy_event($event), $moduleinstance], - [null, null] + [false, false] ); + // The callback will return false for either of the + // min or max cutoffs to indicate that there are no + // valid timestart values. In which case the event is + // not draggable. + if ($min === false || $max === false) { + return ['draggable' => false]; + } + if ($min) { $values = array_merge($values, $this->get_module_timestamp_min_limit($starttime, $min)); } diff --git a/calendar/classes/local/api.php b/calendar/classes/local/api.php index 1b81231c991..6da620e3bda 100644 --- a/calendar/classes/local/api.php +++ b/calendar/classes/local/api.php @@ -260,9 +260,15 @@ class api { 'mod_' . $event->get_course_module()->get('modname'), 'core_calendar_get_valid_event_timestart_range', [$legacyevent, $moduleinstance], - [null, null] + [false, false] ); + // If the callback returns false for either value it means that + // there is no valid time start range. + if ($min === false || $max === false) { + throw new \moodle_exception('The start day of this event can not be modified'); + } + if ($min && $legacyevent->timestart < $min[0]) { throw new \moodle_exception($min[1]); } diff --git a/calendar/templates/month_detailed.mustache b/calendar/templates/month_detailed.mustache index 0cb0d8e782a..9abff8e2c2a 100644 --- a/calendar/templates/month_detailed.mustache +++ b/calendar/templates/month_detailed.mustache @@ -87,7 +87,7 @@ {{^underway}}
  • + {{/draggable}}> diff --git a/calendar/tests/local_api_test.php b/calendar/tests/local_api_test.php index d97c08cac51..ff2e065fb96 100644 --- a/calendar/tests/local_api_test.php +++ b/calendar/tests/local_api_test.php @@ -1239,4 +1239,50 @@ class core_calendar_local_api_testcase extends advanced_testcase { $this->expectException('moodle_exception'); $newevent = \core_calendar\local\api::update_event_start_day($event, $newstartdate); } + + /** + * Updating the start day of an overridden event belonging to an activity + * should result in an exception. This is to prevent the drag and drop + * of override events. + * + * Note: This test uses the quiz activity because it requires + * module callbacks to be in place and override event support to test. + */ + public function test_update_event_start_day_activity_event_override() { + global $CFG, $DB; + require_once($CFG->dirroot . '/calendar/lib.php'); + require_once($CFG->dirroot . '/mod/quiz/lib.php'); + + $this->resetAfterTest(true); + $this->setAdminUser(); + $mapper = container::get_event_mapper(); + $timeopen = new DateTimeImmutable('2017-01-1T15:00:00+08:00'); + $newstartdate = new DateTimeImmutable('2016-02-2T10:00:00+08:00'); + $generator = $this->getDataGenerator(); + $user = $generator->create_user(); + $course = $generator->create_course(); + $quizgenerator = $generator->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => $timeopen->getTimestamp(), + ]); + $event = create_event([ + 'courseid' => $course->id, + 'userid' => $user->id, + 'modulename' => 'quiz', + 'instance' => $quiz->id, + 'eventtype' => QUIZ_EVENT_TYPE_OPEN, + 'timestart' => $timeopen->getTimestamp() + ]); + $event = $mapper->from_legacy_event_to_event($event); + $record = (object) [ + 'quiz' => $quiz->id, + 'userid' => $user->id + ]; + + $DB->insert_record('quiz_overrides', $record); + + $this->expectException('moodle_exception'); + $newevent = \core_calendar\local\api::update_event_start_day($event, $newstartdate); + } } diff --git a/mod/assign/lib.php b/mod/assign/lib.php index 214362b5133..5eeccf327e4 100644 --- a/mod/assign/lib.php +++ b/mod/assign/lib.php @@ -1946,8 +1946,12 @@ function mod_assign_core_calendar_event_action_shows_item_count(calendar_event $ * [1506741172, 'The due date must be before the cutoff date'] * ] * + * If the event does not have a valid timestart range then [false, false] will + * be returned. + * * @param calendar_event $event The calendar event to get the time range for * @param stdClass $instance The module instance to get the range from + * @return array */ function mod_assign_core_calendar_get_valid_event_timestart_range(\calendar_event $event, \stdClass $instance) { global $CFG; diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index 61c6d9b54a7..ae24d0f7db8 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -1002,7 +1002,11 @@ class assign { * [1506741172, 'The due date must be before the cutoff date'] * ] * + * If the event does not have a valid timestart range then [false, false] will + * be returned. + * * @param calendar_event $event The calendar event to get the time range for + * @return array */ function get_valid_calendar_event_timestart_range(\calendar_event $event) { $instance = $this->get_instance(); @@ -1018,8 +1022,9 @@ class assign { // the only events that can be overridden, so we can save a DB // query if we don't bother checking other events. if ($this->is_override_calendar_event($event)) { - // This is an override event so we should ignore it. - return [null, null]; + // This is an override event so there is no valid timestart + // range to set it to. + return [false, false]; } if ($submissionsfromdate) { diff --git a/mod/assign/tests/lib_test.php b/mod/assign/tests/lib_test.php index 5e057eea08f..cadf7660141 100644 --- a/mod/assign/tests/lib_test.php +++ b/mod/assign/tests/lib_test.php @@ -883,8 +883,8 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { $DB->insert_record('assign_overrides', $record); list($min, $max) = mod_assign_core_calendar_get_valid_event_timestart_range($event, $instance); - $this->assertNull($min); - $this->assertNull($max); + $this->assertFalse($min); + $this->assertFalse($max); } /** diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index 49aef8d3a34..9b2b227b8ff 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -2264,6 +2264,9 @@ function mod_quiz_get_completion_active_rule_descriptions($cm) { * If either value isn't set then null will be returned instead to * indicate that there is no cutoff for that value. * + * If the vent has no valid timestart range then [false, false] will + * be returned. This is the case for overriden events. + * * A minimum and maximum cutoff return value will look like: * [ * [1505704373, 'The date must be after this date'], @@ -2279,9 +2282,9 @@ function mod_quiz_core_calendar_get_valid_event_timestart_range(\calendar_event global $CFG, $DB; require_once($CFG->dirroot . '/mod/quiz/locallib.php'); - // No restrictions on override events. + // Overrides do not have a valid timestart range. if (quiz_is_overriden_calendar_event($event)) { - return [null, null]; + return [false, false]; } $mindate = null; diff --git a/mod/quiz/tests/calendar_event_modified_test.php b/mod/quiz/tests/calendar_event_modified_test.php index e514e047a60..4a4cfdf0ffc 100644 --- a/mod/quiz/tests/calendar_event_modified_test.php +++ b/mod/quiz/tests/calendar_event_modified_test.php @@ -416,8 +416,8 @@ class mod_quiz_calendar_event_modified_testcase extends advanced_testcase { list ($min, $max) = mod_quiz_core_calendar_get_valid_event_timestart_range($event, $quiz); - $this->assertNull($min); - $this->assertNull($max); + $this->assertFalse($min); + $this->assertFalse($max); } /**