Repository navigation
fix: stop deleting ClickMeeting rooms that are still in use - #47
Merged
Merged
Conversation
Meetings and recordings were disappearing from ClickMeeting without anyone deleting them on purpose, across unrelated courses and subaccounts. Three separate defects in the deletion path caused it. Deleting a Moodle activity called DELETE on the API straight away. Moodle deletion is reversible - it goes through the recycle bin and runs from cron - so the room was destroyed while the activity could still come back, and the API call landed minutes or hours after the click that caused it. Rooms now outlive their activity by a configurable grace period, five months by default, and a new scheduled task deletes them once it expires. Copying, importing or restoring a course duplicates the activity but both copies keep the same conference_id, so deleting either one destroyed the shared room for the other, in a course nobody had touched. The room is now only deleted once no activity refers to it, which also covers an activity restored from the recycle bin. The API key came from whoever was acting rather than from the room's owner. Under cron that is nobody in particular, so the call fell back to the main account key and could reach a room on a different subaccount. The owner is now carried through to the deletion. Also fixes two defects found along the way: an activity whose conference row was missing could never be deleted at all, and access tokens identifying participants were left behind in the database. Adds a room_deleted event so a missing meeting can be traced to when and why it was removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Moodle Plugin CI workflow has never run green: phpcs reported 154 errors that predate it. This applies phpcbf across the plugin so the checker passes and future failures mean something. Whitespace, argument wrapping, array syntax and interface ordering only - no behaviour is changed. Verified by PHP lint, the plugin test suite, Moodle's privacy checks, and a full v1.1.5 to v1.2.0 upgrade run, since db/upgrade.php was among the reformatted files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI caught this on the MOODLE_401_STABLE jobs: clickmeeting_room_deletions was
named clickmeeting_pending_deletions, 30 characters, and Moodle rejects table
names over 28 characters until 4.2:
Invalid table name {clickmeeting_pending_deletions}: name is too long.
The limit was raised later, so the table installed fine on 4.5 while breaking
every older site. The plugin declares support from Moodle 2.6 and the plugins
directory advertises 2.6 to 4.5, so releasing this would have broken installs
and upgrades for a large share of them.
Adds a test asserting every table in install.xml fits the 28 character limit,
so the next table cannot reintroduce this.
Verified on Moodle 4.1.22 (PHP 8.1) and 4.5.15 (PHP 8.2): clean install and
v1.1.5 to v1.2.0 upgrade both succeed, table and scheduled task created,
existing rows untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The new clickmeeting_room_deletions table records who owned a room so the deletion can run with their API key. That column was declared to the privacy API but nothing acted on it, so an erasure request left the user id behind for the whole grace period - up to five months. The row lives on after its activity, so it belongs to no module context. The provider now reports the system context for anyone named in the table, and clears the owner on erasure. The row itself stays: the room still has to be deleted, and it falls back to the global API key, which is already the behaviour when the owner no longer exists. Covers get_contexts_for_userid, get_users_in_context, delete_data_for_user, delete_data_for_users and delete_data_for_all_users_in_context. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The matrix dated from when Moodle 4.1 was current. Moodle is now on 5.3, and nothing in CI touched 5.x at all - including the public/ directory layout that 5.1 introduced, which changes where plugins are installed. Matrix now runs Moodle 4.1 LTS through 5.3, each branch paired only with PHP versions that branch actually supports, so a red job means the plugin is broken rather than the combination being impossible. PHP 7.4 and 8.0 are gone: both are past end of life and no tested branch accepts them. Services move to postgres 17 and mariadb 11.4, which Moodle 5.3 requires; verified locally that Moodle 4.1 still installs against mariadb 11.4, so the older end of the matrix keeps working. Runner moves to ubuntu-24.04 and the mariadb health check to healthcheck.sh, which the 11.x images ship. release.yml was still on actions/checkout@v2 and action-gh-release@v1; both are bumped. Verified locally before changing anything: clean install and a v1.1.5 to v1.2.0 upgrade both succeed on Moodle 5.3 (PHP 8.3, mariadb 11.4) with no plugin changes needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
release.yml runs with a write-scoped GITHUB_TOKEN and publishes the ZIP that users install, while referencing third-party actions by moving tag. Whoever controls those tags can repoint them at any commit and run code with that token, which reaches both the repository and the published asset. Both actions are now pinned to commit SHAs, with the version kept in a comment so upgrades stay readable. Permissions are declared explicitly: contents:write for the release workflow, contents:read for CI, which writes nothing. Note the @v2 bump in the previous commit introduced this exposure in its current form - the v1 reference had the same weakness. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The plugins directory advertised "2.6 to 4.5" purely because version.php never said otherwise, so Moodle 5.x users saw the plugin as incompatible even though it runs there fine. Declares supported = [39, 503], covering every branch actually verified for this release: 3.9 LTS, 3.11, 4.1 LTS, 4.5 LTS, 5.0, 5.1, 5.2 and 5.3. Each was given a clean install and a v1.1.5 to v1.2.0 upgrade; 4.1 through 5.3 are also covered by CI, 3.9 and 3.11 were checked locally on PHP 7.4. The lower bound stays at 3.9 deliberately. Those branches lost support in December 2023, but the plugin installs and upgrades there, and cutting them off would deny the sites least likely to upgrade Moodle the very fix that stops their meetings and recordings from being deleted. requires is left alone: supported is advisory and only changes what the directory and the admin page report, while requires blocks installation outright, and the boundary below 3.9 was not tested. Verified on live installs that Moodle reports the plugin as supported: branch 39 and branch 311 both return VERSION_SUPPORTED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…self The build did `mkdir mod_clickmeeting` then `cp -R * mod_clickmeeting/`, but the glob already matched the directory it had just created, so cp copied the plugin into itself until it hit the recursion and bailed. The `|| true` swallowed that error, leaving roughly fifty stray files under mod_clickmeeting/mod_clickmeeting/ in the package every user downloads. Checking out directly into the plugin directory name produces the required layout with no copying at all. The ZIP drops from 168K to 92K. Verified by building the archive the way the runner does and installing Moodle 3.11 from the unpacked package rather than from the repository: no nesting, no .git files, correct version, table and scheduled task created. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
chore: fix coding style violations reported by Moodle Code Checker
Backup carried conference_id over verbatim, so a restored or duplicated activity drove the very same room as its original. Two courses sharing one conference is wrong on its own, and deleting either activity took the room out from under the other. Restore now books a fresh room through the API, owned by whoever runs the restore, with a newly generated password; a refusal from the API aborts the restore rather than leaving an activity pointing at nothing. room_type had to join the backup for this: without it the new room could not be created with the type the original had. Backups written before this change carry no type and fall back to webinar. HACK-3336 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renaming or rescheduling an activity changed nothing on the ClickMeeting side. The request did go out, but empty: both edit calls handed their body to curl::put() through the options array, and put() overwrites that very option with its own $params argument, which was passed as an empty array. The API received a PUT with no fields and dutifully changed nothing, without an error to show for it. Passing the body as the $params argument puts it back on the wire. Covered by tests that run a stub HTTP server and read back the request that arrived, since a mocked response cannot tell an empty request from a full one. HACK-3336 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Meetings and recordings were disappearing from ClickMeeting without anyone deleting them on purpose, across unrelated courses and subaccounts. Three separate defects in the deletion path caused it.
Deleting a Moodle activity called DELETE on the API straight away. Moodle deletion is reversible - it goes through the recycle bin and runs from cron - so the room was destroyed while the activity could still come back, and the API call landed minutes or hours after the click that caused it. Rooms now outlive their activity by a configurable grace period, five months by default, and a new scheduled task deletes them once it expires.
Copying, importing or restoring a course duplicates the activity but both copies keep the same conference_id, so deleting either one destroyed the shared room for the other, in a course nobody had touched. The room is now only deleted once no activity refers to it, which also covers an activity restored from the recycle bin.
The API key came from whoever was acting rather than from the room's owner. Under cron that is nobody in particular, so the call fell back to the main account key and could reach a room on a different subaccount. The owner is now carried through to the deletion.
Also fixes two defects found along the way: an activity whose conference row was missing could never be deleted at all, and access tokens identifying participants were left behind in the database.
Adds a room_deleted event so a missing meeting can be traced to when and why it was removed.