Add dynamic monthly sitemaps (#6704) [deploy]

* Initial sitemap work

* More sitemap progress

* Remove files

* Finalize monthly sitemap

* Update spec/models/organization_spec.rb

Co-Authored-By: Vaidehi Joshi <vaidehi.sj@gmail.com>

* Clean up approach

* Change tests

* Fix test not sure what problem was

Co-authored-by: Vaidehi Joshi <vaidehi.sj@gmail.com>

app/controllers/sitemaps_controller.rb

@@ -0,0 +1,32 @@
+class SitemapsController < ApplicationController
+  before_action :set_cache_control_headers, only: %i[show]
+
+  SITEMAP_REGEX = /\Asitemap-(?<date_string>[A-Z][a-z][a-z]-\d{4})\.xml\z/.freeze
+
+  def show
+    match = params[:sitemap].match(SITEMAP_REGEX)
+    not_found unless match && match[:date_string]
+    begin
+      date = Time.zone.parse(match[:date_string]).at_beginning_of_month
+    rescue ArgumentError
+      not_found
+    end
+    @articles = Article.published.where("published_at > ? AND published_at < ? AND score > ?", date, date.end_of_month, 3).pluck(:path, :last_comment_at)
+    set_surrogate_controls(date)
+    response.headers["Surrogate-Control"] = "max-age=#{@max_age}, stale-while-revalidate=#{@stale_while_revalidate}, stale-if-error=#{@stale_if_error}"
+    render layout: false
+  end
+
+  private
+
+  def set_surrogate_controls(date)
+    @stale_if_error = "86400"
+    if date > 1.month.ago
+      @max_age = "8640" # one hour
+      @stale_while_revalidate = "43200" # half a day
+    else
+      @max_age = "259200" # three days
+      @stale_while_revalidate = "432000" # five days
+    end
+  end
+end

app/models/organization.rb

@@ -137,6 +137,6 @@ class Organization < ApplicationRecord
   end
 
   def unique_slug_including_users_and_podcasts
-    errors.add(:slug, "is taken.") if User.find_by(username: slug) || Podcast.find_by(slug: slug) || Page.find_by(slug: slug)
+    errors.add(:slug, "is taken.") if User.find_by(username: slug) || Podcast.find_by(slug: slug) || Page.find_by(slug: slug) || slug.include?("sitemap-")
   end
 end

app/models/page.rb

@@ -38,7 +38,7 @@ class Page < ApplicationRecord
   end
 
   def unique_slug_including_users_and_orgs
-    slug_exists = User.exists?(username: slug) || Organization.exists?(slug: slug) || Podcast.exists?(slug: slug)
+    slug_exists = User.exists?(username: slug) || Organization.exists?(slug: slug) || Podcast.exists?(slug: slug) || slug.include?("sitemap-")
     errors.add(:slug, "is taken.") if slug_exists
   end
 

app/views/service_worker/index.js.erb

@@ -113,6 +113,7 @@
         !url.href.includes('/social_previews') && // Skip for social previews
         !url.href.includes('/users/auth') && // Don't run on authentication.
         !url.href.includes('/enter') && // Don't run on registration.
+        !url.href.includes('/sitemap-') && // Don't run on registration.
         !url.href.includes('/welcome') && // Don't run on welcome reroutes.
 
         // Don't run on search endpoints

app/views/sitemaps/show.xml.erb

@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
+  <%= @articles.each do |path, last_comment_at| %>
+    <url>
+        <loc><%= ApplicationConfig['APP_PROTOCOL'] %><%= ApplicationConfig['APP_DOMAIN'] %><%= path %></loc>
+        <lastmod><%= last_comment_at %></lastmod>
+    </url>
+  <% end %>
+</urlset>

config/routes.rb

@@ -420,6 +420,8 @@ Rails.application.routes.draw do
   get "/:username/:view" => "stories#index",
       :constraints => { view: /comments|moderate|admin/ }
   get "/:username/:slug" => "stories#show"
+  get "/:sitemap" => "sitemaps#show",
+      :constraints => { format: /xml/, sitemap: /sitemap\-.+/ }
   get "/:username" => "stories#index"
 
   root "stories#index"

spec/models/organization_spec.rb

@@ -106,6 +106,12 @@ RSpec.describe Organization, type: :model do
       expect(organization.errors[:slug].to_s.include?("taken")).to be true
     end
 
+    it "takes sitemap into account" do
+      organization = build(:organization, slug: "sitemap-yo")
+      expect(organization).not_to be_valid
+      expect(organization.errors[:slug].to_s.include?("taken")).to be true
+    end
+
     context "when callbacks are triggered after save" do
       let(:organization) { build(:organization) }
 

spec/models/page_spec.rb

@@ -29,6 +29,12 @@ RSpec.describe Page, type: :model do
       expect(page).not_to be_valid
       expect(page.errors[:slug].to_s.include?("taken")).to be true
     end
+
+    it "takes sitemap into account" do
+      page = build(:page, slug: "sitemap-hey")
+      expect(page).not_to be_valid
+      expect(page.errors[:slug].to_s.include?("taken")).to be true
+    end
   end
 
   context "when callbacks are triggered before save" do

spec/requests/sitemaps_spec.rb

@@ -0,0 +1,39 @@
+require "rails_helper"
+
+RSpec.describe "Sitemaps", type: :request do
+  describe "GET /sitemap-*" do
+    it "renders xml file" do
+      get "/sitemap-Mar-2011.xml"
+      expect(response.content_type).to eq("application/xml")
+    end
+
+    it "renders not found if incorrect input" do
+      expect { get "/sitemap-March-2011.xml" }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+
+    it "renders not found if not valid date" do
+      expect { get "/sitemap-Mak-2011.xml" }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+
+    it "renders proper surrogate header for recent sitemap" do
+      get "/sitemap-#{3.days.ago.strftime('%b-%Y')}.xml"
+      expect(response.header["Surrogate-Control"]).to include("8640")
+    end
+
+    it "renders proper surrogate header for older sitemap" do
+      get "/sitemap-#{35.days.ago.strftime('%b-%Y')}.xml"
+      expect(response.header["Surrogate-Control"]).to include("259200")
+    end
+
+    it "sends a surrogate key (for Fastly's user)" do
+      create_list(:article, 4)
+      Article.limit(3).update_all(published_at: 3.months.ago, score: 10)
+      get "/sitemap-#{3.months.ago.strftime('%b-%Y')}.xml"
+      article = Article.first
+      expect(response.body).to include("<loc>#{ApplicationConfig['APP_PROTOCOL']}#{ApplicationConfig['APP_DOMAIN']}#{article.path}</loc>")
+      expect(response.body).to include("<lastmod>#{article.last_comment_at}</lastmod>")
+      expect(response.body).not_to include(Article.last.path)
+      expect(response.content_type).to eq("application/xml")
+    end
+  end
+end