L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'MDL-67548-master' of https://github.com/kabalin/moodle

Browse Source
This commit is contained in:
Adrian Greeve
2020-05-20 08:35:18 +08:00
parent 6754ceae3a a38e1d4968
commit 59d0090fda
7 changed files with 493 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
course/classes/category.php
+56 -18
View File
@@ -171,6 +171,24 @@ class core_course_category implements renderable, cacheable_object, IteratorAggr
debugging('Can not unset core_course_category instance properties!', DEBUG_DEVELOPER);
}
/**
* Get list of plugin callback functions.
*
* @param string $name Callback function name.
* @return [callable] $pluginfunctions
*/
public function get_plugins_callback_function(string $name) : array {
$pluginfunctions = [];
if ($pluginsfunction = get_plugins_with_function($name)) {
foreach ($pluginsfunction as $plugintype => $plugins) {
foreach ($plugins as $pluginfunction) {
$pluginfunctions[] = $pluginfunction;
}
}
}
return $pluginfunctions;
}
/**
* Create an iterator because magic vars can't be seen by 'foreach'.
*
@@ -1900,13 +1918,12 @@ class core_course_category implements renderable, cacheable_object, IteratorAggr
return false;
}
$context = $this->get_context();
if (!$this->is_uservisible() ||
!has_capability('moodle/category:manage', $context)) {
if (!$this->has_manage_capability()) {
return false;
}
// Check all child categories (not only direct children).
$context = $this->get_context();
$sql = context_helper::get_preload_record_columns_sql('ctx');
$childcategories = $DB->get_records_sql('SELECT c.id, c.visible, '. $sql.
' FROM {context} ctx '.
@@ -1936,6 +1953,15 @@ class core_course_category implements renderable, cacheable_object, IteratorAggr
}
}
// Check if plugins permit deletion of category content.
$pluginfunctions = $this->get_plugins_callback_function('can_course_category_delete');
foreach ($pluginfunctions as $pluginfunction) {
// If at least one plugin does not permit deletion, stop and return false.
if (!$pluginfunction($this)) {
return false;
}
}
return true;
}
@@ -1961,13 +1987,9 @@ class core_course_category implements renderable, cacheable_object, IteratorAggr
$settimeout = core_php_time_limit::raise();
// Allow plugins to use this category before we completely delete it.
if ($pluginsfunction = get_plugins_with_function('pre_course_category_delete')) {
$category = $this->get_db_record();
foreach ($pluginsfunction as $plugintype => $plugins) {
foreach ($plugins as $pluginfunction) {
$pluginfunction($category);
}
}
$pluginfunctions = $this->get_plugins_callback_function('pre_course_category_delete');
foreach ($pluginfunctions as $pluginfunction) {
$pluginfunction($this->get_db_record());
}
$deletedcourses = array();
@@ -2076,25 +2098,35 @@ class core_course_category implements renderable, cacheable_object, IteratorAggr
public function can_move_content_to($newcatid) {
global $CFG;
require_once($CFG->libdir . '/questionlib.php');
$context = $this->get_context();
if (!$this->is_uservisible() ||
!has_capability('moodle/category:manage', $context)) {
if (!$this->has_manage_capability()) {
return false;
}
$testcaps = array();
// If this category has courses in it, user must have 'course:create' capability in target category.
if ($this->has_courses()) {
$testcaps[] = 'moodle/course:create';
}
// If this category has subcategories or questions, user must have 'category:manage' capability in target category.
if ($this->has_children() || question_context_has_any_questions($context)) {
if ($this->has_children() || question_context_has_any_questions($this->get_context())) {
$testcaps[] = 'moodle/category:manage';
}
if (!empty($testcaps)) {
return has_all_capabilities($testcaps, context_coursecat::instance($newcatid));
if (!empty($testcaps) && !has_all_capabilities($testcaps, context_coursecat::instance($newcatid))) {
// No sufficient capabilities to perform this task.
return false;
}
// Check if plugins permit moving category content.
$pluginfunctions = $this->get_plugins_callback_function('can_course_category_delete_move');
$newparentcat = self::get($newcatid, MUST_EXIST, true);
foreach ($pluginfunctions as $pluginfunction) {
// If at least one plugin does not permit move on deletion, stop and return false.
if (!$pluginfunction($this, $newparentcat)) {
return false;
}
}
// There is no content but still return true.
return true;
}
@@ -2124,6 +2156,12 @@ class core_course_category implements renderable, cacheable_object, IteratorAggr
$coursesids = $DB->get_fieldset_select('course', 'id', 'category = :category ORDER BY sortorder ASC', $params);
$context = $this->get_context();
// Allow plugins to make necessary changes before we move the category content.
$pluginfunctions = $this->get_plugins_callback_function('pre_course_category_delete_move');
foreach ($pluginfunctions as $pluginfunction) {
$pluginfunction($this, $newparentcat);
}
if ($children) {
foreach ($children as $childcat) {
$childcat->change_parent_raw($newparentcat);
@@ -2186,7 +2224,7 @@ class core_course_category implements renderable, cacheable_object, IteratorAggr
$event = \core\event\course_category_deleted::create(array(
'objectid' => $this->id,
'context' => $context,
'other' => array('name' => $this->name)
'other' => array('name' => $this->name, 'contentmovedcategoryid' => $newparentid)
));
$event->set_coursecat($this);
$event->trigger();
course/classes/deletecategory_form.php
+17 -9
View File
@@ -76,14 +76,23 @@ class core_course_deletecategory_form extends moodleform {
// Describe the contents of this category.
$contents = '';
if ($this->coursecat->has_children()) {
$contents .= '<li>' . get_string('subcategories') . '</li>';
$contents .= html_writer::tag('li', get_string('subcategories'));
}
if ($this->coursecat->has_courses()) {
$contents .= '<li>' . get_string('courses') . '</li>';
$contents .= html_writer::tag('li', get_string('courses'));
}
if (question_context_has_any_questions($categorycontext)) {
$contents .= '<li>' . get_string('questionsinthequestionbank') . '</li>';
$contents .= html_writer::tag('li', get_string('questionsinthequestionbank'));
}
// Check if plugins can provide more info.
$pluginfunctions = $this->coursecat->get_plugins_callback_function('get_course_category_contents');
foreach ($pluginfunctions as $pluginfunction) {
if ($plugincontents = $pluginfunction($this->coursecat)) {
$contents .= html_writer::tag('li', $plugincontents);
}
}
if (!empty($contents)) {
$mform->addElement('static', 'emptymessage', get_string('thiscategorycontains'), html_writer::tag('ul', $contents));
} else {
@@ -92,7 +101,9 @@ class core_course_deletecategory_form extends moodleform {
// Give the options for what to do.
$mform->addElement('select', 'fulldelete', get_string('whattodo'), $options);
if (count($options) == 1) {
// Freeze selector if only one option available.
$optionkeys = array_keys($options);
$option = reset($optionkeys);
$mform->hardFreeze('fulldelete');
@@ -111,10 +122,6 @@ class core_course_deletecategory_form extends moodleform {
$mform->setType('categoryid', PARAM_ALPHANUM);
$mform->addElement('hidden', 'action', 'deletecategory');
$mform->setType('action', PARAM_ALPHANUM);
$mform->addElement('hidden', 'sure');
// This gets set by default to ensure that if the user changes it manually we can detect it.
$mform->setDefault('sure', md5(serialize($this->coursecat)));
$mform->setType('sure', PARAM_ALPHANUM);
$this->add_action_buttons(true, get_string('delete'));
}
@@ -131,10 +138,11 @@ class core_course_deletecategory_form extends moodleform {
if (empty($data['fulldelete']) && empty($data['newparent'])) {
// When they have chosen the move option, they must specify a destination.
$errors['newparent'] = get_string('required');
return $errors;
}
if ($data['sure'] !== md5(serialize($this->coursecat))) {
$errors['categorylabel'] = get_string('categorymodifiedcancel');
if (!empty($data['newparent']) && !$this->coursecat->can_move_content_to($data['newparent'])) {
$errors['newparent'] = get_string('movecatcontentstoselected', 'error');
}
return $errors;
course/tests/category_hooks_test.php
+215
View File
@@ -0,0 +1,215 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
/**
* Tests for class core_course_category methods invoking hooks.
*
* @package core_course
* @category test
* @copyright 2020 Ruslan Kabalin
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
namespace tests\core_course;
defined('MOODLE_INTERNAL') || die();
global $CFG;
require_once($CFG->dirroot . '/course/tests/fixtures/mock_hooks.php');
use PHPUnit\Framework\MockObject\MockObject;
/**
* Functional test for class core_course_category methods invoking hooks.
*/
class core_course_category_hooks_testcase extends \advanced_testcase {
protected function setUp() {
$this->resetAfterTest();
$this->setAdminUser();
}
/**
* Provides mocked category configured for named callback function.
*
* get_plugins_callback_function will return callable prefixed with `tool_unittest_`,
* the actual callbacks are defined in mock_hooks.php fixture file.
*
* @param core_course_category $category Category to mock
* @param string $callback Callback function used in method we test.
* @return MockObject
*/
public function get_mock_category(\core_course_category $category, string $callback = '') : MockObject {
// Setup mock object for \core_course_category.
// Disable original constructor, since we can't use it directly since it is private.
$mockcategory = $this->getMockBuilder(\core_course_category::class)
->setMethods(['get_plugins_callback_function'])
->disableOriginalConstructor()
->getMock();
// Define get_plugins_callback_function use and return value.
if (!empty($callback)) {
$mockcategory->method('get_plugins_callback_function')
->with($this->equalTo($callback))
->willReturn(['tool_unittest_' . $callback]);
}
// Modify constructor visibility and invoke mock object with real object.
// This is used to overcome private constructor.
$reflected = new \ReflectionClass(\core_course_category::class);
$constructor = $reflected->getConstructor();
$constructor->setAccessible(true);
$constructor->invoke($mockcategory, $category->get_db_record());
return $mockcategory;
}
public function test_can_course_category_delete_hook() {
$category1 = \core_course_category::create(array('name' => 'Cat1'));
$category2 = \core_course_category::create(array('name' => 'Cat2', 'parent' => $category1->id));
$category3 = \core_course_category::create(array('name' => 'Cat3'));
$mockcategory2 = $this->get_mock_category($category2, 'can_course_category_delete');
// Add course to mocked clone of category2.
$course1 = $this->getDataGenerator()->create_course(array('category' => $mockcategory2->id));
// Now configure fixture to return false for the callback.
mock_hooks::set_can_course_category_delete_return(false);
$this->assertFalse($mockcategory2->can_delete_full($category3->id));
// Now configure fixture to return true for the callback.
mock_hooks::set_can_course_category_delete_return(true);
$this->assertTrue($mockcategory2->can_delete_full($category3->id));
// Verify passed arguments.
$arguments = mock_hooks::get_calling_arguments();
$this->assertCount(1, $arguments);
// Argument 1 is the same core_course_category instance.
$argument = array_shift($arguments);
$this->assertSame($mockcategory2, $argument);
}
public function test_can_course_category_delete_move_hook() {
$category1 = \core_course_category::create(array('name' => 'Cat1'));
$category2 = \core_course_category::create(array('name' => 'Cat2', 'parent' => $category1->id));
$category3 = \core_course_category::create(array('name' => 'Cat3'));
$mockcategory2 = $this->get_mock_category($category2, 'can_course_category_delete_move');
// Add course to mocked clone of category2.
$course1 = $this->getDataGenerator()->create_course(array('category' => $mockcategory2->id));
// Now configure fixture to return false for the callback.
mock_hooks::set_can_course_category_delete_move_return(false);
$this->assertFalse($mockcategory2->can_move_content_to($category3->id));
// Now configure fixture to return true for the callback.
mock_hooks::set_can_course_category_delete_move_return(true);
$this->assertTrue($mockcategory2->can_move_content_to($category3->id));
// Verify passed arguments.
$arguments = mock_hooks::get_calling_arguments();
$this->assertCount(2, $arguments);
// Argument 1 is the same core_course_category instance.
$argument = array_shift($arguments);
$this->assertSame($mockcategory2, $argument);
// Argument 2 is referring to category 3.
$argument = array_shift($arguments);
$this->assertInstanceOf(\core_course_category::class, $argument);
$this->assertEquals($category3->id, $argument->id);
}
public function test_pre_course_category_delete_hook() {
$category1 = \core_course_category::create(array('name' => 'Cat1'));
$category2 = \core_course_category::create(array('name' => 'Cat2', 'parent' => $category1->id));
$mockcategory2 = $this->get_mock_category($category2, 'pre_course_category_delete');
$mockcategory2->delete_full();
// Verify passed arguments.
$arguments = mock_hooks::get_calling_arguments();
$this->assertCount(1, $arguments);
// Argument 1 is the category object.
$argument = array_shift($arguments);
$this->assertEquals($mockcategory2->get_db_record(), $argument);
}
public function test_pre_course_category_delete_move_hook() {
$category1 = \core_course_category::create(array('name' => 'Cat1'));
$category2 = \core_course_category::create(array('name' => 'Cat2', 'parent' => $category1->id));
$category3 = \core_course_category::create(array('name' => 'Cat3'));
$mockcategory2 = $this->get_mock_category($category2, 'pre_course_category_delete_move');
// Add course to mocked clone of category2.
$course1 = $this->getDataGenerator()->create_course(array('category' => $mockcategory2->id));
$mockcategory2->delete_move($category3->id);
// Verify passed arguments.
$arguments = mock_hooks::get_calling_arguments();
$this->assertCount(2, $arguments);
// Argument 1 is the same core_course_category instance.
$argument = array_shift($arguments);
$this->assertSame($mockcategory2, $argument);
// Argument 2 is referring to category 3.
$argument = array_shift($arguments);
$this->assertInstanceOf(\core_course_category::class, $argument);
$this->assertEquals($category3->id, $argument->id);
}
public function test_get_course_category_contents_hook() {
$category1 = \core_course_category::create(array('name' => 'Cat1'));
$category2 = \core_course_category::create(array('name' => 'Cat2', 'parent' => $category1->id));
$mockcategory2 = $this->get_mock_category($category2);
// Define get_plugins_callback_function use in the mock, it is called twice for different callback in the form.
$mockcategory2->expects($this->exactly(2))
->method('get_plugins_callback_function')
->withConsecutive(
[$this->equalTo('can_course_category_delete')],
[$this->equalTo('get_course_category_contents')]
)
->willReturn(
['tool_unittest_can_course_category_delete'],
['tool_unittest_get_course_category_contents']
);
// Now configure fixture to return string for the callback.
$content = 'Bunch of test artefacts';
mock_hooks::set_get_course_category_contents_return($content);
$mform = new \core_course_deletecategory_form(null, $mockcategory2);
$this->expectOutputRegex("/<li>$content<\/li>/");
$mform->display();
// Verify passed arguments.
$arguments = mock_hooks::get_calling_arguments();
$this->assertCount(1, $arguments);
// Argument 1 is the same core_course_category instance.
$argument = array_shift($arguments);
$this->assertSame($mockcategory2, $argument);
}
}
course/tests/fixtures/mock_hooks.php
Vendored
+184
View File
@@ -0,0 +1,184 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
/**
* Fixture for mocking callbacks used in \core_course_category
*
* @package core_course
* @category test
* @copyright 2020 Ruslan Kabalin
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
namespace tests\core_course {
/**
* Class mock_hooks
*
* @package core_course
* @category test
* @copyright 2020 Ruslan Kabalin
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class mock_hooks {
/** @var bool $cancoursecategorydelete */
private static $cancoursecategorydelete = true;
/** @var bool $cancoursecategorydeletemove */
private static $cancoursecategorydeletemove = true;
/** @var string $getcoursecategorycontents */
private static $getcoursecategorycontents = '';
/** @var array $callingarguments */
private static $callingarguments = [];
/**
* Set calling arguments.
*
* This is supposed to be used in the callbacks to store arguments passed to callback.
*
* @param array $callingarguments
*/
public static function set_calling_arguments($callingarguments) {
self::$callingarguments = $callingarguments;
}
/**
* Get calling arguments.
*
* This is supposed to be used in the test to verify arguments passed to callback.
* This method also reset stored calling arguments.
*
* @return array $callingarguments
*/
public static function get_calling_arguments() {
$callingarguments = self::$callingarguments;
self::$callingarguments = [];
return $callingarguments;
}
/**
* Get can_course_category_delete callback return.
*
* @return bool
*/
public static function get_can_course_category_delete_return() : bool {
return self::$cancoursecategorydelete;
}
/**
* Sets can_course_category_delete callback return.
*
* @param bool $return
*/
public static function set_can_course_category_delete_return(bool $return) {
self::$cancoursecategorydelete = $return;
}
/**
* Get can_course_category_delete_move callback return.
*
* @return bool
*/
public static function get_can_course_category_delete_move_return() : bool {
return self::$cancoursecategorydeletemove;
}
/**
* Sets can_course_category_delete_move callback return.
*
* @param bool $return
*/
public static function set_can_course_category_delete_move_return(bool $return) {
self::$cancoursecategorydeletemove = $return;
}
/**
* Get get_course_category_contents callback return.
*
* @return string
*/
public static function get_get_course_category_contents_return() : string {
return self::$getcoursecategorycontents;
}
/**
* Sets get_course_category_contents callback return.
*
* @param string $return
*/
public static function set_get_course_category_contents_return(string $return) {
self::$getcoursecategorycontents = $return;
}
}
}
namespace {
/**
* Test pre_course_category_delete callback.
*
* @param object $category
*/
function tool_unittest_pre_course_category_delete(object $category) {
\tests\core_course\mock_hooks::set_calling_arguments(func_get_args());
}
/**
* Test pre_course_category_delete_move callback.
*
* @param core_course_category $category
* @param core_course_category $newcategory
*/
function tool_unittest_pre_course_category_delete_move(core_course_category $category, core_course_category $newcategory) {
\tests\core_course\mock_hooks::set_calling_arguments(func_get_args());
}
/**
* Test can_course_category_delete callback.
*
* @param core_course_category $category
* @return bool
*/
function tool_unittest_can_course_category_delete(core_course_category $category) {
\tests\core_course\mock_hooks::set_calling_arguments(func_get_args());
return \tests\core_course\mock_hooks::get_can_course_category_delete_return();
}
/**
* Test can_course_category_delete_move callback.
*
* @param core_course_category $category
* @param core_course_category $newcategory
* @return bool
*/
function tool_unittest_can_course_category_delete_move(core_course_category $category, core_course_category $newcategory) {
\tests\core_course\mock_hooks::set_calling_arguments(func_get_args());
return \tests\core_course\mock_hooks::get_can_course_category_delete_move_return();
}
/**
* Test get_course_category_contents callback.
*
* @param core_course_category $category
* @return string
*/
function tool_unittest_get_course_category_contents(core_course_category $category) {
\tests\core_course\mock_hooks::set_calling_arguments(func_get_args());
return \tests\core_course\mock_hooks::get_get_course_category_contents_return();
}
}
lang/en/error.php
+1
View File
@@ -405,6 +405,7 @@ $string['moduledoesnotexist'] = 'This module does not exist';
$string['moduleinstancedoesnotexist'] = 'The instance of this module does not exist';
$string['modulemissingcode'] = 'Module {$a} is missing the code needed to perform this function';
$string['movecatcontentstoroot'] = 'Moving the category content to root is not allowed. You must move the contents to an existing category!';
$string['movecatcontentstoselected'] = 'Some of the category content can not be moved into selected category.';
$string['movecategorynotpossible'] = 'You cannot move category \'{$a}\' into the selected category.';
$string['movecategoryownparent'] = 'You cannot make category \'{$a}\' a parent of itself.';
$string['movecategoryparentconflict'] = 'You cannot make category \'{$a}\' a subcategory of one of its own subcategories.';
lib/classes/event/course_category_deleted.php
+6 -1
View File
@@ -33,6 +33,7 @@ defined('MOODLE_INTERNAL') || die();
* Extra information about event.
*
* - string name: category name.
* - string contentmovedcategoryid: (optional) category id where content was moved on deletion
* }
*
* @package core
@@ -71,7 +72,11 @@ class course_category_deleted extends base {
* @return string
*/
public function get_description() {
return "The user with id '$this->userid' deleted the course category with id '$this->objectid'.";
$descr = "The user with id '$this->userid' deleted the course category with id '$this->objectid'.";
if (!empty($this->other['contentmovedcategoryid'])) {
$descr .= " Its content has been moved to category with id '{$this->other['contentmovedcategoryid']}'.";
}
return $descr;
}
/**
lib/upgrade.txt
+14
View File
@@ -71,6 +71,20 @@ information provided here is intended especially for developers.
- cron_execute_plugin_type()
- cron_bc_hack_plugin_functions()
Please, use the Task API instead: https://docs.moodle.org/dev/Task_API
* Introduce new hooks for plugin developers:
- <component>_can_course_category_delete($category)
- <component>_can_course_category_delete_move($category, $newcategory)
These hooks allow plugin developers greater control over category deletion. Plugin can return false in those
functions if category deletion or deletion with content move to the new parent category is not permitted.
Both $category and $newcategory params are instances of core_course_category class.
- <component>_pre_course_category_delete_move($category, $newcategory)
This hook is expanding functionality of existing <component>_pre_course_category_delete hook and allow plugin developers
to execute code prior to category deletion when its content is moved to another category.
Both $category and $newcategory params are instances of core_course_category class.
- <component>_get_course_category_contents($category)
This hook allow plugin developers to add information that is displayed on category deletion form. Function should
return string, which will be added to the list of category contents shown on the form. $category param is an instance
of core_course_category class.
=== 3.8 ===
* Add CLI option to notify all cron tasks to stop: admin/cli/cron.php --stop