Skip to content

Commit

Permalink
MDL-26838 rating: multiple fixes after a review of rating validation …
Browse files Browse the repository at this point in the history 
…code
  • Loading branch information
@andyjdavis
andyjdavis committed May 4, 2011
1 parent 2c2ff8d commit 778361c
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 47 deletions.
32 changes: 18 additions & 14 deletions mod/data/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1373,14 +1373,13 @@ function data_rating_permissions($options) {
* rating => int the submitted rating
* rateduserid => int the id of the user whose items have been rated. NOT the user who submitted the ratings. 0 to update all. [required]
* aggregation => int the aggregation method to apply when calculating grades ie RATING_AGGREGATE_AVERAGE [required]
* @return boolean true if the rating is valid
* @return boolean true if the rating is valid. Will throw rating_exception if not
*/
function data_rating_add($params) {
function data_rating_validate($params) {
global $DB, $USER;

if (!array_key_exists('itemid', $params) || !array_key_exists('context', $params)) {
debugging('itemid or context not supplied');
return false;
if (!array_key_exists('itemid', $params) || !array_key_exists('context', $params) || !array_key_exists('rateduserid', $params)) {
throw new rating_exception('missingparameter');
}

$datasql = "SELECT d.id as did, d.course, r.userid as userid, d.approval, r.approved, r.timecreated, d.assesstimestart, d.assesstimefinish, r.groupid
Expand All @@ -1389,24 +1388,29 @@ function data_rating_add($params) {
WHERE r.id = :itemid";
$dataparams = array('itemid'=>$params['itemid']);
if (!$info = $DB->get_record_sql($datasql, $dataparams)) {
//item id doesn't exist
return false;
//item doesn't exist
throw new rating_exception('invaliditemid');
}

if ($info->userid == $USER->id) {
//user is attempting to rate their own glossary entry
return false;
throw new rating_exception('nopermissiontorate');
}

if ($params['rateduserid'] != $info->userid) {
//supplied user ID doesnt match the user ID from the database
throw new rating_exception('invaliduserid');
}

if ($info->approval && !$info->approved) {
//database requires approval but this item isnt approved
return false;
throw new rating_exception('nopermissiontorate');
}

//check the item we're rating was created in the assessable time window
if (!empty($info->assesstimestart) && !empty($info->assesstimefinish)) {
if ($info->timecreated < $info->assesstimestart || $info->timecreated > $info->assesstimefinish) {
return false;
throw new rating_exception('notavailable');
}
}

Expand All @@ -1416,25 +1420,25 @@ function data_rating_add($params) {

$cm = get_coursemodule_from_instance('data', $dataid);
if (empty($cm)) {
return false;
throw new rating_exception('unknowncontext');
}
$context = get_context_instance(CONTEXT_MODULE, $cm->id);

//if the supplied context doesnt match the item's context
if (empty($context) || $context->id != $params['context']->id) {
return false;
throw new rating_exception('invalidcontext');
}

// Make sure groups allow this user to see the item they're rating
$course = $DB->get_record('course', array('id'=>$courseid), '*', MUST_EXIST);
if ($groupid > 0 and $groupmode = groups_get_activity_groupmode($cm, $course)) { // Groups are being used
if (!groups_group_exists($groupid)) { // Can't find group
return false;//something is wrong
throw new rating_exception('cannotfindgroup');//something is wrong
}

if (!groups_is_member($groupid) and !has_capability('moodle/site:accessallgroups', $context)) {
// do not allow rating of posts from other groups when in SEPARATEGROUPS or VISIBLEGROUPS
return false;
throw new rating_exception('notmemberofgroup');
}
}

Expand Down
37 changes: 21 additions & 16 deletions mod/forum/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3465,13 +3465,13 @@ function forum_rating_permissions($contextid) {
* rating => int the submitted rating [required]
* rateduserid => int the id of the user whose items have been rated. NOT the user who submitted the ratings. 0 to update all. [required]
* aggregation => int the aggregation method to apply when calculating grades ie RATING_AGGREGATE_AVERAGE [required]
* @return boolean true if the rating is valid
* @return boolean true if the rating is valid. Will throw rating_exception if not
*/
function forum_rating_add($params) {
function forum_rating_validate($params) {
global $DB, $USER;

if (!array_key_exists('itemid', $params) || !array_key_exists('context', $params)) {
return false;
if (!array_key_exists('itemid', $params) || !array_key_exists('context', $params) || !array_key_exists('rateduserid', $params)) {
throw new rating_exception('missingparameter');
}

$forumsql = "SELECT f.id as fid, f.course, d.id as did, p.userid as userid, p.created, f.assesstimestart, f.assesstimefinish, d.groupid
Expand All @@ -3481,19 +3481,24 @@ function forum_rating_add($params) {
WHERE p.id = :itemid";
$forumparams = array('itemid'=>$params['itemid']);
if (!$info = $DB->get_record_sql($forumsql, $forumparams)) {
//item id doesn't exist
return false;
//item doesn't exist
throw new rating_exception('invaliditemid');
}

if ($info->userid == $USER->id) {
//user is attempting to rate their own post
return false;
throw new rating_exception('nopermissiontorate');
}

if ($params['rateduserid'] != $info->userid) {
//supplied user ID doesnt match the user ID from the database
throw new rating_exception('invaliduserid');
}

//check the item we're rating was created in the assessable time window
if (!empty($info->assesstimestart) && !empty($info->assesstimefinish)) {
if ($info->timecreated < $info->assesstimestart || $info->timecreated > $info->assesstimefinish) {
return false;
throw new rating_exception('notavailable');
}
}

Expand All @@ -3504,43 +3509,43 @@ function forum_rating_add($params) {

$cm = get_coursemodule_from_instance('forum', $forumid);
if (empty($cm)) {
return false;
throw new rating_exception('unknowncontext');
}
$context = get_context_instance(CONTEXT_MODULE, $cm->id);

//if the supplied context doesnt match the item's context
if (empty($context) || $context->id != $params['context']->id) {
return false;
throw new rating_exception('invalidcontext');
}

// Make sure groups allow this user to see the item they're rating
$course = $DB->get_record('course', array('id'=>$courseid), '*', MUST_EXIST);
if ($groupid > 0 and $groupmode = groups_get_activity_groupmode($cm, $course)) { // Groups are being used
if (!groups_group_exists($groupid)) { // Can't find group
return false;//something is wrong
throw new rating_exception('cannotfindgroup');//something is wrong
}

if (!groups_is_member($groupid) and !has_capability('moodle/site:accessallgroups', $context)) {
// do not allow rating of posts from other groups when in SEPARATEGROUPS or VISIBLEGROUPS
return false;
throw new rating_exception('notmemberofgroup');
}
}

//need to load the full objects here as ajax scripts don't like
//the debugging messages produced by forum_user_can_see_post() if you just supply IDs
if (!$forum = $DB->get_record('forum',array('id'=>$forumid))) {
return false;
throw new rating_exception('invalidrecordunknown');
}
if (!$post = $DB->get_record('forum_posts',array('id'=>$params['itemid']))) {
return false;
throw new rating_exception('invalidrecordunknown');
}
if (!$discussion = $DB->get_record('forum_discussions',array('id'=>$discussionid))) {
return false;
throw new rating_exception('invalidrecordunknown');
}

//perform some final capability checks
if( !forum_user_can_see_post($forum, $discussion, $post, $USER, $cm)) {
return false;
throw new rating_exception('nopermissiontorate');
}

return true;
Expand Down
27 changes: 16 additions & 11 deletions mod/glossary/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,13 +479,13 @@ function glossary_rating_permissions($options) {
* rating => int the submitted rating
* rateduserid => int the id of the user whose items have been rated. NOT the user who submitted the ratings. 0 to update all. [required]
* aggregation => int the aggregation method to apply when calculating grades ie RATING_AGGREGATE_AVERAGE [optional]
* @return boolean true if the rating is valid
* @return boolean true if the rating is valid. Will throw rating_exception if not
*/
function glossary_rating_add($params) {
function glossary_rating_validate($params) {
global $DB, $USER;

if (!array_key_exists('itemid', $params) || !array_key_exists('context', $params)) {
return false;
if (!array_key_exists('itemid', $params) || !array_key_exists('context', $params) || !array_key_exists('rateduserid', $params)) {
throw new rating_exception('missingparameter');
}

$glossarysql = "SELECT g.id as gid, e.userid as userid, e.approved, e.timecreated, g.assesstimestart, g.assesstimefinish
Expand All @@ -494,38 +494,43 @@ function glossary_rating_add($params) {
WHERE e.id = :itemid";
$glossaryparams = array('itemid'=>$params['itemid']);
if (!$info = $DB->get_record_sql($glossarysql, $glossaryparams)) {
//item id doesn't exist
return false;
//item doesn't exist
throw new rating_exception('invaliditemid');
}

if ($info->userid == $USER->id) {
//user is attempting to rate their own glossary entry
return false;
throw new rating_exception('nopermissiontorate');
}

if ($params['rateduserid'] != $info->userid) {
//supplied user ID doesnt match the user ID from the database
throw new rating_exception('invaliduserid');
}

if (!$info->approved) {
//item isnt approved
return false;
throw new rating_exception('nopermissiontorate');
}

//check the item we're rating was created in the assessable time window
if (!empty($info->assesstimestart) && !empty($info->assesstimefinish)) {
if ($info->timecreated < $info->assesstimestart || $info->timecreated > $info->assesstimefinish) {
return false;
throw new rating_exception('notavailable');
}
}

$glossaryid = $info->gid;

$cm = get_coursemodule_from_instance('glossary', $glossaryid);
if (empty($cm)) {
return false;
throw new rating_exception('unknowncontext');
}
$context = get_context_instance(CONTEXT_MODULE, $cm->id);

//if the supplied context doesnt match the item's context
if (empty($context) || $context->id != $params['context']->id) {
return false;
throw new rating_exception('invalidcontext');
}

return true;
Expand Down
16 changes: 12 additions & 4 deletions rating/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -607,14 +607,14 @@ public function get_aggregation_method($aggregate) {
}

/**
* Looks for a callback and retrieves permissions from the plugin whose items are being rated
* Looks for a callback like forum_rating_permissions() to retrieve permissions from the plugin whose items are being rated
* @param int $contextid The current context id
* @param string component the name of the component that is using ratings ie 'mod_forum'
* @return array rating related permissions
*/
public function get_plugin_permissions_array($contextid, $component=null) {
$pluginpermissionsarray = null;
$defaultpluginpermissions = array('rate'=>true,'view'=>true,'viewany'=>true,'viewall'=>true);//all true == rely on system level permissions if no plugin callback is defined
$defaultpluginpermissions = array('rate'=>false,'view'=>false,'viewany'=>false,'viewall'=>false);//deny by default
if (!empty($component)) {
list($type, $name) = normalize_component($component);
$pluginpermissionsarray = plugin_callback($type, $name, 'rating', 'permissions', array($contextid), $defaultpluginpermissions);
Expand All @@ -633,14 +633,14 @@ public function get_plugin_permissions_array($contextid, $component=null) {
* rating => int the submitted rating
* rateduserid => int the id of the user whose items have been rated. NOT the user who submitted the ratings. 0 to update all. [required]
* aggregation => int the aggregation method to apply when calculating grades ie RATING_AGGREGATE_AVERAGE [optional]
* @return boolean true if the rating is valid
* @return boolean true if the rating is valid. False if callback wasnt found and will throw rating_exception if rating is invalid
*/
public function check_rating_is_valid($component, $params) {
list($plugintype, $pluginname) = normalize_component($component);

//this looks for a function like forum_rating_is_valid() in mod_forum lib.php
//wrapping the params array in another array as call_user_func_array() expands arrays into multiple arguments
$isvalid = plugin_callback($plugintype, $pluginname, 'rating', 'add', array($params), null);
$isvalid = plugin_callback($plugintype, $pluginname, 'rating', 'validate', array($params), null);

//if null then the callback doesn't exist
if ($isvalid === null) {
Expand All @@ -651,3 +651,11 @@ public function check_rating_is_valid($component, $params) {
return $isvalid;
}
}//end rating_manager class definition

class rating_exception extends moodle_exception {
public $message;
function __construct($errorcode) {
$this->errorcode = $errorcode;
$this->message = get_string($errorcode, 'error');
}
}
2 changes: 1 addition & 1 deletion rating/rate.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
$PAGE->set_context($context);
$PAGE->set_url('/rating/rate.php', array('contextid'=>$context->id));

if (!confirm_sesskey() || $USER->id==$rateduserid || !has_capability('moodle/rating:rate',$context)) {
if (!confirm_sesskey() || !has_capability('moodle/rating:rate',$context)) {
echo $OUTPUT->header();
echo get_string('ratepermissiondenied', 'rating');
echo $OUTPUT->footer();
Expand Down
2 changes: 1 addition & 1 deletion rating/rate_ajax.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
$PAGE->set_context($context);
$PAGE->set_url('/rating/rate_ajax.php', array('contextid'=>$context->id));

if (!confirm_sesskey() || $USER->id==$rateduserid || !has_capability('moodle/rating:rate',$context)) {
if (!confirm_sesskey() || !has_capability('moodle/rating:rate',$context)) {
echo $OUTPUT->header();
echo get_string('ratepermissiondenied', 'rating');
echo $OUTPUT->footer();
Expand Down

0 comments on commit 778361c

Please sign in to comment.