Calendar: Add event dialog fails to open for admin, managers (on large system)

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.

When you click 'Add event' on the calendar screen, it shows a spinner for a while in place of the new dialog that should be appearing, then eventually the request fails with a HTTP 500 and you get a JavaScript error. The failure is because the request hits the PHP memory limit.

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.