Skip to content

Commit

Permalink
fix: properly use the lastmod property in sitemap.xml
Browse files Browse the repository at this point in the history
The sitemap.xml file will now use the last modification date of the
content for videos, playlists and topics, instead of the datetime at
which the sitemap.xml file was generated.

For collection pages like playlists or topics, the last modification
date of the collection will be used (like the last time a playlist was
modified), unless any of the items in the collection is more recent than
the container. In such case, that will be the last modification
timestamp. (Because a title of a video may have changed.)
  • Loading branch information
danirod committed Jul 24, 2023
1 parent eb5e332 commit f7ba73f
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 6 deletions.
6 changes: 5 additions & 1 deletion app/models/playlist.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ def to_s
title
end

def content_updated_at
[updated_at, videos.pluck(:updated_at).max].compact.flatten.max
end

# Returns a HATEOAS-friendly representation of the thumbnails.
def icons
%i[hidef default thumbnail].map do |style|
Expand All @@ -99,7 +103,7 @@ def creative_work_series
description:,
abstract: description,
dateCreated: created_at.iso8601,
dateModified: updated_at.iso8601,
dateModified: content_updated_at.iso8601,
datePublished: videos.first&.published_at&.iso8601,
keywords: videos.pluck(:tags).flatten.uniq,
thumbnailUrl: thumbnail.url(:thumb),
Expand Down
4 changes: 4 additions & 0 deletions app/models/topic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ def playlists_with_children
my_playlists.or(child_playlists)
end

def content_updated_at
[updated_at, playlists.pluck(:updated_at).max].compact.flatten.max
end

def to_s
title
end
Expand Down
10 changes: 5 additions & 5 deletions config/sitemap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,18 @@
add videos_path, priority: 0.9, changefreq: 'daily'

# Playlists
add playlists_path, priority: 0.9, changefreq: 'daily'
add playlists_path, priority: 0.9, changefreq: 'weekly'
Playlist.find_each do |playlist|
add playlist_path(playlist), priority: 0.8, changefreq: 'weekly'
add playlist_path(playlist), priority: 0.8, changefreq: 'weekly', lastmod: playlist.content_updated_at
playlist.videos.visible.find_each do |video|
add playlist_video_path(video, playlist_id: playlist), priority: 0.8
add playlist_video_path(video, playlist_id: playlist), priority: 0.8, lastmod: video.updated_at
end
end

# Topics
add topics_path, priority: 0.8, changefreq: 'daily'
add topics_path, priority: 0.8, changefreq: 'weekly'
Topic.find_each do |topic|
add topic_path(topic), priority: 0.7, changefreq: 'weekly'
add topic_path(topic), priority: 0.7, changefreq: 'weekly', lastmod: topic.content_updated_at
end

add terms_path, priority: 0.5
Expand Down
27 changes: 27 additions & 0 deletions spec/models/playlist_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,31 @@
end
end
end

describe '#content_updated_at' do
let(:playlist) { create(:playlist) }
let(:video) { create(:video, playlist:) }

describe 'when the playlist is more recent than the content' do
before do
playlist.update(updated_at: 2.days.ago)
video.update(updated_at: 3.days.ago)
end

it 'matches the playlist updated at' do
expect(playlist.content_updated_at).to eq(playlist.updated_at)
end
end

describe 'when the content is more recent than the playlist' do
before do
playlist.update(updated_at: 3.days.ago)
video.update(updated_at: 2.days.ago)
end

it 'matches the content updated at' do
expect(playlist.content_updated_at).to eq(video.updated_at)
end
end
end
end
27 changes: 27 additions & 0 deletions spec/models/topic_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,31 @@
end
end
end

describe '#content_updated_at' do
let(:topic) { create(:topic) }
let(:playlist) { create(:playlist, topic:) }

describe 'when the topic is more recent than the playlist' do
before do
topic.update(updated_at: 2.days.ago)
playlist.update(updated_at: 3.days.ago)
end

it 'matches the topic updated at' do
expect(topic.content_updated_at).to eq(topic.updated_at)
end
end

describe 'when the playlist is more recent than the topic' do
before do
topic.update(updated_at: 3.days.ago)
playlist.update(updated_at: 2.days.ago)
end

it 'matches the playlist updated at' do
expect(topic.content_updated_at).to eq(playlist.updated_at)
end
end
end
end

0 comments on commit f7ba73f

Please sign in to comment.