Talk:Ratings 2.0

1. I think it would be clearer to move the userid column later in the database table, to just before timecreated.
2. What value will be in the userid field if one user sets one rating, then a different user changes it?
3. I think the content column would be easier to understand if you called it 'comment'.
4. Is there any way of knowing which scale the rating is against, other than by asking the plugin? Have you considered a scaleid column in the table?
5. class rating really should be split into class rating for the back-end, and care_rating_renderer for the GUI.
6. Why do plugins need to implement so many callbacks to use ratings?
7. You say the interface won't be changed, but that AJAX could be implemented in future. Well, forum already has AJAX rating!
8. Manual grading in the quiz have never been handled in the same ways as rating elsewhere, for good reasons. However, you might like to look at how manual grading of quiz questions works.
9. What happens in the scenario where a teacher sets a forum to be rated using Scale A, then rates some posts, then tries to change the forum to be rated by Scale B?

--Tim Hunt 11:03, 20 January 2010 (UTC)

Hi Tim, I've added comments in brackets. If there is a smarter way for me to comment on your comments let me know :)

1. done
2. User A rating an item is independent of user B rating an item. Two users rating something will result in two rows.
3. Ccontent column? I started with a copy of the Comments 2.0 document. You may have read it while I was still working on the first version.
4. Adding a scaleid column could result in potentially thousands of rows containing the scaleid which seems odd. Actually, that would help with what to do if the scale gets changed. I have added a scaleid column.
5. I'm now reading through the code to get a sense of how renderers work.
6. I'm not sure the plugins do need to implement so many callbacks. That was largely copied from Comments.
7. I'll add more detail as to what I mean re ajax. Thanks for the heads up.
8. re manual grading in the quiz be aware that rating and grading are largely unrelated. Grading is a teacher assigning a single grade to a single item. Rating is a student or teacher giving a mark to a thing based on how good they think it is. There will be numerous ratings for a single item. I'm not sure if you're getting them mixed up or if I'm not understanding what you're saying :)
9. re the teacher changing the scale after students have begun to give ratings I think we will store the scale id in every row. That way we can say "here are the N ratings using scale A. Here are the N ratings after you changed to scale B. We could potentially offer to scale up or down the previous ratings ie if they move from 1-5 to 1-100 multiply the rating by 20 and update the scaleid column.

--Andrew Davis 6:06 AM, 25 January 2010 (UTC)

Good answers.

5. Sam can explain renderers to you. I think the renderer should go in mod/ratings/renderer.php. The class would be called core_rating_renderer, and that is a core renderer with subtype rating. That is, you create one with a call to ...->get_renderer($PAGE, 'core', 'rating');

8. I was not getting mixed up. I just thought it was worth deciding how similar, or not, the two were. The answer seems to be not.

--Tim Hunt 10:30, 25 January 2010 (UTC)

Inline comments

I cleaned the mess of the main page. It is bad practice by me to put my comments inline in the first place.--Tim Hunt 14:20, 1 February 2010 (UTC)

Rating loading API

We can definitely fetch all ratings in a single query. Fetching forum posts and their ratings in one query however seems like binding the two too tight. That would require the forum, glossary and anywhere else we want to use ratings to do the same. $forumpost->rating would be handy but $ratings[$postid] isn't bad and allows us to add ratings to just about anything without making major changes to the thing being rated:--Andrew Davis 02:25, 01 February 2010 (UTC)

A case where we do this is in some of the queries that load things like lists of modules and list of activities. We join on the context table, to get columns called with the right names, then call the function make_context_subobj. So, for people who want to use this tight binding to save DB queries, the extent of the coupling is to make sure that the query returns the right data in columns with the right names. I think it is worth providing this option. An API that involves having to load some data, then build a huge array of id, then call another function, are sucky.--Tim Hunt 14:20, 1 February 2010 (UTC)

I see your point. Is there a specific point in the code you can point me to where we do this? --Andrew Davis 03:00, 2 February 2010 (UTC)

