From 69ade5b131981d3cefe62ae999398ecc65ea305c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 20 May 2021 14:24:50 +0200 Subject: [PATCH 1/2] Fix crash with budgets with disabled current phase There's an edge case where the current phase of the budget was disabled. In this case, the application was crashing. I'm not sure what we should do regarding this case. Is it OK to allow disabling the current phase? Is it OK to allow selecting a disabled phase as the current phase? Since I'm not sure about it, for now I'm leaving it the same way it was. Co-authored-by: Julian Herrero --- .../admin/budgets/index_component.rb | 6 +++++- .../admin/budgets/index_component_spec.rb | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 spec/components/admin/budgets/index_component_spec.rb diff --git a/app/components/admin/budgets/index_component.rb b/app/components/admin/budgets/index_component.rb index 1bf411989..58203a206 100644 --- a/app/components/admin/budgets/index_component.rb +++ b/app/components/admin/budgets/index_component.rb @@ -19,7 +19,11 @@ class Admin::Budgets::IndexComponent < ApplicationComponent end def current_enabled_phase_number(budget) - budget.phases.enabled.order(:id).pluck(:kind).index(budget.phase) + 1 + current_enabled_phase_index(budget) + 1 + end + + def current_enabled_phase_index(budget) + budget.phases.enabled.order(:id).pluck(:kind).index(budget.phase) || -1 end def dates(budget) diff --git a/spec/components/admin/budgets/index_component_spec.rb b/spec/components/admin/budgets/index_component_spec.rb new file mode 100644 index 000000000..c6da3183a --- /dev/null +++ b/spec/components/admin/budgets/index_component_spec.rb @@ -0,0 +1,18 @@ +require "rails_helper" + +describe Admin::Budgets::IndexComponent, type: :component do + before do + allow(ViewComponent::Base).to receive(:test_controller).and_return("Admin::BudgetsController") + allow_any_instance_of(Admin::BudgetsController).to receive(:valid_filters).and_return(["all"]) + allow_any_instance_of(Admin::BudgetsController).to receive(:current_filter).and_return("all") + end + + it "displays current phase zero for budgets with no current phase" do + budget = create(:budget, :accepting, name: "Not enabled phase") + budget.phases.accepting.update!(enabled: false) + + render_inline Admin::Budgets::IndexComponent.new(Budget.page(1)) + + expect(page.find("tr", text: "Not enabled phase")).to have_content "0/8" + end +end From 93a7d28a82d39fcc51aad451cf52f06c827704b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 20 May 2021 14:45:53 +0200 Subject: [PATCH 2/2] Fix crash with budgets with no published phases In this case, the duration of the budget cannot be determined, and the application was crashing when trying to do so. Now we're just returning `nil` as duration. --- .../admin/budgets/duration_component.rb | 2 +- app/models/budget.rb | 4 ++-- .../admin/budgets/duration_component_spec.rb | 18 +++++++++++++++++- .../admin/budgets/index_component_spec.rb | 9 +++++++++ 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/app/components/admin/budgets/duration_component.rb b/app/components/admin/budgets/duration_component.rb index 8af1a3ad0..3e7a80207 100644 --- a/app/components/admin/budgets/duration_component.rb +++ b/app/components/admin/budgets/duration_component.rb @@ -10,7 +10,7 @@ class Admin::Budgets::DurationComponent < ApplicationComponent end def duration - distance_of_time_in_words(durable.starts_at, durable.ends_at) + distance_of_time_in_words(durable.starts_at, durable.ends_at) if durable.starts_at && durable.ends_at end private diff --git a/app/models/budget.rb b/app/models/budget.rb index c667ada15..c685323f5 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -72,11 +72,11 @@ class Budget < ApplicationRecord end def starts_at - phases.published.first.starts_at + phases.published.first&.starts_at end def ends_at - phases.published.last.ends_at + phases.published.last&.ends_at end def description diff --git a/spec/components/admin/budgets/duration_component_spec.rb b/spec/components/admin/budgets/duration_component_spec.rb index 193d45e7c..fbbd51b73 100644 --- a/spec/components/admin/budgets/duration_component_spec.rb +++ b/spec/components/admin/budgets/duration_component_spec.rb @@ -43,6 +43,22 @@ describe Admin::Budgets::DurationComponent, type: :component do expect(page.text).to eq "about 1 year" end + + it "is not defined when no end date is defined" do + durable = double(starts_at: Time.zone.local(2015, 8, 1, 12, 0, 0), ends_at: nil) + + render Admin::Budgets::DurationComponent.new(durable).duration + + expect(page.text).to be_empty + end + + it "is not defined when no start date is defined" do + durable = double(starts_at: nil, ends_at: Time.zone.local(2016, 9, 30, 16, 30, 00)) + + render Admin::Budgets::DurationComponent.new(durable).duration + + expect(page.text).to be_empty + end end attr_reader :content @@ -52,6 +68,6 @@ describe Admin::Budgets::DurationComponent, type: :component do end def page - Capybara::Node::Simple.new(content) + Capybara::Node::Simple.new(content.to_s) end end diff --git a/spec/components/admin/budgets/index_component_spec.rb b/spec/components/admin/budgets/index_component_spec.rb index c6da3183a..c8e94ed6a 100644 --- a/spec/components/admin/budgets/index_component_spec.rb +++ b/spec/components/admin/budgets/index_component_spec.rb @@ -15,4 +15,13 @@ describe Admin::Budgets::IndexComponent, type: :component do expect(page.find("tr", text: "Not enabled phase")).to have_content "0/8" end + + it "displays phase zero out of zero for budgets with no enabled phases" do + budget = create(:budget, name: "Without phases") + budget.phases.each { |phase| phase.update!(enabled: false) } + + render_inline Admin::Budgets::IndexComponent.new(Budget.page(1)) + + expect(page.find("tr", text: "Without phases")).to have_content "0/0" + end end