diff --git a/.upgradenotes/MDL-85754-2025070810511425.yml b/.upgradenotes/MDL-85754-2025070810511425.yml new file mode 100644 index 00000000000..50c107ece6e --- /dev/null +++ b/.upgradenotes/MDL-85754-2025070810511425.yml @@ -0,0 +1,11 @@ +issueNumber: MDL-85754 +notes: + core_question: + - message: >- + `question_edit_contexts` now only considers the provided context when + checking permissions, rather than all parent contexts as well. As + questions now exist only at the activity module context level, + permissions can be inherited or overridden as normal for each question + bank. The previous pattern of checking for a permission in any parent + context circumvented the override system, and no longer makes sense. + type: changed diff --git a/mod/quiz/tests/behat/editing_add_from_question_bank.feature b/mod/quiz/tests/behat/editing_add_from_question_bank.feature index 33fb35b7a48..e5c72df50cf 100644 --- a/mod/quiz/tests/behat/editing_add_from_question_bank.feature +++ b/mod/quiz/tests/behat/editing_add_from_question_bank.feature @@ -216,3 +216,32 @@ Feature: Adding questions to a quiz from the question bank When I am on the "Quiz 1" "mod_quiz > Edit" page logged in as teacher1 Then I should see "Question Bank A" in the "TF1" "list_item" And I should see "Question Bank B" in the "TF2" "list_item" + + Scenario: User doesn't see the option to switch to a bank they can't use + Given the "multilang" filter is "on" + And the "multilang" filter applies to "content and headings" + And the following "users" exist: + | username | firstname | lastname | email | + | teacher2 | Teacher | 2 | teacher2@example.com | + And the following "role" exists: + | shortname | noquestions | + | name | Cannot use questions | + | moodle/question:usemine | prohibit | + | moodle/question:useall | prohibit | + And the following "course enrolments" exist: + | user | course | role | + | teacher2 | C1 | editingteacher | + And I am on the "Quiz 1" "mod_quiz > Edit" page logged in as "teacher2" + And I open the "last" add to quiz menu + And I follow "from question bank" + And I click on "Switch bank" "button" + And I click on "Qbank 1 & < > \" ' &" "link" in the "Select question bank" "dialogue" + And I click on "Select" "checkbox" in the "question 03 name" "table_row" + And I click on "Add selected questions to the quiz" "button" + When the following "role assigns" exist: + | user | role | contextlevel | reference | + | teacher2 | noquestions | Activity module | qbank1 | + And I open the "Page 1" add to quiz menu + And I follow "from question bank" + And I click on "Switch bank" "button" + Then "Qbank 1 & < > \" ' &" "link" should not exist in the "Select question bank" "dialogue" diff --git a/question/classes/local/bank/question_bank_helper.php b/question/classes/local/bank/question_bank_helper.php index 1d90a3f5329..5756a6541a7 100644 --- a/question/classes/local/bank/question_bank_helper.php +++ b/question/classes/local/bank/question_bank_helper.php @@ -339,6 +339,7 @@ class question_bank_helper { int $userid, int $notincourseid = 0, ?context $filtercontext = null, + array $havingcap = [], ): array { $prefs = get_user_preferences(self::RECENTLY_VIEWED, null, $userid); $contextids = !empty($prefs) ? explode(',', $prefs) : []; @@ -360,6 +361,9 @@ class question_bank_helper { if (!empty($notincourseid) && $notincourseid == $cm->course) { continue; } + if (!empty($havingcap) && !(new question_edit_contexts($context))->have_one_cap($havingcap)) { + continue; + } $record = self::get_formatted_bank($cm, filtercontext: $filtercontext); $banks[] = $record; } diff --git a/question/classes/local/bank/question_edit_contexts.php b/question/classes/local/bank/question_edit_contexts.php index c3776223c34..392fc015619 100644 --- a/question/classes/local/bank/question_edit_contexts.php +++ b/question/classes/local/bank/question_edit_contexts.php @@ -66,7 +66,7 @@ class question_edit_contexts { * @param \context $thiscontext the current context. */ public function __construct(\context $thiscontext) { - $this->allcontexts = array_values($thiscontext->get_parent_contexts(true)); + $this->allcontexts = [$thiscontext]; } /** diff --git a/question/classes/output/switch_question_bank.php b/question/classes/output/switch_question_bank.php index ab64b51994a..b958249be8b 100644 --- a/question/classes/output/switch_question_bank.php +++ b/question/classes/output/switch_question_bank.php @@ -62,12 +62,13 @@ class switch_question_bank implements \renderable, \templatable { [, $cm] = get_module_from_cmid($this->quizcmid); $cminfo = cm_info::create($cm); + $capabilities = ['moodle/question:useall', 'moodle/question:usemine']; $coursesharedbanks = question_bank_helper::get_activity_instances_with_shareable_questions( incourseids: [$this->courseid], - havingcap: ['moodle/question:managecategory'], + havingcap: $capabilities, filtercontext: $cminfo->context, ); - $recentlyviewedbanks = question_bank_helper::get_recently_used_open_banks($this->userid); + $recentlyviewedbanks = question_bank_helper::get_recently_used_open_banks($this->userid, havingcap: $capabilities); return [ 'quizname' => $cminfo->get_formatted_name(), diff --git a/question/tests/local/bank/context_to_string_translator_test.php b/question/tests/local/bank/context_to_string_translator_test.php index 1eef4667da6..c55a24c5640 100644 --- a/question/tests/local/bank/context_to_string_translator_test.php +++ b/question/tests/local/bank/context_to_string_translator_test.php @@ -50,7 +50,7 @@ final class context_to_string_translator_test extends \advanced_testcase { $quizcontext = context_module::instance($quiz->cmid); // Create the context_to_string_translator. - $translator = new context_to_string_translator((new question_edit_contexts($quizcontext))->all()); + $translator = new context_to_string_translator([$systemcontext, $categorycontext, $coursecontext, $quizcontext]); // Verify its behaviour. $this->assertEquals('module', $translator->context_to_string($quizcontext->id)); diff --git a/question/tests/local/bank/question_bank_helper_test.php b/question/tests/local/bank/question_bank_helper_test.php index e08cb776449..78f7b69d560 100644 --- a/question/tests/local/bank/question_bank_helper_test.php +++ b/question/tests/local/bank/question_bank_helper_test.php @@ -319,6 +319,7 @@ final class question_bank_helper_test extends \advanced_testcase { $user = self::getDataGenerator()->create_user(); $course1 = self::getDataGenerator()->create_course(); $course2 = self::getDataGenerator()->create_course(); + self::getDataGenerator()->enrol_user($user->id, $course1->id, 'editingteacher'); $banks = []; $banks[] = self::getDataGenerator()->create_module('qbank', ['course' => $course1->id]); $banks[] = self::getDataGenerator()->create_module('qbank', ['course' => $course1->id]); @@ -340,6 +341,16 @@ final class question_bank_helper_test extends \advanced_testcase { // Check that the courseid filter works. $recentlyviewed = question_bank_helper::get_recently_used_open_banks($user->id, $course1->id); $this->assertCount(3, $recentlyviewed); + // We should have the viewed banks in course 2. + $courseviewed = array_slice($banks, 3, 3); + $this->assertEqualsCanonicalizing(array_column($recentlyviewed, 'modid'), array_column($courseviewed, 'cmid')); + + // Check that the capability filter works. + $recentlyviewed = question_bank_helper::get_recently_used_open_banks($user->id, havingcap: ['moodle/question:useall']); + $this->assertCount(2, $recentlyviewed); + // We should have the 2 most recently viewed banks in course 1. + $capabilityviewed = array_slice($banks, 1, 2); + $this->assertEqualsCanonicalizing(array_column($recentlyviewed, 'modid'), array_column($capabilityviewed, 'cmid')); $recentlyviewed = question_bank_helper::get_recently_used_open_banks($user->id);