-
Improvement
-
Resolution: Fixed
-
Minor
-
3.8
-
MOODLE_38_STABLE
-
MOODLE_39_STABLE
-
MDL-67548-master -
When manager deletes course category, there is a confirmation dialogue that says something like this: "This category contains: ...", "What to do"
Plugins also may have contents associated with the category. Would be good if they can hook into this form and tell the manager about it. Also plugins need to know what is the selected action and perform their checks/execute code in:
core_course_category::can_move_content_to()
core_course_category::delete_move()
etc.
Currently if "Move content to" was selected, there is no information about the new category in the "category_deleted" event and plugins can not distinguish it from the full deletion event.
====
A viable plan to better allow plugins to intercept coursecat actions (delete & move right now) can be something like:
A) hooks will be used to both modify the UI/form. It's not clear if we just need to "display" information or, also, be able to get new data on form submission. That will define how many hooks the UI/form need.
B) hook will be also used to check permissions. The idea is to have one unique hook called something like (disclaimer, name invented, just showing what it will do): xxx_delete_category_can_action($category, $action) that will enable plugins to decide if a given action can happen.
C1) For the execution itself... there was a little bit of debate. Finally, based on how course delete/reset is doing currently, it was agreed that we would be using the existing cat_delete and cat_update events to implement the action execution in plugins. That's the primary idea.
C2) If, for any reason, the event is not enough (because it's created AFTER core has performed the execution... it's possible to use the already existing xxxx__pre_course_category_delete() hook, that happens BEFORE core execution.
D) It's acknowledged by all parties that event observers are forbidden solution for core (and that hook callbacks, when possible, should be also avoided in core, though they are not forbidden). So, in case any core plugin wants some day to take part on the "delete cat" & "move cat" actions... we should create a new xxxx__pre_course_category_move() to avoid any event observer in core.
Associated aspects to decide:
- TODO1: Decide who wins when checking permissions. Surely the easier (and also logic) is to require all the hook permissions to return "true" (allowed). If there is just one plugin not allowing the action to happen (false) then the action should be prevented. Note that's a simplification (true/false), surely the check can come with a textual reason too.
- TODO2: Some small refactoring, keeping BC will be needed, for example the update category event needs to be enriched to easily know which action has happened, and associated information (like the new parent category in a move and other details). That way, observers can fine-decide when they have something to do or no.
- TODO3: Point D... if that new callback for move (or update in general, although maybe that's too much) action was created now... then we wouldn't need to perform any refactor like the TODO2 above. and the solution here would be 100% hook-based (no need for events at all). This should be considered, IMO.
- TODO4: Consider how recursion affects to all this, related to the "delete" action. Both if the existing hook and/or the events are used.
That's it, so far. Please, feel free to amend it any time, discuss, whatever. Ciao