From 408422891eb4401c84ad80a8175312e3ff144a6d Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 24 Mar 2021 19:15:53 +0100 Subject: [PATCH 1/2] Adding consistency in banner scopes Since the :post_started_at and :post_ended_at fields are of type Date, we check with Date.current and not with Time.current. This change has been caused because some test suites were failing (https://github.com/consul/consul/runs/2170798218?check_suite_focus=true). The code we had was causing the banners to be available a few hours earlier or later than they should be depending on the time zone of the application. --- app/models/banner.rb | 6 ++--- spec/models/banner_spec.rb | 46 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/app/models/banner.rb b/app/models/banner.rb index b5484e397..45f2ca916 100644 --- a/app/models/banner.rb +++ b/app/models/banner.rb @@ -19,9 +19,7 @@ class Banner < ApplicationRecord has_many :sections has_many :web_sections, through: :sections - scope :with_active, -> { where("post_started_at <= ?", Time.current).where("post_ended_at >= ?", Time.current) } - - scope :with_inactive, -> { where("post_started_at > ? or post_ended_at < ?", Time.current, Time.current) } - + scope :with_active, -> { where("post_started_at <= :date and post_ended_at >= :date", date: Date.current) } + scope :with_inactive, -> { where.not(id: with_active) } scope :in_section, ->(section_name) { joins(:web_sections, :sections).where("web_sections.name ilike ?", section_name) } end diff --git a/spec/models/banner_spec.rb b/spec/models/banner_spec.rb index 75c05d918..733c4362d 100644 --- a/spec/models/banner_spec.rb +++ b/spec/models/banner_spec.rb @@ -18,4 +18,50 @@ describe Banner do expect(banner.background_color).to be_present expect(banner.font_color).to be_present end + + describe "scope" do + describe ".with_active" do + it "works when UTC date is different", :with_non_utc_time_zone do + banner = create(:banner, post_started_at: Date.current, post_ended_at: Date.current) + + travel_to((Date.current - 1.day).end_of_day) do + expect(Banner.with_active).to be_empty + end + + travel_to(Date.current.beginning_of_day) do + expect(Banner.with_active).to eq [banner] + end + + travel_to(Date.current.end_of_day) do + expect(Banner.with_active).to eq [banner] + end + + travel_to((Date.current + 1.day).beginning_of_day) do + expect(Banner.with_active).to be_empty + end + end + end + + describe ".with_inactive" do + it "works when UTC date is different", :with_non_utc_time_zone do + banner = create(:banner, post_started_at: Date.current, post_ended_at: Date.current) + + travel_to((Date.current - 1.day).end_of_day) do + expect(Banner.with_inactive).to eq [banner] + end + + travel_to(Date.current.beginning_of_day) do + expect(Banner.with_inactive).to be_empty + end + + travel_to(Date.current.end_of_day) do + expect(Banner.with_inactive).to be_empty + end + + travel_to((Date.current + 1.day).beginning_of_day) do + expect(Banner.with_inactive).to eq [banner] + end + end + end + end end From 85611a5103d975d33adb02fe3bc0681a8cba2c31 Mon Sep 17 00:00:00 2001 From: taitus Date: Thu, 8 Apr 2021 16:52:57 +0200 Subject: [PATCH 2/2] Adding consistency for post_started_at and post_ended_at on specs These two fields are of type Date and in some specs they are being filled as Time or DateTime. --- .../shared/banner_component_spec.rb | 8 ++--- spec/factories/administration.rb | 4 +-- spec/system/admin/banners_spec.rb | 32 +++++++++---------- spec/system/banners_spec.rb | 4 +-- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/spec/components/shared/banner_component_spec.rb b/spec/components/shared/banner_component_spec.rb index 0309afc67..dcf4511c1 100644 --- a/spec/components/shared/banner_component_spec.rb +++ b/spec/components/shared/banner_component_spec.rb @@ -6,8 +6,8 @@ describe Shared::BannerComponent, type: :component do title: "Vote now!", description: "Banner description", target_url: "http://www.url.com", - post_started_at: (Time.current - 4.days), - post_ended_at: (Time.current + 10.days), + post_started_at: (Date.current - 4.days), + post_ended_at: (Date.current + 10.days), background_color: "#FF0000", font_color: "#FFFFFF" ) @@ -69,7 +69,7 @@ describe Shared::BannerComponent, type: :component do end it "only renders active banners" do - Banner.first.update!(post_ended_at: 1.day.ago) + Banner.first.update!(post_ended_at: Date.current - 1.day) render_inline Shared::BannerComponent.new("debates") @@ -78,7 +78,7 @@ describe Shared::BannerComponent, type: :component do end it "does not render anything with no active banners" do - Banner.all.each { |banner| banner.update!(post_ended_at: 1.day.ago) } + Banner.all.each { |banner| banner.update!(post_ended_at: Date.current - 1.day) } render_inline Shared::BannerComponent.new("debates") diff --git a/spec/factories/administration.rb b/spec/factories/administration.rb index a596d45dd..0b56b5364 100644 --- a/spec/factories/administration.rb +++ b/spec/factories/administration.rb @@ -18,8 +18,8 @@ FactoryBot.define do sequence(:title) { |n| "Banner title #{n}" } sequence(:description) { |n| "This is the text of Banner #{n}" } target_url { ["/proposals", "/debates"].sample } - post_started_at { Time.current - 7.days } - post_ended_at { Time.current + 7.days } + post_started_at { Date.current - 7.days } + post_ended_at { Date.current + 7.days } background_color { "#FF0000" } font_color { "#FFFFFF" } end diff --git a/spec/system/admin/banners_spec.rb b/spec/system/admin/banners_spec.rb index 9e262169c..29f0fcb12 100644 --- a/spec/system/admin/banners_spec.rb +++ b/spec/system/admin/banners_spec.rb @@ -6,40 +6,40 @@ describe "Admin banners magement", :admin do create(:banner, title: "Banner number one", description: "This is the text of banner number one and is not active yet", target_url: "http://www.url.com", - post_started_at: (Time.current + 4.days), - post_ended_at: (Time.current + 10.days), + post_started_at: (Date.current + 4.days), + post_ended_at: (Date.current + 10.days), background_color: "#FF0000", font_color: "#FFFFFF") create(:banner, title: "Banner number two", description: "This is the text of banner number two and is not longer active", target_url: "http://www.url.com", - post_started_at: (Time.current - 10.days), - post_ended_at: (Time.current - 3.days), + post_started_at: (Date.current - 10.days), + post_ended_at: (Date.current - 3.days), background_color: "#00FF00", font_color: "#FFFFFF") create(:banner, title: "Banner number three", description: "This is the text of banner number three", target_url: "http://www.url.com", - post_started_at: (Time.current - 1.day), - post_ended_at: (Time.current + 10.days), + post_started_at: (Date.current - 1.day), + post_ended_at: (Date.current + 10.days), background_color: "#0000FF", font_color: "#FFFFFF") create(:banner, title: "Banner number four", description: "This is the text of banner number four", target_url: "http://www.url.com", - post_started_at: (DateTime.current - 10.days), - post_ended_at: (DateTime.current + 10.days), + post_started_at: (Date.current - 10.days), + post_ended_at: (Date.current + 10.days), background_color: "#FFF000", font_color: "#FFFFFF") create(:banner, title: "Banner number five", description: "This is the text of banner number five", target_url: "http://www.url.com", - post_started_at: (DateTime.current - 10.days), - post_ended_at: (DateTime.current + 10.days), + post_started_at: (Date.current - 10.days), + post_ended_at: (Date.current + 10.days), background_color: "#FFFF00", font_color: "#FFFFFF") end @@ -100,8 +100,8 @@ describe "Admin banners magement", :admin do fill_in "Title", with: "En Français" fill_in "Description", with: "Link en Français" fill_in "Link", with: "https://www.url.com" - fill_in "Post started at", with: Time.current - 1.week - fill_in "Post ended at", with: Time.current + 1.week + fill_in "Post started at", with: Date.current - 1.week + fill_in "Post ended at", with: Date.current + 1.week click_button "Save changes" click_link "Edit banner" @@ -125,8 +125,8 @@ describe "Admin banners magement", :admin do create(:banner, title: "Hello", description: "Wrong text", target_url: "http://www.url.com", - post_started_at: (Time.current + 4.days), - post_ended_at: (Time.current + 10.days), + post_started_at: (Date.current + 4.days), + post_ended_at: (Date.current + 10.days), background_color: "#FF0000", font_color: "#FFFFFF") @@ -164,8 +164,8 @@ describe "Admin banners magement", :admin do create(:banner, title: "Ugly banner", description: "Bad text", target_url: "http://www.url.com", - post_started_at: (Time.current + 4.days), - post_ended_at: (Time.current + 10.days), + post_started_at: (Date.current + 4.days), + post_ended_at: (Date.current + 10.days), background_color: "#FF0000", font_color: "#FFFFFF") diff --git a/spec/system/banners_spec.rb b/spec/system/banners_spec.rb index 3278e8603..93c557fb3 100644 --- a/spec/system/banners_spec.rb +++ b/spec/system/banners_spec.rb @@ -5,8 +5,8 @@ describe "Banner" do create(:banner, web_sections: [WebSection.find_by!(name: "homepage")], description: "Banner description", - post_started_at: (Time.current - 4.days), - post_ended_at: (Time.current + 10.days)) + post_started_at: (Date.current - 4.days), + post_ended_at: (Date.current + 10.days)) visit root_path