diff --git a/lib/questionlib.php b/lib/questionlib.php
index 4f0124c648a..1cbfa7b05a3 100644
--- a/lib/questionlib.php
+++ b/lib/questionlib.php
@@ -1606,61 +1606,40 @@ class context_to_string_translator{
/**
* Check capability on category
*
- * @param mixed $question object or id
- * @param string $cap 'add', 'edit', 'view', 'use', 'move'
- * @param integer $cachecat useful to cache all question records in a category
+ * @param mixed $questionorid object or id. If an object is passed, it should include ->contextid and ->createdby.
+ * @param string $cap 'add', 'edit', 'view', 'use', 'move' or 'tag'.
+ * @param integer $notused no longer used.
* @return boolean this user has the capability $cap for this question $question?
*/
-function question_has_capability_on($question, $cap, $cachecat = -1) {
- global $USER, $DB;
+function question_has_capability_on($questionorid, $cap, $notused = -1) {
+ global $USER;
+
+ if (is_numeric($questionorid)) {
+ $question = question_bank::load_question_data((int)$questionorid);
+ } else if (is_object($questionorid)) {
+ if (isset($questionorid->contextid) && isset($questionorid->createdby)) {
+ $question = $questionorid;
+ }
+
+ if (!isset($question) && isset($questionorid->id) && $questionorid->id != 0) {
+ $question = question_bank::load_question_data($questionorid->id);
+ }
+ } else {
+ throw new coding_exception('$questionorid parameter needs to be an integer or an object.');
+ }
+
+ $context = context::instance_by_id($question->contextid);
// these are capabilities on existing questions capabilties are
//set per category. Each of these has a mine and all version. Append 'mine' and 'all'
- $question_questioncaps = array('edit', 'view', 'use', 'move', 'tag');
- static $questions = array();
- static $categories = array();
- static $cachedcat = array();
- if ($cachecat != -1 && array_search($cachecat, $cachedcat) === false) {
- $questions += $DB->get_records('question', array('category' => $cachecat), '', 'id,category,createdby');
- $cachedcat[] = $cachecat;
- }
- if (!is_object($question)) {
- if (!isset($questions[$question])) {
- if (!$questions[$question] = $DB->get_record('question',
- array('id' => $question), 'id,category,createdby')) {
- print_error('questiondoesnotexist', 'question');
- }
- }
- $question = $questions[$question];
- }
- if (empty($question->category)) {
- // This can happen when we have created a fake 'missingtype' question to
- // take the place of a deleted question.
- return false;
- }
- if (!isset($categories[$question->category])) {
- if (!$categories[$question->category] = $DB->get_record('question_categories',
- array('id'=>$question->category))) {
- print_error('invalidcategory', 'question');
- }
- }
- $category = $categories[$question->category];
- $context = context::instance_by_id($category->contextid);
+ $capabilitieswithallandmine = ['edit' => 1, 'view' => 1, 'use' => 1, 'move' => 1, 'tag' => 1];
- if (array_search($cap, $question_questioncaps)!== false) {
- if (!has_capability('moodle/question:' . $cap . 'all', $context)) {
- if ($question->createdby == $USER->id) {
- return has_capability('moodle/question:' . $cap . 'mine', $context);
- } else {
- return false;
- }
- } else {
- return true;
- }
- } else {
+ if (!isset($capabilitieswithallandmine[$cap])) {
return has_capability('moodle/question:' . $cap, $context);
+ } else {
+ return has_capability('moodle/question:' . $cap . 'all', $context) ||
+ ($question->createdby == $USER->id && has_capability('moodle/question:' . $cap . 'mine', $context));
}
-
}
/**
diff --git a/lib/tests/questionlib_test.php b/lib/tests/questionlib_test.php
index 1d79dcdd320..b946524e552 100644
--- a/lib/tests/questionlib_test.php
+++ b/lib/tests/questionlib_test.php
@@ -1498,4 +1498,469 @@ class core_questionlib_testcase extends advanced_testcase {
}
}
+ /**
+ * Data provider for tests of question_has_capability_on_context and question_require_capability_on_context.
+ *
+ * @return array
+ */
+ public function question_capability_on_question_provider() {
+ return [
+ 'Unrelated capability which is present' => [
+ 'capabilities' => [
+ 'moodle/question:config' => CAP_ALLOW,
+ ],
+ 'testcapability' => 'config',
+ 'isowner' => true,
+ 'expect' => true,
+ ],
+ 'Unrelated capability which is present (not owner)' => [
+ 'capabilities' => [
+ 'moodle/question:config' => CAP_ALLOW,
+ ],
+ 'testcapability' => 'config',
+ 'isowner' => false,
+ 'expect' => true,
+ ],
+ 'Unrelated capability which is not set' => [
+ 'capabilities' => [
+ ],
+ 'testcapability' => 'config',
+ 'isowner' => true,
+ 'expect' => false,
+ ],
+ 'Unrelated capability which is not set (not owner)' => [
+ 'capabilities' => [
+ ],
+ 'testcapability' => 'config',
+ 'isowner' => false,
+ 'expect' => false,
+ ],
+ 'Unrelated capability which is prevented' => [
+ 'capabilities' => [
+ 'moodle/question:config' => CAP_PREVENT,
+ ],
+ 'testcapability' => 'config',
+ 'isowner' => true,
+ 'expect' => false,
+ ],
+ 'Unrelated capability which is prevented (not owner)' => [
+ 'capabilities' => [
+ 'moodle/question:config' => CAP_PREVENT,
+ ],
+ 'testcapability' => 'config',
+ 'isowner' => false,
+ 'expect' => false,
+ ],
+ 'Related capability which is not set' => [
+ 'capabilities' => [
+ ],
+ 'testcapability' => 'edit',
+ 'isowner' => true,
+ 'expect' => false,
+ ],
+ 'Related capability which is not set (not owner)' => [
+ 'capabilities' => [
+ ],
+ 'testcapability' => 'edit',
+ 'isowner' => false,
+ 'expect' => false,
+ ],
+ 'Related capability which is allowed at all, unset at mine' => [
+ 'capabilities' => [
+ 'moodle/question:editall' => CAP_ALLOW,
+ ],
+ 'testcapability' => 'edit',
+ 'isowner' => true,
+ 'expect' => true,
+ ],
+ 'Related capability which is allowed at all, unset at mine (not owner)' => [
+ 'capabilities' => [
+ 'moodle/question:editall' => CAP_ALLOW,
+ ],
+ 'testcapability' => 'edit',
+ 'isowner' => false,
+ 'expect' => true,
+ ],
+ 'Related capability which is allowed at all, prevented at mine' => [
+ 'capabilities' => [
+ 'moodle/question:editall' => CAP_ALLOW,
+ 'moodle/question:editmine' => CAP_PREVENT,
+ ],
+ 'testcapability' => 'edit',
+ 'isowner' => true,
+ 'expect' => true,
+ ],
+ 'Related capability which is allowed at all, prevented at mine (not owner)' => [
+ 'capabilities' => [
+ 'moodle/question:editall' => CAP_ALLOW,
+ 'moodle/question:editmine' => CAP_PREVENT,
+ ],
+ 'testcapability' => 'edit',
+ 'isowner' => false,
+ 'expect' => true,
+ ],
+ 'Related capability which is unset all, allowed at mine' => [
+ 'capabilities' => [
+ 'moodle/question:editall' => CAP_PREVENT,
+ 'moodle/question:editmine' => CAP_ALLOW,
+ ],
+ 'testcapability' => 'edit',
+ 'isowner' => true,
+ 'expect' => true,
+ ],
+ 'Related capability which is unset all, allowed at mine (not owner)' => [
+ 'capabilities' => [
+ 'moodle/question:editall' => CAP_PREVENT,
+ 'moodle/question:editmine' => CAP_ALLOW,
+ ],
+ 'testcapability' => 'edit',
+ 'isowner' => false,
+ 'expect' => false,
+ ],
+ ];
+ }
+
+ /**
+ * Tests for the deprecated question_has_capability_on function when passing a stdClass.
+ *
+ * @dataProvider question_capability_on_question_provider
+ * @param array $capabilities The capability assignments to set.
+ * @param string $capability The capability to test
+ * @param bool $expectall The expectation when passing false to checkmine.
+ * @param bool $expectmine The expectation when passing true to checkmine.
+ */
+ public function test_question_has_capability_on_using_stdClass($capabilities, $capability, $isowner, $expect) {
+ $this->resetAfterTest();
+
+ // Create the test data.
+ $user = $this->getDataGenerator()->create_user();
+ $otheruser = $this->getDataGenerator()->create_user();
+ $roleid = $this->getDataGenerator()->create_role();
+ $category = $this->getDataGenerator()->create_category();
+ $context = context_coursecat::instance($category->id);
+
+ // Assign the user to the role.
+ role_assign($roleid, $user->id, $context->id);
+
+ // Assign the capabilities to the role.
+ foreach ($capabilities as $capname => $capvalue) {
+ assign_capability($capname, $capvalue, $roleid, $context->id);
+ }
+ $context->mark_dirty();
+
+ $this->setUser($user);
+
+ // The current fake question we make use of is always a stdClass and typically has no ID.
+ $fakequestion = (object) [
+ 'contextid' => $context->id,
+ ];
+
+ if ($isowner) {
+ $fakequestion->createdby = $user->id;
+ } else {
+ $fakequestion->createdby = $otheruser->id;
+ }
+
+ $result = question_has_capability_on($fakequestion, $capability);
+ $this->assertEquals($expect, $result);
+ }
+
+ /**
+ * Tests for the deprecated question_has_capability_on function when using a real question.
+ *
+ * @dataProvider question_capability_on_question_provider
+ * @param array $capabilities The capability assignments to set.
+ * @param string $capability The capability to test
+ * @param bool $expectall The expectation when passing false to checkmine.
+ * @param bool $expectmine The expectation when passing true to checkmine.
+ */
+ public function test_question_has_capability_on_using_question_definition($capabilities, $capability, $isowner, $expect) {
+ $this->resetAfterTest();
+
+ // Create the test data.
+ $generator = $this->getDataGenerator();
+ $questiongenerator = $generator->get_plugin_generator('core_question');
+ $user = $generator->create_user();
+ $otheruser = $generator->create_user();
+ $roleid = $generator->create_role();
+ $category = $generator->create_category();
+ $context = context_coursecat::instance($category->id);
+ $questioncat = $questiongenerator->create_question_category([
+ 'contextid' => $context->id,
+ ]);
+
+ // Assign the user to the role.
+ role_assign($roleid, $user->id, $context->id);
+
+ // Assign the capabilities to the role.
+ foreach ($capabilities as $capname => $capvalue) {
+ assign_capability($capname, $capvalue, $roleid, $context->id);
+ }
+ $context->mark_dirty();
+
+ // Create the question.
+ $qtype = 'truefalse';
+ $overrides = [
+ 'category' => $questioncat->id,
+ ];
+
+ $question = $questiongenerator->create_question($qtype, null, $overrides);
+
+ // The question generator does not support setting of the createdby for some reason.
+ $question->createdby = ($isowner) ? $user->id : $otheruser->id;
+ $fromform = test_question_maker::get_question_form_data($qtype, null);
+ $fromform = (object) $generator->combine_defaults_and_record((array) $fromform, $overrides);
+ question_bank::get_qtype($qtype)->save_question($question, $fromform);
+
+ $this->setUser($user);
+ $result = question_has_capability_on($question, $capability);
+ $this->assertEquals($expect, $result);
+ }
+
+ /**
+ * Tests for the deprecated question_has_capability_on function when using a real question.
+ *
+ * @dataProvider question_capability_on_question_provider
+ * @param array $capabilities The capability assignments to set.
+ * @param string $capability The capability to test
+ * @param bool $expectall The expectation when passing false to checkmine.
+ * @param bool $expectmine The expectation when passing true to checkmine.
+ */
+ public function test_question_has_capability_on_using_question_id($capabilities, $capability, $isowner, $expect) {
+ $this->resetAfterTest();
+
+ // Create the test data.
+ $generator = $this->getDataGenerator();
+ $questiongenerator = $generator->get_plugin_generator('core_question');
+ $user = $generator->create_user();
+ $otheruser = $generator->create_user();
+ $roleid = $generator->create_role();
+ $category = $generator->create_category();
+ $context = context_coursecat::instance($category->id);
+ $questioncat = $questiongenerator->create_question_category([
+ 'contextid' => $context->id,
+ ]);
+
+ // Assign the user to the role.
+ role_assign($roleid, $user->id, $context->id);
+
+ // Assign the capabilities to the role.
+ foreach ($capabilities as $capname => $capvalue) {
+ assign_capability($capname, $capvalue, $roleid, $context->id);
+ }
+ $context->mark_dirty();
+
+ // Create the question.
+ $qtype = 'truefalse';
+ $overrides = [
+ 'category' => $questioncat->id,
+ ];
+
+ $question = $questiongenerator->create_question($qtype, null, $overrides);
+
+ // The question generator does not support setting of the createdby for some reason.
+ $question->createdby = ($isowner) ? $user->id : $otheruser->id;
+ $fromform = test_question_maker::get_question_form_data($qtype, null);
+ $fromform = (object) $generator->combine_defaults_and_record((array) $fromform, $overrides);
+ question_bank::get_qtype($qtype)->save_question($question, $fromform);
+
+ $this->setUser($user);
+ $result = question_has_capability_on($question->id, $capability);
+ $this->assertEquals($expect, $result);
+ }
+
+ /**
+ * Tests for the deprecated question_has_capability_on function when using a real question.
+ *
+ * @dataProvider question_capability_on_question_provider
+ * @param array $capabilities The capability assignments to set.
+ * @param string $capability The capability to test
+ * @param bool $expectall The expectation when passing false to checkmine.
+ * @param bool $expectmine The expectation when passing true to checkmine.
+ */
+ public function test_question_has_capability_on_using_question_string_id($capabilities, $capability, $isowner, $expect) {
+ $this->resetAfterTest();
+
+ // Create the test data.
+ $generator = $this->getDataGenerator();
+ $questiongenerator = $generator->get_plugin_generator('core_question');
+ $user = $generator->create_user();
+ $otheruser = $generator->create_user();
+ $roleid = $generator->create_role();
+ $category = $generator->create_category();
+ $context = context_coursecat::instance($category->id);
+ $questioncat = $questiongenerator->create_question_category([
+ 'contextid' => $context->id,
+ ]);
+
+ // Assign the user to the role.
+ role_assign($roleid, $user->id, $context->id);
+
+ // Assign the capabilities to the role.
+ foreach ($capabilities as $capname => $capvalue) {
+ assign_capability($capname, $capvalue, $roleid, $context->id);
+ }
+ $context->mark_dirty();
+
+ // Create the question.
+ $qtype = 'truefalse';
+ $overrides = [
+ 'category' => $questioncat->id,
+ ];
+
+ $question = $questiongenerator->create_question($qtype, null, $overrides);
+
+ // The question generator does not support setting of the createdby for some reason.
+ $question->createdby = ($isowner) ? $user->id : $otheruser->id;
+ $fromform = test_question_maker::get_question_form_data($qtype, null);
+ $fromform = (object) $generator->combine_defaults_and_record((array) $fromform, $overrides);
+ question_bank::get_qtype($qtype)->save_question($question, $fromform);
+
+ $this->setUser($user);
+ $result = question_has_capability_on((string) $question->id, $capability);
+ $this->assertEquals($expect, $result);
+ }
+
+ /**
+ * Tests for the deprecated question_has_capability_on function when using a real question.
+ *
+ * @dataProvider question_capability_on_question_provider
+ * @param array $capabilities The capability assignments to set.
+ * @param string $capability The capability to test
+ * @param bool $expectall The expectation when passing false to checkmine.
+ * @param bool $expectmine The expectation when passing true to checkmine.
+ */
+ public function test_question_has_capability_on_using_moved_question($capabilities, $capability, $isowner, $expect) {
+ $this->resetAfterTest();
+
+ // Create the test data.
+ $generator = $this->getDataGenerator();
+ $questiongenerator = $generator->get_plugin_generator('core_question');
+ $user = $generator->create_user();
+ $otheruser = $generator->create_user();
+ $roleid = $generator->create_role();
+ $category = $generator->create_category();
+ $context = context_coursecat::instance($category->id);
+ $questioncat = $questiongenerator->create_question_category([
+ 'contextid' => $context->id,
+ ]);
+
+ $newcategory = $generator->create_category();
+ $newcontext = context_coursecat::instance($newcategory->id);
+ $newquestioncat = $questiongenerator->create_question_category([
+ 'contextid' => $newcontext->id,
+ ]);
+
+ // Assign the user to the role in the _new_ context..
+ role_assign($roleid, $user->id, $newcontext->id);
+
+ // Assign the capabilities to the role in the _new_ context.
+ foreach ($capabilities as $capname => $capvalue) {
+ assign_capability($capname, $capvalue, $roleid, $newcontext->id);
+ }
+ $context->mark_dirty();
+ $newcontext->mark_dirty();
+
+ // Create the question.
+ $qtype = 'truefalse';
+ $overrides = [
+ 'category' => $questioncat->id,
+ ];
+
+ $question = $questiongenerator->create_question($qtype, null, $overrides);
+
+ // The question generator does not support setting of the createdby for some reason.
+ $question->createdby = ($isowner) ? $user->id : $otheruser->id;
+ $fromform = test_question_maker::get_question_form_data($qtype, null);
+ $fromform = (object) $generator->combine_defaults_and_record((array) $fromform, $overrides);
+ question_bank::get_qtype($qtype)->save_question($question, $fromform);
+
+ // Move the question.
+ question_move_questions_to_category([$question->id], $newquestioncat->id);
+
+ // Test that the capability is correct after the question has been moved.
+ $this->setUser($user);
+ $result = question_has_capability_on($question->id, $capability);
+ $this->assertEquals($expect, $result);
+ }
+
+ /**
+ * Tests for the deprecated question_has_capability_on function when using a real question.
+ *
+ * @dataProvider question_capability_on_question_provider
+ * @param array $capabilities The capability assignments to set.
+ * @param string $capability The capability to test
+ * @param bool $isowner The expectation when passing false to checkmine.
+ * @param bool $expectmine The expectation when passing true to checkmine.
+ */
+ public function test_question_has_capability_on_using_question($capabilities, $capability, $isowner, $expect) {
+ $this->resetAfterTest();
+
+ // Create the test data.
+ $generator = $this->getDataGenerator();
+ $questiongenerator = $generator->get_plugin_generator('core_question');
+ $user = $generator->create_user();
+ $otheruser = $generator->create_user();
+ $roleid = $generator->create_role();
+ $category = $generator->create_category();
+ $context = context_coursecat::instance($category->id);
+ $questioncat = $questiongenerator->create_question_category([
+ 'contextid' => $context->id,
+ ]);
+
+ // Assign the user to the role.
+ role_assign($roleid, $user->id, $context->id);
+
+ // Assign the capabilities to the role.
+ foreach ($capabilities as $capname => $capvalue) {
+ assign_capability($capname, $capvalue, $roleid, $context->id);
+ }
+ $context->mark_dirty();
+
+ // Create the question.
+ $question = $questiongenerator->create_question('truefalse', null, [
+ 'category' => $questioncat->id,
+ ]);
+ $question = question_bank::load_question_data($question->id);
+
+ // The question generator does not support setting of the createdby for some reason.
+ $question->createdby = ($isowner) ? $user->id : $otheruser->id;
+
+ $this->setUser($user);
+ $result = question_has_capability_on($question, $capability);
+ $this->assertEquals($expect, $result);
+ }
+
+ /**
+ * Tests that question_has_capability_on throws an exception for wrong parameter types.
+ */
+ public function test_question_has_capability_on_wrong_param_type() {
+ // Create the test data.
+ $generator = $this->getDataGenerator();
+ $questiongenerator = $generator->get_plugin_generator('core_question');
+ $user = $generator->create_user();
+
+ $category = $generator->create_category();
+ $context = context_coursecat::instance($category->id);
+ $questioncat = $questiongenerator->create_question_category([
+ 'contextid' => $context->id,
+ ]);
+
+ // Create the question.
+ $question = $questiongenerator->create_question('truefalse', null, [
+ 'category' => $questioncat->id,
+ ]);
+ $question = question_bank::load_question_data($question->id);
+
+ // The question generator does not support setting of the createdby for some reason.
+ $question->createdby = $user->id;
+
+ $this->setUser($user);
+ $result = question_has_capability_on((string)$question->id, 'tag');
+ $this->assertFalse($result);
+
+ $this->expectException('coding_exception');
+ $this->expectExceptionMessage('$questionorid parameter needs to be an integer or an object.');
+ question_has_capability_on('one', 'tag');
+ }
}
diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php
index 9350ec95985..888addf7b09 100644
--- a/mod/quiz/locallib.php
+++ b/mod/quiz/locallib.php
@@ -1333,12 +1333,12 @@ function quiz_question_edit_button($cmid, $question, $returnurl, $contentafteric
// What sort of icon should we show?
$action = '';
if (!empty($question->id) &&
- (question_has_capability_on($question, 'edit', $question->category) ||
- question_has_capability_on($question, 'move', $question->category))) {
+ (question_has_capability_on($question, 'edit') ||
+ question_has_capability_on($question, 'move'))) {
$action = $stredit;
$icon = 't/edit';
} else if (!empty($question->id) &&
- question_has_capability_on($question, 'view', $question->category)) {
+ question_has_capability_on($question, 'view')) {
$action = $strview;
$icon = 'i/info';
}
@@ -1390,7 +1390,7 @@ function quiz_question_preview_url($quiz, $question, $variant = null) {
*/
function quiz_question_preview_button($quiz, $question, $label = false, $variant = null) {
global $PAGE;
- if (!question_has_capability_on($question, 'use', $question->category)) {
+ if (!question_has_capability_on($question, 'use')) {
return '';
}
diff --git a/question/amd/build/edit_tags.min.js b/question/amd/build/edit_tags.min.js
index cb3dc1915b0..f452866bfe7 100644
--- a/question/amd/build/edit_tags.min.js
+++ b/question/amd/build/edit_tags.min.js
@@ -1 +1 @@
-define(["jquery","core/fragment","core/str","core/modal_events","core/modal_factory","core/notification","core/custom_interaction_events","core_question/repository","core_question/selectors"],function(a,b,c,d,e,f,g,h,i){var j=function(a){a.find(i.actions.save).prop("disabled",!1)},k=function(a){a.find(i.actions.save).prop("disabled",!0)},l=function(a){return a.getBody().find("form").serialize()},m=function(a){var b=a.find(i.containers.loadingIcon);b.removeClass("hidden")},n=function(a){var b=a.find(i.containers.loadingIcon);b.addClass("hidden")},o=function(h){var l=e.create({type:e.types.SAVE_CANCEL,large:!1},[h,i.actions.edittags]).then(function(a){return c.get_string("questiontags","question").then(function(b){return a.setTitle(b),b}).fail(f.exception),a.getRoot().on(d.save,function(b){var c=a.getBody().find("form");c.submit(),b.preventDefault()}),a.getRoot().on("submit","form",function(b){p(a,h).then(function(){a.hide()}).fail(f.exception),b.preventDefault(),b.stopPropagation()}),a});h.on(g.events.activate,i.actions.edittags,function(c){var d=a(c.currentTarget),e=d.data("questionid"),g=!!d.data("cantag"),o=d.data("contextid");l.then(function(a){k(h),m(h);var c={id:e},d=b.loadFragment("question","tags_form",o,c);return a.setBody(d),d.then(function(){j(h)}).always(function(){n(h)}).fail(f.exception),g?a.getRoot().find(i.actions.save).show():a.getRoot().find(i.actions.save).hide(),a}).fail(f.exception),c.preventDefault()})},p=function(a,b){k(b),m(b);var c=l(a);return h.submitTagCreateUpdateForm(c).always(function(){n(b),j(b)}).fail(f.exception)};return{init:function(b){b=a(b),o(b)}}});
\ No newline at end of file
+define(["jquery","core/fragment","core/str","core/modal_events","core/modal_factory","core/notification","core/custom_interaction_events","core_question/repository","core_question/selectors"],function(a,b,c,d,e,f,g,h,i){var j=function(a){a.find(i.actions.save).prop("disabled",!1)},k=function(a){a.find(i.actions.save).prop("disabled",!0)},l=function(a){return a.getBody().find("form").serialize()},m=function(a){var b=a.find(i.containers.loadingIcon);b.removeClass("hidden")},n=function(a){var b=a.find(i.containers.loadingIcon);b.addClass("hidden")},o=function(a,b){a.getBody().attr("data-contextid",b)},p=function(a){return a.getBody().data("contextid")},q=function(a,b){a.getBody().attr("data-questionid",b)},r=function(a){return a.getBody().data("questionid")},s=function(h){var l=e.create({type:e.types.SAVE_CANCEL,large:!1},[h,i.actions.edittags]).then(function(a){return c.get_string("questiontags","question").then(function(b){return a.setTitle(b),b}).fail(f.exception),a.getRoot().on(d.save,function(b){var c=a.getBody().find("form");c.submit(),b.preventDefault()}),a.getRoot().on("submit","form",function(b){t(a,h).then(function(){a.hide()}).fail(f.exception),b.preventDefault(),b.stopPropagation()}),a});h.on(g.events.activate,i.actions.edittags,function(c){var d=a(c.currentTarget),e=d.data("questionid"),g=!!d.data("cantag"),p=d.data("contextid");l.then(function(a){k(h),m(h);var c={id:e},d=b.loadFragment("question","tags_form",p,c);return a.setBody(d),d.then(function(){j(h)}).always(function(){n(h)}).fail(f.exception),g?a.getRoot().find(i.actions.save).show():a.getRoot().find(i.actions.save).hide(),q(a,e),o(a,p),a}).fail(f.exception),c.preventDefault()})},t=function(a,b){k(b),m(b);var c=l(a),d=r(a),e=p(a);return h.submitTagCreateUpdateForm(d,e,c).always(function(){n(b),j(b)}).fail(f.exception)};return{init:function(b){b=a(b),s(b)}}});
\ No newline at end of file
diff --git a/question/amd/build/repository.min.js b/question/amd/build/repository.min.js
index 35de00b2255..b86aedc4656 100644
--- a/question/amd/build/repository.min.js
+++ b/question/amd/build/repository.min.js
@@ -1 +1 @@
-define(["jquery","core/ajax"],function(a,b){var c=function(a){var c={methodname:"core_question_submit_tags_form",args:{formdata:a}};return b.call([c])[0]};return{submitTagCreateUpdateForm:c}});
\ No newline at end of file
+define(["jquery","core/ajax"],function(a,b){var c=function(a,c,d){var e={methodname:"core_question_submit_tags_form",args:{questionid:a,contextid:c,formdata:d}};return b.call([e])[0]};return{submitTagCreateUpdateForm:c}});
\ No newline at end of file
diff --git a/question/amd/src/edit_tags.js b/question/amd/src/edit_tags.js
index 6bfabe03822..f4aabc1fa67 100644
--- a/question/amd/src/edit_tags.js
+++ b/question/amd/src/edit_tags.js
@@ -98,6 +98,46 @@ define([
loadingIconContainer.addClass('hidden');
};
+ /**
+ * Set the context Id data attribute on the modal.
+ *
+ * @param {Promise} modal The modal promise.
+ * @param {int} contextId The context id.
+ */
+ var setContextId = function(modal, contextId) {
+ modal.getBody().attr('data-contextid', contextId);
+ };
+
+ /**
+ * Get the context Id data attribute value from the modal body.
+ *
+ * @param {Promise} modal The modal promise.
+ * @return {int} The context id.
+ */
+ var getContextId = function(modal) {
+ return modal.getBody().data('contextid');
+ };
+
+ /**
+ * Set the question Id data attribute on the modal.
+ *
+ * @param {Promise} modal The modal promise.
+ * @param {int} questionId The question Id.
+ */
+ var setQuestionId = function(modal, questionId) {
+ modal.getBody().attr('data-questionid', questionId);
+ };
+
+ /**
+ * Get the question Id data attribute value from the modal body.
+ *
+ * @param {Promise} modal The modal promise.
+ * @return {int} The question Id.
+ */
+ var getQuestionId = function(modal) {
+ return modal.getBody().data('questionid');
+ };
+
/**
* Register event listeners for the module.
*
@@ -186,6 +226,9 @@ define([
modal.getRoot().find(QuestionSelectors.actions.save).hide();
}
+ setQuestionId(modal, questionId);
+ setContextId(modal, contextId);
+
return modal;
}).fail(Notification.exception);
@@ -207,9 +250,11 @@ define([
startLoading(root);
var formData = getFormData(modal);
+ var questionId = getQuestionId(modal);
+ var contextId = getContextId(modal);
// Send the form data to the server for processing.
- return Repository.submitTagCreateUpdateForm(formData)
+ return Repository.submitTagCreateUpdateForm(questionId, contextId, formData)
.always(function() {
// Regardless of success or error we should always stop
// the loading icon and re-enable the buttons.
diff --git a/question/amd/src/repository.js b/question/amd/src/repository.js
index 42e72726bf4..6848577777d 100644
--- a/question/amd/src/repository.js
+++ b/question/amd/src/repository.js
@@ -31,10 +31,12 @@ define(['jquery', 'core/ajax'], function($, Ajax) {
* @param {string} formdata The URL encoded values from the form
* @return {promise}
*/
- var submitTagCreateUpdateForm = function(formdata) {
+ var submitTagCreateUpdateForm = function(questionId, contextId, formdata) {
var request = {
methodname: 'core_question_submit_tags_form',
args: {
+ questionid: questionId,
+ contextid: contextId,
formdata: formdata
}
};
diff --git a/question/classes/bank/tags_action_column.php b/question/classes/bank/tags_action_column.php
index 7ff372bb048..79a7cbca5b7 100644
--- a/question/classes/bank/tags_action_column.php
+++ b/question/classes/bank/tags_action_column.php
@@ -56,10 +56,11 @@ class tags_action_column extends action_column_base {
question_has_capability_on($question, 'view')) {
$cantag = question_has_capability_on($question, 'tag');
- $category = $DB->get_record('question_categories', ['id' => $question->category], 'contextid');
- $url = $this->qbank->edit_question_url($question->id);
+ $qbank = $this->qbank;
+ $url = $qbank->edit_question_url($question->id);
+ $editingcontext = $qbank->get_most_specific_context();
- $this->print_tag_icon($question->id, $url, $cantag, $category->contextid);
+ $this->print_tag_icon($question->id, $url, $cantag, $editingcontext->id);
}
}
diff --git a/question/classes/bank/view.php b/question/classes/bank/view.php
index e8df555b457..518e36c0687 100644
--- a/question/classes/bank/view.php
+++ b/question/classes/bank/view.php
@@ -929,7 +929,7 @@ class view {
if (preg_match('!^q([0-9]+)$!', $key, $matches)) {
$key = $matches[1];
$questionlist .= $key.',';
- question_require_capability_on($key, 'edit');
+ question_require_capability_on((int)$key, 'edit');
if (questions_in_use(array($key))) {
$questionnames .= '* ';
$inuse = true;
diff --git a/question/classes/external.php b/question/classes/external.php
index ecc48767739..f2011d45e6b 100644
--- a/question/classes/external.php
+++ b/question/classes/external.php
@@ -122,6 +122,8 @@ class core_question_external extends external_api {
*/
public static function submit_tags_form_parameters() {
return new external_function_parameters([
+ 'questionid' => new external_value(PARAM_INT, 'The question id'),
+ 'contextid' => new external_value(PARAM_INT, 'The editing context id'),
'formdata' => new external_value(PARAM_RAW, 'The data from the tag form'),
]);
}
@@ -129,51 +131,67 @@ class core_question_external extends external_api {
/**
* Handles the tags form submission.
*
+ * @param int $questionid The question id.
+ * @param int $contextid The editing context id.
* @param string $formdata The question tag form data in a URI encoded param string
* @return array The created or modified question tag
- * @throws moodle_exception
*/
- public static function submit_tags_form($formdata) {
- global $USER, $DB, $CFG;
+ public static function submit_tags_form($questionid, $contextid, $formdata) {
+ global $DB, $CFG;
$data = [];
$result = ['status' => false];
// Parameter validation.
- $params = self::validate_parameters(self::submit_tags_form_parameters(), ['formdata' => $formdata]);
- $context = \context_user::instance($USER->id);
+ $params = self::validate_parameters(self::submit_tags_form_parameters(), [
+ 'questionid' => $questionid,
+ 'contextid' => $contextid,
+ 'formdata' => $formdata
+ ]);
- self::validate_context($context);
+ $editingcontext = \context::instance_by_id($contextid);
+ self::validate_context($editingcontext);
parse_str($params['formdata'], $data);
- if (!empty($data['id'])) {
- $questionid = clean_param($data['id'], PARAM_INT);
- $question = $DB->get_record('question', array('id' => $questionid));
+ if (!$question = $DB->get_record_sql('
+ SELECT q.*, qc.contextid
+ FROM {question} q
+ JOIN {question_categories} qc ON qc.id = q.category
+ WHERE q.id = ?', [$questionid])) {
+ print_error('questiondoesnotexist', 'question');
+ }
- require_once($CFG->libdir . '/questionlib.php');
- $cantag = question_has_capability_on($question, 'tag');
+ require_once($CFG->libdir . '/questionlib.php');
+ require_once($CFG->dirroot . '/question/type/tags_form.php');
- require_once($CFG->dirroot . '/question/type/tags_form.php');
- $mform = new \core_question\form\tags(null, null, 'post', '', null, $cantag, $data);
+ $cantag = question_has_capability_on($question, 'tag');
+ $questioncontext = \context::instance_by_id($question->contextid);
+ $formoptions = [
+ 'editingcontext' => $editingcontext,
+ 'questioncontext' => $questioncontext
+ ];
- if ($validateddata = $mform->get_data()) {
- // Due to a mform bug, if there's no tags set on the tag element, it submits the name as the value.
- // The only way to discover is checking if the tag element is an array.
- if ($cantag) {
- if (is_array($validateddata->tags)) {
- $categorycontext = context::instance_by_id($validateddata->contextid);
+ $mform = new \core_question\form\tags(null, $formoptions, 'post', '', null, $cantag, $data);
- core_tag_tag::set_item_tags('core_question', 'question', $validateddata->id,
- $categorycontext, $validateddata->tags);
+ if ($validateddata = $mform->get_data()) {
+ if ($cantag) {
+ if (isset($validateddata->tags)) {
+ // Due to a mform bug, if there's no tags set on the tag element, it submits the name as the value.
+ // The only way to discover is checking if the tag element is an array.
+ $tags = is_array($validateddata->tags) ? $validateddata->tags : [];
- $result['status'] = true;
- } else {
- // If the tags element is not array, this means we don't have any tags to be set.
- // This is the only way to assume the user removed all tags from the question.
- core_tag_tag::remove_all_item_tags('core_question', 'question', $validateddata->id);
+ core_tag_tag::set_item_tags('core_question', 'question', $validateddata->id,
+ $questioncontext, $tags);
- $result['status'] = true;
- }
+ $result['status'] = true;
+ }
+
+ if (isset($validateddata->coursetags)) {
+ $coursetags = is_array($validateddata->coursetags) ? $validateddata->coursetags : [];
+ core_tag_tag::set_item_tags('core_question', 'question', $validateddata->id,
+ $editingcontext->get_course_context(false), $coursetags);
+
+ $result['status'] = true;
}
}
}
diff --git a/question/engine/tests/helpers.php b/question/engine/tests/helpers.php
index 8a89633b879..0614f606779 100644
--- a/question/engine/tests/helpers.php
+++ b/question/engine/tests/helpers.php
@@ -119,6 +119,7 @@ abstract class question_test_helper {
$catcontext = context::instance_by_id($cat->contextid, MUST_EXIST);
$contexts = new question_edit_contexts($catcontext);
$dataforformconstructor = new stdClass();
+ $dataforformconstructor->createdby = $questiondata->createdby;
$dataforformconstructor->qtype = $questiondata->qtype;
$dataforformconstructor->contextid = $questiondata->contextid = $catcontext->id;
$dataforformconstructor->category = $questiondata->category = $cat->id;
diff --git a/question/format.php b/question/format.php
index 08689f5b700..a3257cf8694 100644
--- a/question/format.php
+++ b/question/format.php
@@ -857,7 +857,7 @@ class qformat_default {
// export the question displaying message
$count++;
- if (question_has_capability_on($question, 'view', $question->category)) {
+ if (question_has_capability_on($question, 'view')) {
$expout .= $this->writequestion($question, $contextid) . "\n";
}
}
diff --git a/question/lib.php b/question/lib.php
index 842e050a95e..5a731c492c2 100644
--- a/question/lib.php
+++ b/question/lib.php
@@ -41,26 +41,42 @@ function core_question_output_fragment_tags_form($args) {
require_once($CFG->dirroot . '/question/type/tags_form.php');
require_once($CFG->libdir . '/questionlib.php');
$id = clean_param($args['id'], PARAM_INT);
+ $editingcontext = $args['context'];
$question = $DB->get_record('question', ['id' => $id]);
- $category = $DB->get_record('question_categories', array('id' => $question->category));
- $context = \context::instance_by_id($category->contextid);
- $toform = new stdClass();
- $toform->id = $question->id;
- $toform->questioncategory = $category->name;
- $toform->questionname = $question->name;
- $toform->categoryid = $category->id;
- $toform->contextid = $category->contextid;
- $toform->context = $context->get_context_name();
-
- if (core_tag_tag::is_enabled('core_question', 'question')) {
- $toform->tags = core_tag_tag::get_item_tags_array('core_question', 'question', $question->id);
+ if ($coursecontext = $editingcontext->get_course_context(false)) {
+ $course = $DB->get_record('course', ['id' => $coursecontext->instanceid]);
+ $filtercourses = [$course];
+ } else {
+ $filtercourses = null;
}
+ // Load the question tags and filter the course tags by the current
+ // course.
+ get_question_options($question, true, $filtercourses);
+
+ $category = $question->categoryobject;
+ $questioncontext = \context::instance_by_id($category->contextid);
+
+ $formoptions = [
+ 'editingcontext' => $editingcontext,
+ 'questioncontext' => $questioncontext
+ ];
+ $data = [
+ 'id' => $question->id,
+ 'questioncategory' => $category->name,
+ 'questionname' => $question->name,
+ 'categoryid' => $category->id,
+ 'contextid' => $category->contextid,
+ 'context' => $questioncontext->get_context_name(),
+ 'tags' => isset($question->tags) ? $question->tags : [],
+ 'coursetags' => isset($question->coursetags) ? $question->coursetags : [],
+ ];
+
$cantag = question_has_capability_on($question, 'tag');
- $mform = new \core_question\form\tags(null, null, 'post', '', null, $cantag, $toform);
- $mform->set_data($toform);
+ $mform = new \core_question\form\tags(null, $formoptions, 'post', '', null, $cantag, $data);
+ $mform->set_data($data);
return $mform->render();
}
diff --git a/question/question.php b/question/question.php
index acbabcec399..2a02d92b126 100644
--- a/question/question.php
+++ b/question/question.php
@@ -162,6 +162,7 @@ if (isset($question->categoryobject)) {
$question->formoptions = new stdClass();
$categorycontext = context::instance_by_id($category->contextid);
+$question->contextid = $category->contextid;
$addpermission = has_capability('moodle/question:add', $categorycontext);
if ($id) {
diff --git a/question/tests/externallib_test.php b/question/tests/externallib_test.php
index 5fcf3989f5c..9d297b40503 100644
--- a/question/tests/externallib_test.php
+++ b/question/tests/externallib_test.php
@@ -102,4 +102,374 @@ class core_question_external_testcase extends externallib_advanced_testcase {
$this->assertEquals('errorsavingflags', $e->errorcode);
}
}
-}
\ No newline at end of file
+
+ /**
+ * submit_tags_form should throw an exception when the question id doesn't match
+ * a question.
+ */
+ public function test_submit_tags_form_incorrect_question_id() {
+ $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
+ list ($category, $course, $qcat, $questions) = $questiongenerator->setup_course_and_questions();
+ $questioncontext = context::instance_by_id($qcat->contextid);
+ $editingcontext = $questioncontext;
+ $question = $questions[0];
+ // Generate an id for a question that doesn't exist.
+ $missingquestionid = $questions[1]->id * 2;
+ $question->id = $missingquestionid;
+ $formdata = $this->generate_encoded_submit_tags_form_string($question, $qcat, $questioncontext, [], []);
+
+ // We should receive an exception if the question doesn't exist.
+ $this->expectException('moodle_exception');
+ core_question_external::submit_tags_form($missingquestionid, $editingcontext->id, $formdata);
+ }
+
+ /**
+ * submit_tags_form should throw an exception when the context id doesn't match
+ * a context.
+ */
+ public function test_submit_tags_form_incorrect_context_id() {
+ $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
+ list ($category, $course, $qcat, $questions) = $questiongenerator->setup_course_and_questions();
+ $questioncontext = context::instance_by_id($qcat->contextid);
+ $editingcontext = $questioncontext;
+ $question = $questions[0];
+ // Generate an id for a context that doesn't exist.
+ $missingcontextid = $editingcontext->id * 200;
+ $formdata = $this->generate_encoded_submit_tags_form_string($question, $qcat, $questioncontext, [], []);
+
+ // We should receive an exception if the question doesn't exist.
+ $this->expectException('moodle_exception');
+ core_question_external::submit_tags_form($question->id, $missingcontextid, $formdata);
+ }
+
+ /**
+ * submit_tags_form should return false when tags are disabled.
+ */
+ public function test_submit_tags_form_tags_disabled() {
+ global $CFG;
+
+ $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
+ list ($category, $course, $qcat, $questions) = $questiongenerator->setup_course_and_questions();
+ $questioncontext = context::instance_by_id($qcat->contextid);
+ $editingcontext = $questioncontext;
+ $question = $questions[0];
+ $user = $this->create_user_can_tag($course);
+ $formdata = $this->generate_encoded_submit_tags_form_string($question, $qcat, $questioncontext, [], []);
+
+ $this->setUser($user);
+ $CFG->usetags = false;
+ $result = core_question_external::submit_tags_form($question->id, $editingcontext->id, $formdata);
+ $CFG->usetags = true;
+
+ $this->assertFalse($result['status']);
+ }
+
+ /**
+ * submit_tags_form should return false if the user does not have any capability
+ * to tag the question.
+ */
+ public function test_submit_tags_form_no_tag_permissions() {
+ global $DB;
+
+ $generator = $this->getDataGenerator();
+ $user = $generator->create_user();
+ $teacherrole = $DB->get_record('role', ['shortname' => 'editingteacher']);
+ $questiongenerator = $generator->get_plugin_generator('core_question');
+ list ($category, $course, $qcat, $questions) = $questiongenerator->setup_course_and_questions();
+ $questioncontext = context::instance_by_id($qcat->contextid);
+ $editingcontext = $questioncontext;
+ $question = $questions[0];
+ $formdata = $this->generate_encoded_submit_tags_form_string(
+ $question,
+ $qcat,
+ $questioncontext,
+ ['foo'],
+ ['bar']
+ );
+
+ // Prohibit all of the tag capabilities.
+ assign_capability('moodle/question:tagmine', CAP_PROHIBIT, $teacherrole->id, $questioncontext->id);
+ assign_capability('moodle/question:tagall', CAP_PROHIBIT, $teacherrole->id, $questioncontext->id);
+
+ $generator->enrol_user($user->id, $course->id, $teacherrole->id, 'manual');
+ $user->ignoresesskey = true;
+ $this->setUser($user);
+
+ $result = core_question_external::submit_tags_form($question->id, $editingcontext->id, $formdata);
+
+ $this->assertFalse($result['status']);
+ }
+
+ /**
+ * submit_tags_form should return false if the user only has the capability to
+ * tag their own questions and the question is not theirs.
+ */
+ public function test_submit_tags_form_tagmine_permission_non_owner_question() {
+ global $DB;
+
+ $generator = $this->getDataGenerator();
+ $user = $generator->create_user();
+ $teacherrole = $DB->get_record('role', ['shortname' => 'editingteacher']);
+ $questiongenerator = $generator->get_plugin_generator('core_question');
+ list ($category, $course, $qcat, $questions) = $questiongenerator->setup_course_and_questions();
+ $questioncontext = context::instance_by_id($qcat->contextid);
+ $editingcontext = $questioncontext;
+ $question = $questions[0];
+ $formdata = $this->generate_encoded_submit_tags_form_string(
+ $question,
+ $qcat,
+ $questioncontext,
+ ['foo'],
+ ['bar']
+ );
+
+ // Make sure the question isn't created by the user.
+ $question->createdby = $user->id + 1;
+
+ // Prohibit all of the tag capabilities.
+ assign_capability('moodle/question:tagmine', CAP_ALLOW, $teacherrole->id, $questioncontext->id);
+ assign_capability('moodle/question:tagall', CAP_PROHIBIT, $teacherrole->id, $questioncontext->id);
+
+ $generator->enrol_user($user->id, $course->id, $teacherrole->id, 'manual');
+ $user->ignoresesskey = true;
+ $this->setUser($user);
+
+ $result = core_question_external::submit_tags_form($question->id, $editingcontext->id, $formdata);
+
+ $this->assertFalse($result['status']);
+ }
+
+ /**
+ * Data provided for the submit_tags_form test to check that course tags are
+ * only created in the correct editing and question context combinations.
+ *
+ * @return array Test cases
+ */
+ public function get_submit_tags_form_testcases() {
+ return [
+ 'course - course' => [
+ 'editingcontext' => 'course',
+ 'questioncontext' => 'course',
+ 'questiontags' => ['foo'],
+ 'coursetags' => ['bar'],
+ 'expectcoursetags' => false
+ ],
+ 'course - course - empty tags' => [
+ 'editingcontext' => 'course',
+ 'questioncontext' => 'course',
+ 'questiontags' => [],
+ 'coursetags' => ['bar'],
+ 'expectcoursetags' => false
+ ],
+ 'course - course category' => [
+ 'editingcontext' => 'course',
+ 'questioncontext' => 'category',
+ 'questiontags' => ['foo'],
+ 'coursetags' => ['bar'],
+ 'expectcoursetags' => true
+ ],
+ 'course - system' => [
+ 'editingcontext' => 'course',
+ 'questioncontext' => 'system',
+ 'questiontags' => ['foo'],
+ 'coursetags' => ['bar'],
+ 'expectcoursetags' => true
+ ],
+ 'course category - course' => [
+ 'editingcontext' => 'category',
+ 'questioncontext' => 'course',
+ 'questiontags' => ['foo'],
+ 'coursetags' => ['bar'],
+ 'expectcoursetags' => false
+ ],
+ 'course category - course category' => [
+ 'editingcontext' => 'category',
+ 'questioncontext' => 'category',
+ 'questiontags' => ['foo'],
+ 'coursetags' => ['bar'],
+ 'expectcoursetags' => false
+ ],
+ 'course category - system' => [
+ 'editingcontext' => 'category',
+ 'questioncontext' => 'system',
+ 'questiontags' => ['foo'],
+ 'coursetags' => ['bar'],
+ 'expectcoursetags' => false
+ ],
+ 'system - course' => [
+ 'editingcontext' => 'system',
+ 'questioncontext' => 'course',
+ 'questiontags' => ['foo'],
+ 'coursetags' => ['bar'],
+ 'expectcoursetags' => false
+ ],
+ 'system - course category' => [
+ 'editingcontext' => 'system',
+ 'questioncontext' => 'category',
+ 'questiontags' => ['foo'],
+ 'coursetags' => ['bar'],
+ 'expectcoursetags' => false
+ ],
+ 'system - system' => [
+ 'editingcontext' => 'system',
+ 'questioncontext' => 'system',
+ 'questiontags' => ['foo'],
+ 'coursetags' => ['bar'],
+ 'expectcoursetags' => false
+ ],
+ ];
+ }
+
+ /**
+ * Tests that submit_tags_form only creates course tags when the correct combination
+ * of editing context and question context is provided.
+ *
+ * Course tags can only be set on a course category or system context question that
+ * is being editing in a course context.
+ *
+ * @dataProvider get_submit_tags_form_testcases()
+ * @param string $editingcontext The type of the context the question is being edited in
+ * @param string $questioncontext The type of the context the question belongs to
+ * @param string[] $questiontags The tag names to set as question tags
+ * @param string[] $coursetags The tag names to set as course tags
+ * @param bool $expectcoursetags If the given course tags should have been set or not
+ */
+ public function test_submit_tags_form_context_combinations(
+ $editingcontext,
+ $questioncontext,
+ $questiontags,
+ $coursetags,
+ $expectcoursetags
+ ) {
+ $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
+ list ($category, $course, $qcat, $questions) = $questiongenerator->setup_course_and_questions($questioncontext);
+ $coursecontext = context_course::instance($course->id);
+ $questioncontext = context::instance_by_id($qcat->contextid);
+
+ switch($editingcontext) {
+ case 'system':
+ $editingcontext = context_system::instance();
+ break;
+
+ case 'category':
+ $editingcontext = context_coursecat::instance($category->id);
+ break;
+
+ default:
+ $editingcontext = context_course::instance($course->id);
+ }
+
+ $user = $this->create_user_can_tag($course);
+ $question = $questions[0];
+ $formdata = $this->generate_encoded_submit_tags_form_string(
+ $question,
+ $qcat,
+ $questioncontext,
+ $questiontags, // Question tags.
+ $coursetags // Course tags.
+ );
+
+ $this->setUser($user);
+
+ $result = core_question_external::submit_tags_form($question->id, $editingcontext->id, $formdata);
+
+ $this->assertTrue($result['status']);
+
+ $tagobjects = core_tag_tag::get_item_tags('core_question', 'question', $question->id);
+ $coursetagobjects = [];
+ $questiontagobjects = [];
+
+ if ($expectcoursetags) {
+ // If the use case is expecting course tags to be created then split
+ // the tags into course tags and question tags and ensure we have
+ // the correct number of course tags.
+
+ while ($tagobject = array_shift($tagobjects)) {
+ if ($tagobject->taginstancecontextid == $questioncontext->id) {
+ $questiontagobjects[] = $tagobject;
+ } else if ($tagobject->taginstancecontextid == $coursecontext->id) {
+ $coursetagobjects[] = $tagobject;
+ }
+ }
+
+ $this->assertCount(count($coursetags), $coursetagobjects);
+ } else {
+ $questiontagobjects = $tagobjects;
+ }
+
+ // Ensure the expected number of question tags was created.
+ $this->assertCount(count($questiontags), $questiontagobjects);
+
+ foreach ($questiontagobjects as $tagobject) {
+ // If we have any question tags then make sure they are in the list
+ // of expected tags and have the correct context.
+ $this->assertContains($tagobject->name, $questiontags);
+ $this->assertEquals($questioncontext->id, $tagobject->taginstancecontextid);
+ }
+
+ foreach ($coursetagobjects as $tagobject) {
+ // If we have any course tags then make sure they are in the list
+ // of expected course tags and have the correct context.
+ $this->assertContains($tagobject->name, $coursetags);
+ $this->assertEquals($coursecontext->id, $tagobject->taginstancecontextid);
+ }
+ }
+
+ /**
+ * Build the encoded form data expected by the submit_tags_form external function.
+ *
+ * @param stdClass $question The question record
+ * @param stdClass $questioncategory The question category record
+ * @param context $questioncontext Context for the question category
+ * @param array $tags A list of tag names for the question
+ * @param array $coursetags A list of course tag names for the question
+ * @return string HTML encoded string of the data
+ */
+ protected function generate_encoded_submit_tags_form_string($question, $questioncategory,
+ $questioncontext, $tags = [], $coursetags = []) {
+ global $CFG;
+
+ require_once($CFG->dirroot . '/question/type/tags_form.php');
+
+ $data = [
+ 'id' => $question->id,
+ 'categoryid' => $questioncategory->id,
+ 'contextid' => $questioncontext->id,
+ 'questionname' => $question->name,
+ 'questioncategory' => $questioncategory->name,
+ 'context' => $questioncontext->get_context_name(false),
+ 'tags' => $tags,
+ 'coursetags' => $coursetags
+ ];
+ $data = core_question\form\tags::mock_generate_submit_keys($data);
+
+ return http_build_query($data, '', '&');
+ }
+
+ /**
+ * Create a user, enrol them in the course, and give them the capability to
+ * tag all questions in the system context.
+ *
+ * @param stdClass $course The course record to enrol in
+ * @return stdClass The user record
+ */
+ protected function create_user_can_tag($course) {
+ global $DB;
+
+ $generator = $this->getDataGenerator();
+ $user = $generator->create_user();
+ $roleid = $generator->create_role();
+ $teacherrole = $DB->get_record('role', ['shortname' => 'editingteacher']);
+ $systemcontext = context_system::instance();
+
+ $generator->role_assign($roleid, $user->id, $systemcontext->id);
+ $generator->enrol_user($user->id, $course->id, $teacherrole->id, 'manual');
+
+ // Give the user global ability to tag questions.
+ assign_capability('moodle/question:tagall', CAP_ALLOW, $roleid, $systemcontext, true);
+ // Allow the user to submit form data.
+ $user->ignoresesskey = true;
+
+ return $user;
+ }
+}
diff --git a/question/tests/generator/lib.php b/question/tests/generator/lib.php
index 21840f5b338..eb866ea3823 100644
--- a/question/tests/generator/lib.php
+++ b/question/tests/generator/lib.php
@@ -89,4 +89,43 @@ class core_question_generator extends component_generator_base {
$question->createdby = 0;
return question_bank::get_qtype($qtype)->save_question($question, $fromform);
}
+
+ /**
+ * Setup a course category, course, a question category, and 2 questions
+ * for testing.
+ *
+ * @param string $type The type of question category to create.
+ * @return array The created data objects
+ */
+ public function setup_course_and_questions($type = 'course') {
+ $datagenerator = $this->datagenerator;
+ $category = $datagenerator->create_category();
+ $course = $datagenerator->create_course([
+ 'numsections' => 5,
+ 'category' => $category->id
+ ]);
+
+ switch ($type) {
+ case 'category':
+ $context = context_coursecat::instance($category->id);
+ break;
+
+ case 'system':
+ $context = context_system::instance();
+ break;
+
+ default:
+ $context = context_course::instance($course->id);
+ break;
+ }
+
+ $qcat = $this->create_question_category(['contextid' => $context->id]);
+
+ $questions = array(
+ $this->create_question('shortanswer', null, ['category' => $qcat->id]),
+ $this->create_question('shortanswer', null, ['category' => $qcat->id]),
+ );
+
+ return array($category, $course, $qcat, $questions);
+ }
}
diff --git a/question/type/calculatedsimple/tests/helper.php b/question/type/calculatedsimple/tests/helper.php
index 53cb8e4282d..5218994a2ae 100644
--- a/question/type/calculatedsimple/tests/helper.php
+++ b/question/type/calculatedsimple/tests/helper.php
@@ -209,9 +209,11 @@ class qtype_calculatedsimple_test_helper extends question_test_helper {
}
public function get_calculatedsimple_question_data_sumwithvariants() {
+ global $USER;
$q = new stdClass();
$q->name = 'Calculated simple';
+ $q->createdby = $USER->id;
$q->questiontext = 'This is a simple sum of two variables.
';
$q->questiontextformat = '1';
$q->generalfeedback = 'The answer is {a} + {b}
';
diff --git a/question/type/gapselect/tests/edit_form_test.php b/question/type/gapselect/tests/edit_form_test.php
index 97ee86d62ac..7c6dc3850c9 100644
--- a/question/type/gapselect/tests/edit_form_test.php
+++ b/question/type/gapselect/tests/edit_form_test.php
@@ -43,6 +43,8 @@ class qtype_gapselect_edit_form_base_testable extends qtype_gapselect_edit_form_
$category = question_make_default_categories(array($syscontext));
$fakequestion = new stdClass();
$fakequestion->qtype = 'stack';
+ $fakequestion->contextid = $syscontext->id;
+ $fakequestion->createdby = 2;
$fakequestion->category = $category->id;
$fakequestion->questiontext = 'Test [[1]] question [[2]]';
$fakequestion->options = new stdClass();
diff --git a/question/type/tags_form.php b/question/type/tags_form.php
index a3af411f8dc..bcc2fc502a0 100644
--- a/question/type/tags_form.php
+++ b/question/type/tags_form.php
@@ -41,6 +41,8 @@ class tags extends \moodleform {
*/
public function definition() {
$mform = $this->_form;
+ $customdata = $this->_customdata;
+
$mform->disable_form_change_checker();
$mform->addElement('hidden', 'id');
@@ -56,7 +58,29 @@ class tags extends \moodleform {
$mform->addElement('static', 'questioncategory', get_string('categorycurrent', 'question'));
$mform->addElement('static', 'context', '');
- $mform->addElement('tags', 'tags', get_string('tags'),
- ['itemtype' => 'question', 'component' => 'core_question']);
+ if (\core_tag_tag::is_enabled('core_question', 'question')) {
+ $mform->addElement('tags', 'tags', get_string('tags'),
+ ['itemtype' => 'question', 'component' => 'core_question']);
+
+ // Is the question category in a course context?
+ $qcontext = $customdata['questioncontext'];
+ $qcoursecontext = $qcontext->get_course_context(false);
+ $iscourseoractivityquestion = !empty($qcoursecontext);
+ // Is the current context we're editing in a course context?
+ $editingcontext = $customdata['editingcontext'];
+ $editingcoursecontext = $editingcontext->get_course_context(false);
+ $iseditingcontextcourseoractivity = !empty($editingcoursecontext);
+
+ if ($iseditingcontextcourseoractivity && !$iscourseoractivityquestion) {
+ // If the question is being edited in a course or activity context
+ // and the question isn't a course or activity level question then
+ // allow course tags to be added to the course.
+ $coursetagheader = get_string('questionformtagheader', 'core_question',
+ $editingcoursecontext->get_context_name(true));
+ $mform->addElement('tags', 'coursetags', $coursetagheader,
+ array('itemtype' => 'question', 'component' => 'core_question'));
+
+ }
+ }
}
}