diff --git a/lib/govuk_web_banners/recruitment_banner.rb b/lib/govuk_web_banners/recruitment_banner.rb index f95c1ed..2570513 100644 --- a/lib/govuk_web_banners/recruitment_banner.rb +++ b/lib/govuk_web_banners/recruitment_banner.rb @@ -1,5 +1,7 @@ module GovukWebBanners class RecruitmentBanner + BANNER_CONFIG_FILE = "../../config/govuk_web_banners/recruitment_banners.yml".freeze + def self.for_path(path) active_banners.find do |banner| return banner if banner.page_paths.include?(path) @@ -11,8 +13,7 @@ def self.active_banners end def self.all_banners - recruitment_banners_urls_file_path = Rails.root.join(__dir__, - "../../config/govuk_web_banners/recruitment_banners.yml") + recruitment_banners_urls_file_path = Rails.root.join(__dir__, BANNER_CONFIG_FILE) recruitment_banners_data = YAML.load_file(recruitment_banners_urls_file_path) recruitment_banners_data["banners"].map { |attributes| RecruitmentBanner.new(attributes:) } end @@ -25,12 +26,14 @@ def initialize(attributes:) @suggestion_link_text = attributes["suggestion_link_text"] @survey_url = attributes["survey_url"] @page_paths = attributes["page_paths"] - @start_date = attributes["start_date"] ? Date.parse(attributes["start_date"]) : Time.zone.at(0) - @end_date = attributes["end_date"] ? Date.parse(attributes["end_date"]) : Time.zone.now + 10.years + @start_date = attributes["start_date"] ? Time.parse(attributes["start_date"]) : Time.at(0) + @end_date = attributes["end_date"] ? Time.parse(attributes["end_date"]) : Time.now + 10.years end + # NB: .between? is inclusive. To make it exclude the end date, we set the end range as + # 1 second earlier. def active? - Time.zone.now.between?(start_date, end_date) + Time.zone.now.between?(start_date, end_date - 1.second) end end end diff --git a/spec/units/recruitment_banner_spec.rb b/spec/units/recruitment_banner_spec.rb index 5fc5c0b..bc88c35 100644 --- a/spec/units/recruitment_banner_spec.rb +++ b/spec/units/recruitment_banner_spec.rb @@ -1,46 +1,23 @@ require "govuk_web_banners/test_validators/recruitment_banner" RSpec.describe GovukWebBanners::RecruitmentBanner do - let(:validator_class) { GovukWebBanners::TestValidators::RecruitmentBanner } - - context "live configuration" do - it "contains the root banners: key (or banners: [] if there are no entries)" do - expect { described_class.all_banners }.not_to raise_error - end - - it "does not contain invalid banners" do - expect(described_class.all_banners.all? { |banner| validator_class.valid?(banner) }).to eq(true) - end - - it "does not contain banners that are past their end date" do - expect(described_class.all_banners.all? { |banner| banner.end_date >= Time.zone.now }).to eq(true) - end - - it "does not contain banners active at the same time and pointing to the same path" do - all_banners = described_class.all_banners - expect(all_banners.all? { |banner| validator_class.does_not_include_path_clashes?(banner, all_banners - [banner]) }).to eq(true) - end + before do + original_path = Rails.root.join(__dir__, described_class::BANNER_CONFIG_FILE) + allow(YAML).to receive(:load_file).with(original_path).and_return(replacement_file) end - context "test configurations" do - before do - original_path = Rails.root.join(__dir__, "../../config/govuk_web_banners/recruitment_banners.yml") - allow(YAML).to receive(:load_file).with(original_path).and_return(replacement_file) - end - + describe ".for_path" do context "with banners" do let(:replacement_file) do YAML.load_file(Rails.root.join(__dir__, "../../spec/fixtures/active_recruitment_banners.yml")) end - describe ".for_path?" do - it "returns banner that includes the path" do - expect(described_class.for_path("/foreign-travel-advice")).to be_instance_of(GovukWebBanners::RecruitmentBanner) - end + it "returns banner that includes the path" do + expect(described_class.for_path("/foreign-travel-advice")).to be_instance_of(described_class) + end - it "returns nil for a path without a banner" do - expect(described_class.for_path("/foreign-climbing-advice")).to be_nil - end + it "returns nil for a path without a banner" do + expect(described_class.for_path("/foreign-climbing-advice")).to be_nil end end @@ -49,88 +26,57 @@ YAML.load_file(Rails.root.join(__dir__, "../../spec/fixtures/timed_recruitment_banners.yml")) end - before { travel_to Time.local(2025, 1, 1) } after { travel_back } - describe ".active_banners" do - it "returns an array with only banners that are active now" do - expect(described_class.active_banners.count).to eq(2) - expect(described_class.active_banners.first.name).to eq("Banner that has started and hasn't ended yet") - expect(described_class.active_banners.second.name).to eq("Banner that has started and has no end date") - end - end + context "Before timed banners are active" do + before { travel_to Time.local(2024, 12, 31) } - describe ".all_banners" do - it "returns an array with all banners in the configuration " do - expect(described_class.all_banners.count).to eq(4) + it "finds only banners with no start time" do + expect(described_class.for_path("/page-1")).to be_nil + expect(described_class.for_path("/page-2")).to be_instance_of(described_class) + expect(described_class.for_path("/page-3")).to be_nil + expect(described_class.for_path("/page-4")).to be_instance_of(described_class) end end - end - context "with broken banners" do - let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../spec/fixtures/broken_recruitment_banners.yml")) - end + context "The day banners 1 and 3 become active and banner 2 ends" do + before { travel_to Time.local(2025, 1, 1) } - describe ".all_banners" do - it "confirms invalid banners exist" do - expect(described_class.all_banners.none? { |banner| validator_class.valid?(banner) }).to eq(true) + it "finds only banners active on that date" do + expect(described_class.for_path("/page-1")).to be_instance_of(described_class) + expect(described_class.for_path("/page-2")).to be_nil + expect(described_class.for_path("/page-3")).to be_instance_of(described_class) + expect(described_class.for_path("/page-4")).to be_instance_of(described_class) end - end - end - context "with expired banners" do - let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../spec/fixtures/expired_recruitment_banners.yml")) - end - - describe ".all_banners" do - it "detects expired banners" do - expect(described_class.all_banners.all? { |banner| banner.end_date >= Time.zone.now }).to eq(false) + it "finds the first banner for /page-3" do + expect(described_class.for_path("/page-3").name).to eq("Banner with start and end date") end end - describe ".active_banners" do - it "does not include expired banners in the active_banners list" do - expect(described_class.active_banners.count).to eq(0) - end - end - end - - context "with path clashes" do - let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../spec/fixtures/path_clash_recruitment_banners.yml")) - end + context "The day the page-3 banner swaps" do + before { travel_to Time.local(2025, 2, 1) } - describe ".all_banners" do - it "detects path clashes" do - all_banners = described_class.all_banners - expect(all_banners.any? { |banner| validator_class.does_not_include_path_clashes?(banner, all_banners - [banner]) }).to eq(false) + it "finds only banners active on that date" do + expect(described_class.for_path("/page-1")).to be_instance_of(described_class) + expect(described_class.for_path("/page-2")).to be_nil + expect(described_class.for_path("/page-3")).to be_instance_of(described_class) + expect(described_class.for_path("/page-4")).to be_instance_of(described_class) end - end - end - context "with identical paths, but not overlapping timess" do - let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../spec/fixtures/no_path_clash_recruitment_banners.yml")) - end - - describe ".all_banners" do - it "ignores path clashes that don't overlap in time" do - all_banners = described_class.all_banners - expect(all_banners.all? { |banner| validator_class.does_not_include_path_clashes?(banner, all_banners - [banner]) }).to eq(true) + it "finds the second banner for /page-3" do + expect(described_class.for_path("/page-3").name).to eq("Banner with start and end date abutting previous banner") end end - end - context "with an empty file" do - let(:replacement_file) do - YAML.load_file(Rails.root.join(__dir__, "../../spec/fixtures/empty_recruitment_banners.yml")) - end + context "After all timed banners end" do + before { travel_to Time.local(2025, 3, 1) } - describe ".all_banners" do - it "raises an error" do - expect { described_class.all_banners }.to raise_error(NoMethodError) + it "finds only banners active on that date" do + expect(described_class.for_path("/page-1")).to be_instance_of(described_class) + expect(described_class.for_path("/page-2")).to be_nil + expect(described_class.for_path("/page-3")).to be_nil + expect(described_class.for_path("/page-4")).to be_instance_of(described_class) end end end