Opened 13 years ago

Last modified 13 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:


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

` <>`_

Attachments (2)

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

Download all attachments as: .zip

Change History (10)

by Michele Azzolari, 13 years ago

Attachment: issue_724.patch added


comment:1 by Michele Azzolari, 13 years ago

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 by Elrond, 13 years ago

Component: Programming
Status: NewIn Progress

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 by Christopher Allan Webber, 13 years ago

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,' + + ',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:

` <>`_

For example:


    --- a/mediagoblin/listings/
    +++ b/mediagoblin/listings/
    @@ -81,14 +81,25 @@ def tag_atom_feed(request):
             "MediaGoblin: Feed for tag '%s'" % tag_slug,
             id='tag:'',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'),
    -            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)},
    -            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 by Michele Azzolari, 13 years ago

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)

`\_URI <>`_
` <>`_

by Michele Azzolari, 13 years ago

Attachment: issue_724.patch.gz added


comment:5 by Michele Azzolari, 13 years ago

Ok, it should be fine now.

Let me know

comment:6 by Shawn Khan, 13 years ago

Status: In ProgressFeedback

comment:6 by Christopher Allan Webber, 13 years ago

Status: FeedbackClosed
Merged! Thanks!

comment:7 by Will Kahn-Greene, 12 years ago

The original url for this bug was .

Note: See TracTickets for help on using tickets.