296541 - ratings not updated after flagged comment removed

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

After a flagged comment is removed, the overall application rating is not
recomputed, as it is after a comment is added.  Thus it remains incorrect until
someone adds a new comment.

The rating should be recomputed after a comment is removed just as it is
recomputed after a comment is added.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

See f.e. Forumzilla (as of 2005 June 3), which had two comments, one with five
stars, and one with one star, until the latter comment was removed:

https://addons.mozilla.org/extensions/moreinfo.php?application=thunderbird&id=133&&page=comments

The overall rating for the extension has remained at three stars (1+5/2) even
though it should now be at five stars (5/1).

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: factors out recomputing code

I don't know much about PHP,
I'm not so skilled in update wizardry.
But I do know, that this seems true,
And I know that if you give review,
Then what a wonderful world this will be.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Attachment: same song, different dance--err, branch

Comment 4

[Giorgio Maone [:ma1]]

I've scanned the code for possible problems: there's an appearance of dangerous
usage of $id (used as a temp var in patch code, and as request var in the
original code), but $id is assigned explicetely to $_GET['id'] *after* patch, so
there's no real danger: only the usual UMO mess ;-)

Comment 5

[Giorgio Maone [:ma1]]

Comment on attachment 185278 [details] [diff] [review]
patch v1: factors out recomputing code

wrong source code

Comment 6

[Colin Ogilvie [:cso]]

This actually seems to be quite a problem...

Running Myk's function on the entire database dump at 2000 GMT today produces
these results:

Total Changed: 262
Total Checked: 715
Percentage: 36.6%

Some of these are actually going down, a few go up.

Adding kveton to the CC list to see if we should take this patch...

Comment 7

[Colin Ogilvie [:cso]]

Attachment: Patch v3

This patch follows on from discussions on the umo-dev mailing list when I did
more investigation earlier on... It is essentially the same as Myk's patch.

The reason the values in my previous comment is so high is that the average
rating is based over the last 30 days (stored in the database in this way) but
it only updates on a new comment, so for a lot of extensions it will be wrong.

We discussed it on the mailing list and agreement seemed to be that we should
switch to using AVG() instead, which isn't thought to be a big database hit.

*note*

For this patch to work you need to run the following on the database:

ALTER TABLE `feedback` CHANGE `CommentVote` `CommentVote` TINYINT UNSIGNED
DEFAULT NULL;

UPDATE feedback SET `CommentVote` = `CommentVote` -1;

(The second is due to the conversion from ENUM which makes 0 = 1 and 5 = 6, ask
morgamic for more details)

I'll also attach my script that I've used to mass update the ratings etc. from
the old values.

Comment 8

[Colin Ogilvie [:cso]]

Attachment: Script

Script used to update the entire database.

Comment 9

[Colin Ogilvie [:cso]]

Comment on attachment 185286 [details] [diff] [review]
same song, different dance--err, branch

Obsoleting patch (and clearing review request) based on mailing list discussion
- although most of Myk's patch has actually made it to the other patch.

Comment 10

[Colin Ogilvie [:cso]]

Comment on attachment 185286 [details] [diff] [review]
same song, different dance--err, branch

Obsoleting patch (and clearing review request) based on mailing list discussion
- although most of Myk's patch has actually made it to the other patch.

Comment 11

[Colin Ogilvie [:cso]]

Comment on attachment 186157 [details] [diff] [review]
Patch v3

Marking patch as obsolete, and r- because there needs to be more done in
developers/commentsmanager.php - there are two code paths to delete comments
and this only catches one.

Comment 12

[Colin Ogilvie [:cso]]

Attachment: Patch v4

Fixes the duplicate code issue in commentsmanager... as much as it pains me to
leave it like that, I don't fancy rewriting the whole file at this time.

Comment 13

[Colin Ogilvie [:cso]]

Attachment: Patch v5

Woops, a local hack to make testing easier creeped in there.

Comment 14

[Colin Ogilvie [:cso]]

Checking in core/inc_global.php;
/cvsroot/mozilla/webtools/update/core/inc_global.php,v  <--  inc_global.php
new revision: 1.12.2.9; previous revision: 1.12.2.8
done
Checking in core/postfeedback.php;
/cvsroot/mozilla/webtools/update/core/postfeedback.php,v  <--  postfeedback.php
new revision: 1.6.2.6; previous revision: 1.6.2.5
done
Checking in developers/commentsmanager.php;
/cvsroot/mozilla/webtools/update/developers/commentsmanager.php,v  <-- 
commentsmanager.php
new revision: 1.4.2.3; previous revision: 1.4.2.2
done

Comment 15

[Colin Ogilvie [:cso]]

Attachment: Script v2

Uploading for justdave.

Comment 16

[Michael Morgan [:morgamic]]

AMO BUGSPAM FOR COMPONENT MOVE AND DELETE (FILTER ME)

Comment 17

[Reed Loden [:reed]]

AMO BUGSPAM FOR QACONTACT FIX (FILTER ME)