From a5224e1f83eb03e0627d9082fb123c1d24a87276 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Wed, 22 Jun 2011 01:01:06 +0200 Subject: [PATCH 1/4] rating/lib.php unit tests 1st cut --- rating/simpletest/testrating.php | 203 +++++++++++++++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 rating/simpletest/testrating.php diff --git a/rating/simpletest/testrating.php b/rating/simpletest/testrating.php new file mode 100644 index 00000000000..73accba3ee1 --- /dev/null +++ b/rating/simpletest/testrating.php @@ -0,0 +1,203 @@ +. + +/** + * Unit tests for rating/lib.php + * + * @package moodlecore + * @subpackage rating + * @copyright 2011 onwards Eloy Lafuente (stronk7) {@link http://stronk7.com} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +// Include all the needed stuff +require_once($CFG->dirroot . '/rating/lib.php'); + +/** + * Unit test case for all the rating/lib.php requiring DB mockup & manipulation + */ +class rating_db_test extends UnitTestCaseUsingDatabase { + + public static $includecoverage = array( + 'rating/lib.php' + ); + + protected $testtables = array( + 'lib' => array( + 'rating', 'scale')); + + public function setUp() { + parent::setUp(); + + $this->switch_to_test_db(); // Switch to test DB for all the execution + + foreach ($this->testtables as $dir => $tables) { + $this->create_test_tables($tables, $dir); // Create tables + } + } + + /** + * Test the current get_ratings method main sql + */ + function test_get_ratings_sql() { + + // We load 6 ratings, 2 performed by each (1, 2, 3) user to items corresponding to the other users + $ctxid = SYSCONTEXTID; + $this->load_test_data('rating', + array('contextid', 'component', 'ratingarea', 'itemid', 'scaleid', 'rating', 'userid', 'timecreated', 'timemodified'), array( + array( $ctxid , 'mod_forum', 'post', 2 , 10 , 1 , 1 , 1 , 1), + array( $ctxid , 'mod_forum', 'post', 3 , 10 , 3 , 1 , 1 , 1), + array( $ctxid , 'mod_forum', 'post', 1 , 10 , 1 , 2 , 1 , 1), + array( $ctxid , 'mod_forum', 'post', 3 , 10 , 5 , 2 , 1 , 1), + array( $ctxid , 'mod_forum', 'post', 1 , 10 , 3 , 3 , 1 , 1), + array( $ctxid , 'mod_forum', 'post', 2 , 10 , 5 , 3 , 1 , 1))); + + // Create 1 post (item) by user 1 (rated above by user 2 and 3 with average = 2) + $user1posts = array( + (object)array('id' => 1, 'userid' => 1, 'message' => 'hello')); + // Create 1 post (item) by user 2 (rated above by user 1 and 3 with average = 3) + $user2posts = array( + (object)array('id' => 2, 'userid' => 2, 'message' => 'world')); + // Create 1 post (item) by user 3 (rated above by user 1 and 2 with average = 4) + $user3posts = array( + (object)array('id' => 3, 'userid' => 3, 'message' => 'moodle')); + + // Prepare the default options + $defaultoptions = array ( + 'context' => get_context_instance(CONTEXT_SYSTEM), + 'component' => 'mod_forum', + 'ratingarea' => 'post', + 'scaleid' => 10, + 'aggregate' => RATING_AGGREGATE_AVERAGE); + + $rm = new rating_manager(); + + // STEP 1: Aparently, agreggation and counts are always returned ok + + // Get results for items of user 1 (expected average 1 + 3 / 2 = 2) + $toptions = (object)array_merge($defaultoptions, array('items' => $user1posts)); + $result = $rm->get_ratings($toptions); + $this->assertEqual(count($result), count($user1posts)); + $this->assertEqual($result[0]->id, $user1posts[0]->id); + $this->assertEqual($result[0]->userid, $user1posts[0]->userid); + $this->assertEqual($result[0]->message, $user1posts[0]->message); + $this->assertEqual($result[0]->rating->count, 2); + $this->assertEqual($result[0]->rating->aggregate, 2); + + // Get results for items of user 2 (expected average 1 + 5 / 2 = 3) + $toptions = (object)array_merge($defaultoptions, array('items' => $user2posts)); + $result = $rm->get_ratings($toptions); + $this->assertEqual(count($result), count($user2posts)); + $this->assertEqual($result[0]->id, $user2posts[0]->id); + $this->assertEqual($result[0]->userid, $user2posts[0]->userid); + $this->assertEqual($result[0]->message, $user2posts[0]->message); + $this->assertEqual($result[0]->rating->count, 2); + $this->assertEqual($result[0]->rating->aggregate, 3); + + // Get results for items of user 3 (expected average 3 + 5 / 2 = 4) + $toptions = (object)array_merge($defaultoptions, array('items' => $user3posts)); + $result = $rm->get_ratings($toptions); + $this->assertEqual(count($result), count($user3posts)); + $this->assertEqual($result[0]->id, $user3posts[0]->id); + $this->assertEqual($result[0]->userid, $user3posts[0]->userid); + $this->assertEqual($result[0]->message, $user3posts[0]->message); + $this->assertEqual($result[0]->rating->count, 2); + $this->assertEqual($result[0]->rating->aggregate, 4); + + // Get results for items of user 1 & 2 together (expected averages are 2 and 3, as tested above) + $posts = array_merge($user1posts, $user2posts); + $toptions = (object)array_merge($defaultoptions, array('items' => $posts)); + $result = $rm->get_ratings($toptions); + $this->assertEqual(count($result), count($posts)); + $this->assertEqual($result[0]->id, $posts[0]->id); + $this->assertEqual($result[0]->userid, $posts[0]->userid); + $this->assertEqual($result[0]->message, $posts[0]->message); + $this->assertEqual($result[0]->rating->count, 2); + $this->assertEqual($result[0]->rating->aggregate, 2); + + $this->assertEqual($result[1]->id, $posts[1]->id); + $this->assertEqual($result[1]->userid, $posts[1]->userid); + $this->assertEqual($result[1]->message, $posts[1]->message); + $this->assertEqual($result[1]->rating->count, 2); + $this->assertEqual($result[1]->rating->aggregate, 3); + + // STEP 2: But what happens if only ratings performed by one user are requested? Do we expect + // complete aggregations and counts? + + // Get results for items of user 1 rated by user 2 (averages and counts must be the complete ones?) + $toptions = (object)array_merge($defaultoptions, array('items' => $user1posts, 'userid' => 2)); + $result = $rm->get_ratings($toptions); + $this->assertEqual(count($result), count($user1posts)); + $this->assertEqual($result[0]->id, $user1posts[0]->id); + $this->assertEqual($result[0]->userid, $user1posts[0]->userid); + $this->assertEqual($result[0]->message, $user1posts[0]->message); + $this->assertEqual($result[0]->rating->count, 2); + $this->assertEqual($result[0]->rating->aggregate, 2); + $this->assertEqual($result[0]->rating->userid, $toptions->userid); // Must be the passed userid + + // Get results for items of user 1 rated by user 3 (averages and counts must be the complete ones?) + $toptions = (object)array_merge($defaultoptions, array('items' => $user1posts, 'userid' => 3)); + $result = $rm->get_ratings($toptions); + $this->assertEqual(count($result), count($user1posts)); + $this->assertEqual($result[0]->id, $user1posts[0]->id); + $this->assertEqual($result[0]->userid, $user1posts[0]->userid); + $this->assertEqual($result[0]->message, $user1posts[0]->message); + $this->assertEqual($result[0]->rating->count, 2); + $this->assertEqual($result[0]->rating->aggregate, 2); + $this->assertEqual($result[0]->rating->userid, $toptions->userid); // Must be the passed userid + + // Get results for items of user 1 & 2 together rated by user 3 (averages and counts must be the complete ones?) + $posts = array_merge($user1posts, $user2posts); + $toptions = (object)array_merge($defaultoptions, array('items' => $posts, 'userid' => 3)); + $result = $rm->get_ratings($toptions); + $this->assertEqual(count($result), count($posts)); + $this->assertEqual($result[0]->id, $posts[0]->id); + $this->assertEqual($result[0]->userid, $posts[0]->userid); + $this->assertEqual($result[0]->message, $posts[0]->message); + $this->assertEqual($result[0]->rating->count, 2); + $this->assertEqual($result[0]->rating->aggregate, 2); + $this->assertEqual($result[0]->rating->userid, $toptions->userid); // Must be the passed userid + + $this->assertEqual($result[1]->id, $posts[1]->id); + $this->assertEqual($result[1]->userid, $posts[1]->userid); + $this->assertEqual($result[1]->message, $posts[1]->message); + $this->assertEqual($result[1]->rating->count, 2); + $this->assertEqual($result[1]->rating->aggregate, 3); + $this->assertEqual($result[1]->rating->userid, $toptions->userid); // Must be the passed userid + + // STEP 3: But there are a lot of information that shouldn't be there at all, these shouldn't fail + + // Get results for items of user 1 (expected average 1 + 3 / 2 = 2) + // (we have tested this above, but now we are going to look to other variables) + $toptions = (object)array_merge($defaultoptions, array('items' => $user1posts)); + $result = $rm->get_ratings($toptions); + // This test fails! Let's name it T1 + $this->assertNull($result[0]->rating->userid); // There are 2 users rating this, no way we are returning "randomly" one here + // Same for rating->[scaleid, permissions, rating, id.... all them are not unique in this example (there are 2 ratings) so we should nullify them + // or they will be failing here + + // Get results for items of user 2 (expected average 1 + 5 / 2 = 3) + // (we have tested this above, but now we are going to look to other variables) + $toptions = (object)array_merge($defaultoptions, array('items' => $user2posts)); + $result = $rm->get_ratings($toptions); + // This test pass! It's exactly the same than T1 above, so random results are returned!! + $this->assertNull($result[0]->rating->userid); // There are 2 users rating this, no way we are returning "randomly" one here + // Same for rating->[scaleid, permissions, rating, id.... all them are not unique in this example (there are 2 ratings) so we should nullify them + // or they will be failing here + } +} From fc391624f06ebe28830148080d8e51c8c368001c Mon Sep 17 00:00:00 2001 From: "Andrew Davis (andyjdavis)" Date: Fri, 22 Jul 2011 15:57:14 +0800 Subject: [PATCH 2/4] MDL-27845 rating: updated rating unit tests --- rating/simpletest/testrating.php | 80 ++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/rating/simpletest/testrating.php b/rating/simpletest/testrating.php index 73accba3ee1..910544c08da 100644 --- a/rating/simpletest/testrating.php +++ b/rating/simpletest/testrating.php @@ -56,24 +56,31 @@ class rating_db_test extends UnitTestCaseUsingDatabase { */ function test_get_ratings_sql() { - // We load 6 ratings, 2 performed by each (1, 2, 3) user to items corresponding to the other users + // We load 3 items. Each is rated twice. For simplicity itemid == user id of the item owner $ctxid = SYSCONTEXTID; $this->load_test_data('rating', array('contextid', 'component', 'ratingarea', 'itemid', 'scaleid', 'rating', 'userid', 'timecreated', 'timemodified'), array( - array( $ctxid , 'mod_forum', 'post', 2 , 10 , 1 , 1 , 1 , 1), - array( $ctxid , 'mod_forum', 'post', 3 , 10 , 3 , 1 , 1 , 1), - array( $ctxid , 'mod_forum', 'post', 1 , 10 , 1 , 2 , 1 , 1), - array( $ctxid , 'mod_forum', 'post', 3 , 10 , 5 , 2 , 1 , 1), - array( $ctxid , 'mod_forum', 'post', 1 , 10 , 3 , 3 , 1 , 1), - array( $ctxid , 'mod_forum', 'post', 2 , 10 , 5 , 3 , 1 , 1))); - // Create 1 post (item) by user 1 (rated above by user 2 and 3 with average = 2) + //user 1's items. Average == 2 + array( $ctxid , 'mod_forum', 'post', 1 , 10 , 1 , 2 , 1 , 1), + array( $ctxid , 'mod_forum', 'post', 1 , 10 , 3 , 3 , 1 , 1), + + //user 2's items. Average == 3 + array( $ctxid , 'mod_forum', 'post', 2 , 10 , 1 , 1 , 1 , 1), + array( $ctxid , 'mod_forum', 'post', 2 , 10 , 5 , 3 , 1 , 1), + + //user 3's items. Average == 4 + array( $ctxid , 'mod_forum', 'post', 3 , 10 , 3 , 1 , 1 , 1), + array( $ctxid , 'mod_forum', 'post', 3 , 10 , 5 , 2 , 1 , 1) + )); + + // a post (item) by user 1 (rated above by user 2 and 3 with average = 2) $user1posts = array( (object)array('id' => 1, 'userid' => 1, 'message' => 'hello')); - // Create 1 post (item) by user 2 (rated above by user 1 and 3 with average = 3) + // a post (item) by user 2 (rated above by user 1 and 3 with average = 3) $user2posts = array( (object)array('id' => 2, 'userid' => 2, 'message' => 'world')); - // Create 1 post (item) by user 3 (rated above by user 1 and 2 with average = 4) + // a post (item) by user 3 (rated above by user 1 and 2 with average = 4) $user3posts = array( (object)array('id' => 3, 'userid' => 3, 'message' => 'moodle')); @@ -87,9 +94,9 @@ class rating_db_test extends UnitTestCaseUsingDatabase { $rm = new rating_manager(); - // STEP 1: Aparently, agreggation and counts are always returned ok + // STEP 1: Retreive ratings using the current user - // Get results for items of user 1 (expected average 1 + 3 / 2 = 2) + // Get results for user 1's item (expected average 1 + 3 / 2 = 2) $toptions = (object)array_merge($defaultoptions, array('items' => $user1posts)); $result = $rm->get_ratings($toptions); $this->assertEqual(count($result), count($user1posts)); @@ -98,6 +105,8 @@ class rating_db_test extends UnitTestCaseUsingDatabase { $this->assertEqual($result[0]->message, $user1posts[0]->message); $this->assertEqual($result[0]->rating->count, 2); $this->assertEqual($result[0]->rating->aggregate, 2); + // Note that $result[0]->rating->rating is somewhat random + // We didn't supply a user ID so $USER was used which will vary depending on who runs the tests // Get results for items of user 2 (expected average 1 + 5 / 2 = 3) $toptions = (object)array_merge($defaultoptions, array('items' => $user2posts)); @@ -108,6 +117,8 @@ class rating_db_test extends UnitTestCaseUsingDatabase { $this->assertEqual($result[0]->message, $user2posts[0]->message); $this->assertEqual($result[0]->rating->count, 2); $this->assertEqual($result[0]->rating->aggregate, 3); + // Note that $result[0]->rating->rating is somewhat random + // We didn't supply a user ID so $USER was used which will vary depending on who runs the tests // Get results for items of user 3 (expected average 3 + 5 / 2 = 4) $toptions = (object)array_merge($defaultoptions, array('items' => $user3posts)); @@ -118,6 +129,8 @@ class rating_db_test extends UnitTestCaseUsingDatabase { $this->assertEqual($result[0]->message, $user3posts[0]->message); $this->assertEqual($result[0]->rating->count, 2); $this->assertEqual($result[0]->rating->aggregate, 4); + // Note that $result[0]->rating->rating is somewhat random + // We didn't supply a user ID so $USER was used which will vary depending on who runs the tests // Get results for items of user 1 & 2 together (expected averages are 2 and 3, as tested above) $posts = array_merge($user1posts, $user2posts); @@ -129,17 +142,21 @@ class rating_db_test extends UnitTestCaseUsingDatabase { $this->assertEqual($result[0]->message, $posts[0]->message); $this->assertEqual($result[0]->rating->count, 2); $this->assertEqual($result[0]->rating->aggregate, 2); + // Note that $result[0]->rating->rating is somewhat random + // We didn't supply a user ID so $USER was used which will vary depending on who runs the tests $this->assertEqual($result[1]->id, $posts[1]->id); $this->assertEqual($result[1]->userid, $posts[1]->userid); $this->assertEqual($result[1]->message, $posts[1]->message); $this->assertEqual($result[1]->rating->count, 2); $this->assertEqual($result[1]->rating->aggregate, 3); + // Note that $result[0]->rating->rating is somewhat random + // We didn't supply a user ID so $USER was used which will vary depending on who runs the tests - // STEP 2: But what happens if only ratings performed by one user are requested? Do we expect - // complete aggregations and counts? + // STEP 2: Retrieve ratings by a specified user + // We still expect complete aggregations and counts - // Get results for items of user 1 rated by user 2 (averages and counts must be the complete ones?) + // Get results for items of user 1 rated by user 2 (avg 2, rating 1) $toptions = (object)array_merge($defaultoptions, array('items' => $user1posts, 'userid' => 2)); $result = $rm->get_ratings($toptions); $this->assertEqual(count($result), count($user1posts)); @@ -148,9 +165,10 @@ class rating_db_test extends UnitTestCaseUsingDatabase { $this->assertEqual($result[0]->message, $user1posts[0]->message); $this->assertEqual($result[0]->rating->count, 2); $this->assertEqual($result[0]->rating->aggregate, 2); + $this->assertEqual($result[0]->rating->rating, 1); //user 2 rated user 1 "1" $this->assertEqual($result[0]->rating->userid, $toptions->userid); // Must be the passed userid - // Get results for items of user 1 rated by user 3 (averages and counts must be the complete ones?) + // Get results for items of user 1 rated by user 3 $toptions = (object)array_merge($defaultoptions, array('items' => $user1posts, 'userid' => 3)); $result = $rm->get_ratings($toptions); $this->assertEqual(count($result), count($user1posts)); @@ -159,9 +177,10 @@ class rating_db_test extends UnitTestCaseUsingDatabase { $this->assertEqual($result[0]->message, $user1posts[0]->message); $this->assertEqual($result[0]->rating->count, 2); $this->assertEqual($result[0]->rating->aggregate, 2); + $this->assertEqual($result[0]->rating->rating, 3); //user 3 rated user 1 "3" $this->assertEqual($result[0]->rating->userid, $toptions->userid); // Must be the passed userid - // Get results for items of user 1 & 2 together rated by user 3 (averages and counts must be the complete ones?) + // Get results for items of user 1 & 2 together rated by user 3 $posts = array_merge($user1posts, $user2posts); $toptions = (object)array_merge($defaultoptions, array('items' => $posts, 'userid' => 3)); $result = $rm->get_ratings($toptions); @@ -171,6 +190,7 @@ class rating_db_test extends UnitTestCaseUsingDatabase { $this->assertEqual($result[0]->message, $posts[0]->message); $this->assertEqual($result[0]->rating->count, 2); $this->assertEqual($result[0]->rating->aggregate, 2); + $this->assertEqual($result[0]->rating->rating, 3); //user 3 rated user 1 "3" $this->assertEqual($result[0]->rating->userid, $toptions->userid); // Must be the passed userid $this->assertEqual($result[1]->id, $posts[1]->id); @@ -178,26 +198,28 @@ class rating_db_test extends UnitTestCaseUsingDatabase { $this->assertEqual($result[1]->message, $posts[1]->message); $this->assertEqual($result[1]->rating->count, 2); $this->assertEqual($result[1]->rating->aggregate, 3); + $this->assertEqual($result[0]->rating->rating, 3); //user 3 rated user 2 "5" $this->assertEqual($result[1]->rating->userid, $toptions->userid); // Must be the passed userid - // STEP 3: But there are a lot of information that shouldn't be there at all, these shouldn't fail + // STEP 3: Some special cases - // Get results for items of user 1 (expected average 1 + 3 / 2 = 2) - // (we have tested this above, but now we are going to look to other variables) + // Get results for user 1's items (expected average 1 + 3 / 2 = 2) + // supplying a non-existent user id so no rating from that user should be found $toptions = (object)array_merge($defaultoptions, array('items' => $user1posts)); + $toptions->userid = 123456; //non-existent user $result = $rm->get_ratings($toptions); - // This test fails! Let's name it T1 - $this->assertNull($result[0]->rating->userid); // There are 2 users rating this, no way we are returning "randomly" one here - // Same for rating->[scaleid, permissions, rating, id.... all them are not unique in this example (there are 2 ratings) so we should nullify them - // or they will be failing here + $this->assertNull($result[0]->rating->userid); + $this->assertNull($result[0]->rating->rating); + $this->assertEqual($result[0]->rating->aggregate, 2);//should still get the aggregate // Get results for items of user 2 (expected average 1 + 5 / 2 = 3) - // (we have tested this above, but now we are going to look to other variables) + // Supplying the user id of the user who owns the items so no rating should be found $toptions = (object)array_merge($defaultoptions, array('items' => $user2posts)); + $toptions->userid = 2; //user 2 viewing the ratings of their own item $result = $rm->get_ratings($toptions); - // This test pass! It's exactly the same than T1 above, so random results are returned!! - $this->assertNull($result[0]->rating->userid); // There are 2 users rating this, no way we are returning "randomly" one here - // Same for rating->[scaleid, permissions, rating, id.... all them are not unique in this example (there are 2 ratings) so we should nullify them - // or they will be failing here + //these should be null as the user is viewing their own item and thus cannot rate + $this->assertNull($result[0]->rating->userid); + $this->assertNull($result[0]->rating->rating); + $this->assertEqual($result[0]->rating->aggregate, 3);//should still get the aggregate } } From 2f74865eef25c0a46f1146af1f931df6babc4cc5 Mon Sep 17 00:00:00 2001 From: "Andrew Davis (andyjdavis)" Date: Mon, 25 Jul 2011 13:46:49 +0800 Subject: [PATCH 3/4] MDL-27845 rating: split the get_ratings() query into two to reduce query complexity --- rating/lib.php | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/rating/lib.php b/rating/lib.php index f1a511e6ec3..00b30464b5e 100644 --- a/rating/lib.php +++ b/rating/lib.php @@ -567,22 +567,25 @@ class rating_manager { $params['component'] = $options->component; $params['ratingarea'] = $options->ratingarea; - $sql = "SELECT r.itemid, r.component, r.ratingarea, r.contextid, - $aggregatestr(r.rating) AS aggrrating, COUNT(r.rating) AS numratings, - ur.id, ur.userid, ur.scaleid, ur.rating AS usersrating + $sql = "SELECT r.itemid, r.id, r.userid, r.scaleid, r.rating AS usersrating + FROM {rating} r + WHERE r.userid = :userid AND + r.contextid = :contextid AND + r.itemid {$itemidtest} AND + r.component = :component AND + r.ratingarea = :ratingarea + ORDER BY r.itemid"; + $userratings = $DB->get_records_sql($sql, $params); + + $sql = "SELECT r.itemid, $aggregatestr(r.rating) AS aggrrating, COUNT(r.rating) AS numratings FROM {rating} r - LEFT JOIN {rating} ur ON ur.contextid = r.contextid AND - ur.itemid = r.itemid AND - ur.component = r.component AND - ur.ratingarea = r.ratingarea AND - ur.userid = :userid WHERE r.contextid = :contextid AND r.itemid {$itemidtest} AND r.component = :component AND r.ratingarea = :ratingarea - GROUP BY r.itemid, r.component, r.ratingarea, r.contextid, ur.id, ur.userid, ur.scaleid + GROUP BY r.itemid, r.component, r.ratingarea, r.contextid ORDER BY r.itemid"; - $ratingsrecords = $DB->get_records_sql($sql, $params); + $aggregateratings = $DB->get_records_sql($sql, $params); $ratingoptions = new stdClass; $ratingoptions->context = $options->context; @@ -590,25 +593,29 @@ class rating_manager { $ratingoptions->ratingarea = $options->ratingarea; $ratingoptions->settings = $this->generate_rating_settings_object($options); foreach ($options->items as $item) { - if (array_key_exists($item->{$itemidcol}, $ratingsrecords)) { + if (array_key_exists($item->{$itemidcol}, $userratings)) { // Note: rec->scaleid = the id of scale at the time the rating was submitted // may be different from the current scale id - $rec = $ratingsrecords[$item->{$itemidcol}]; - $ratingoptions->itemid = $item->{$itemidcol}; + $rec = $userratings[$item->{$itemidcol}]; $ratingoptions->scaleid = $rec->scaleid; $ratingoptions->userid = $rec->userid; $ratingoptions->id = $rec->id; - $ratingoptions->aggregate = min($rec->aggrrating, $ratingoptions->settings->scale->max); - $ratingoptions->count = $rec->numratings; $ratingoptions->rating = min($rec->usersrating, $ratingoptions->settings->scale->max); } else { - $ratingoptions->itemid = $item->{$itemidcol}; $ratingoptions->scaleid = null; $ratingoptions->userid = null; $ratingoptions->id = null; + $ratingoptions->rating = null; + } + if (array_key_exists($item->{$itemidcol}, $aggregateratings)) { + $rec = $aggregateratings[$item->{$itemidcol}]; + $ratingoptions->itemid = $item->{$itemidcol}; + $ratingoptions->aggregate = min($rec->aggrrating, $ratingoptions->settings->scale->max); + $ratingoptions->count = $rec->numratings; + } else { + $ratingoptions->itemid = $item->{$itemidcol}; $ratingoptions->aggregate = null; $ratingoptions->count = 0; - $ratingoptions->rating = null; } $rating = new rating($ratingoptions); From b312305cccb92dfbdbb92d3c9f031d8b6919ce9a Mon Sep 17 00:00:00 2001 From: "Andrew Davis (andyjdavis)" Date: Fri, 5 Aug 2011 11:12:44 +0800 Subject: [PATCH 4/4] MDL-27845 rating: altered the get_ratings() query to avoid possible future duplicate key problems --- rating/lib.php | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/rating/lib.php b/rating/lib.php index 00b30464b5e..f75a548438e 100644 --- a/rating/lib.php +++ b/rating/lib.php @@ -567,7 +567,7 @@ class rating_manager { $params['component'] = $options->component; $params['ratingarea'] = $options->ratingarea; - $sql = "SELECT r.itemid, r.id, r.userid, r.scaleid, r.rating AS usersrating + $sql = "SELECT r.id, r.itemid, r.userid, r.scaleid, r.rating AS usersrating FROM {rating} r WHERE r.userid = :userid AND r.contextid = :contextid AND @@ -593,20 +593,28 @@ class rating_manager { $ratingoptions->ratingarea = $options->ratingarea; $ratingoptions->settings = $this->generate_rating_settings_object($options); foreach ($options->items as $item) { - if (array_key_exists($item->{$itemidcol}, $userratings)) { - // Note: rec->scaleid = the id of scale at the time the rating was submitted - // may be different from the current scale id - $rec = $userratings[$item->{$itemidcol}]; - $ratingoptions->scaleid = $rec->scaleid; - $ratingoptions->userid = $rec->userid; - $ratingoptions->id = $rec->id; - $ratingoptions->rating = min($rec->usersrating, $ratingoptions->settings->scale->max); - } else { + $founduserrating = false; + foreach($userratings as $userrating) { + //look for an existing rating from this user of this item + if ($item->{$itemidcol} == $userrating->itemid) { + // Note: rec->scaleid = the id of scale at the time the rating was submitted + // may be different from the current scale id + $ratingoptions->scaleid = $userrating->scaleid; + $ratingoptions->userid = $userrating->userid; + $ratingoptions->id = $userrating->id; + $ratingoptions->rating = min($userrating->usersrating, $ratingoptions->settings->scale->max); + + $founduserrating = true; + break; + } + } + if (!$founduserrating) { $ratingoptions->scaleid = null; $ratingoptions->userid = null; $ratingoptions->id = null; $ratingoptions->rating = null; } + if (array_key_exists($item->{$itemidcol}, $aggregateratings)) { $rec = $aggregateratings[$item->{$itemidcol}]; $ratingoptions->itemid = $item->{$itemidcol};