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]);