https://addons.mozilla.org/en-US/firefox/recommended/format:rss This should be an RSS feed of recommended add-ons, but no add-ons are actually returned.
Created attachment 414059 [details] [diff] [review] Recommended RSS Fix v1 You can't order by Addon.name(which is what the RSS feed is trying to do) without including the Addon table in the query. That's the only thing that this unbindFully removes; Addon table data isn't especially large or anything so just removing this should work fine. Without the Addons table, the $order parameter for getRecommendedAddons doesn't have much meaning, since you can't really order by anything in the Features table at all. Asking wenzel for review since it appears you were looking at this code recently!
Attachment #414059 - Flags: review?(fwenzel)
Comment on attachment 414059 [details] [diff] [review] Recommended RSS Fix v1 Hm, I don't think removing the unbindFully() call is a good solution. If sorting by add-on name doesn't work anyway, the easiest thing would be sorting the RSS feed differently. How about the recommendation start date? If that doesn't make sense, then we need to write code to actually sort the feed by name (oh, the beauty of Cake), but just pulling in more data and keeping it broken otherwise is probably not what we want.
Attachment #414059 - Flags: review?(fwenzel) → review-
I think maybe I miscommunicated something, sorting by add-on name DOES work -- it's just that if you don't bind the addons table, it SQL errors trying to sort by addon name, which is why the recommended page doesn't display any data currently. Removing the unbindFully allows it to keep it's binding to the addons table, which means the SQL is fine, and it does in fact perform the sort correctly.
Hm, but the RSS feed isn't actually sorted by name -- I assume it's rather sorted by the translation ID which is probably not very helpful :(
You're right, I didn't notice that because my very small pool of test addons happens to have ids that correspond with ordering by name. To make an ordering by name actually work, I'd need to write a completely new custom query that pulls in the translations, looks like.
As I said, not sure if it's worth the effort. RSS feeds are usually somewhat chronological, so we could do the same.
Created attachment 414141 [details] [diff] [review] Recommended RSS Fix v2 I thought about it and I think that chronological order is better than alphabetical by name, since, as you said, it's an RSS feed. So might as well go with the easy and better solution.
Comment on attachment 414141 [details] [diff] [review] Recommended RSS Fix v2 Cool attachment number, 414141. That looks like a good solution. Checked it into r56741, thanks!
Attachment #414141 - Flags: review?(fwenzel) → review+
Status: NEW → RESOLVED
Last Resolved: 9 years ago
OS: Windows XP → All
Hardware: x86 → All
Resolution: --- → FIXED
Verified FIXED on https://preview.addons.mozilla.org/en-US/firefox/recommended/format:rss.
Flags: in-litmus? → in-litmus+
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.