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

atom_feed() sets wrong update timestamp for the feed with :preserve_order => true #1007

Closed
th-h opened this Issue Dec 5, 2016 · 2 comments

Comments

Projects
None yet
2 participants
@th-h

th-h commented Dec 5, 2016

atom_feed tries to set the <updated></updated> element for the whole atom feed to the <published></published> date of the youngest feed element, relying on the feed list being sorted descending by date. This assumption may fail when :preserve_order is set to true.

Steps to reproduce

  1. The <updated></updated> element is set on line 112 of lib/nanoc/helpers/blogging.rb:
xml.updated(attribute_to_time(last_article[:created_at]).__nanoc_to_iso8601_time)
  1. last_article is set on line 71-73 to the first element of the feed (after sorting the relevant articles):
def last_article
    sorted_relevant_articles.first
end
  1. That works fine in the default case where all articles are sorted by their publishing date descending, as the youngest article will always be the first one.
    But that is not necessarily the case when :preserve_order is set to true. The articles may e.g. be sorted ascending by date, so the first article is the oldest. In this case the feed may contain elements published e.g. at 2016-11-01 and 2016-12-01, but the feed itself will have its latest update wrongly set to 2016-11-01.
    See sorted_relevant_articles on line 61-69.

Expected behavior

The <updated></updated> element of the feed should not be older than the publishing date of the newest feed element.

It could even reflect the latest update time of any feed element, not just the publication time.

Actual behavior

The <updated></updated> element of the feed will be set to the publication date of the first feed element, whether that may be the newest, oldest or any other element.

@th-h

This comment has been minimized.

Show comment
Hide comment
@th-h

th-h Dec 5, 2016

Ah, I see it's documented:

:preserve_order (Boolean)
Whether or not the ordering of the list of articles should be preserved. If false or unspecified, the articles will be sorted by created_at. If true, the list of articles will be used as-is, and is assumed to end with the most recent article.

Anyway, it would be nice if that restriction could be lifted.

th-h commented Dec 5, 2016

Ah, I see it's documented:

:preserve_order (Boolean)
Whether or not the ordering of the list of articles should be preserved. If false or unspecified, the articles will be sorted by created_at. If true, the list of articles will be used as-is, and is assumed to end with the most recent article.

Anyway, it would be nice if that restriction could be lifted.

@ddfreyne

This comment has been minimized.

Show comment
Hide comment
@ddfreyne

ddfreyne Dec 17, 2016

Member

Fixed in #1014.

Member

ddfreyne commented Dec 17, 2016

Fixed in #1014.

@ddfreyne ddfreyne added this to the 4.4.3 milestone Dec 17, 2016

ddfreyne added a commit that referenced this issue Dec 17, 2016

Merge pull request #1014 from nanoc/gh-1007-most-recent
Use most recent timestamp for <updated> in Atom feed

@ddfreyne ddfreyne closed this Dec 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment