course sync and cohort sync task optimisation

[weilai-irl]

No description provided.

[Copilot]

Pull request overview

This PR optimizes course sync / cohort sync by consolidating the separate Teams and Groups caches into a single local_o365_groups_cache table and updating tasks/UI codepaths to use the unified cache.

Changes:

• Merge local_o365_teams_cache into local_o365_groups_cache (schema + data migration + new indexes).
• Update cache refresh logic to handle both Groups and Teams, add rate limiting, and add backoff/refresh tracking for fetching team details (URL/lock status).
• Replace many reads/writes of local_o365_teams_cache with filtered reads against local_o365_groups_cache (has_team = 1).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
local/o365/version.php	Bumps plugin version to trigger upgrade.
local/o365/db/upgrade.php	Adds migration to consolidate cache tables + new fields/indexes/config.
local/o365/db/install.xml	Updates install-time schema to the unified cache table (removes teams cache table).
local/o365/classes/utils.php	Unifies cache update/cleanup logic; adds rate limiting and team detail refresh.
local/o365/classes/task/processcourserequestapproval.php	Updates team lock write to unified cache table.
local/o365/classes/task/coursesync.php	Switches scheduled task to unified cache update flow.
local/o365/classes/page/acp.php	Updates ACP teamconnections reads to use unified cache table.
local/o365/classes/feature/coursesync/utils.php	Reads teams list from unified cache table.
local/o365/classes/feature/coursesync/main.php	Deprecates old teams-cache updater and updates team-lock checks to unified cache.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

local/o365/classes/page/acp.php:1069

