Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use option url: false to allow entries without a link tag #18437

Merged
merged 1 commit into from
Jan 16, 2015

Conversation

eldano
Copy link
Contributor

@eldano eldano commented Jan 10, 2015

It should be possible to omit the link tag.
According to https://tools.ietf.org/html/rfc4287#section-4.1.2

atom:entry elements that contain no child atom:content element MUST contain at least one atom:link element with a rel attribute value of "alternate".

@xml.link(:rel => 'alternate', :type => type, :href => options[:url] || @view.polymorphic_url(record))
unless options[:url] == false
@xml.link(:rel => 'alternate', :type => type, :href => options[:url] || @view.polymorphic_url(record))
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd do something like this ...

url = options.fetch(:url) { @view.polymorphic_url(record) }
@xml.link(:rel => 'alternate', :type => type, :href => url) if url

Because you want @view.polymorphic_url(record) to be executed if options[:url] isn't set. But if somebody does set it, regardless if it's nil or false you want to skip that.

@spastorino
Copy link
Contributor

Other than that looks good 👍

@eldano
Copy link
Contributor Author

eldano commented Jan 16, 2015

@spastorino I agree with your comment, it's changed now

@@ -174,7 +174,7 @@ def updated(date_or_time = nil)
#
# * <tt>:published</tt>: Time first published. Defaults to the created_at attribute on the record if one such exists.
# * <tt>:updated</tt>: Time of update. Defaults to the updated_at attribute on the record if one such exists.
# * <tt>:url</tt>: The URL for this entry. Defaults to the polymorphic_url for the record.
# * <tt>:url</tt>: The URL for this entry or false for not having a link tag. Defaults to the polymorphic_url for the record.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eldano it should say that url: nil will omit the tag as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sdebarros Thanks for spotting this!

spastorino added a commit that referenced this pull request Jan 16, 2015
Use option url: false to allow entries without a link tag
@spastorino spastorino merged commit fb82da3 into rails:master Jan 16, 2015
@eldano eldano deleted the feed_entry_no_link_option branch January 16, 2015 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants