Skip to content

Add 3 subplugins and amended pre-existing Step: coursedelete#297

Draft
ccaewan wants to merge 9 commits intolearnweb:mainfrom
ccaewan:lifecycle-workflows
Draft

Add 3 subplugins and amended pre-existing Step: coursedelete#297
ccaewan wants to merge 9 commits intolearnweb:mainfrom
ccaewan:lifecycle-workflows

Conversation

@ccaewan
Copy link
Copy Markdown

@ccaewan ccaewan commented Mar 31, 2026

🔀 Purpose of this PR:

  • Fixes a bug
  • Adds new functionality (Triggers: coursefreeze, coursedelete) (Step: uclcontextfreeze)

📝 Description:

Course Delete Fix

What bug does it address?

When the deletecourse step deleted a course, an orphaned tool_lifecycle_process record was occasionally left behind pointing to the now-deleted course. On subsequent cron runs the LC would throw a fatal dml_missing_record_exception (course ID not found) error, breaking the entire lifecycle workflow and preventing any further processing or manual intervention via the UI.

Why is this change necessary?

The fatal exception blocked the workflow completely and could not be resolved without direct DB access. The never-accessed course exclusion meant courses eligible for deletion were silently skipped indefinitely.

Expected behaviour after the change:

  • Orphaned process records (where the course no longer exists) are detected in get_processes_by_workflow() and moved to the tool_lifecycle_proc_error table rather than crashing the workflow
  • process::from_record() uses IGNORE_MISSING when loading the course context, so it never throws on a deleted course
  • The errors.php UI handles deleted course records gracefully when proceeding or rolling back, preventing a crash when actioning error records
  • The deletecourse step self-cleans its own orphaned process record if it encounters an already-deleted course

New subplugins

The archival workflow:

  • I created a configurable trigger subplugin (coursefreeze) to target courses with (default values) last acc > 1yr and creation date > 2yrs.

  • Step subplugin (uclcontextfreeze) then calls the manager/service method used by the context freeze task to successfully archive (make read-only) the triggered courses directly from Lifecycle.

The deletion workflow:

  • I created a configurable trigger subplugin (coursedelete) to target archived courses with (default values) Last acc > 4 years and creation date > 5 years. This currently also includes courses that have never been accessed and courses where enrolled users last access record is set to 'Never'.

The deletion workflow will use the pre existing deletecourse step in the lifecycle.


📋 Checklist

  • I have phpunit and/or behat tests that cover my changes or additions.
  • Code passes the code checker without errors and warnings.
  • Code does not have var_dump() or var_export or any other debugging statements that should not appear on the productive branch.
  • Code only uses language strings instead of hard-coded strings.
  • If there are changes in the database: I updated/created the necessary upgrade steps in db/upgrade.php and updated the version.php.

📋 Files changed

  • classes/local/entity/process.php — use IGNORE_MISSING in context_course::instance()
  • classes/local/manager/process_manager.php — orphan detection in get_processes_by_workflow()
  • errors.php — safe get_course() calls with try/catch for deleted courses
  • db/upgrade.php — cleanup step for orphaned process records (version 2026012005)
  • version.php — bumped to 2026012005
  • steps/deletecourse/lib.php — self-cleaning guard for already-deleted courses

Files added

  • trigger/coursedelete/lib.php — a configurable trigger to target archived courses that have Last access > x and creation date > x
  • trigger/coursefreeze/lib.php — a configurable trigger to target courses that have Last access > x and creation date > x
  • step/coursefreeze/lib.php — calls a uclcontextfreeze manager to archive courses

Notes for reviewers

  • This is a draft PR for early feedback and review
  • Implementation follows existing lifecycle step/trigger structure
  • Minor changes to existing functionality (error handling and course delete changes)

Testing

  • Code structure mirrors existing steps/triggers
  • Ready for functional testing within main Extend environment

🔍 Related Issues

  • Related to #[294]
  • Related to #[240]

🧾 Additional Information

The root of the deletion error was that delete_course() removes the course record but does not guarantee atomic cleanup of tool_lifecycle_process entries, particularly if a previous cron run was interrupted. The fix applies defence in depth across three layers (entity, manager, step) so the system self-heals regardless of which code path encounters the orphaned record first.

ccaewan@ucl.ac.uk and others added 8 commits March 31, 2026 17:05
Comment out plugin dependencies for future reference.
Updated context retrieval to handle missing courses gracefully.
Updated documentation for get_processes_by_workflow method to clarify orphaned process handling and fixed typos.
Added new upgrade steps for version 2026012004 and 2026012005, including cleanup of orphaned process records.
fix to prevent error message when proceeding or rolling back deleted course
@ccaewan ccaewan changed the title Add coursefreeze, coursedelete triggers and uclcontextfreeze step Add 3 subplugins and amended pre-existing Step: coursedelete Apr 14, 2026
@aspark21
Copy link
Copy Markdown

@ccaewan very quick scan

  1. probably worth considering the impact of issue#178 Refactor Subplugins Calls #293 and rebasing on that
  2. step uclcontextfreeze - is the assumption that we would start making use of the lifecycle block in UCL Extend? (we don't currently)
  3. step uclcontextfreeze - these are sub-plugins so we don't need to have this merged in the upstream repo, this could be a standalone repo. especially if it's something bespoke to UCL. That being said I can see most of the subplugin repos have ended up being merged into this
  4. revert changes to customfieldsemester/version.php

Comment thread trigger/customfieldsemester/version.php Outdated
Initially commented out to prevent error during instance upgrade
@ccaewan
Copy link
Copy Markdown
Author

ccaewan commented Apr 15, 2026

@aspark21

Response to 1 + 3

The changes to deletecourse could be useful for the LC plugin in general as they offer a fix for the preexisting subplugin, so I'll keep them within the PR. The issue was similar to #294 except the deletion was not manual in my case but was via the LC deletecourse step.

As for the 3 new subplugins I agree with the rebase. The best course of action will be to eventually move them to separate repos (once the recent #293 changes have been approved), considering the UCL niche of the subplugins.

Response to 2

I designed uclcontextfreeze taking https://ucldata.atlassian.net/browse/CTP-4885 into consideration - so the step has a hard dependency on block_lifecycle to action the archival of courses. I wasn't aware that this wasn't being used on live extend already.

Should block_lifecycle be installed on live Extend to facilitate this? or should the context freeze logic be implemented directly into the archival step?

@ccaewan
Copy link
Copy Markdown
Author

ccaewan commented Apr 16, 2026

Moved the 3 plugins to individual repos:

Step:

Triggers:

Will rebase onto #293 once it has been merged onto main or 405_stable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants