From 2cf5ab1028bf99fcf632aedfa6e3cde1160ba90d Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Tue, 23 Oct 2012 10:26:33 +0800 Subject: [PATCH] MDL-30545 messaging prefs: show all relevant message providers. Some message providers have an associated capability. For example the mod/quiz:emailconfirmsubmission capability. At the moment, we only show the preferences for the capabilities that the user has in the system context. That does not work for the example capability above. Typically the capability will be overridden in one quiz, to allow for Students, and the user will have the Student role in the course context. Therefore, this message type does not appear on the preferences page. This fix corrects the logic, so that all message types that are to the current user are displayed. --- lib/messagelib.php | 99 ++++++++++++++++++++++++----- lib/tests/messagelib_test.php | 96 ++++++++++++++++++++++++++++ mod/quiz/lib.php | 2 + mod/quiz/tests/generator/lib.php | 106 +++++++++++++++++++++++++++++++ 4 files changed, 286 insertions(+), 17 deletions(-) create mode 100644 lib/tests/messagelib_test.php create mode 100644 mod/quiz/tests/generator/lib.php diff --git a/lib/messagelib.php b/lib/messagelib.php index d007432ab68..43db2546dfb 100644 --- a/lib/messagelib.php +++ b/lib/messagelib.php @@ -414,32 +414,97 @@ function message_get_my_providers() { function message_get_providers_for_user($userid) { global $DB, $CFG; - $systemcontext = context_system::instance(); - $providers = get_message_providers(); - // Remove all the providers we aren't allowed to see now - foreach ($providers as $providerid => $provider) { - if (!empty($provider->capability)) { - if (!has_capability($provider->capability, $systemcontext, $userid)) { - unset($providers[$providerid]); // Not allowed to see this - continue; + // Ensure user is not allowed to configure instantmessage if it is globally disabled. + if (!$CFG->messaging) { + foreach ($providers as $providerid => $provider) { + if ($provider->name == 'instantmessage') { + unset($providers[$providerid]); + break; } } + } - // Ensure user is not allowed to configure instantmessage if it is globally disabled. - if (!$CFG->messaging && $provider->name == 'instantmessage') { + // If the component is an enrolment plugin, check it is enabled + foreach ($providers as $providerid => $provider) { + list($type, $name) = normalize_component($provider->component); + if ($type == 'enrol' && !enrol_is_enabled($name)) { unset($providers[$providerid]); + } + } + + // Now we need to check capabilities. We need to eliminate the providers + // where the user does not have the corresponding capability anywhere. + // Here we deal with the common simple case of the user having the + // capability in the system context. That handles $CFG->defaultuserroleid. + // For the remaining providers/capabilities, we need to do a more complex + // query involving all overrides everywhere. + $unsureproviders = array(); + $unsurecapabilities = array(); + $systemcontext = context_system::instance(); + foreach ($providers as $providerid => $provider) { + if (empty($provider->capability) || has_capability($provider->capability, $systemcontext, $userid)) { + // The provider is relevant to this user. continue; } - // If the component is an enrolment plugin, check it is enabled - list($type, $name) = normalize_component($provider->component); - if ($type == 'enrol') { - if (!enrol_is_enabled($name)) { - unset($providers[$providerid]); - continue; - } + $unsureproviders[$providerid] = $provider; + $unsurecapabilities[$provider->capability] = 1; + unset($providers[$providerid]); + } + + if (empty($unsureproviders)) { + // More complex checks are not required. + return $providers; + } + + // Now check the unsure capabilities. + list($capcondition, $params) = $DB->get_in_or_equal( + array_keys($unsurecapabilities), SQL_PARAMS_NAMED); + $params['userid'] = $userid; + + $sql = "SELECT DISTINCT rc.capability, 1 + + FROM {role_assignments} ra + JOIN {context} actx ON actx.id = ra.contextid + JOIN {role_capabilities} rc ON rc.roleid = ra.roleid + JOIN {context} cctx ON cctx.id = rc.contextid + + WHERE ra.userid = :userid + AND rc.capability $capcondition + AND rc.permission > 0 + AND (CONCAT(actx.path, '/') LIKE CONCAT(cctx.path, '/%') OR CONCAT(cctx.path, '/') LIKE CONCAT(actx.path, '/%'))"; + + if (!empty($CFG->defaultfrontpageroleid)) { + $frontpagecontext = context_course::instance(SITEID); + + list($capcondition2, $params2) = $DB->get_in_or_equal( + array_keys($unsurecapabilities), SQL_PARAMS_NAMED); + $params = array_merge($params, $params2); + $params['frontpageroleid'] = $CFG->defaultfrontpageroleid; + $params['frontpagepathpattern'] = $frontpagecontext->path . '/'; + + $sql .= " + UNION DISTINCT + + SELECT DISTINCT rc.capability, 1 + + FROM {role_capabilities} rc + JOIN {context} cctx ON cctx.id = rc.contextid + + WHERE rc.roleid = :frontpageroleid + AND rc.capability $capcondition2 + AND rc.permission > 0 + AND CONCAT(cctx.path, '/') LIKE :frontpagepathpattern"; + } + + $relevantcapabilities = $DB->get_records_sql_menu($sql, $params); + + // Add back any providers based on the detailed capability check. + foreach ($unsureproviders as $providerid => $provider) { + if (array_key_exists($provider->capability, $relevantcapabilities)) { + $providers[$providerid] = $provider; } } diff --git a/lib/tests/messagelib_test.php b/lib/tests/messagelib_test.php new file mode 100644 index 00000000000..94d9df106a9 --- /dev/null +++ b/lib/tests/messagelib_test.php @@ -0,0 +1,96 @@ +. + +/** + * Tests for messagelib.php. + * + * @package core_message + * @copyright 2012 The Open Universtiy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +class messagelib_testcase extends advanced_testcase { + + public function test_message_get_providers_for_user() { + global $CFG, $DB; + + $this->resetAfterTest(true); + + $generator = $this->getDataGenerator(); + + // Create a course category and course + $cat = $generator->create_category(array('parent' => 0)); + $course = $generator->create_course(array('category' => $cat->id)); + $quiz = $generator->create_module('quiz', array('course' => $course->id)); + $user = $generator->create_user(); + + $coursecontext = context_course::instance($course->id); + $quizcontext = context_module::instance($quiz->cmid); + $frontpagecontext = context_course::instance(SITEID); + + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + + // The user is a student in a course, and has the capability for quiz + // confirmation emails in one quiz in that course. + role_assign($studentrole->id, $user->id, $coursecontext->id); + assign_capability('mod/quiz:emailconfirmsubmission', CAP_ALLOW, $studentrole->id, $quizcontext->id); + + // Give this message type to the front page role. + assign_capability('mod/quiz:emailwarnoverdue', CAP_ALLOW, $CFG->defaultfrontpageroleid, $frontpagecontext->id); + + $providers = message_get_providers_for_user($user->id); + $this->assertTrue($this->message_type_present('mod_forum', 'posts', $providers)); + $this->assertTrue($this->message_type_present('mod_quiz', 'confirmation', $providers)); + $this->assertTrue($this->message_type_present('mod_quiz', 'attempt_overdue', $providers)); + $this->assertFalse($this->message_type_present('mod_quiz', 'submission', $providers)); + + // A user is a student in a different course, they should not get confirmation. + $course2 = $generator->create_course(array('category' => $cat->id)); + $user2 = $generator->create_user(); + $coursecontext2 = context_course::instance($course2->id); + role_assign($studentrole->id, $user2->id, $coursecontext2->id); + accesslib_clear_all_caches_for_unit_testing(); + $providers = message_get_providers_for_user($user2->id); + $this->assertTrue($this->message_type_present('mod_forum', 'posts', $providers)); + $this->assertFalse($this->message_type_present('mod_quiz', 'confirmation', $providers)); + + // Now remove the frontpage role id, and attempt_overdue message should go away. + unset_config('defaultfrontpageroleid'); + accesslib_clear_all_caches_for_unit_testing(); + + $providers = message_get_providers_for_user($user->id); + $this->assertTrue($this->message_type_present('mod_quiz', 'confirmation', $providers)); + $this->assertFalse($this->message_type_present('mod_quiz', 'attempt_overdue', $providers)); + $this->assertFalse($this->message_type_present('mod_quiz', 'submission', $providers)); + } + + /** + * Is a particular message type in the list of message types. + * @param string $name a message name. + * @param array $providers as returned by message_get_providers_for_user. + * @return bool whether the message type is present. + */ + protected function message_type_present($component, $name, $providers) { + foreach ($providers as $provider) { + if ($provider->component == $component && $provider->name == $name) { + return true; + } + } + return false; + } +} diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index f006e75584a..43733d46a0e 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -1026,6 +1026,8 @@ function quiz_process_options($quiz) { $quiz->feedbackboundaries[-1] = $quiz->grade + 1; $quiz->feedbackboundaries[$numboundaries] = 0; $quiz->feedbackboundarycount = $numboundaries; + } else { + $quiz->feedbackboundarycount = -1; } // Combing the individual settings into the review columns. diff --git a/mod/quiz/tests/generator/lib.php b/mod/quiz/tests/generator/lib.php new file mode 100644 index 00000000000..24ed645b8d8 --- /dev/null +++ b/mod/quiz/tests/generator/lib.php @@ -0,0 +1,106 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * Quiz module test data generator class + * + * @package mod_quiz + * @copyright 2012 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class mod_quiz_generator extends phpunit_module_generator { + + /** + * Create new quiz module instance. + * @param array|stdClass $record + * @param array $options (mostly course_module properties) + * @return stdClass activity record with extra cmid field + */ + public function create_instance($record = null, array $options = null) { + global $CFG; + require_once("$CFG->dirroot/mod/quiz/locallib.php"); + + $this->instancecount++; + $i = $this->instancecount; + + $record = (object)(array)$record; + $options = (array)$options; + + if (empty($record->course)) { + throw new coding_exception('module generator requires $record->course'); + } + if (isset($options['idnumber'])) { + $record->cmidnumber = $options['idnumber']; + } else { + $record->cmidnumber = ''; + } + + $alwaysvisible = mod_quiz_display_options::DURING | mod_quiz_display_options::IMMEDIATELY_AFTER | + mod_quiz_display_options::LATER_WHILE_OPEN | mod_quiz_display_options::AFTER_CLOSE; + + $defaultquizsettings = array( + 'name' => get_string('pluginname', 'data').' '.$i, + 'intro' => 'Test quiz ' . $i, + 'introformat' => FORMAT_MOODLE, + 'timeopen' => 0, + 'timeclose' => 0, + 'preferredbehaviour' => 'deferredfeedback', + 'attempts' => 0, + 'attemptonlast' => 0, + 'grademethod' => QUIZ_GRADEHIGHEST, + 'decimalpoints' => 2, + 'questiondecimalpoints' => -1, + 'reviewattempt' => $alwaysvisible, + 'reviewcorrectness' => $alwaysvisible, + 'reviewmarks' => $alwaysvisible, + 'reviewspecificfeedback' => $alwaysvisible, + 'reviewgeneralfeedback' => $alwaysvisible, + 'reviewrightanswer' => $alwaysvisible, + 'reviewoverallfeedback' => $alwaysvisible, + 'questionsperpage' => 1, + 'shufflequestions' => 0, + 'shuffleanswers' => 1, + 'questions' => '', + 'sumgrades' => 0, + 'grade' => 0, + 'timecreated' => time(), + 'timemodified' => time(), + 'timelimit' => 0, + 'overduehandling' => 'autoabandon', + 'graceperiod' => 86400, + 'quizpassword' => '', + 'subnet' => '', + 'browsersecurity' => '', + 'delay1' => 0, + 'delay2' => 0, + 'showuserpicture' => 0, + 'showblocks' => 0, + 'navmethod' => QUIZ_NAVMETHOD_FREE, + ); + + foreach ($defaultquizsettings as $name => $value) { + if (!isset($record->{$name})) { + $record->{$name} = $value; + } + } + + $record->coursemodule = $this->precreate_course_module($record->course, $options); + $id = quiz_add_instance($record); + return $this->post_add_instance($id, $record->coursemodule); + } +}