Merge branch 'MDL-85431_500_STABLE' of https://github.com/marxjohnson/moodle into MOODLE_500_STABLE

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 <strong>"{$a->tocontext}"</strong>?<br /> We have detected <strong>{$a->urlcount} files</strong> 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 <strong>"{$a->tocontext}"</strong>?<br /> There are <strong>no files</strong> 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.';

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 {

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,

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,
         ];
     }
 }

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