429139 - Reviews count maintenance script counts developer replies

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Maintenance Scripts, defect)

Description

[Fred Wenzel [:wenzel]]

The maintenance script that counts the reviews for each add-on should not include developer replies (i.e. reviews for which reply_to is not null).

The same goes for calculating the average ratings: Developer replies (not allowed to rate anyway) must not influence an add-ons average rating.

Comment 2

[Fred Wenzel [:wenzel]]

Attachment: Maintenance script fix

This patch makes sure not to count developer replies and not-yet-moderated reviews. For the ratings, it ignores reviews without ratings and only counts ratings for valid versions.

To test, you may need to manually calculate the correct rating for an add-on (preferably with few ratings ;)) and compare it to the maintenance script output: I did so for https://addons.mozilla.org/en-US/firefox/addon/2942 .

Comment 3

[Michael Morgan [:morgamic]]

Comment on attachment 316862 [details] [diff] [review]
Maintenance script fix

Works, amazingly... :)  Need to ask fligtar if he's ready to update /bin though.

Comment 4

[Fred Wenzel [:wenzel]]

Does that mean I can or cannot commit yet? CCing fligtar for confirmation.

Comment 5

[Justin Scott [:fligtar]]

/bin should be up to date in production, so it's fine.

Comment 6

[Fred Wenzel [:wenzel]]

Thanks, Justin. This is in SVN, r12537.

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

Jeremy: "wenzel: stephend|lunch: you can confirm that the script works if oremj runs the "ratings" and "reviews" maintenance tasks once, respectively, on preview." -- would you mind running this?

Comment 8

[Jeremy Orem [:oremj]]

(In reply to comment #7)
> Jeremy: "wenzel: stephend|lunch: you can confirm that the script works if oremj
> runs the "ratings" and "reviews" maintenance tasks once, respectively, on
> preview." -- would you mind running this?
> 
Done.

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

Still not seeing this working on preview: on the main page, "Hyperwords" was featured, and had 36 reviews; its details page says it has 43 :-(

Comment 10

[Fred Wenzel [:wenzel]]

Attachment: Count reviews with no rating

I noticed the difference between the counting code on the details page and the one used in the maintenance script is that the latter did not count reviews without rating. However, there's no need to not count them there. The only place where they must not be included is the average rating calculation.

Comment 11

[Michael Morgan [:morgamic]]

Comment on attachment 318121 [details] [diff] [review]
Count reviews with no rating

Yeah, I'm okay with this as long as there is a message and we're not messing up people's ratings by adding this to AVG().  Looks good.

Comment 12

[Fred Wenzel [:wenzel]]

r12593.

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

This still doesn't fix bug 428094, so reopening :-(

Comment 14

[Justin Scott [:fligtar]]

(In reply to comment #9)
> Still not seeing this working on preview:

This fix isn't going to show up in preview - maintenance scripts are only run in production against the production database.

Comment 15

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #14)
> (In reply to comment #9)
> > Still not seeing this working on preview:
> 
> This fix isn't going to show up in preview - maintenance scripts are only run
> in production against the production database.

Comment 7 mislead me, then :-(

Comment 16

[Justin Scott [:fligtar]]

The first patch might show up on preview depending on how the script was run, but the second patch won't.

Comment 17

[Fred Wenzel [:wenzel]]

stephend said he'll reconfirm once this is in production. That's sad we're giving him such a hard time verifying, but since this is not a critical (read: dangerous) bug, I'm fine with waiting until the push.

Comment 18

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #17)
> stephend said he'll reconfirm once this is in production. That's sad we're
> giving him such a hard time verifying, but since this is not a critical (read:
> dangerous) bug, I'm fine with waiting until the push.

Tested on production, and this is fine; I've been clicking around since we deployed earlier tonight.

Verified FIXED (/me crosses fingers)