ratings not updated after flagged comment removed

RESOLVED FIXED in 1.0

Status

RESOLVED FIXED
14 years ago
3 years ago

People

(Reporter: myk, Assigned: cso)

Tracking

unspecified

Details

(Whiteboard: [landme] WHEN CHECKING IN SEE COMMENT 4! Also, land with bug 297724)

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

14 years ago
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.
(Reporter)

Comment 1

14 years ago
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).
(Reporter)

Comment 2

14 years ago
Created attachment 185278 [details] [diff] [review]
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.
Assignee: Bugzilla-alanjstrBugs → myk
Status: NEW → ASSIGNED
Attachment #185278 - Flags: first-review?(g.maone)
(Reporter)

Comment 3

14 years ago
Created attachment 185286 [details] [diff] [review]
same song, different dance--err, branch
Attachment #185286 - Flags: first-review?(g.maone)

Updated

14 years ago
Attachment #185278 - Flags: first-review?(g.maone) → first-review+

Comment 4

14 years ago
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

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

wrong source code
Attachment #185278 - Attachment is obsolete: true
Attachment #185278 - Flags: first-review+ → first-review-
(Assignee)

Comment 6

14 years ago
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...
(Assignee)

Comment 7

14 years ago
Created attachment 186157 [details] [diff] [review]
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.
Attachment #186157 - Flags: first-review?(cst)
(Assignee)

Comment 8

14 years ago
Created attachment 186158 [details]
Script

Script used to update the entire database.
(Assignee)

Updated

14 years ago
Attachment #186158 - Attachment mime type: text/x-delimtext → text/plain
(Assignee)

Comment 9

14 years ago
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.
Attachment #185286 - Attachment is obsolete: true
Attachment #185286 - Flags: first-review?(g.maone)
(Assignee)

Comment 10

14 years ago
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.
(Assignee)

Comment 11

14 years ago
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.
Attachment #186157 - Attachment is obsolete: true
Attachment #186157 - Flags: first-review?(cst) → first-review-
(Assignee)

Comment 12

14 years ago
Created attachment 186305 [details] [diff] [review]
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.
(Assignee)

Updated

14 years ago
Attachment #186305 - Flags: first-review?(cst)
(Assignee)

Updated

14 years ago
Attachment #186305 - Flags: first-review?(cst) → first-review-
(Assignee)

Comment 13

14 years ago
Created attachment 186306 [details] [diff] [review]
Patch v5

Woops, a local hack to make testing easier creeped in there.
Assignee: myk → colin.ogilvie
Attachment #186305 - Attachment is obsolete: true
Attachment #186306 - Flags: first-review?(cst)
Attachment #186306 - Flags: first-review?(cst) → first-review+
Whiteboard: [landme] WHEN CHECKING IN SEE COMMENT 4! Also, land with bug 297724
(Assignee)

Comment 14

14 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
(Assignee)

Comment 15

14 years ago
Created attachment 187309 [details]
Script v2

Uploading for justdave.
Attachment #186158 - Attachment is obsolete: true
AMO BUGSPAM FOR COMPONENT MOVE AND DELETE (FILTER ME)
Component: Listings → Web Site
AMO BUGSPAM FOR QACONTACT FIX (FILTER ME)
QA Contact: listings → web-ui
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.