Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

MDL-26838 rating: multiple fixes after a review of rating validation …

…code
  • Loading branch information...
commit 778361c3306de1799116162193cc259ec87a1fb2 1 parent 2c2ff8d
@andyjdavis andyjdavis authored
View
32 mod/data/lib.php
@@ -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
@@ -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');
}
}
@@ -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');
}
}
View
37 mod/forum/lib.php
@@ -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
@@ -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');
}
}
@@ -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;
View
27 mod/glossary/lib.php
@@ -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
@@ -494,24 +494,29 @@ 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');
}
}
@@ -519,13 +524,13 @@ function glossary_rating_add($params) {
$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;
View
16 rating/lib.php
@@ -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);
@@ -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) {
@@ -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');
+ }
+}
View
2  rating/rate.php
@@ -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();
View
2  rating/rate_ajax.php
@@ -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();
Please sign in to comment.
Something went wrong with that request. Please try again.