Recommended Addons RSS feed is broken



10 years ago
3 years ago


(Reporter: sancus, Assigned: sancus)


Bug Flags:
in-testsuite ?
in-litmus +




(1 attachment, 1 obsolete attachment)



10 years ago

This should be an RSS feed of recommended add-ons, but no add-ons are actually returned.

Comment 1

10 years ago
Posted patch Recommended RSS Fix v1 (obsolete) — Splinter Review
You can't order by 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-

Comment 3

10 years ago
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 :(

Comment 5

10 years ago
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.

Comment 7

10 years ago
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.
Attachment #414059 - Attachment is obsolete: true
Attachment #414141 - Flags: review?(fwenzel)
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+
Last Resolved: 10 years ago
Keywords: push-needed
OS: Windows XP → All
Hardware: x86 → All
Resolution: --- → FIXED

Comment 10

9 years ago
Flags: in-litmus? → in-litmus+
Product: → Graveyard
You need to log in before you can comment on or make changes to this bug.