diff --git a/lang/en/question.php b/lang/en/question.php index f4ea5608c02..e2cda045133 100644 --- a/lang/en/question.php +++ b/lang/en/question.php @@ -252,6 +252,7 @@ $string['movingquestions'] = 'Moving questions and any files'; $string['movingquestionsandfiles'] = 'Are you sure you want to move question(s) {$a->questions} to context for "{$a->tocontext}"?
We have detected {$a->urlcount} files linked from these question(s) in {$a->fromareaname}, would you like to copy or move these to {$a->toareaname}?'; $string['movingquestionsnofiles'] = 'Are you sure you want to move question(s) {$a->questions} to context for "{$a->tocontext}"?
There are no files linked from these question(s) in {$a->fromareaname}.'; $string['needtochoosecat'] = 'You need to choose a category to move this question to or press \'cancel\'.'; +$string['nobankpermissions'] = 'You do not have permission to access any question banks on this course.'; $string['nobanks'] = 'This course doesn\'t have any question banks yet.'; $string['nocate'] = 'No such category {$a}!'; $string['nopermissionadd'] = 'You don\'t have permission to add questions here.'; diff --git a/lib/questionlib.php b/lib/questionlib.php index 5dc2dbb7c64..ba047195774 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -1416,14 +1416,33 @@ function question_extend_settings_navigation(navigation_node $navigationnode, $c $iscourse = $context->contextlevel === CONTEXT_COURSE; - if ($iscourse && has_capability('moodle/course:manageactivities', $context)) { - return $navigationnode->add( - get_string('questionbank_plural', 'question'), - new moodle_url($baseurl, ['courseid' => $context->instanceid]), - navigation_node::TYPE_CONTAINER, - null, - 'questionbank' - ); + if ($iscourse) { + $viewquestionbanks = has_capability('moodle/course:manageactivities', $context); + if (!$viewquestionbanks) { + // If the user can view any activities with shared questions, display the Question banks node. + // If they can access activities with private questions (such as quiz) they can be accessed elsewhere on the course. + $modtypes = \core_question\local\bank\question_bank_helper::get_activity_types_with_shareable_questions(); + $modinfo = get_fast_modinfo($context->instanceid); + foreach ($modtypes as $modtype) { + foreach ($modinfo->get_instances_of($modtype) as $mod) { + if (has_capability("mod/{$modtype}:view", $mod->context)) { + $viewquestionbanks = true; + break 2; + } + } + } + } + if ($viewquestionbanks) { + return $navigationnode->add( + get_string('questionbank_plural', 'question'), + new moodle_url($baseurl, ['courseid' => $context->instanceid]), + navigation_node::TYPE_CONTAINER, + null, + 'questionbank', + ); + } else { + return; + } } else if ($context->contextlevel == CONTEXT_MODULE) { $params = ['cmid' => $context->instanceid]; } else { diff --git a/question/banks.php b/question/banks.php index a3fe5a98b5c..9b8e929ba36 100644 --- a/question/banks.php +++ b/question/banks.php @@ -38,7 +38,7 @@ $course = get_course($courseid); $coursecontext = context_course::instance($course->id); require_login($course, false); -require_capability('moodle/course:manageactivities', \context_course::instance($course->id)); +$canmanage = has_capability('moodle/course:manageactivities', \context_course::instance($course->id)); if (empty(question_bank_helper::get_activity_types_with_shareable_questions())) { throw new moodle_exception('disabledbanks', 'question'); @@ -48,13 +48,19 @@ $allcaps = array_merge(question_edit_contexts::$caps['editq'], question_edit_con $sharedbanks = question_bank_helper::get_activity_instances_with_shareable_questions([$course->id], [], $allcaps); $privatebanks = question_bank_helper::get_activity_instances_with_private_questions([$course->id], [], $allcaps); +if (!$canmanage && empty($sharedbanks)) { + // Only allow non-managing users access to this page if they can access at least 1 activity with shared questions. + // If they can access activities with private questions (such as quiz) they can be accessed elsewhere on the course. + throw new \core\exception\moodle_exception('nobankpermissions', 'question'); +} + $pageurl = question_bank_helper::get_url_for_qbank_list($course->id); $PAGE->set_url($pageurl); $PAGE->add_body_class('limitedwidth'); $PAGE->set_heading(format_string($course->fullname, true, ['context' => $coursecontext])); $PAGE->set_title(get_string('questionbank_plural', 'question')); -if ($createdefault) { +if ($canmanage && $createdefault) { require_sesskey(); question_bank_helper::create_default_open_instance( $course, diff --git a/question/classes/output/view_banks.php b/question/classes/output/view_banks.php index 7352ee665ba..2320af06d8b 100644 --- a/question/classes/output/view_banks.php +++ b/question/classes/output/view_banks.php @@ -84,16 +84,16 @@ class view_banks implements \templatable, \renderable { get_string('createdefault', 'question') ); - $cancreatedefault = has_capability('moodle/course:manageactivities', context_course::instance($this->course->id)); + $cancreate = has_capability('moodle/course:manageactivities', context_course::instance($this->course->id)); return [ - 'addqbank' => $addqbanklink->export_for_template($output), + 'addqbank' => $cancreate ? $addqbanklink->export_for_template($output) : false, 'hassharedbanks' => !empty($sharedbankscontext), 'sharedbanks' => $sharedbankscontext, 'hasprivatebanks' => !empty($privatebankscontext), 'privatebanks' => $privatebankscontext, - 'addcustombanks' => $addcustombanksrenderable->export_for_template($output), - 'createdefault' => $cancreatedefault ? $createdefaultrenderable->export_for_template($output) : false, + 'addcustombanks' => $cancreate ? $addcustombanksrenderable->export_for_template($output) : false, + 'createdefault' => $cancreate ? $createdefaultrenderable->export_for_template($output) : false, ]; } } diff --git a/question/tests/behat/bank_manage.feature b/question/tests/behat/bank_manage.feature index 29978dac746..cca9f025139 100644 --- a/question/tests/behat/bank_manage.feature +++ b/question/tests/behat/bank_manage.feature @@ -17,9 +17,9 @@ Feature: Manage question banks | teacher1 | C1 | editingteacher | | student1 | C1 | student | And the following "activities" exist: - | activity | name | course | section | intro | showdescription | - | qbank | bank1 | C1 | 0 | Bank 1 intro | 0 | - | qbank | bank2 | C1 | 0 | Bank 2 intro | 0 | + | activity | name | course | section | intro | showdescription | idnumber | + | qbank | bank1 | C1 | 0 | Bank 1 intro | 0 | qbank1 | + | qbank | bank2 | C1 | 0 | Bank 2 intro | 0 | qbank2 | @javascript Scenario: Show description when show description checkbox ticked @@ -56,3 +56,28 @@ Feature: Manage question banks And I click on "Delete" "button" Then I should not see "bank1" But I should see "bank2" + + Scenario: A student without permissions to access a bank cannot access the question banks page + Given I am on the "C1" "Course" page logged in as "student1" + Then "Question banks" "link" should not exist + + Scenario: A student with permissions to access a bank can access the question banks page + Given the following "role assigns" exist: + | user | role | contextlevel | reference | + | student1 | editingteacher | Activity module | qbank2 | + And I am on the "C1" "Course" page logged in as "student1" + When I navigate to "Question banks" in current page administration + Then I should see "bank2" + And I should not see "bank1" + And "Add" "button" should not exist in the "region-main" "region" + + Scenario: A teacher can access the question banks page when there are no question banks yet + Given the following "courses" exist: + | fullname | shortname | category | + | Course 2 | C2 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C2 | editingteacher | + When I am on the "C2" "Course" page logged in as "teacher1" + And I navigate to "Question banks" in current page administration + Then "Create default question bank" "button" should exist