From acc15bfafd296f16c4f11aa6dcb8a2248d10cee6 Mon Sep 17 00:00:00 2001 From: Goulven Champenois Date: Tue, 4 Mar 2025 10:28:15 +0100 Subject: [PATCH] List sites by most recent audit checked_at date --- app/controllers/sites_controller.rb | 2 +- app/models/audit.rb | 2 +- app/models/site.rb | 8 ++++ spec/models/audit_spec.rb | 8 ++-- spec/models/site_spec.rb | 58 +++++++++++++++++------------ 5 files changed, 48 insertions(+), 30 deletions(-) diff --git a/app/controllers/sites_controller.rb b/app/controllers/sites_controller.rb index bde7e3e..9c63ea7 100644 --- a/app/controllers/sites_controller.rb +++ b/app/controllers/sites_controller.rb @@ -4,7 +4,7 @@ class SitesController < ApplicationController # GET /sites def index - @pagy, @sites = pagy Site.sort_by_audit_url.includes(:audit) + @pagy, @sites = pagy Site.sort_by_audit_date.includes(:audit) end # GET /sites/1 diff --git a/app/models/audit.rb b/app/models/audit.rb index 4dba6c2..c2ff222 100644 --- a/app/models/audit.rb +++ b/app/models/audit.rb @@ -14,7 +14,7 @@ class Audit < ApplicationRecord "failed", # All checks failed ].index_by(&:itself), validate: true, default: :pending - scope :sort_by_newest, -> { order(created_at: :desc) } + scope :sort_by_newest, -> { order(checked_at: :desc) } scope :sort_by_url, -> { order(Arel.sql("REGEXP_REPLACE(audits.url, '^https?://(www\.)?', '') ASC")) } scope :past, -> { where.not(status: :pending) } diff --git a/app/models/site.rb b/app/models/site.rb index ed6220f..453fd8f 100644 --- a/app/models/site.rb +++ b/app/models/site.rb @@ -15,6 +15,14 @@ class Site < ApplicationRecord .order("sites.id, sortable_url") from(subquery, :sites).order(:sortable_url) end + scope :sort_by_audit_date, -> do + joins("LEFT JOIN ( + SELECT site_id, MAX(checked_at) as latest_check + FROM audits + GROUP BY site_id + ) latest_audits ON sites.id = latest_audits.site_id") + .order("latest_audits.latest_check DESC NULLS LAST, sites.created_at DESC") + end class << self def find_or_create_by_url(attributes) diff --git a/spec/models/audit_spec.rb b/spec/models/audit_spec.rb index ffe29be..23b6822 100644 --- a/spec/models/audit_spec.rb +++ b/spec/models/audit_spec.rb @@ -43,10 +43,10 @@ describe "scopes" do before { site.audit.destroy } - it ".sort_by_newest returns audits in descending order by creation date" do - oldest = create(:audit, site:, created_at: 3.days.ago) - older = create(:audit, site:, created_at: 2.days.ago) - newer = create(:audit, site:, created_at: 1.day.ago) + it ".sort_by_newest returns audits in descending order by checked_at date" do + oldest = create(:audit, site:, checked_at: 3.days.ago) + older = create(:audit, site:, checked_at: 2.days.ago) + newer = create(:audit, site:, checked_at: 1.day.ago) expect(described_class.sort_by_newest).to eq([newer, older, oldest]) end diff --git a/spec/models/site_spec.rb b/spec/models/site_spec.rb index ae59a96..0fc9e41 100644 --- a/spec/models/site_spec.rb +++ b/spec/models/site_spec.rb @@ -20,37 +20,47 @@ end end - describe ".find_or_create_by_url" do - let(:http_url) { "http://example.com" } + describe "scopes" do + it ".sort_by_audit_date orders sites by their most recent audit checked_at date" do + site1 = build(:site, audits: [build(:audit, checked_at: 1.day.ago)]).tap(&:save) + site2 = build(:site, audits: [build(:audit, checked_at: 5.days.ago)]).tap(&:save) + site3 = build(:site, audits: [build(:audit, checked_at: 2.days.ago)]).tap(&:save) - context "when site with URL exists" do - let!(:existing_site) { described_class.create(url:) } + expect(described_class.sort_by_audit_date).to eq([site1, site3, site2]) + end - it "returns existing site for exact URL match" do - expect(described_class.find_or_create_by_url(url:)).to eq(existing_site) - end + describe ".find_or_create_by_url" do + let(:http_url) { "http://example.com" } - it "returns existing site when only scheme differs" do - expect(described_class.find_or_create_by_url(url: http_url)).to eq(existing_site) - end + context "when site with URL exists" do + let!(:existing_site) { described_class.create(url:) } - it "finds site with historical URLs" do - new_url = "https://new-example.com" - existing_site.audits.create!(url: new_url) + it "returns existing site for exact URL match" do + expect(described_class.find_or_create_by_url(url:)).to eq(existing_site) + end - expect(described_class.find_or_create_by_url(url:)).to eq(existing_site) - expect(described_class.find_or_create_by_url(url: new_url)).to eq(existing_site) + it "returns existing site when only scheme differs" do + expect(described_class.find_or_create_by_url(url: http_url)).to eq(existing_site) + end + + it "finds site with historical URLs" do + new_url = "https://new-example.com" + existing_site.audits.create!(url: new_url) + + expect(described_class.find_or_create_by_url(url:)).to eq(existing_site) + expect(described_class.find_or_create_by_url(url: new_url)).to eq(existing_site) + end end - end - context "when site does not exist" do - it "creates a new site with audit" do - expect { - site = described_class.find_or_create_by_url(url:) - expect(site).to be_persisted - expect(site.audit.url).to eq(url) - }.to change(described_class, :count).by(1) - .and change(Audit, :count).by(1) + context "when site does not exist" do + it "creates a new site with audit" do + expect { + site = described_class.find_or_create_by_url(url:) + expect(site).to be_persisted + expect(site.audit.url).to eq(url) + }.to change(described_class, :count).by(1) + .and change(Audit, :count).by(1) + end end end end