394394 - Theme count is incorrect

Status: RESOLVED FIXED

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

Description

[Basil Hashem [:baz]]

It appears that the theme count at the bottom of this page is incorrect. Also, clicking on next/Page # gives an unstyled page. May be related to bug 394374.

Comment 1

[Michael Morgan [:morgamic]]

Should not show additional results if less than # per page.

Comment 2

[Fred Wenzel [:wenzel]]

The culprit here is an INNER JOIN against the files table (that was not executed for result counting), resulting in sandboxed addons without files being counted yet not shown.

I will make a patch for this.

Comment 3

[Fred Wenzel [:wenzel]]

Attachment: execute joins on count time also

This executes the JOINs for addon lists also if we are just counting. This avoids overcounting add-ons that don't have versions with files.

I am also likely fixed bug 394374, but I haven't checked that yet.

Fligtar, can you review this please?

Comment 4

[Justin Scott [:fligtar]]

This didn't fix the problem in the URL above for me after applying the patch.

http://fligtar.khan-vm.mozilla.org/addons-reviews/site/en-US/firefox/browse/type:2/cat:32/status:1

Comment 6

[Fred Wenzel [:wenzel]]

Attachment: Handling empty categories in the sandbox gracefully

This patch does fix the problem for me (and also fixed bug 394374). The error message did still render the rest of the page for the sandbox. I fixed it so it stops rendering (and redirects instead) in public and sandbox equally now.

Please review, thanks.

Comment 7

[Justin Scott [:fligtar]]

The fix still doesn't seem to be working for me. When I go to http://fligtar.khan-vm.mozilla.org/addons-reviews/site/en-US/firefox/browse/type:2/cat:32/status:1 and click next at the bottom, or on any of the page numbers, I still get the error message.

Comment 8

[Fred Wenzel [:wenzel]]

I cannot log into your dev copy. If you get an error message (no addons found) this means the fix *does* work. You are not supposed to find any addons in an empty category :)

Comment 9

[Justin Scott [:fligtar]]

Sorry, forgot I changed the database for stats stuff.
http://fligtar.khan-vm.mozilla.org/addons/trunk/site/en-US/firefox/browse/type:2/cat:32/status:1

As I understand it, the bug is that there are 3 add-ons displayed on this page, but the count at the bottom says 31. Clicking next or on any page says there are no add-ons in this category, so the count and pagination is wrong because there are apparently only 3. This still exists with this patch - the only difference I can see is the order of the add-ons it does find and the style fix with the error messages.

Comment 10

[Fred Wenzel [:wenzel]]

Attachment: Only show add-ons that have files to offer.

Ouch, how stupid -- the inner join was still not executed, and I didn't notice because I did not use the current data on my dev copy. Sorry!

Anyways, this will fix it (I promise ;)) but now it won't display any add-ons anymore that do not offer any versions and files. This means, the number of addons in this category has shrunk to 3 (also for the other list types). I think this is reasonable because how good is an addon whose description you can read but that doesn't have any version to offer. But ymmv, so speak up if you disagree.

Thanks.

Comment 11

[Justin Scott [:fligtar]]

Comment on attachment 288637 [details] [diff] [review]
Only show add-ons that have files to offer.

Yay!

Comment 12

[Fred Wenzel [:wenzel]]

This is in SVN r8034. Thanks, all!

Comment 13

[Justin Scott [:fligtar]]

These bugs were pushed last Thursday.