Simplify rendering a banner
Now the banner component accepts either a banner or a section and loads the banner if it's a section, so we don't have to add the `@banners` variable in several controllers.
This commit is contained in:
@@ -1,8 +1,16 @@
|
||||
class Shared::BannerComponent < ApplicationComponent
|
||||
attr_reader :banner
|
||||
attr_reader :banner_or_section
|
||||
|
||||
def initialize(banner)
|
||||
@banner = banner
|
||||
def initialize(banner_or_section)
|
||||
@banner_or_section = banner_or_section
|
||||
end
|
||||
|
||||
def banner
|
||||
@banner ||= if banner_or_section.respond_to?(:sections)
|
||||
banner_or_section
|
||||
else
|
||||
Banner.in_section(banner_or_section).with_active.sample
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -17,7 +17,6 @@ class BudgetsController < ApplicationController
|
||||
def index
|
||||
@finished_budgets = @budgets.finished.order(created_at: :desc)
|
||||
@budgets_coordinates = current_budget_map_locations
|
||||
@banners = Banner.in_section("budgets").with_active
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -16,7 +16,6 @@ module CommentableActions
|
||||
index_customization
|
||||
|
||||
@tag_cloud = tag_cloud
|
||||
@banners = Banner.in_section(section(resource_model.name)).with_active
|
||||
|
||||
set_resource_votes(@resources)
|
||||
|
||||
@@ -110,15 +109,6 @@ module CommentableActions
|
||||
nil
|
||||
end
|
||||
|
||||
def section(resource_name)
|
||||
case resource_name
|
||||
when "Proposal"
|
||||
"proposals"
|
||||
when "Debate"
|
||||
"debates"
|
||||
end
|
||||
end
|
||||
|
||||
def featured_proposals
|
||||
@featured_proposals ||= []
|
||||
end
|
||||
|
||||
@@ -6,7 +6,6 @@ class PagesController < ApplicationController
|
||||
|
||||
def show
|
||||
@custom_page = SiteCustomization::Page.published.find_by(slug: params[:id])
|
||||
@banners = Banner.in_section("help_page").with_active
|
||||
|
||||
if @custom_page.present?
|
||||
@cards = @custom_page.cards
|
||||
|
||||
@@ -11,7 +11,6 @@ class WelcomeController < ApplicationController
|
||||
@header = Widget::Card.header.first
|
||||
@feeds = Widget::Feed.active
|
||||
@cards = Widget::Card.body
|
||||
@banners = Banner.in_section("homepage").with_active
|
||||
@remote_translations = detect_remote_translations(@feeds,
|
||||
@recommended_debates,
|
||||
@recommended_proposals)
|
||||
|
||||
@@ -79,6 +79,6 @@
|
||||
</div>
|
||||
|
||||
<div class="row">
|
||||
<%= render "shared/banner", banner: @banner %>
|
||||
<%= render Shared::BannerComponent.new(@banner) %>
|
||||
</div>
|
||||
<% end %>
|
||||
|
||||
@@ -30,7 +30,7 @@
|
||||
<tr>
|
||||
<td colspan="3">
|
||||
<span class="help-text"><%= t("admin.banners.index.preview") %></span>
|
||||
<%= render "shared/banner", banner: banner %>
|
||||
<%= render Shared::BannerComponent.new(banner) %>
|
||||
</td>
|
||||
</tr>
|
||||
</tbody>
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
<%= render "shared/banner" %>
|
||||
<%= render Shared::BannerComponent.new("budgets") %>
|
||||
|
||||
<% provide :title do %><%= t("budgets.index.title") %><% end %>
|
||||
|
||||
|
||||
@@ -40,7 +40,7 @@
|
||||
<div class="row">
|
||||
<div id="debates" class="debates-list small-12 medium-9 column">
|
||||
|
||||
<%= render "shared/banner" %>
|
||||
<%= render Shared::BannerComponent.new("debates") %>
|
||||
|
||||
<% unless @search_terms || !has_featured? %>
|
||||
<%= render "featured_debates" %>
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
<%= render "shared/canonical", href: help_url %>
|
||||
<% end %>
|
||||
|
||||
<%= render "shared/banner" %>
|
||||
<%= render Shared::BannerComponent.new("help_page") %>
|
||||
|
||||
<div class="jumbo light">
|
||||
<div class="row">
|
||||
|
||||
@@ -49,7 +49,7 @@
|
||||
<div class="row">
|
||||
<div id="proposals" class="proposals-list small-12 medium-9 column">
|
||||
|
||||
<%= render "shared/banner" %>
|
||||
<%= render Shared::BannerComponent.new("proposals") %>
|
||||
|
||||
<% if show_featured_proposals? %>
|
||||
<div id="featured-proposals" class="row featured-proposals">
|
||||
|
||||
@@ -1,3 +0,0 @@
|
||||
<% banner ||= @banners&.sample %>
|
||||
|
||||
<%= render Shared::BannerComponent.new(banner) %>
|
||||
@@ -4,7 +4,7 @@
|
||||
<%= render "shared/canonical", href: root_url %>
|
||||
<% end %>
|
||||
|
||||
<%= render "shared/banner" %>
|
||||
<%= render Shared::BannerComponent.new("homepage") %>
|
||||
|
||||
<% provide :social_media_meta_tags do %>
|
||||
<%= render "shared/social_media_meta_tags",
|
||||
|
||||
@@ -7,6 +7,70 @@ describe Shared::BannerComponent, type: :component do
|
||||
expect(page.find(".banner")).to have_content "Vote now!"
|
||||
end
|
||||
|
||||
it "renders given a section" do
|
||||
create(:banner, web_sections: [WebSection.find_by!(name: "debates")], title: "Debate banner")
|
||||
create(:banner, web_sections: [WebSection.find_by!(name: "proposals")], title: "Proposal banner")
|
||||
|
||||
render_inline Shared::BannerComponent.new("debates")
|
||||
|
||||
expect(page.find(".banner")).to have_content "Debate banner"
|
||||
expect(page).not_to have_content "Proposal banner"
|
||||
end
|
||||
|
||||
context "several banners available in the same section" do
|
||||
before do
|
||||
create(:banner,
|
||||
web_sections: [WebSection.find_by!(name: "debates")],
|
||||
title: "First banner",
|
||||
description: "First description",
|
||||
target_url: "/first_target"
|
||||
)
|
||||
|
||||
create(:banner,
|
||||
web_sections: [WebSection.find_by!(name: "debates")],
|
||||
title: "Second banner",
|
||||
description: "Second description",
|
||||
target_url: "/second_target"
|
||||
)
|
||||
end
|
||||
|
||||
it "only renders one banner" do
|
||||
render_inline Shared::BannerComponent.new("debates")
|
||||
|
||||
expect(page).to have_css ".banner", count: 1
|
||||
end
|
||||
|
||||
it "consistently renders one banner" do
|
||||
render_inline Shared::BannerComponent.new("debates")
|
||||
|
||||
if page.has_content?("First banner")
|
||||
expect(page).to have_content "First description"
|
||||
expect(page).to have_css "[href='/first_target']"
|
||||
expect(page).not_to have_content "Second banner"
|
||||
else
|
||||
expect(page).to have_content "Second description"
|
||||
expect(page).to have_css "[href='/second_target']"
|
||||
end
|
||||
end
|
||||
|
||||
it "only renders active banners" do
|
||||
Banner.first.update!(post_ended_at: 1.day.ago)
|
||||
|
||||
render_inline Shared::BannerComponent.new("debates")
|
||||
|
||||
expect(page).to have_content "Second banner"
|
||||
expect(page).not_to have_content "First banner"
|
||||
end
|
||||
|
||||
it "does not render anything with no active banners" do
|
||||
Banner.all.each { |banner| banner.update!(post_ended_at: 1.day.ago) }
|
||||
|
||||
render_inline Shared::BannerComponent.new("debates")
|
||||
|
||||
expect(page).not_to have_css ".banner"
|
||||
end
|
||||
end
|
||||
|
||||
it "does not render anything given nil" do
|
||||
render_inline Shared::BannerComponent.new(nil)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user