From 20ff15e22b4f0abebe1ab5fbfd1d681c88765e2a Mon Sep 17 00:00:00 2001 From: Ankit Agarwal Date: Mon, 8 Jun 2015 14:33:26 +0530 Subject: [PATCH] MDL-50173 ratings: Use proper checks to ensure ratings are viewable. Mainly to verify groups visibility this new callback has been created. Note this was originally 2 commits but for amending purposes they have been squashed. --- mod/data/lib.php | 47 +++++++++++++++++++ mod/data/tests/lib_test.php | 103 +++++++++++++++++++++++++++++++++++++++++- mod/forum/lib.php | 42 +++++++++++++++++ mod/forum/tests/lib_test.php | 104 +++++++++++++++++++++++++++++++++++++++++++ mod/upgrade.txt | 5 +++ rating/index.php | 8 +++- 6 files changed, 306 insertions(+), 3 deletions(-) diff --git a/mod/data/lib.php b/mod/data/lib.php index c972a6c1f59..bc037e1932d 100644 --- a/mod/data/lib.php +++ b/mod/data/lib.php @@ -1500,6 +1500,53 @@ function data_rating_validate($params) { return true; } +/** + * Can the current user see ratings for a given itemid? + * + * @param array $params submitted data + * contextid => int contextid [required] + * component => The component for this module - should always be mod_data [required] + * ratingarea => object the context in which the rated items exists [required] + * itemid => int the ID of the object being rated [required] + * scaleid => int scale id [optional] + * @return bool + * @throws coding_exception + * @throws rating_exception + */ +function mod_data_rating_can_see_item_ratings($params) { + global $DB; + + // Check the component is mod_data. + if (!isset($params['component']) || $params['component'] != 'mod_data') { + throw new rating_exception('invalidcomponent'); + } + + // Check the ratingarea is entry (the only rating area in data). + if (!isset($params['ratingarea']) || $params['ratingarea'] != 'entry') { + throw new rating_exception('invalidratingarea'); + } + + if (!isset($params['itemid'])) { + throw new rating_exception('invaliditemid'); + } + + $datasql = "SELECT d.id as dataid, d.course, r.groupid + FROM {data_records} r + JOIN {data} d ON r.dataid = d.id + WHERE r.id = :itemid"; + $dataparams = array('itemid' => $params['itemid']); + if (!$info = $DB->get_record_sql($datasql, $dataparams)) { + // Item doesn't exist + throw new rating_exception('invaliditemid'); + } + + $course = $DB->get_record('course', array('id' => $info->course), '*', MUST_EXIST); + $cm = get_coursemodule_from_instance('data', $info->dataid, $course->id, false, MUST_EXIST); + + // Make sure groups allow this user to see the item they're rating. + return groups_group_visible($info->groupid, $course, $cm); +} + /** * function that takes in the current data, number of items per page, diff --git a/mod/data/tests/lib_test.php b/mod/data/tests/lib_test.php index 78ab7ed9b71..fbd744c5c68 100644 --- a/mod/data/tests/lib_test.php +++ b/mod/data/tests/lib_test.php @@ -35,9 +35,9 @@ require_once($CFG->dirroot . '/mod/data/lib.php'); * @copyright 2013 Adrian Greeve * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class data_lib_testcase extends advanced_testcase { +class mod_data_lib_testcase extends advanced_testcase { - function test_data_delete_record() { + public function test_data_delete_record() { global $DB; $this->resetAfterTest(); @@ -231,4 +231,103 @@ class data_lib_testcase extends advanced_testcase { $this->assertEquals($url, $event->get_url()); $this->assertEventContextNotUsed($event); } + + /** + * Tests for mod_data_rating_can_see_item_ratings(). + * + * @throws coding_exception + * @throws rating_exception + */ + public function test_mod_data_rating_can_see_item_ratings() { + global $DB; + + $this->resetAfterTest(); + + // Setup test data. + $course = new stdClass(); + $course->groupmode = SEPARATEGROUPS; + $course->groupmodeforce = true; + $course = $this->getDataGenerator()->create_course($course); + $data = $this->getDataGenerator()->create_module('data', array('course' => $course->id)); + $cm = get_coursemodule_from_instance('data', $data->id); + $context = context_module::instance($cm->id); + + // Create users. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(); + $user4 = $this->getDataGenerator()->create_user(); + + // Groups and stuff. + $role = $DB->get_record('role', array('shortname' => 'teacher'), '*', MUST_EXIST); + $this->getDataGenerator()->enrol_user($user1->id, $course->id, $role->id); + $this->getDataGenerator()->enrol_user($user2->id, $course->id, $role->id); + $this->getDataGenerator()->enrol_user($user3->id, $course->id, $role->id); + $this->getDataGenerator()->enrol_user($user4->id, $course->id, $role->id); + + $group1 = $this->getDataGenerator()->create_group(array('courseid' => $course->id)); + $group2 = $this->getDataGenerator()->create_group(array('courseid' => $course->id)); + groups_add_member($group1, $user1); + groups_add_member($group1, $user2); + groups_add_member($group2, $user3); + groups_add_member($group2, $user4); + + // Add data. + $field = data_get_field_new('text', $data); + + $fielddetail = new stdClass(); + $fielddetail->name = 'Name'; + $fielddetail->description = 'Some name'; + + $field->define_field($fielddetail); + $field->insert_field(); + $recordid = data_add_record($data, $group1->id); + + $datacontent = array(); + $datacontent['fieldid'] = $field->field->id; + $datacontent['recordid'] = $recordid; + $datacontent['content'] = 'Asterix'; + $DB->insert_record('data_content', $datacontent); + + // Now try to access it as various users. + unassign_capability('moodle/site:accessallgroups', $role->id); + $params = array('contextid' => 2, + 'component' => 'mod_data', + 'ratingarea' => 'entry', + 'itemid' => $recordid, + 'scaleid' => 2); + $this->setUser($user1); + $this->assertTrue(mod_data_rating_can_see_item_ratings($params)); + $this->setUser($user2); + $this->assertTrue(mod_data_rating_can_see_item_ratings($params)); + $this->setUser($user3); + $this->assertFalse(mod_data_rating_can_see_item_ratings($params)); + $this->setUser($user4); + $this->assertFalse(mod_data_rating_can_see_item_ratings($params)); + + // Now try with accessallgroups cap and make sure everything is visible. + assign_capability('moodle/site:accessallgroups', CAP_ALLOW, $role->id, $context->id); + $this->setUser($user1); + $this->assertTrue(mod_data_rating_can_see_item_ratings($params)); + $this->setUser($user2); + $this->assertTrue(mod_data_rating_can_see_item_ratings($params)); + $this->setUser($user3); + $this->assertTrue(mod_data_rating_can_see_item_ratings($params)); + $this->setUser($user4); + $this->assertTrue(mod_data_rating_can_see_item_ratings($params)); + + // Change group mode and verify visibility. + $course->groupmode = VISIBLEGROUPS; + $DB->update_record('course', $course); + unassign_capability('moodle/site:accessallgroups', $role->id); + $this->setUser($user1); + $this->assertTrue(mod_data_rating_can_see_item_ratings($params)); + $this->setUser($user2); + $this->assertTrue(mod_data_rating_can_see_item_ratings($params)); + $this->setUser($user3); + $this->assertTrue(mod_data_rating_can_see_item_ratings($params)); + $this->setUser($user4); + $this->assertTrue(mod_data_rating_can_see_item_ratings($params)); + + } } diff --git a/mod/forum/lib.php b/mod/forum/lib.php index a0eeb1fead1..8317269bd7e 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -3664,6 +3664,48 @@ function forum_rating_validate($params) { return true; } +/** + * Can the current user see ratings for a given itemid? + * + * @param array $params submitted data + * contextid => int contextid [required] + * component => The component for this module - should always be mod_forum [required] + * ratingarea => object the context in which the rated items exists [required] + * itemid => int the ID of the object being rated [required] + * scaleid => int scale id [optional] + * @return bool + * @throws coding_exception + * @throws rating_exception + */ +function mod_forum_rating_can_see_item_ratings($params) { + global $DB, $USER; + + // Check the component is mod_forum. + if (!isset($params['component']) || $params['component'] != 'mod_forum') { + throw new rating_exception('invalidcomponent'); + } + + // Check the ratingarea is post (the only rating area in forum). + if (!isset($params['ratingarea']) || $params['ratingarea'] != 'post') { + throw new rating_exception('invalidratingarea'); + } + + if (!isset($params['itemid'])) { + throw new rating_exception('invaliditemid'); + } + + $post = $DB->get_record('forum_posts', array('id' => $params['itemid']), '*', MUST_EXIST); + $discussion = $DB->get_record('forum_discussions', array('id' => $post->discussion), '*', MUST_EXIST); + $forum = $DB->get_record('forum', array('id' => $discussion->forum), '*', MUST_EXIST); + $course = $DB->get_record('course', array('id' => $forum->course), '*', MUST_EXIST); + $cm = get_coursemodule_from_instance('forum', $forum->id, $course->id , false, MUST_EXIST); + + // Perform some final capability checks. + if (!forum_user_can_see_post($forum, $discussion, $post, $USER, $cm)) { + return false; + } + return true; +} /** * This function prints the overview of a discussion in the forum listing. diff --git a/mod/forum/tests/lib_test.php b/mod/forum/tests/lib_test.php index 53c1716d92f..1bcae2c935a 100644 --- a/mod/forum/tests/lib_test.php +++ b/mod/forum/tests/lib_test.php @@ -26,6 +26,7 @@ defined('MOODLE_INTERNAL') || die(); global $CFG; require_once($CFG->dirroot . '/mod/forum/lib.php'); +require_once($CFG->dirroot . '/rating/lib.php'); class mod_forum_lib_testcase extends advanced_testcase { @@ -1317,4 +1318,107 @@ class mod_forum_lib_testcase extends advanced_testcase { return $discussion; } + /** + * Tests for mod_forum_rating_can_see_item_ratings(). + * + * @throws coding_exception + * @throws rating_exception + */ + public function test_mod_forum_rating_can_see_item_ratings() { + global $DB; + + $this->resetAfterTest(); + + // Setup test data. + $course = new stdClass(); + $course->groupmode = SEPARATEGROUPS; + $course->groupmodeforce = true; + $course = $this->getDataGenerator()->create_course($course); + $forum = $this->getDataGenerator()->create_module('forum', array('course' => $course->id)); + $generator = self::getDataGenerator()->get_plugin_generator('mod_forum'); + $cm = get_coursemodule_from_instance('forum', $forum->id); + $context = context_module::instance($cm->id); + + // Create users. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(); + $user4 = $this->getDataGenerator()->create_user(); + + // Groups and stuff. + $role = $DB->get_record('role', array('shortname' => 'teacher'), '*', MUST_EXIST); + $this->getDataGenerator()->enrol_user($user1->id, $course->id, $role->id); + $this->getDataGenerator()->enrol_user($user2->id, $course->id, $role->id); + $this->getDataGenerator()->enrol_user($user3->id, $course->id, $role->id); + $this->getDataGenerator()->enrol_user($user4->id, $course->id, $role->id); + + $group1 = $this->getDataGenerator()->create_group(array('courseid' => $course->id)); + $group2 = $this->getDataGenerator()->create_group(array('courseid' => $course->id)); + groups_add_member($group1, $user1); + groups_add_member($group1, $user2); + groups_add_member($group2, $user3); + groups_add_member($group2, $user4); + + $record = new stdClass(); + $record->course = $forum->course; + $record->forum = $forum->id; + $record->userid = $user1->id; + $record->groupid = $group1->id; + $discussion = $generator->create_discussion($record); + + // Retrieve the first post. + $post = $DB->get_record('forum_posts', array('discussion' => $discussion->id)); + + $ratingoptions = new stdClass; + $ratingoptions->context = $context; + $ratingoptions->ratingarea = 'post'; + $ratingoptions->component = 'mod_forum'; + $ratingoptions->itemid = $post->id; + $ratingoptions->scaleid = 2; + $ratingoptions->userid = $user2->id; + $rating = new rating($ratingoptions); + $rating->update_rating(2); + + // Now try to access it as various users. + unassign_capability('moodle/site:accessallgroups', $role->id); + $params = array('contextid' => 2, + 'component' => 'mod_forum', + 'ratingarea' => 'post', + 'itemid' => $post->id, + 'scaleid' => 2); + $this->setUser($user1); + $this->assertTrue(mod_forum_rating_can_see_item_ratings($params)); + $this->setUser($user2); + $this->assertTrue(mod_forum_rating_can_see_item_ratings($params)); + $this->setUser($user3); + $this->assertFalse(mod_forum_rating_can_see_item_ratings($params)); + $this->setUser($user4); + $this->assertFalse(mod_forum_rating_can_see_item_ratings($params)); + + // Now try with accessallgroups cap and make sure everything is visible. + assign_capability('moodle/site:accessallgroups', CAP_ALLOW, $role->id, $context->id); + $this->setUser($user1); + $this->assertTrue(mod_forum_rating_can_see_item_ratings($params)); + $this->setUser($user2); + $this->assertTrue(mod_forum_rating_can_see_item_ratings($params)); + $this->setUser($user3); + $this->assertTrue(mod_forum_rating_can_see_item_ratings($params)); + $this->setUser($user4); + $this->assertTrue(mod_forum_rating_can_see_item_ratings($params)); + + // Change group mode and verify visibility. + $course->groupmode = VISIBLEGROUPS; + $DB->update_record('course', $course); + unassign_capability('moodle/site:accessallgroups', $role->id); + $this->setUser($user1); + $this->assertTrue(mod_forum_rating_can_see_item_ratings($params)); + $this->setUser($user2); + $this->assertTrue(mod_forum_rating_can_see_item_ratings($params)); + $this->setUser($user3); + $this->assertTrue(mod_forum_rating_can_see_item_ratings($params)); + $this->setUser($user4); + $this->assertTrue(mod_forum_rating_can_see_item_ratings($params)); + + } + } diff --git a/mod/upgrade.txt b/mod/upgrade.txt index d380281885f..472ea5039b8 100644 --- a/mod/upgrade.txt +++ b/mod/upgrade.txt @@ -1,6 +1,11 @@ This files describes API changes in /mod/* - activity modules, information provided here is intended especially for developers. +=== 2.8.8 === + +* Modules using rating component must implement a callback mod_x_rating_can_see_item_ratings(). Refer + to mod_forum_rating_can_see_item_ratings() for example. + === 2.8 === * Constant FEATURE_GROUPMEMBERSONLY is deprecated. Modules should remove this diff --git a/rating/index.php b/rating/index.php index ae516100d32..e61857290b9 100644 --- a/rating/index.php +++ b/rating/index.php @@ -55,7 +55,13 @@ if ($popup) { $PAGE->set_pagelayout('popup'); } -if (!has_capability('moodle/rating:view', $context)) { +$params = array('contextid' => $contextid, + 'component' => $component, + 'ratingarea' => $ratingarea, + 'itemid' => $itemid, + 'scaleid' => $scaleid); +if (!has_capability('moodle/rating:view', $context) || + !component_callback($component, 'rating_can_see_item_ratings', array($params), true)) { print_error('noviewrate', 'rating'); } -- 2.11.4.GIT