Sorry, don't know. I think you'll just have to grep the code for calls to make_context_subobj, or the column names it expects. Both of those are quite distinctive things to grep for, however.--Tim Hunt 11:15, 2 February 2010 (UTC)

Discussed this with Sam as I wasnt sure what you meant. I think I get it now. I've described the two methods of fetching ratings side by side. Method 1 is going to lead to more hard to follow code but it will require less trips to the database. --Andrew Davis 09:00, 8 February 2010 (UTC)

Permission checks

Is there a better way to check the user's permission to add ratings? Checking and filtering ratings seems somewhat pointless. Do we need to do them at all?:--Andrew Davis 03:10, 01 February 2010 (UTC)

The really tricky issue is when we want AJAX to work, which we will sooner, rather than later. An approach I took for a similar thing can be seen in question/toggleflag.php, and the functions that uses in lib/questionlib.php. However, the situation is not quite the same. The question flagging code does not know the context id. Your code does know the context id, so perhaps you can just use a has_capability check.--Tim Hunt 14:20, 1 February 2010 (UTC)

Rating Aggregation

How do we do rating aggregation? Two options:

1. When the user has 'view' permissions retrieve an array of ratings and aggregate on the fly? I'm not sure how this would work with the first method of retrieving ratings (joining the rating table with the forum post table or whatever).
2. Do the aggregation when a rating is submitted then store it somewhere. Possibly in another new table.

I think rating on the fly is likely to be less buggy and easier to maintain. Should just be a matter of doing a GROUP BY and an AVERAGE in the query that loads the combined ratings.

--Tim Hunt 11:13, 10 February 2010 (UTC)

