Skip to content

Commit

Permalink
Merge pull request #5800 from rubyforgood/5604-banner-expiration
Browse files Browse the repository at this point in the history
Add expiration to banners
  • Loading branch information
compwron authored Jun 1, 2024
2 parents 504af69 + 599f2e8 commit 45444b7
Show file tree
Hide file tree
Showing 14 changed files with 221 additions and 2 deletions.
2 changes: 2 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ def set_active_banner
return nil unless current_organization

@active_banner = current_organization.banners.active.first

@active_banner = nil if session[:dismissed_banner] == @active_banner&.id
@active_banner = nil if @active_banner&.expired?
end

protected
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/banners_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def set_banner
end

def banner_params
params.require(:banner).permit(:active, :content, :name).merge(user: current_user)
BannerParameters.new(params, current_user, cookies[:browser_time_zone])
end

def deactivate_alternate_active_banner
Expand Down
10 changes: 10 additions & 0 deletions app/helpers/banner_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,14 @@ def conditionally_add_hidden_class(current_banner_is_active)
"d-none"
end
end

def banner_expiration_time_in_words(banner)
if banner.expired?
"Yes"
elsif banner.expires_at
"in #{distance_of_time_in_words(Time.now, banner.expires_at)}"
else
"No"
end
end
end
11 changes: 11 additions & 0 deletions app/models/banner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ class Banner < ApplicationRecord
validates_presence_of :name
validate :only_one_banner_is_active_per_organization

def expired?
expires_at && Time.current > expires_at
end

# `expires_at` is stored in the database as UTC, but timezone information will be stripped before displaying on frontend
# so this method converts the time to the user's timezone before displaying it
def expires_at_in_time_zone(timezone)
expires_at&.in_time_zone(timezone)
end

private

def only_one_banner_is_active_per_organization
Expand All @@ -25,6 +35,7 @@ def only_one_banner_is_active_per_organization
#
# id :bigint not null, primary key
# active :boolean default(FALSE)
# expires_at :datetime
# name :string
# created_at :datetime not null
# updated_at :datetime not null
Expand Down
25 changes: 25 additions & 0 deletions app/values/banner_parameters.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Calculate values when using banner parameters
class BannerParameters < SimpleDelegator
def initialize(params, user, timezone)
new_params = params.require(:banner).permit(:active, :content, :name, :expires_at).merge(user: user)

if params.dig(:banner, :expires_at)
new_params[:expires_at] = convert_expires_at_with_user_time_zone(params, timezone)
end

super(new_params)
end

private

# `expires_at` comes from the frontend without any timezone information, so we use `in_time_zone` to attach
# timezone information to it before saving to the database. If we don't do this, the time will be stored at UTC
# by default.
def convert_expires_at_with_user_time_zone(params, timezone)
params[:banner][:expires_at].in_time_zone(timezone)
end

def params
__getobj__
end
end
4 changes: 4 additions & 0 deletions app/views/banners/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
<%= form.label :active, "Active?", class: 'form-check-label' %>
<span data-reveal-target="item" class="text-danger <%= conditionally_add_hidden_class(form.object.active) %>">Warning: This will replace your current active banner</span>
</div>
<span class="input-style-1">
<%= form.label :expires_at, "Expires at (optional)" %>
<%= form.datetime_field :expires_at, value: banner.expires_at_in_time_zone(cookies[:browser_time]), required: false %>
</span>
<div class="input-style-1">
<%= form.label :content %>
<%= form.rich_text_area :content %>
Expand Down
4 changes: 4 additions & 0 deletions app/views/banners/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
<tr>
<th>Name</th>
<th>Active?</th>
<th>Expires At</th>
<th>Last Updated By</th>
<th>Updated At</th>
<th>Actions</th>
Expand All @@ -43,6 +44,9 @@
No
<% end %>
</td>
<td class="min-width">
<%= banner_expiration_time_in_words(banner) %>
</td>
<td class="min-width">
<%= banner.user.display_name %>
</td>
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20240531172823_add_expires_at_to_banner.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddExpiresAtToBanner < ActiveRecord::Migration[7.1]
def change
add_column :banners, :expires_at, :datetime, null: true
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.1].define(version: 2024_05_23_101303) do
ActiveRecord::Schema[7.1].define(version: 2024_05_31_172823) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"

Expand Down Expand Up @@ -95,6 +95,7 @@
t.boolean "active", default: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.datetime "expires_at"
t.index ["casa_org_id"], name: "index_banners_on_casa_org_id"
t.index ["user_id"], name: "index_banners_on_user_id"
end
Expand Down
28 changes: 28 additions & 0 deletions spec/helpers/banner_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,32 @@
expect(helper.conditionally_add_hidden_class(true)).to eq(nil)
end
end

