579198 - Have your way with /addon/:id/versions/format:rss

Status: RESOLVED FIXED

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

Description

[Wil Clouser [:clouserw]]

The old remora page:
https://preview.addons.mozilla.org/en-US/firefox/addons/versions/10868/format:rss

The new zamboni page (apparently works without a /z/?):
https://preview.addons.mozilla.org/en-US/firefox/addon/10868/versions/format:rss

Also note that the versions listing page (take format:rss off the end) now has a link to the RSS feed as an alternate view.

Comment 1

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

https://preview.addons.mozilla.org/en-US/firefox/addon/10868/versions/?page=2 goes back to:

Version 0.2.105  February 23, 2009, but https://preview.addons.mozilla.org/en-US/firefox/addon/10868/versions/format:rss doesn't have that.

Does the RSS feed have a max limit, or something?

Comment 2

[Sam Keen]

currently feed is set to limit to the latest 30 results

I believe it is common to limit RSS feeds as they are never ending streams

i.e. Twitter RSS feeds limit to 20

Comment 3

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

Do we want to make the links in the feeds specific to the versions?  Right now, they all just go to http://preview.addons.mozilla.org/en-US/firefox/addon/10868/versions/, but we have:

* https://preview.addons.mozilla.org/en-US/firefox/addon/10868/versions/1.2.2, which redirects to
* https://preview.addons.mozilla.org/en-US/firefox/addon/10868/versions/?page=1#version-1.2.2

Comment 4

[Sam Keen]

thx, i fully agree,
pushed fix to master
http://github.com/jbalogh/zamboni/commit/14c832130dea4ab1897eb32545e7baecc367fc88

Comment 5

[Tobias (:Tobbi) Markus]

W3C feed validator mentions the following:
http://validator.w3.org/feed/check.cgi?url=https%3A%2F%2Fpreview.addons.mozilla.org%2Fen-US%2Ffirefox%2Faddon%2F10868%2Fversions%2Fformat%3Arss

"Self reference doesn't match document location"

Would be a cool thing if you could fix that as well.

Comment 6

[Sam Keen]

tracked down the issue to the href in the atom:link has http rather than https
<atom:link href="http://preview.addons.mozilla.org/en-US/firefox/addon/10868/versions/format:rss" rel="self"></atom:link> 

unfortunately django syndication framework automagically creates the atom:link element

I'll start digging in the django src to see what's up

Comment 7

[Jeff Balogh (:jbalogh)]

'feed_url': iri_to_uri(feed_url),

if self.feed['feed_url'] is not None:
    handler.addQuickElement(u"link", "", {u"rel": u"self", u"href": self.feed['feed_url']})

Comment 8

[Sam Keen]

so what i understand to this point is (as seen in jbalogh's comment) this link element is being created with:
handler.addQuickElement(u"link", "", {u"rel": u"self", u"href":
self.feed['feed_url']})

I debugged it and in this call, self.feed['feed_url'] is an http url (even if on https)

feed_url gets passed into __init__ (do they call it a constructor in python?) for the surrounding class: SyndicationFeed 

that's where I am at.  
further debugging to see why we always get http rather than https.

Comment 9

[Sam Keen]

for a quick fix on this, i may need an assist from a python'r more experienced than I.

From my perspective it seems 'buggy' that the __init__ for SyndicationFeed is getting 'http...' rather than https for feed_url when we are on https

so far it is not totally clear where to 'hook in' and overwrite this value, so suggestions are very welcome.  I'll continue to poke around in the meantime.

Comment 10

[Jeff Balogh (:jbalogh)]

feed_url comes from http://github.com/django/django/blob/master/django/contrib/syndication/views.py#L108, which calls http://github.com/django/django/blob/master/django/contrib/syndication/views.py#L11, which has http:// hardcoded.  I would call that a Django bug and give them a patch if I wasn't so lazy.

diff --git a/apps/versions/feeds.py b/apps/versions/feeds.py
index 4e46099..b54957e 100644
--- a/apps/versions/feeds.py
+++ b/apps/versions/feeds.py
@@ -17,6 +17,9 @@ class VersionsRss(Feed):
 
     addon = None
 
+    def feed_url(self, addon):
+        return absolutify(reverse('addons.versions.rss', args=[addon.id]))
+
     def get_object(self, request, addon_id):
         """Get the Addon for which we are about to output
            the RSS feed of it Versions"""

Comment 11

[Sam Keen]

i'll file and post a link here
thanks for the help jbalogh

Comment 12

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

(In reply to comment #11)
> i'll file and post a link here
> thanks for the help jbalogh

For our side/code in AMO, I spun off bug 581170; wasn't sure if comment 11 was referring to filing a bug on us or Django itself (assumed the latter, so we can file against Django, and if they patch, we can remove the hack in comment 10).

Comment 13

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

QA's done with this bug; we feel confident in pushing Version's RSS feed out with bug 581170 filed.