Opened 9 years ago

Last modified 9 years ago

#366 closed defect (FIXED)

atom feed not valid

Reported by: Michele Azzolari Owned by: Michele Azzolari
Priority: major Milestone: 0.2.1
Component: programming Keywords:
Cc: Parent Tickets:

Description

ATOM feed is not valid, entry id must be an URL

`http://validator.w3.org/feed/check.cgi?url=http%3A%2F%2Fmediagoblin.com%2Fu%2Felrond%2Fatom%2F <http://validator.w3.org/feed/check.cgi?url=http://mediagoblin.com/u/elrond/atom/>`_



Subtickets

Attachments (2)

issue_724.patch (4.1 KB) - added by Michele Azzolari 9 years ago.
issue_724.patch
issue_724.patch.gz (1.4 KB) - added by Michele Azzolari 9 years ago.
issue_724.patch.gz

Download all attachments as: .zip

Change History (10)

Changed 9 years ago by Michele Azzolari

Attachment: issue_724.patch added

issue_724.patch

comment:1 Changed 9 years ago by Michele Azzolari

I made a few changes to fix the id bug and then I added some more
infos to the atom (author uri and links to the html version)



comment:2 Changed 9 years ago by Elrond

Component: Programming
Status: NewIn Progress
Hi,

I have looked over the patch!

Looks mostly good.

Just two things I wonder:


1. The "url=" from the AtomFeed isn't needed any more, because the
   new links thing does all the needed stuff?
2. This ``2011`` looks very suspicious? What is it doing there?



comment:3 Changed 9 years ago by Christopher Allan Webber

Milestone: 0.2.1
Owner: set to Michele Azzolari
Priority: NormalHigh
Yes, I'd like to merge this ASAP... however I'm not sure what's
going on here:

::

       id='tag:%s,' + request.host + ',2011:UserGallery:' + request.matchdict['user'],

What is this?

Also, your code is good, but it would be good if it followed the
style of the rest of our code, which follows PEP-8:

`http://www.python.org/dev/peps/pep-0008/ <http://www.python.org/dev/peps/pep-0008/>`_

For example:

::

    --- a/mediagoblin/listings/views.py
    +++ b/mediagoblin/listings/views.py
    @@ -81,14 +81,25 @@ def tag_atom_feed(request):
             "MediaGoblin: Feed for tag '%s'" % tag_slug,
             feed_url=request.url,
             id='tag:'+request.host+',2011:TagGallery:%s' % tag_slug,
    -        links=[{'href': request.urlgen('mediagoblin.listings.tags_listing', qualified=True,tag=tag_slug ), 'rel': 'alternate','type': 'text/html'}])
    +        links=[{'href': request.urlgen(
    +                    'mediagoblin.listings.tags_listing',
    +                    qualified=True,tag=tag_slug ),
    +                'rel': 'alternate',
    +                'type': 'text/html'}])
         for entry in cursor:
    -        feed.add(entry.get('title'),
    +        feed.add(
    +            entry.get('title'),
                 entry.get('description_html'),
                 content_type='html',
    -            author={'name': entry.get_uploader.username,'uri': request.urlgen('mediagoblin.user_pages.user_home', qualified=True, user=entry.get_uploader.username)},
    +            author={
    +                'name': entry.get_uploader.username,
    +                'uri': request.urlgen(
    +                    'mediagoblin.user_pages.user_home',
    +                    qualified=True, user=entry.get_uploader.username)},
                 updated=entry.get('created'),
    -            links=[{'href':entry.url_for_self(request.urlgen,qualified=True), 'rel': 'alternate','type': 'text/html'}],
    +            links=[
    +                {'href': entry.url_for_self(request.urlgen,qualified=True),
    +                 'rel': 'alternate','type': 'text/html'}],
                 url=entry.url_for_self(request.urlgen, qualified=True))
    
         return feed.get_response()

Breaking those lines up and doing spacing like above really helps
code be a lot easier to read!

Bumping priority up to "high".



comment:4 Changed 9 years ago by Michele Azzolari

The ATOM id is a tag URI [1][2]

The year I choose is when the mediagoblin project started (blogger
uses 1999 for example)

Sorry about my code style, I'll try to follow PEP-8 in future (I'm
not really into python so please help me if I do it wrong)

[1]
`http://en.wikipedia.org/wiki/Tag\_URI <http://en.wikipedia.org/wiki/Tag_URI>`_
[2]
`http://www.intertwingly.net/wiki/pie/IdElement?action=highlight&value=tag%3A <http://www.intertwingly.net/wiki/pie/IdElement?action=highlight&value=tag:>`_



Changed 9 years ago by Michele Azzolari

Attachment: issue_724.patch.gz added

issue_724.patch.gz

comment:5 Changed 9 years ago by Michele Azzolari

Ok, it should be fine now.

Let me know



comment:6 Changed 9 years ago by Shawn Khan

Status: In ProgressFeedback

comment:6 Changed 9 years ago by Christopher Allan Webber

Status: FeedbackClosed
Merged! Thanks!



comment:7 Changed 9 years ago by Will Kahn-Greene

The original url for this bug was http://bugs.foocorp.net/issues/724 .

Note: See TracTickets for help on using tickets.