Opened 15 years ago

Closed 9 years ago

Last modified 8 years ago

#13110 closed New feature (fixed)

Allow multiple enclosures in Atom feeds

Reported by: Piaume Owned by: Unai Zalakain
Component: contrib.syndication Version: dev
Severity: Normal Keywords: multiple, enclosures, atom, feed 1.9
Cc: kotenko@…, unai@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

For now, Feed.item_enclosure_url represents a single enclosure for an item. It should be nice to support many enclosures per item. Something like that:

class ExampleFeed(Feed):
    def item_enclosure_url(self, item):
        return [video.url for video in item.videos]

Change History (25)

comment:1 by Eric Holscher, 15 years ago

milestone: 1.21.3

Feature Request.

comment:2 by Russell Keith-Magee, 15 years ago

milestone: 1.3
Triage Stage: UnreviewedAccepted

comment:3 by Jason Kotenko, 14 years ago

Cc: kotenko@… added
Needs documentation: set
Needs tests: set
Owner: changed from nobody to Jason Kotenko
Status: newassigned

comment:4 by Jason Kotenko, 14 years ago

Triage Stage: AcceptedDesign decision needed

It appears that RSS only officially supports one enclosure per item {1}, and Atom supports multiple. Since RSS is the default feed, actually the use case presented in the bug report is invalid. The question then becomes, should we support multiple enclosures for Atom? As of now the enclosure is handled in the same general way in both types of feeds.

Changing to "Design Decision Needed" before I do anything else... Please let me know if that was the wrong thing to do in this situation, as I'm pretty new to the Django community.

{1} - http://www.rssboard.org/rss-profile#element-channel-item-enclosure

comment:5 by Luke Plant, 14 years ago

Type: New feature

comment:6 by Luke Plant, 14 years ago

Severity: Normal

comment:7 by Aymeric Augustin, 13 years ago

Easy pickings: unset
Triage Stage: Design decision neededAccepted
UI/UX: unset

It should be possible to accept either a string or a list of string, and to validate that there's only one enclosure for RSS feeds.

in reply to:  7 comment:8 by Unai Zalakain, 9 years ago

Replying to aaugustin:

It should be possible to accept either a string or a list of string, and to validate that there's only one enclosure for RSS feeds.

Currently Feed builds the Enclosure from item_enclosure_url, item_enclosure_length and item_enclosure_mime_type. Accepting an optional list for one of them would mean accepting optional lists for the three of them and that would become quite a strange and cumbersome API.

Instead, I propose creating a item_enclosures attr/func (the same way all the other attr/funcs work) which, by default, returns a list with a single Enclosure in it. That enclosure would be built using the url, length and mime_type gotten from the corresponding attrs.

RSS feeds could then validate that item_enclosures is no longer than 1.

Last edited 9 years ago by Unai Zalakain (previous) (diff)

comment:9 by Unai Zalakain, 9 years ago

Cc: unai@… added
Has patch: set
Keywords: multiple enclosures atom feed added
Owner: changed from Jason Kotenko to Unai Zalakain
Version: 1.1master

comment:11 by Unai Zalakain, 9 years ago

I updated the commit with the proposed changes and opened a PR:
https://github.com/django/django/pull/5200

comment:12 by Markus Holtermann, 9 years ago

Patch needs improvement: set

comment:13 by Unai Zalakain, 9 years ago

Needs documentation: unset
Patch needs improvement: unset

Updated PR with docs.

comment:14 by Unai Zalakain, 9 years ago

Needs tests: unset

Basic tests added, PR updated, ready for review!

comment:15 by Tim Graham, 9 years ago

Patch needs improvement: set
Summary: Multiple enclosures per feed's itemAllow multiple enclosures in Atom feeds

I left comments for improvement on the pull request.

comment:16 by Unai Zalakain, 9 years ago

Patch needs improvement: unset

PR updated!

comment:17 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:18 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In aac2a2d:

Fixed #13110 -- Added support for multiple enclosures in Atom feeds.

The item_enclosures hook returns a list of Enclosure objects which is
then used by the feed builder. If the feed is a RSS feed, an exception is
raised as RSS feeds don't allow multiple enclosures per feed item.

The item_enclosures hook defaults to an empty list or, if the
item_enclosure_url hook is defined, to a list with a single Enclosure
built from the item_enclosure_url, item_enclosure_length, and
item_enclosure_mime_type hooks.

comment:19 by Tim Graham, 9 years ago

Has patch: unset
Keywords: 1.9 added
Resolution: fixed
Status: closednew
Triage Stage: Ready for checkinAccepted

Reopening per comments on the commit.

comment:20 by Unai Zalakain, 9 years ago

Has patch: set

Sorry about that…
New PR: https://github.com/django/django/pull/5315

comment:21 by Unai Zalakain <unai@…>, 9 years ago

In a4b80e24:

Refs #13110 -- Fixed mistakes in the new multiple enclosure feed tests

comment:22 by Tim Graham, 9 years ago

Resolution: fixed
Status: newclosed

comment:23 by Tim Graham <timograham@…>, 9 years ago

In 2ec23a3d:

Refs #13110 -- Fixed typo in Feed class reference.

comment:24 by Tim Graham <timograham@…>, 9 years ago

In 54295a95:

[1.9.x] Refs #13110 -- Fixed typo in Feed class reference.

Backport of 2ec23a3d41be2ba5df9f46b17e0a05eb1b051c41 from master

comment:25 by Tim Graham <timograham@…>, 8 years ago

In 75cf9b5a:

Refs #13110 -- Removed SyndicationFeed.add_item()'s enclosure argument.

Per deprecation timeline.

Note: See TracTickets for help on using tickets.
Back to Top