Fix crash voting on a heading with a content block

When voting investment projects, the sidebar was rendered without the
`@heading_content_blocks` being set. That resulted in a 500 error when
the heading had content blocks.

By extracting the logic to a component, we make sure the heading content
blocks are properly set every time this code is rendered, no matter
which controller is rendering the view.
This commit is contained in:
Javi Martín
2022-11-28 13:23:53 +01:00
parent 937a86e345
commit 236796406a
7 changed files with 75 additions and 16 deletions

View File

@@ -0,0 +1,5 @@
<ul class="no-bullet categories">
<% content_blocks.each do |content_block| %>
<%= raw content_block.body %>
<% end %>
</ul>

View File

@@ -0,0 +1,17 @@
class Budgets::Investments::ContentBlocksComponent < ApplicationComponent
attr_reader :heading
def initialize(heading)
@heading = heading
end
def render?
heading&.allow_custom_content && content_blocks.any?
end
private
def content_blocks
heading.content_blocks.where(locale: I18n.locale)
end
end

View File

@@ -25,7 +25,6 @@ module Budgets
before_action :load_categories, only: :index before_action :load_categories, only: :index
before_action :set_default_investment_filter, only: :index before_action :set_default_investment_filter, only: :index
before_action :set_view, only: :index before_action :set_view, only: :index
before_action :load_content_blocks, only: :index
skip_authorization_check only: :json_data skip_authorization_check only: :json_data
@@ -149,10 +148,6 @@ module Budgets
@categories = Tag.category.order(:name) @categories = Tag.category.order(:name)
end end
def load_content_blocks
@heading_content_blocks = @heading.content_blocks.where(locale: I18n.locale) if @heading
end
def tag_cloud def tag_cloud
TagCloud.new(Budget::Investment, params[:search]) TagCloud.new(Budget::Investment, params[:search])
end end

View File

@@ -1,7 +0,0 @@
<% if @heading.allow_custom_content %>
<ul class="no-bullet categories">
<% @heading_content_blocks.each do |content_block| %>
<%= raw content_block.body %>
<% end %>
</ul>
<% end %>

View File

@@ -59,9 +59,7 @@
</div> </div>
<% end %> <% end %>
<% if @heading && !@heading.content_blocks.where(locale: I18n.locale).empty? %> <%= render Budgets::Investments::ContentBlocksComponent.new(@heading) %>
<%= render "budgets/investments/content_blocks" %>
<% end %>
<% if @map_location&.available? %> <% if @map_location&.available? %>
<%= render "budgets/investments/map" %> <%= render "budgets/investments/map" %>

View File

@@ -0,0 +1,46 @@
require "rails_helper"
describe Budgets::Investments::ContentBlocksComponent do
it "is not rendered without a heading" do
render_inline Budgets::Investments::ContentBlocksComponent.new(nil)
expect(page).not_to be_rendered
end
it "is not rendered with a heading without custom blocks" do
heading = Budget::Heading.new(allow_custom_content: true)
render_inline Budgets::Investments::ContentBlocksComponent.new(heading)
expect(page).not_to be_rendered
end
it "is not rendered with a heading with custom blocks in other languages" do
heading = create(:budget_heading, allow_custom_content: true)
create(:heading_content_block, heading: heading, locale: :es)
render_inline Budgets::Investments::ContentBlocksComponent.new(heading)
expect(page).not_to be_rendered
end
it "is not rendered with a heading not allowing custom content" do
heading = create(:budget_heading, allow_custom_content: false)
create(:heading_content_block, heading: heading, locale: :en)
render_inline Budgets::Investments::ContentBlocksComponent.new(heading)
expect(page).not_to be_rendered
end
it "renders content blocks for the current language" do
heading = create(:budget_heading, allow_custom_content: true)
create(:heading_content_block, heading: heading, locale: :en, body: "<li>Heading block</li>")
render_inline Budgets::Investments::ContentBlocksComponent.new(heading)
page.find("ul") do |list|
expect(page).to have_css "li", exact_text: "Heading block"
end
end
end

View File

@@ -160,13 +160,16 @@ describe "Ballots" do
end end
end end
scenario "the Map shoud be visible before and after" do scenario "map and content block shoud be visible before and after" do
create(:budget_investment, :selected, heading: new_york, price: 10000, title: "More bridges") create(:budget_investment, :selected, heading: new_york, price: 10000, title: "More bridges")
create(:heading_content_block, heading: new_york, body: "<li>New Block</li>")
new_york.update!(allow_custom_content: true)
visit budget_investments_path(budget, heading_id: new_york) visit budget_investments_path(budget, heading_id: new_york)
within("#sidebar") do within("#sidebar") do
expect(page).to have_content "OpenStreetMap" expect(page).to have_content "OpenStreetMap"
expect(page).to have_content "New Block"
end end
add_to_ballot("More bridges") add_to_ballot("More bridges")
@@ -174,6 +177,7 @@ describe "Ballots" do
within("#sidebar") do within("#sidebar") do
expect(page).to have_content "More bridges" expect(page).to have_content "More bridges"
expect(page).to have_content "OpenStreetMap" expect(page).to have_content "OpenStreetMap"
expect(page).to have_content "New Block"
end end
within(".budget-investment", text: "More bridges") do within(".budget-investment", text: "More bridges") do
@@ -183,6 +187,7 @@ describe "Ballots" do
within("#sidebar") do within("#sidebar") do
expect(page).not_to have_content "More bridges" expect(page).not_to have_content "More bridges"
expect(page).to have_content "OpenStreetMap" expect(page).to have_content "OpenStreetMap"
expect(page).to have_content "New Block"
end end
end end
end end