• When creating links to team URLs on lines 1026 and 1069, there's no check for null or empty URL. During the 24-hour backoff period after a failed URL fetch, or if the team details fetch has never succeeded, the url field could be null. This would result in creating an HTML link with a null href attribute, which is invalid HTML. Consider adding a check like if (!empty($teamscache->url)) before creating the link, and displaying an appropriate message if the URL is not available.

                        $existingconnection = html_writer::link($teamscache->url, $teamscache->name);
                        if (
                            !$DB->record_exists(
                                'local_o365_objects',
                                ['type' => 'sdssection', 'subtype' => 'course', 'moodleid' => $course->id]
                            )
                        ) {
                            $updateurl = new moodle_url(
                                '/local/o365/acp.php',
                                ['mode' => 'teamconnections_update', 'course' => $course->id, 'sesskey' => sesskey()]
                            );
                            $updatelabel = get_string('acp_teamconnections_table_update', 'local_o365');

                            $actions = [html_writer::link($updateurl, $updatelabel)];
                        } else {
                            $actions = [get_string('acp_coursesynccustom_sds_course', 'local_o365')];
                        }
                    } else {
                        // A matching record exists in local_o365_objects, but the team cannot be found.
                        $existingconnection = $grouprecord->o365name . get_string('acp_teamconnections_team_missing', 'local_o365');

                        $actions = [html_writer::span(get_string('acp_teamconnections_table_missing_team', 'local_o365'))];
                    }
                } else {
                    // Connected to group only.
                    $metadata = (!empty($grouprecord->metadata)) ? json_decode($grouprecord->metadata, true) : [];
                    if (is_array($metadata) && !empty($metadata['softdelete'])) {
                        // Deleted group connection.
                        $existingconnection = get_string('acp_teamconnections_not_connected', 'local_o365');
                        $connecturl = new moodle_url(
                            '/local/o365/acp.php',
                            ['mode' => 'teamconnections_connect', 'course' => $course->id, 'sesskey' => sesskey()]
                        );
                        $connectlabel = get_string('acp_teamconnections_table_connect', 'local_o365');

                        $actions = [html_writer::link($connecturl, $connectlabel)];
                    } else if ($teamscache = $DB->get_record('local_o365_groups_cache', $teamscachedata)) {
                        // Connect the course with the team.
                        $teamobjectrecord = ['type' => 'group', 'subtype' => 'courseteam', 'objectid' => $teamscache->objectid,
                            'moodleid' => $course->id, 'o365name' => $teamscache->name, 'timecreated' => time(),
                            'timemodified' => time()];
                        $teamobjectrecord['id'] = $DB->insert_record('local_o365_objects', (object) $teamobjectrecord);

                        $existingconnection = html_writer::link($teamscache->url, $teamscache->name);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

local/o365/classes/feature/coursesync/main.php:1186

• The condition if (!$existingcachebyoid[$team['id']]->locked) on line 1176 is checking for falsy values, which will match 0 (TEAM_LOCKED_STATUS_UNKNOWN), null, and false. This could be problematic if the locked field can have different falsy values with different meanings. Consider using a more explicit check like if ($existingcachebyoid[$team['id']]->locked === null || $existingcachebyoid[$team['id']]->locked === TEAM_LOCKED_STATUS_UNKNOWN) to make the intent clearer and avoid unintended matches.

Additionally, note that this deprecated function's logic differs from the new utils::update_groups_cache() which has a more sophisticated lock status refresh mechanism (periodic 7-day refresh). This inconsistency could cause confusion if the deprecated function is still being used in the admin UI.

                if (!$existingcachebyoid[$team['id']]->locked) {
                    // Need to update lock status.
                    try {
                        [$rawteam, $teamurl, $lockstatus] = $this->graphclient->get_team($team['id']);
                        $lockstatuschecked = true;
                    } catch (moodle_exception $e) {
                        continue;
                    }
                } else {
                    $lockstatus = $existingcachebyoid[$team['id']]->locked;
                }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

local/o365/classes/page/acp.php:1088

• The values in teamscache->name and grouprecord->o365name are taken from Microsoft 365 (group/team display names) and are rendered directly into HTML via html_writer::link and string concatenation without escaping. An attacker who can control a Team or Group display name in the connected tenant can inject HTML/JavaScript into the “Teams connections” admin page, leading to stored XSS in the context of a Moodle administrator. To fix this, ensure these names are properly HTML-escaped or passed through the appropriate Moodle formatting/escaping functions before being used as link text or concatenated into $existingconnection.

                    if ($teamscache = $DB->get_record('local_o365_groups_cache', $teamscachedata)) {
                        // Team record can be found in cache.
                        $existingconnection = html_writer::link($teamscache->url, $teamscache->name);
                        if (
                            !$DB->record_exists(
                                'local_o365_objects',
                                ['type' => 'sdssection', 'subtype' => 'course', 'moodleid' => $course->id]
                            )
                        ) {
                            $updateurl = new moodle_url(
                                '/local/o365/acp.php',
                                ['mode' => 'teamconnections_update', 'course' => $course->id, 'sesskey' => sesskey()]
                            );
                            $updatelabel = get_string('acp_teamconnections_table_update', 'local_o365');

                            $actions = [html_writer::link($updateurl, $updatelabel)];
                        } else {
                            $actions = [get_string('acp_coursesynccustom_sds_course', 'local_o365')];
                        }
                    } else {
                        // A matching record exists in local_o365_objects, but the team cannot be found.
                        $existingconnection = $grouprecord->o365name . get_string('acp_teamconnections_team_missing', 'local_o365');

                        $actions = [html_writer::span(get_string('acp_teamconnections_table_missing_team', 'local_o365'))];
                    }
                } else {
                    // Connected to group only.
                    $metadata = (!empty($grouprecord->metadata)) ? json_decode($grouprecord->metadata, true) : [];
                    if (is_array($metadata) && !empty($metadata['softdelete'])) {
                        // Deleted group connection.
                        $existingconnection = get_string('acp_teamconnections_not_connected', 'local_o365');
                        $connecturl = new moodle_url(
                            '/local/o365/acp.php',
                            ['mode' => 'teamconnections_connect', 'course' => $course->id, 'sesskey' => sesskey()]
                        );
                        $connectlabel = get_string('acp_teamconnections_table_connect', 'local_o365');

                        $actions = [html_writer::link($connecturl, $connectlabel)];
                    } else if ($teamscache = $DB->get_record('local_o365_groups_cache', $teamscachedata)) {
                        // Connect the course with the team.
                        $teamobjectrecord = ['type' => 'group', 'subtype' => 'courseteam', 'objectid' => $teamscache->objectid,
                            'moodleid' => $course->id, 'o365name' => $teamscache->name, 'timecreated' => time(),
                            'timemodified' => time()];
                        $teamobjectrecord['id'] = $DB->insert_record('local_o365_objects', (object) $teamobjectrecord);

                        $existingconnection = html_writer::link($teamscache->url, $teamscache->name);

                        if (
                            !$DB->record_exists(
                                'local_o365_objects',
                                ['type' => 'sdssection', 'subtype' => 'course', 'moodleid' => $course->id]
                            )
                        ) {
                            $updateurl = new moodle_url(
                                '/local/o365/acp.php',
                                ['mode' => 'teamconnections_update', 'course' => $course->id, 'sesskey' => sesskey()]
                            );
                            $updatelabel = get_string('acp_teamconnections_table_update', 'local_o365');

                            $actions = [html_writer::link($updateurl, $updatelabel)];
                        }
                    } else {
                        // A team does not exist for the synced group.
                        $existingconnection = $grouprecord->o365name . get_string('acp_teamconnections_group_only', 'local_o365');

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.