Affects Version/s: 3.4.3
Fix Version/s: None
On any large system, the 'Add event' dialog in the calendar is likely to fail for admin and manager staff: everyone with moodle/calendar:manageentries at system level.
This appears to be because it loads all group members across the entire system.
Note this happens whether you click 'add event' from the general view (without a course selected) or whether you select a course first. It doesn't make any difference.
The specific cause is as follows:
Fragment function calendar_output_fragment_event_form()...
. Calls \core_calendar\local\event\forms\create...
. . Calls calendar_get_all_allowed_types...
. . . Calls calendar_get_default_courses, passing TRUE for $canmanage
BAD THING 1: If you have the capability moodle/calendar:manageentries, then it will load all courses into memory. However, as it only loads selected fields, this probably won't run out of memory assuming you have a reasonably low number of courses (we have about 6,000).
. . . Passes the result (all courses) to groups_get_all_groups_for_courses
BAD THING 2: This function will now get all groups for the listed courses, which is all courses. Not only does it load all the groups across the entire system (already quite bad - that's at the hundreds-of-thousands level in our system), it also loads all the group members (five million). Most likely this is the point where it hits the PHP memory limit.
I don't have sufficient confidence in the calendar code to fix this myself (unless somebody here decides it's a priority, obviously) but it would be great if somebody else could. Specifically, there are two things really wrong:
1. It retrieves all the group members from the database, but we don't care about any of them unless it's the current user.
2. It reads masses of data from the database, when it would be much more efficient to have the database do the calculation and only return the results.
So I think calendar_get_all_allowed_types should be rewritten to use a single (efficient) SQL query instead of calling those complex API functions. Even when it's not doing it for all the courses, it seems an inefficient way to do it; for example on a course with say 5,000 students, you're going to load the information about all those people for no reason.