From f7ba73f536a3e8016a851bdae0ae11d21c84a6ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dani=20Rodri=CC=81guez?= <dani@danirod.es>
Date: Tue, 25 Jul 2023 00:27:54 +0200
Subject: [PATCH] fix: properly use the lastmod property in sitemap.xml

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.)
---
 app/models/playlist.rb       |  6 +++++-
 app/models/topic.rb          |  4 ++++
 config/sitemap.rb            | 10 +++++-----
 spec/models/playlist_spec.rb | 27 +++++++++++++++++++++++++++
 spec/models/topic_spec.rb    | 27 +++++++++++++++++++++++++++
 5 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/app/models/playlist.rb b/app/models/playlist.rb
index eeb00a1f..857ba0b3 100644
--- a/app/models/playlist.rb
+++ b/app/models/playlist.rb
@@ -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|
@@ -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),
diff --git a/app/models/topic.rb b/app/models/topic.rb
index b2605a97..a0acde2b 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -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
diff --git a/config/sitemap.rb b/config/sitemap.rb
index 7de43255..20c02f74 100644
--- a/config/sitemap.rb
+++ b/config/sitemap.rb
@@ -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
diff --git a/spec/models/playlist_spec.rb b/spec/models/playlist_spec.rb
index ee2c146f..839845ac 100644
--- a/spec/models/playlist_spec.rb
+++ b/spec/models/playlist_spec.rb
@@ -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
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index 268f616b..a0e35b9a 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -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