diff --git a/lib/questionlib.php b/lib/questionlib.php index f717ebdcb7c..438fe683aa3 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -389,13 +389,6 @@ function question_delete_question($questionid): void { // Delete questiontype-specific data. question_bank::get_qtype($question->qtype, false)->delete_question($question->id, $questiondata->contextid); - // Delete all tag instances. - core_tag_tag::remove_all_item_tags('core_question', 'question', $question->id); - - // Delete the custom filed data for the question. - $customfieldhandler = qbank_customfields\customfield\question_handler::create(); - $customfieldhandler->delete_instance($question->id); - // Now recursively delete all child questions if ($children = $DB->get_records('question', array('parent' => $questionid), '', 'id, qtype')) { @@ -406,9 +399,6 @@ function question_delete_question($questionid): void { } } - // Delete question comments. - $DB->delete_records('comments', ['itemid' => $questionid, 'component' => 'qbank_comment', - 'commentarea' => 'question']); // Finally delete the question record itself. $DB->delete_records('question', ['id' => $question->id]); $DB->delete_records('question_versions', ['id' => $questiondata->versionid]); @@ -421,6 +411,7 @@ function question_delete_question($questionid): void { question_bank::notify_question_edited($question->id); // Log the deletion of this question. + // Any qbank plugins storing additional question data should observe this event and perform the necessary deletion. $question->category = $questiondata->categoryid; $question->contextid = $questiondata->contextid; $event = \core\event\question_deleted::create_from_question_instance($question); diff --git a/lib/upgrade.txt b/lib/upgrade.txt index d62ac3243ae..41aa2e2fe90 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -62,6 +62,8 @@ information provided here is intended especially for developers. * New events \core\event\qbank_plugin_enabled and \core\event\qbank_plugin_disabled are triggered when a qbank plugin is enabled or disabled respectively, with the plugin's frankenstyle name. Any plugins that need to perform an action in response to a qbank plugin being enabled or disabled should observe these events. +* Code calling to qbank plugins was moved from question_delete_question in questionlib.php into the plugins themselves. Any plugins + that need to perform processing when a question is deleted should observe the \core\event\question_deleted event instead. === 4.2 === diff --git a/question/bank/comment/classes/event/question_deleted_observer.php b/question/bank/comment/classes/event/question_deleted_observer.php new file mode 100644 index 00000000000..ff53c04b6bb --- /dev/null +++ b/question/bank/comment/classes/event/question_deleted_observer.php @@ -0,0 +1,49 @@ +. + +namespace qbank_comment\event; + +defined('MOODLE_INTERNAL') || die(); + +require_once($CFG->dirroot . '/comment/lib.php'); + +use core\event\question_deleted; + +/** + * Event observer for question deletion + * + * @package qbank_comment + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class question_deleted_observer { + + /** + * Delete any comments for the deleted question. + * + * @param question_deleted $event + * @return void + */ + public static function delete_question_comments(question_deleted $event): void { + \comment::delete_comments([ + 'contextid' => \context_system::instance()->id, + 'component' => 'qbank_comment', + 'commentarea' => 'question', + 'itemid' => $event->objectid, + ]); + } +} diff --git a/question/bank/comment/db/events.php b/question/bank/comment/db/events.php new file mode 100644 index 00000000000..7efa63e9f2e --- /dev/null +++ b/question/bank/comment/db/events.php @@ -0,0 +1,33 @@ +. + +/** + * Question custom fields events + * + * @package qbank_comment + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$observers = [ + [ + 'eventname' => '\core\event\question_deleted', + 'callback' => '\qbank_comment\event\question_deleted_observer::delete_question_comments', + ] +]; diff --git a/question/bank/comment/tests/event/question_deleted_observer_test.php b/question/bank/comment/tests/event/question_deleted_observer_test.php new file mode 100644 index 00000000000..f31c3f71ef0 --- /dev/null +++ b/question/bank/comment/tests/event/question_deleted_observer_test.php @@ -0,0 +1,66 @@ +. + +namespace qbank_comment\event; + +/** + * Tests for question_deleted_observer + * + * @package qbank_comment + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qbank_comment\event\question_deleted_observer + */ +class question_deleted_observer_test extends \advanced_testcase { + + /** + * Deleting a question with comments should also delete the comments + * + * @return void + */ + public function test_delete_question_with_comments(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + [, , , $questions] = $questiongenerator->setup_course_and_questions(); + $question = reset($questions); + + $context = \context_system::instance(); + $commentgenerator = $this->getDataGenerator()->get_plugin_generator('core_comment'); + /** @var \comment $comment */ + $comment = $commentgenerator->create_comment([ + 'context' => $context, + 'component' => 'qbank_comment', + 'area' => 'question', + 'itemid' => $question->id, + 'content' => random_string(), + ]); + + $this->assertEquals(1, $comment->count()); + + question_delete_question($question->id); + + $newcomment = new \comment((object)[ + 'context' => $context, + 'component' => 'qbank_comment', + 'area' => 'question', + 'itemid' => $question->id, + ]); + + $this->assertEquals(0, $newcomment->count()); + } +} diff --git a/question/bank/comment/version.php b/question/bank/comment/version.php index d94f8bd1b75..dd34079b7c6 100644 --- a/question/bank/comment/version.php +++ b/question/bank/comment/version.php @@ -26,6 +26,6 @@ defined('MOODLE_INTERNAL') || die(); $plugin->component = 'qbank_comment'; -$plugin->version = 2023042400; +$plugin->version = 2023042401; $plugin->requires = 2023041800; $plugin->maturity = MATURITY_STABLE; diff --git a/question/bank/customfields/classes/event/question_deleted_observer.php b/question/bank/customfields/classes/event/question_deleted_observer.php new file mode 100644 index 00000000000..853c5a79119 --- /dev/null +++ b/question/bank/customfields/classes/event/question_deleted_observer.php @@ -0,0 +1,41 @@ +. + +namespace qbank_customfields\event; + +use core\event\question_deleted; +use qbank_customfields\customfield\question_handler; + +/** + * Event observer for question deletion + * + * @package qbank_customfields + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class question_deleted_observer { + + /** + * Delete any custom field data for the deleted question. + * + * @param question_deleted $event + * @return void + */ + public static function delete_question_customfields(question_deleted $event): void { + question_handler::create()->delete_instance($event->objectid); + } +} diff --git a/question/bank/customfields/db/events.php b/question/bank/customfields/db/events.php new file mode 100644 index 00000000000..311d7d34cab --- /dev/null +++ b/question/bank/customfields/db/events.php @@ -0,0 +1,33 @@ +. + +/** + * Question custom fields events + * + * @package qbank_customfields + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$observers = [ + [ + 'eventname' => '\core\event\question_deleted', + 'callback' => '\qbank_customfields\event\question_deleted_observer::delete_question_customfields' + ] +]; diff --git a/question/bank/customfields/tests/event/question_deleted_observer_test.php b/question/bank/customfields/tests/event/question_deleted_observer_test.php new file mode 100644 index 00000000000..bcbde6aa7b9 --- /dev/null +++ b/question/bank/customfields/tests/event/question_deleted_observer_test.php @@ -0,0 +1,68 @@ +. + +namespace qbank_customfields\event; + +/** + * Tests for question_deleted_observer + * + * @package qbank_customfields + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qbank_customfields\event\question_deleted_observer + */ +class question_deleted_observer_test extends \advanced_testcase { + + /** + * Deleting a question with customfield data should also delete the data. + * + * @return void + */ + public function test_delete_question_with_customfields(): void { + $this->resetAfterTest(); + $generator = self::getDataGenerator(); + $data = [ + 'component' => 'qbank_customfields', + 'area' => 'question' + ]; + + $categoryid = $generator->create_custom_field_category($data)->get('id'); + $generator->create_custom_field(['categoryid' => $categoryid, 'type' => 'text', 'shortname' => 'f1']); + + $questiongenerator = $generator->get_plugin_generator('core_question'); + [, , , $questions] = $questiongenerator->setup_course_and_questions(); + $question = reset($questions); + + $customfieldhandler = \qbank_customfields\customfield\question_handler::create(); + $questiondata = (object)[ + 'id' => $question->id, + 'customfield_f1' => random_string() + ]; + + $customfieldhandler->instance_form_save($questiondata); + + $customdata = $customfieldhandler->get_instance_data($question->id); + $this->assertCount(1, $customdata); + $this->assertEquals($questiondata->customfield_f1, reset($customdata)->get_value()); + + question_delete_question($question->id); + + $customdata = $customfieldhandler->get_instance_data($question->id); + $this->assertCount(1, $customdata); + $this->assertEmpty(reset($customdata)->get_value()); + } +} diff --git a/question/bank/customfields/version.php b/question/bank/customfields/version.php index 4ee627e03d1..077a36e5d51 100644 --- a/question/bank/customfields/version.php +++ b/question/bank/customfields/version.php @@ -26,6 +26,6 @@ defined('MOODLE_INTERNAL') || die(); $plugin->component = 'qbank_customfields'; -$plugin->version = 2023042400; +$plugin->version = 2023042401; $plugin->requires = 2023041800; $plugin->maturity = MATURITY_STABLE; diff --git a/question/bank/tagquestion/classes/event/question_deleted_observer.php b/question/bank/tagquestion/classes/event/question_deleted_observer.php new file mode 100644 index 00000000000..056acb40bd2 --- /dev/null +++ b/question/bank/tagquestion/classes/event/question_deleted_observer.php @@ -0,0 +1,45 @@ +. + +namespace qbank_tagquestion\event; + +use core\context; +use core\event\question_deleted; + +/** + * Event observer for question deletion + * + * @package qbank_tagquestion + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class question_deleted_observer { + + /** + * Delete any tags defined for the deleted question. + * + * This uses {@see \core_tag_tag::set_item_tags} rather than {@see \core_tag_tag::remove_all_item_tags} since the latter + * will always pass the system context, not the question context that the tag was set in. + * + * @param question_deleted $event + * @return void + */ + public static function delete_question_tags(question_deleted $event): void { + $questioncontext = context::instance_by_id($event->contextid); + \core_tag_tag::set_item_tags('core_question', 'question', $event->objectid, $questioncontext, null, $event->userid); + } +} diff --git a/question/bank/tagquestion/db/events.php b/question/bank/tagquestion/db/events.php new file mode 100644 index 00000000000..911b9e9b591 --- /dev/null +++ b/question/bank/tagquestion/db/events.php @@ -0,0 +1,33 @@ +. + +/** + * Question tag events + * + * @package qbank_tagquestion + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$observers = [ + [ + 'eventname' => '\core\event\question_deleted', + 'callback' => '\qbank_tagquestion\event\question_deleted_observer::delete_question_tags' + ] +]; diff --git a/question/bank/tagquestion/tests/event/question_deleted_observer_test.php b/question/bank/tagquestion/tests/event/question_deleted_observer_test.php new file mode 100644 index 00000000000..a4d13efa22b --- /dev/null +++ b/question/bank/tagquestion/tests/event/question_deleted_observer_test.php @@ -0,0 +1,50 @@ +. + +namespace qbank_tagquestion\event; + +/** + * Tests for question_deleted_observer + * + * @package qbank_tagquestion + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qbank_tagquestion\event\question_deleted_observer + */ +class question_deleted_observer_test extends \advanced_testcase { + + /** + * Deleting a question with tags should also delete the tags. + * + * @return void + */ + public function test_delete_question_with_tags(): void { + $this->resetAfterTest(); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + [, , $qcat, $questions] = $questiongenerator->setup_course_and_questions(); + $questioncontext = \context::instance_by_id($qcat->contextid); + $question = reset($questions); + $tag = random_string(); + \core_tag_tag::add_item_tag('core_question', 'question', $question->id, $questioncontext, $tag); + + $this->assertCount(1, \core_tag_tag::get_item_tags('core_question', 'question', $question->id)); + + question_delete_question($question->id); + + $this->assertEmpty(\core_tag_tag::get_item_tags('core_question', 'question', $question->id)); + } +} diff --git a/question/bank/tagquestion/version.php b/question/bank/tagquestion/version.php index ff37e0241a4..eda3baf8ad2 100644 --- a/question/bank/tagquestion/version.php +++ b/question/bank/tagquestion/version.php @@ -26,6 +26,6 @@ defined('MOODLE_INTERNAL') || die(); $plugin->component = 'qbank_tagquestion'; -$plugin->version = 2023042400; +$plugin->version = 2023042401; $plugin->requires = 2023041800; $plugin->maturity = MATURITY_STABLE;