379161 - "Newest" RSS feed does not filter by application

Status: RESOLVED FIXED

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

Description

[Fred Wenzel [:wenzel]]

From bug 379145 comment 2:

https://addons.mozilla.org/en-US/firefox/addons/rss/newest
lists Thunderbird extensions (e.g. "Get All Mails")

and the feed from
https://addons.mozilla.org/en-US/thunderbird/addons/rss/newest
lists Firefox extensions (e.g. "Bookmark Permissions").

In other words, the two feeds are the same and do not filter by application.

Comment 2

[Fred Wenzel [:wenzel]]

Attachment: Sort addons by creation date

This patch allows to sort the browse pages by addon creation date, and it removes the global "newest addons" feed so we don't duplicate stuff.

However:
- this will break the current "newest addons" link (...addons/rss/newest)
- the "newest addons" RSS feeds will only contain one addon type at a time (only extensions, for example), due to their connection with the browse pages.

Thoughts?

Comment 3

[Justin Scott [:fligtar]]

Hmm. It's not possible for us to filter the current app in the main page rss that's there now? If it's not, this looks good.

I am wondering if Addon.created is supposed to be Addon.created DESC though?

Also, maybe we should do a rewrite for the existing rss to go to Firefox extensions. Maybe we can also do rewrites for the old site rss feeds, which are still getting a good number of 404s.

Comment 4

[Fred Wenzel [:wenzel]]

(In reply to comment #3)
> Hmm. It's not possible for us to filter the current app in the main page rss
> that's there now? If it's not, this looks good.

Hm. There probably is, with some more code (needs some JOINs I figure), but I remember that we wanted to remove these "special case" RSSes since a long time, this is the last remaining one.

> I am wondering if Addon.created is supposed to be Addon.created DESC though?

I looked at it again, thanks. But the DESC is added later in the code.

> Also, maybe we should do a rewrite for the existing rss to go to Firefox
> extensions. Maybe we can also do rewrites for the old site rss feeds, which are
> still getting a good number of 404s.

Good call. I will make sure to add these as well.

Will finish that ASAP so the localizers can translate the two new strings.

Comment 5

[Mike Shaver (:shaver emeritus)]

If you want it to go in before the localizers finish, could you just land the back-end pieces and point the RSS links at them?  We can surface the explicit UI when it's ready and localized, but fix the meat of this bug earlier.

This also doesn't seem like it fixes the bug as summarized, since there's still no replacement for the "all things Firefox" RSS.  Maybe that's fine, but I want to make sure that that's intentional.

Comment 6

[Fred Wenzel [:wenzel]]

Mike: Good call about the UI vs. backend stuff, will do that.

Also, I agree that replacing the feeds as shown, there won't be an "all things Firefox" feed anymore. Do you guys think this is necessary? In that case I will not delete the global RSS feed section and rather add some app filtering logic there.

I slowly get to the conclusion that this might be closer to what the user might expect to find, rather than single feeds (only).

Comment 7

[Fred Wenzel [:wenzel]]

Assigning to myself because the patch for this is about ready to go.

Comment 8

[Fred Wenzel [:wenzel]]

Attachment: "Newest" sort order for browse pages plus app awareness for global RSS feed

This patch implements everything talked about above, in particular:
- "newest" sort order for browse pages, keeping the UI hidden until it is translated enough, but enabling the RSS feeds
- teaching global "newest" RSS feed app awareness

Comment 9

[Fred Wenzel [:wenzel]]

Attachment: htaccess redirects for legacy RSS feeds with "newest" sort order

and these are the redirects so legacy RSS subscribers don't get errors anymore.

Comment 10

[Fred Wenzel [:wenzel]]

Attachment: New RSS Tests

So yeah, here are the tests as well, cause that's how we roll.

Comment 11

[Wil Clouser [:clouserw]]

Comment on attachment 266075 [details] [diff] [review]
"Newest" sort order for browse pages plus app awareness for global RSS feed

Looks good to me.  Instead of commenting out the lines in addon_list_options.thtml, we could add something like:

if ('list_sortby_newest' == _('list_sortby_newest')) {
  // print english
} else {
  // print _('list_sortby_newest')
}

That would make the option available right away.

Comment 12

[Fred Wenzel [:wenzel]]

Good call, clouserw. I'll add this, then commit. And then later we can clean that up once it's localized well.

Comment 13

[Fred Wenzel [:wenzel]]

RSS code is in SVN r4117, tests in r4118, htaccess redirects in r4119 and localization changes in r4120.

Closing this, will go online with the next push. Marking this fixed.

The code to avoid the tags showing up for untranslated locales can be removed once it becomes widely translated, of course.