diff --git a/calendar/lib.php b/calendar/lib.php index 3bf8caf9cae..83d4089b84f 100644 --- a/calendar/lib.php +++ b/calendar/lib.php @@ -235,9 +235,18 @@ class calendar_event { $data->eventtype = 'user'; } - // Default to the current user. + // Only user and user override type of events should record the user id. + // For all other event types we set userid to 0 as they are considered shared events. if (empty($data->userid)) { - $data->userid = $USER->id; + if ($this->allow_custom_userid($data)) { + $data->userid = $USER->id; + } else { + $data->userid = 0; + } + } + + if (empty($data->courseid) && $data->eventtype == 'site') { + $data->courseid = SITEID; } if (!empty($data->timeduration) && is_array($data->timeduration)) { @@ -315,6 +324,28 @@ class calendar_event { return !empty($this->properties->{$key}); } + /** + * Decide whether user id should be stored on the event or not. + * + * Only user events and user overrides type of events should retain user id. + * + * @param stdClass $data The event data object. + * @return bool + */ + protected function allow_custom_userid(stdClass $data): bool { + if ($data->eventtype === 'user') { + return true; + } + + $isactionevent = !empty($data->type) && $data->type == CALENDAR_EVENT_TYPE_ACTION; + $isuseroverride = !empty($data->priority) && $data->priority == CALENDAR_EVENT_USER_OVERRIDE_PRIORITY; + if ($isactionevent && $isuseroverride) { + return true; + } + + return false; + } + /** * Calculate the context value needed for an event. * @@ -489,49 +520,18 @@ class calendar_event { } if ($usingeditor) { - switch ($this->properties->eventtype) { - case 'user': - $this->properties->courseid = 0; - $this->properties->course = 0; - $this->properties->groupid = 0; - $this->properties->userid = $USER->id; - break; - case 'site': - $this->properties->courseid = SITEID; - $this->properties->course = SITEID; - $this->properties->groupid = 0; - $this->properties->userid = $USER->id; - break; - case 'course': - $this->properties->groupid = 0; - $this->properties->userid = $USER->id; - break; - case 'category': - $this->properties->groupid = 0; - $this->properties->category = 0; - $this->properties->userid = $USER->id; - break; - case 'group': - $this->properties->userid = $USER->id; - break; - default: - // We should NEVER get here, but just incase we do lets fail gracefully. - $usingeditor = false; - break; - } - // If we are actually using the editor, we recalculate the context because some default values // were set when calculate_context() was called from the constructor. - if ($usingeditor) { - $this->properties->context = $this->calculate_context(); - $this->editorcontext = $this->get_context(); - } + $this->properties->context = $this->calculate_context(); + $this->editorcontext = $this->get_context(); $editor = $this->properties->description; $this->properties->format = $this->properties->description['format']; $this->properties->description = $this->properties->description['text']; } + $this->set_default_event_ids(); + // Insert the event into the database. $this->properties->id = $DB->insert_record('event', $this->properties); @@ -676,6 +676,8 @@ class calendar_event { $event->trigger(); } } else { + $this->set_default_event_ids(); + $DB->update_record('event', $this->properties); $event = self::load($this->properties->id); $this->properties = $event->properties(); @@ -870,6 +872,49 @@ class calendar_event { return $properties; } + /** + * Set the default event ids. + */ + protected function set_default_event_ids(): void { + global $USER; + $userid = (!empty($this->properties->userid)) ? $this->properties->userid : $USER->id; + switch ($this->properties->eventtype) { + case 'user': + $this->properties->courseid = 0; + $this->properties->course = 0; + $this->properties->groupid = 0; + $this->properties->userid = $userid; + break; + case 'site': + $this->properties->courseid = SITEID; + $this->properties->course = SITEID; + $this->properties->groupid = 0; + $this->properties->userid = 0; + break; + case 'course': + $this->properties->groupid = 0; + $this->properties->userid = 0; + break; + case 'category': + $this->properties->groupid = 0; + $this->properties->category = 0; + $this->properties->userid = 0; + break; + case 'group': + $this->properties->userid = 0; + break; + } + + // Only user overrides type of events should store user's id. + $isactionevent = !empty($this->properties->type) && $this->properties->type == CALENDAR_EVENT_TYPE_ACTION; + $isuseroverride = $isactionevent && !empty($this->properties->priority) && + $this->properties->priority == CALENDAR_EVENT_USER_OVERRIDE_PRIORITY; + + if ($isuseroverride) { + $this->properties->userid = $userid; + } + } + /** * Toggles the visibility of an event * diff --git a/calendar/tests/event_vault_test.php b/calendar/tests/event_vault_test.php index 5d90f93b3d8..83d0bdbab16 100644 --- a/calendar/tests/event_vault_test.php +++ b/calendar/tests/event_vault_test.php @@ -608,8 +608,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 1; $i < 6; $i++) { create_event([ 'name' => sprintf('Event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course1->id, @@ -619,8 +619,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 6; $i < 12; $i++) { create_event([ 'name' => sprintf('Event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course2->id, @@ -663,8 +663,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 1; $i < 6; $i++) { create_event([ 'name' => sprintf('Event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course1->id, @@ -674,8 +674,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 6; $i < 12; $i++) { create_event([ 'name' => sprintf('Event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course2->id, @@ -719,8 +719,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 1; $i < 6; $i++) { create_event([ 'name' => sprintf('Event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course1->id, @@ -730,8 +730,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 6; $i < 12; $i++) { create_event([ 'name' => sprintf('Event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course2->id, @@ -773,8 +773,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 1; $i < 21; $i++) { $records[] = create_event([ 'name' => sprintf('Event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course1->id, @@ -784,8 +784,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 21; $i < 41; $i++) { $records[] = create_event([ 'name' => sprintf('Event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course2->id, @@ -830,8 +830,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 1; $i < 41; $i++) { create_event([ 'name' => sprintf('Event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course1->id, @@ -901,8 +901,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 1; $i < 21; $i++) { create_event([ 'name' => sprintf('Event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course1->id, @@ -912,8 +912,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 21; $i < 41; $i++) { create_event([ 'name' => sprintf('Event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course2->id, @@ -979,8 +979,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 1; $i < 11; $i++) { $records[] = create_event([ 'name' => sprintf('1 event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course1->id, @@ -990,8 +990,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 1; $i < 11; $i++) { $records[] = create_event([ 'name' => sprintf('2 event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course1->id, @@ -1002,8 +1002,8 @@ class core_calendar_event_vault_testcase extends advanced_testcase { for ($i = 1; $i < 11; $i++) { $records[] = create_event([ 'name' => sprintf('3 event %d', $i), - 'eventtype' => 'user', - 'userid' => $user->id, + 'eventtype' => 'due', + 'userid' => 0, 'timesort' => $i, 'type' => CALENDAR_EVENT_TYPE_ACTION, 'courseid' => $course2->id, diff --git a/calendar/tests/externallib_test.php b/calendar/tests/externallib_test.php index bc33c032785..46af548aa55 100644 --- a/calendar/tests/externallib_test.php +++ b/calendar/tests/externallib_test.php @@ -96,13 +96,6 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { } else { $prop->repeat = 1; } - if (empty($prop->userid)) { - if (!empty($userid)) { - $prop->userid = $userid; - } else { - $prop->userid = 0; - } - } if (!isset($prop->courseid)) { $prop->courseid = $SITE->id; } @@ -390,44 +383,54 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $this->setUser($user); $events = core_calendar_external::get_calendar_events($paramevents, $options); $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); - $this->assertEquals(2, count($events['events'])); // site, user. - $this->assertEquals(2, count($events['warnings'])); // course, group. + // There should be a single user event. + $this->assertEquals(1, count($events['events'])); + // There should be only two events (course and group). + $this->assertEquals(2, count($events['warnings'])); $role = $DB->get_record('role', array('shortname' => 'student')); $this->getDataGenerator()->enrol_user($user->id, $course->id, $role->id); $events = core_calendar_external::get_calendar_events($paramevents, $options); $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); - $this->assertEquals(4, count($events['events'])); // site, user, both course events. - $this->assertEquals(1, count($events['warnings'])); // group. + // There should be a site and both course events. + $this->assertEquals(3, count($events['events'])); + // There should be a warning related to group event. + $this->assertEquals(1, count($events['warnings'])); $options = array ('siteevents' => true, 'userevents' => true, 'timeend' => time() + HOURSECS); $events = core_calendar_external::get_calendar_events($paramevents, $options); $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); - $this->assertEquals(3, count($events['events'])); // site, user, one course event. - $this->assertEquals(1, count($events['warnings'])); // group. + // There should be a site and a course event. + $this->assertEquals(2, count($events['events'])); + // There should be a warning related to group event. + $this->assertEquals(1, count($events['warnings'])); groups_add_member($group, $user); $events = core_calendar_external::get_calendar_events($paramevents, $options); $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); - $this->assertEquals(4, count($events['events'])); // site, user, group, one course event. + // There should be a site, group, one course event. + $this->assertEquals(3, count($events['events'])); $this->assertEquals(0, count($events['warnings'])); $paramevents = array ('courseids' => array($course->id), 'groupids' => array($group->id)); $events = core_calendar_external::get_calendar_events($paramevents, $options); $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); - $this->assertEquals(4, count($events['events'])); // site, user, group, one course event. + // There should be only site, course and group events. + $this->assertEquals(3, count($events['events'])); $this->assertEquals(0, count($events['warnings'])); $paramevents = array ('groupids' => array($group->id, 23)); $events = core_calendar_external::get_calendar_events($paramevents, $options); $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); - $this->assertEquals(3, count($events['events'])); // site, user, group. + // There should be only site and group events. + $this->assertEquals(2, count($events['events'])); $this->assertEquals(1, count($events['warnings'])); $paramevents = array ('courseids' => array(23)); $events = core_calendar_external::get_calendar_events($paramevents, $options); $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); - $this->assertEquals(2, count($events['events'])); // site, user. + // There should be a single site event. + $this->assertEquals(1, count($events['events'])); $this->assertEquals(1, count($events['warnings'])); $paramevents = array (); @@ -468,7 +471,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $events = core_calendar_external::get_calendar_events($paramevents, $options); $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); - $this->assertCount(5, $events['events']); + $this->assertCount(4, $events['events']); // Hide the assignment. set_coursemodule_visible($assign->cmid, 0); @@ -479,7 +482,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $events = core_calendar_external::get_calendar_events($paramevents, $options); $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); // Expect one less. - $this->assertCount(4, $events['events']); + $this->assertCount(3, $events['events']); // Create some category events. $this->setAdminUser(); @@ -838,7 +841,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $now = time(); // Create two events - one for everybody in the course and one only for the first student. $event1 = $this->create_calendar_event('Base event', 0, 'due', 0, $now + DAYSECS, $params + ['courseid' => $course->id]); - $event2 = $this->create_calendar_event('User event', $user->id, 'due', 0, $now + 2*DAYSECS, $params + ['courseid' => 0]); + $event2 = $this->create_calendar_event('User event', 0, 'user', 0, $now + 2 * DAYSECS, $params + ['courseid' => 0]); // Retrieve course events for the second student - only one "Base event" is returned. $this->setUser($user2); @@ -850,14 +853,13 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $this->assertEquals(0, count($events['warnings'])); $this->assertEquals('Base event', $events['events'][0]['name']); - // Retrieve events for the first student - both events are returned. + // Retrieve events for the first student. $this->setUser($user); $events = core_calendar_external::get_calendar_events($paramevents, $options); $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); - $this->assertEquals(2, count($events['events'])); + $this->assertEquals(1, count($events['events'])); $this->assertEquals(0, count($events['warnings'])); $this->assertEquals('Base event', $events['events'][0]['name']); - $this->assertEquals('User event', $events['events'][1]['name']); // Retrieve events by id as a teacher, 'User event' should be returned since teacher has access to this course. $this->setUser($teacher); @@ -1720,7 +1722,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $timedurationuntil->add($interval); $formdata = [ 'id' => 0, - 'userid' => $user->id, + 'userid' => 0, 'modulename' => '', 'instance' => 0, 'visible' => 1, @@ -1781,7 +1783,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $timedurationuntil->add($interval); $formdata = [ 'id' => 0, - 'userid' => $user->id, + 'userid' => 0, 'modulename' => '', 'instance' => 0, 'visible' => 1, @@ -1827,7 +1829,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { ); $event = $result['event']; - $this->assertEquals($user->id, $event['userid']); + $this->assertEquals(0, $event['userid']); $this->assertEquals($formdata['eventtype'], $event['eventtype']); $this->assertEquals($formdata['name'], $event['name']); } @@ -1847,7 +1849,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $timedurationuntil->add($interval); $formdata = [ 'id' => 0, - 'userid' => $user->id, + 'userid' => 0, 'modulename' => '', 'instance' => 0, 'visible' => 1, @@ -1958,7 +1960,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { ); $event = $result['event']; - $this->assertEquals($user->id, $event['userid']); + $this->assertEquals(0, $event['userid']); $this->assertEquals($formdata['eventtype'], $event['eventtype']); $this->assertEquals($formdata['name'], $event['name']); $this->assertEquals($formdata['courseid'], $event['course']['id']); @@ -2046,7 +2048,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $timedurationuntil->add($interval); $formdata = [ 'id' => 0, - 'userid' => $user->id, + 'userid' => 0, 'modulename' => '', 'instance' => 0, 'visible' => 1, @@ -2162,7 +2164,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { ); $event = $result['event']; - $this->assertEquals($user->id, $event['userid']); + $this->assertEquals(0, $event['userid']); $this->assertEquals($formdata['eventtype'], $event['eventtype']); $this->assertEquals($formdata['name'], $event['name']); $this->assertEquals($group->id, $event['groupid']); @@ -2185,7 +2187,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $timedurationuntil->add($interval); $formdata = [ 'id' => 0, - 'userid' => $user->id, + 'userid' => 0, 'modulename' => '', 'instance' => 0, 'visible' => 1, @@ -2236,7 +2238,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { ); $event = $result['event']; - $this->assertEquals($user->id, $event['userid']); + $this->assertEquals(0, $event['userid']); $this->assertEquals($formdata['eventtype'], $event['eventtype']); $this->assertEquals($formdata['name'], $event['name']); $this->assertEquals($group->id, $event['groupid']); @@ -2259,7 +2261,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $timedurationuntil->add($interval); $formdata = [ 'id' => 0, - 'userid' => $user->id, + 'userid' => 0, 'modulename' => '', 'instance' => 0, 'visible' => 1, @@ -2309,7 +2311,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { ); $event = $result['event']; - $this->assertEquals($user->id, $event['userid']); + $this->assertEquals(0, $event['userid']); $this->assertEquals($formdata['eventtype'], $event['eventtype']); $this->assertEquals($formdata['name'], $event['name']); $this->assertEquals($group->id, $event['groupid']); @@ -2581,8 +2583,10 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $category = $generator->create_category(['visible' => 0]); $name = 'Category Event (category: ' . $category->id . ')'; $record = new stdClass(); + $record->eventtype = 'category'; $record->categoryid = $category->id; - $categoryevent = $this->create_calendar_event($name, $USER->id, 'category', 0, time(), $record); + $record->userid = 0; + $categoryevent = $this->create_calendar_event($name, 0, 'category', 0, time(), $record); $events = [ 'eventids' => [$categoryevent->id] diff --git a/calendar/tests/local_api_test.php b/calendar/tests/local_api_test.php index 987de7b3915..934ef414adc 100644 --- a/calendar/tests/local_api_test.php +++ b/calendar/tests/local_api_test.php @@ -154,8 +154,8 @@ class core_calendar_local_api_testcase extends advanced_testcase { 'courseid' => $course->id, 'modulename' => 'assign', 'instance' => $moduleinstance->id, - 'userid' => 1, - 'eventtype' => 'user', + 'userid' => 0, + 'eventtype' => 'due', 'repeats' => 0, 'timestart' => 1, ]; @@ -205,8 +205,8 @@ class core_calendar_local_api_testcase extends advanced_testcase { 'courseid' => $course->id, 'modulename' => 'assign', 'instance' => $moduleinstance->id, - 'userid' => 1, - 'eventtype' => 'user', + 'userid' => 0, + 'eventtype' => 'due', 'repeats' => 0, 'timestart' => 1, ]; @@ -257,8 +257,8 @@ class core_calendar_local_api_testcase extends advanced_testcase { 'courseid' => $course->id, 'modulename' => 'assign', 'instance' => $moduleinstance->id, - 'userid' => 1, - 'eventtype' => 'user', + 'userid' => 0, + 'eventtype' => 'due', 'repeats' => 0, 'timestart' => 1, ]; diff --git a/lib/testing/generator/data_generator.php b/lib/testing/generator/data_generator.php index a5697b50ee5..600cc762a2c 100644 --- a/lib/testing/generator/data_generator.php +++ b/lib/testing/generator/data_generator.php @@ -1139,19 +1139,23 @@ EOD; break; case 'group': unset($record->categoryid); + unset($record->userid); break; case 'course': unset($record->categoryid); unset($record->groupid); + unset($record->userid); break; case 'category': unset($record->courseid); unset($record->groupid); + unset($record->userid); break; case 'site': unset($record->categoryid); unset($record->courseid); unset($record->groupid); + unset($record->userid); break; }