describe "#banner_expiration_time_in_words" do
let(:banner) { create(:banner, expires_at: expires_at) }

context "when expires_at isn't set" do
let(:expires_at) { nil }

it "returns No" do
expect(helper.banner_expiration_time_in_words(banner)).to eq("No")
end
end

context "when expires_at is in the future" do
let(:expires_at) { 7.days.from_now }

it "returns a word description of how far in the future" do
expect(helper.banner_expiration_time_in_words(banner)).to eq("in 7 days")
end
end

context "when expires_at is in the past" do
let(:expires_at) { 7.days.ago }

it "returns yes" do
expect(helper.banner_expiration_time_in_words(banner)).to eq("Yes")
end
end
end
end
34 changes: 34 additions & 0 deletions spec/models/banner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,38 @@
expect(banner).to be_valid
end
end

describe "#expired?" do
it "is false when expires_at is nil" do
banner = create(:banner, expires_at: nil)

expect(banner).not_to be_expired
end

it "is false when expires_at is set but is after today" do
banner = create(:banner, expires_at: 7.days.from_now)

expect(banner).not_to be_expired
end

it "is true when expires_at is set but is before today" do
banner = create(:banner, expires_at: 7.days.ago)

expect(banner).to be_expired
end
end

describe "#expires_at_in_time_zone" do
it "can shift time by timezone for equivalent times" do
banner = create(:banner, expires_at: "2024-06-13 12:00:00 UTC")

expires_at_in_pacific_time = banner.expires_at_in_time_zone("America/Los_Angeles")
expect(expires_at_in_pacific_time.to_s).to eq("2024-06-13 05:00:00 -0700")

expires_at_in_eastern_time = banner.expires_at_in_time_zone("America/New_York")
expect(expires_at_in_eastern_time.to_s).to eq("2024-06-13 08:00:00 -0400")

expect(expires_at_in_pacific_time).to eq(expires_at_in_eastern_time)
end
end
end
24 changes: 24 additions & 0 deletions spec/requests/banners_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,28 @@
end
end
end

context "when a banner has expires_at" do
let!(:active_banner) { create(:banner, casa_org: casa_org, expires_at: expires_at) }

context "when expires_at is after today" do
let(:expires_at) { 7.days.from_now }

it "displays the banner" do
sign_in volunteer
get casa_cases_path
expect(response.body).to include "Please fill out this survey"
end
end

context "when expires_at is before today" do
let(:expires_at) { 7.days.ago }

it "does not display the banner" do
sign_in volunteer
get casa_cases_path
expect(response.body).not_to include "Please fill out this survey"
end
end
end
end
28 changes: 28 additions & 0 deletions spec/system/banners/new_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,34 @@
expect(page).to have_text("Please fill out this survey.")
end

it "lets you create banner with expiration time and edit it" do
sign_in admin

visit banners_path
click_on "New Banner"
fill_in "Name", with: "Expiring Announcement"
check "Active?"
fill_in "banner_expires_at", with: 7.days.from_now.strftime("%m%d%Y\t%I%M%P")
fill_in_rich_text_area "banner_content", with: "Please fill out this survey."
click_on "Submit"

visit banners_path
expect(page).to have_text("Expiring Announcement")

visit banners_path
within "#banners" do
click_on "Edit", match: :first
end
fill_in "banner_expires_at", with: 7.days.ago.strftime("%m%d%Y\t%I%M%P")
click_on "Submit"

visit banners_path
expect(page).to have_text("Expiring Announcement")

visit root_path
expect(page).not_to have_text("Please fill out this survey.")
end

describe "when an organization has an active banner" do
let(:admin) { create(:casa_admin) }
let(:organization) { create(:casa_org) }
Expand Down
43 changes: 43 additions & 0 deletions spec/values/banner_parameters_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
require "rails_helper"

RSpec.describe BannerParameters do
subject { described_class.new(params, user, timezone) }

let(:params) {
ActionController::Parameters.new(
banner: ActionController::Parameters.new(
active: "1",
content: "content",
name: "name"
)
)
}

let(:user) { create(:user) }

let(:timezone) { nil }

it "returns data" do
expect(subject["active"]).to eq("1")
expect(subject["content"]).to eq("content")
expect(subject["name"]).to eq("name")
expect(subject["expires_at"]).to be_blank
expect(subject["user"]).to eq(user)
end

context "when expires_at is set" do
let(:params) {
ActionController::Parameters.new(
banner: ActionController::Parameters.new(
expires_at: "2024-06-10T12:12"
)
)
}

let(:timezone) { "America/Los_Angeles" }

it "attaches timezone information to expires_at" do
expect(subject["expires_at"]).to eq("2024-06-10 12:12:00 -0700")
end
end
end

0 comments on commit 45444b7

Please sign in to comment.