We need to load the rating the current user previously gave plus the aggregate of all ratings given. That seems like it would be either two queries (one for the current user's ratings and one for the aggregates) or one query for all the ratings then calculate the aggregates in php.

http://tracker.moodle.org/browse/MDL-20514 The way we calculate the aggregate can vary. For forums at least. In the forum table the fields assessed, assesstimestart and assesstimefinish govern when the user can rate and how to calculate aggregates. assessed can be set to things like maximum rating, minimum, average of ratings, count of ratings. I'm not sure what we should do with these settings. Adding support for the different aggregation methods to the glossary means either adding those rows to the glossary table or shifting them into another module independent table.

We could add a table called rating_settings and access the settings when we need to calculate aggregates something like this:

$rating_settings = rating_settings::fetch(array('itemtype'=>'mod', 'itemmodule'=>'forum', 'iteminstance'=>1))
if($rating_setting->assessed==AGGREGATE_AVG) {
//calculate average by either calculating an average in php or by doing "select AVG(rating) from rating where..."
}

Currently aggregation in the forums is done at mod/forum/lib.php in forum_print_ratings(). All the ratings are retrieved from the database then the aggregate is calculated in php by the forum. The ratings code should be figuring out the aggregate. The forum should not be doing it.

With the need to get/calculate aggregates I'm not sure that method 1 (https://docs.moodle.org/en/Development:Ratings_2.0#Method_1) will work. As soon as we need to load multiple ratings per forum post it doesnt seem like it would work. With the example there anyway surely youll wind up with multiple rows ie

forumpost1 rating1

forumpost1 rating2

forumpost2 rating1

It could work if we do "select * from forum_posts inner join ratings inner join rating_aggregates where ..." but then we would need to make sure we keep the aggregate table up to date.

--Andrew Davis 04:00, 11 February 2010 (UTC)

OK. I think you are right that we cannot hope to load the forum posts and ratings at the same time, if we need aggregates too.

However, I think we can do this user's ratings and aggregates together. The following it untested, but looks about right:

function ratings_load_ratings($context, $itemids,
        $aggregate = 'AVG', $userid = null) {

    global $DB, $USER;

    if (isnull($userid)) {
        $userid = $USER->id;
    }

    list($itemidtest, $params) = $DB->get_in_or_equal(
            $itemids, SQL_PARAMS_NAMED, 'itemid0000');

    $sql = <<<ENDSQL
SELECT r.itemid,
    $aggregate(r.rating) AS aggrrating,
    COUNT(r.rating) AS numratings,
    ur.rating AS usersrating

FROM {ratings} r
LEFT JOIN {ratings} ur ON ur.contextid = r.contextid AND
        ur.itemid = r.itemid

WHERE
    r.contextid = :contextid AND
    r.itemid $itemidtest AND
    ur.userid = :userid

GROUP BY r.itemid, ur.rating
ENDSQL;

    $params['userid'] = $userid;
    $params['contextid'] = $context->id;

    return $DB->get_records_sql($sql, $params);
}

As an added sophistication, you could allow $itemids to be either an array of ids, or a string SQL fragment, which would then be used as a sub-select. That is, let people pass something like 'SELECT postid FROM {forum_posts} WHERE ...' as itemids, instead of an array. Of course, in that case, you would also need a $params argument to the function.

On another matter, I know you say Ajax is not important, but if you look at sam marshall's ForumNG, you will see he has done a fairly sweet ajax rating system there. You might like to steal it (even though it is currently YUI2.0 / Moodle 1.9 code). You can see it demoed in his iMoot talk.

--Tim Hunt 06:51, 11 February 2010 (UTC)

I made a dummy rating table to play with the sql. Came up with this although this version won't let you get aggregates on an item the user hasnt themselves rated.

select ur.itemid, ur.rating, 

(select avg(ar.rating) from rating as ar where contextid=1 and itemid=ur.itemid) as aggregate,

(select count(ar.rating) from rating as ar where contextid=1 and itemid=ur.itemid) as count

from rating as ur where contextid=1 and itemid in (1,2,3) and userid=4

order by itemid

I'll track down sam marshall's ForumNG.

--Andrew Davis 07:57, 12 February 2010 (UTC)

What was wrong with my code, which should get all of the averages and counts, wheterh or not the current user has ragted. Also, a join will be much more efficient than sub-selects.--Tim Hunt 09:14, 12 February 2010 (UTC)

I was just seeing some odd behaviour with it which I think I've isolated. It seems there is a quirk to left joins that I wasn't aware of. Your suggested query returns no rows if the user has not rated the item. It looks like it should but it doesn't. With one small modification it seems to work.

SELECT r.itemid,
AVG(r.rating) AS aggrrating,
COUNT(r.rating) AS numratings,
ur.rating AS usersrating 
FROM ratings r 
LEFT JOIN ratings ur 
ON r.contextid = ur.contextid AND r.itemid = ur.itemid and ur.userid = 4
WHERE r.contextid = 1 
AND r.itemid = 3 
GROUP BY r.itemid

Note that I've moved the condition stipulating the user id into the left join on clause. I think mysql at least, does the left join THEN applies the where condition knocking out rows where the user isnt 4. That includes those that are null because the user hasn't submitted a ranking. If it applied the where condition to ur then did the left join it would work fine. Putting the user id condition in the left join condition looks weird but seems to make it work how it should. --Andrew Davis 02:44, 12 February 2010 (UTC)

Sorry, yes. The userid condition has to be in the ON clause of the JOIN. (Alternatively you can do (userid = 4 OR userid IS NULL) in the where, but I think it is easier to understand doing it in the join condition.--Tim Hunt 11:00, 15 February 2010 (UTC)

I've done more work on this document following another meeting with Martin. There's now so much in here I'm finding it hard to keep it all straight in my head :\--Andrew Davis 07:27, 18 February 2010 (UTC)

Ratings not wanted in the gradebook

I think there should be an easy way for teachers to specify that ratings DON'T end up in the gradebook e.g. when students are allowed to rate, I guess quite often teachers would not want their ratings in the gradebook. --Helen Foster 13:37, 10 March 2010 (UTC)