In certain (unexpected, and out of the ordinary) circumstances where `mdl_course_modules` rows have been removed, but the corresponding plugin's rows have not, users may not be able to delete the course if the plugin references the course module context as part of the [plugin]_delete_instance() method.

I understand the rarity of the situation where `mdl_course_modules` rows are removed and the plugin's corresponding rows are not. I also understand that it may not be typical to reference the course module context when deleting an instance. The problem is, when coping with varying degrees of quality from third-party sources, the problem becomes less rare, and more typical--rather than what should be, we are left with what is.

Our production installation has just such a problem using Blackboard's Elluminate plugin. Several courses could not be deleted because an error would be generated by the plugin when attempting to delete Elluminate sessions—the error is caused by the absence of the `mdl_course_modules` row, and hence the absence of the course module’s context, because Elluminate does a capability check as part of the instance deletion.

As easy as it would be to just blame Blackboard, and say “they shouldn’t do it that way,” I don’t think that’d be fair. When you consider that Moodle should use the `mdl_course_modules` entries as the foundation for what needs to be removed from a course, in the same way it uses `mdl_course_modules` entries as the foundation for what should be displayed in a course, the weakness is in the remove_course_contents() function in moodlelib.php.

The current approach is to get a list of mod plugins, and query each of the main plugin tables (e.g. `mdl_elluminate`, `mdl_forum`) for any rows that have a particular `course` column value. Then for each of those entries, work backward to the corresponding `mdl_course_modules` entries—and from the current code, there is an expectation that the `mdl_course_modules` row might not exit (v. 2.8 line 5115 – if($cm) test before calling context_helper::delete_instance).

Yeah, it’s easy to say a plugin should test the course module context for null before calling a routine that requires it. I get that. But if the core Moodle code could do things in such a way that circumvented the problem, while being just as efficient, shouldn’t it?

So, my suggested improvement to the remove_course_contents is to drive the process from the entries in `mdl_course_modules` rather than the individual plugins’ main table. Attached is the code.

Firstly, one side benefit is that the lib.php files are loaded only for mods that are known to be used in the course, rather than all of them.

Secondly, the Moodle core routine does not query the plugin’s main table directly, rather it uses the `mdl_course_modules` row to identify the plugin and then delegates to the plugin. As a fallback, it will still attempt to delete the row from the plugin’s main table directly—even though, in my opinion, it should not do this because of the likely orphaning of related rows in other tables. The remove_course_contents() function shouldn’t need to delete plugin table entities directly; if the plugin fails to delete the entity in its own tables (e.g. failed webservice call, or some other bug in the plugin), then so be it, that problem is on the plugin, but the core Moodle routine should remove the `mdl_course_modules` rows and remove the course.

Is there anywhere else in Moodle that the different plugins’ main tables are queried directly and the assumption that it has a column named `course` ? Isn’t that what the `mdl_course_modules` table was meant to avoid?