From 0de8c884ac4acc03895d20930101c29104a67de7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 13 Nov 2018 12:50:28 +0100 Subject: [PATCH 1/4] Extract method to sort investments by heading --- .../budgets/executions_controller.rb | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/app/controllers/budgets/executions_controller.rb b/app/controllers/budgets/executions_controller.rb index e17763090..cf5b0c8f8 100644 --- a/app/controllers/budgets/executions_controller.rb +++ b/app/controllers/budgets/executions_controller.rb @@ -7,32 +7,32 @@ module Budgets def show authorize! :read_executions, @budget @statuses = Milestone::Status.all - - if params[:status].present? - @investments_by_heading = @budget.investments.winners - .joins(:milestones).includes(:milestones) - .select { |i| i.milestones.published.with_status - .order_by_publication_date.last - .try(:status_id) == params[:status].to_i } - .uniq - .group_by(&:heading) - else - @investments_by_heading = @budget.investments.winners - .joins(:milestones).includes(:milestones) - .distinct.group_by(&:heading) - end - - @investments_by_heading = reorder_alphabetically_with_city_heading_first.to_h + @investments_by_heading = investments_by_heading_ordered_alphabetically_with_city_heading_first.to_h end private + def investments_by_heading + if params[:status].present? + @budget.investments.winners + .joins(:milestones).includes(:milestones) + .select { |i| i.milestones.published.with_status + .order_by_publication_date.last + .try(:status_id) == params[:status].to_i } + .uniq + .group_by(&:heading) + else + @budget.investments.winners + .joins(:milestones).includes(:milestones) + .distinct.group_by(&:heading) + end + end def load_budget @budget = Budget.find_by(slug: params[:id]) || Budget.find_by(id: params[:id]) end - def reorder_alphabetically_with_city_heading_first - @investments_by_heading.sort do |a, b| + def investments_by_heading_ordered_alphabetically_with_city_heading_first + investments_by_heading.sort do |a, b| if a[0].name == 'Toda la ciudad' -1 elsif b[0].name == 'Toda la ciudad' From 91d91f2ebf3ee666d1fcb744bcb1073270642231 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 13 Nov 2018 12:53:02 +0100 Subject: [PATCH 2/4] Remove city heading specific code This code accidentally made it to consul, and it's specific to Madrid. --- app/controllers/budgets/executions_controller.rb | 14 +++----------- spec/features/budgets/executions_spec.rb | 13 ------------- 2 files changed, 3 insertions(+), 24 deletions(-) diff --git a/app/controllers/budgets/executions_controller.rb b/app/controllers/budgets/executions_controller.rb index cf5b0c8f8..9af8893b4 100644 --- a/app/controllers/budgets/executions_controller.rb +++ b/app/controllers/budgets/executions_controller.rb @@ -7,7 +7,7 @@ module Budgets def show authorize! :read_executions, @budget @statuses = Milestone::Status.all - @investments_by_heading = investments_by_heading_ordered_alphabetically_with_city_heading_first.to_h + @investments_by_heading = investments_by_heading_ordered_alphabetically.to_h end private @@ -31,16 +31,8 @@ module Budgets @budget = Budget.find_by(slug: params[:id]) || Budget.find_by(id: params[:id]) end - def investments_by_heading_ordered_alphabetically_with_city_heading_first - investments_by_heading.sort do |a, b| - if a[0].name == 'Toda la ciudad' - -1 - elsif b[0].name == 'Toda la ciudad' - 1 - else - a[0].name <=> b[0].name - end - end + def investments_by_heading_ordered_alphabetically + investments_by_heading.sort { |a, b| a[0].name <=> b[0].name } end end end diff --git a/spec/features/budgets/executions_spec.rb b/spec/features/budgets/executions_spec.rb index 226d01cfe..53d0ef6b6 100644 --- a/spec/features/budgets/executions_spec.rb +++ b/spec/features/budgets/executions_spec.rb @@ -230,19 +230,6 @@ feature 'Executions' do heading end - scenario 'City heading is displayed first' do - heading.destroy! - other_heading1 = create_heading_with_investment_with_milestone(group: group, name: 'Other 1') - city_heading = create_heading_with_investment_with_milestone(group: group, name: 'Toda la ciudad') - other_heading2 = create_heading_with_investment_with_milestone(group: group, name: 'Other 2') - - visit budget_executions_path(budget) - - expect(page).to have_css('.budget-execution', count: 3) - expect(city_heading.name).to appear_before(other_heading1.name) - expect(city_heading.name).to appear_before(other_heading2.name) - end - scenario 'Non-city headings are displayed in alphabetical order' do heading.destroy! z_heading = create_heading_with_investment_with_milestone(group: group, name: 'Zzz') From b4b0b18a2d6157ae2da331fc7c77b216cb0f9d11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 13 Nov 2018 13:25:47 +0100 Subject: [PATCH 3/4] Extract method to get investment milestone status --- app/controllers/budgets/executions_controller.rb | 4 +--- app/helpers/budget_executions_helper.rb | 6 +++--- app/models/budget/investment.rb | 4 ++++ 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/controllers/budgets/executions_controller.rb b/app/controllers/budgets/executions_controller.rb index 9af8893b4..52da2a7ed 100644 --- a/app/controllers/budgets/executions_controller.rb +++ b/app/controllers/budgets/executions_controller.rb @@ -15,9 +15,7 @@ module Budgets if params[:status].present? @budget.investments.winners .joins(:milestones).includes(:milestones) - .select { |i| i.milestones.published.with_status - .order_by_publication_date.last - .try(:status_id) == params[:status].to_i } + .select { |i| i.milestone_status_id == params[:status].to_i } .uniq .group_by(&:heading) else diff --git a/app/helpers/budget_executions_helper.rb b/app/helpers/budget_executions_helper.rb index d77867052..45175e806 100644 --- a/app/helpers/budget_executions_helper.rb +++ b/app/helpers/budget_executions_helper.rb @@ -1,9 +1,9 @@ module BudgetExecutionsHelper def filters_select_counts(status) - @budget.investments.winners.with_milestones.select { |i| i.milestones - .published.with_status.order_by_publication_date - .last.status_id == status rescue false }.count + @budget.investments.winners.with_milestones.select do |investment| + investment.milestone_status_id == status + end.count end def first_milestone_with_image(investment) diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 3ed96da5f..91a9f2219 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -342,6 +342,10 @@ class Budget self.valuator_groups.collect(&:name).compact.join(', ').presence end + def milestone_status_id + milestones.published.with_status.order_by_publication_date.last&.status_id + end + private def set_denormalized_ids From e6a609e6e59882ec958fc90c38c96edd8eee5c4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 13 Nov 2018 13:32:35 +0100 Subject: [PATCH 4/4] Extract method to filter investments by status --- app/controllers/budgets/executions_controller.rb | 3 +-- app/helpers/budget_executions_helper.rb | 4 +--- app/models/budget/investment.rb | 6 ++++++ 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/controllers/budgets/executions_controller.rb b/app/controllers/budgets/executions_controller.rb index 52da2a7ed..25f27b1ca 100644 --- a/app/controllers/budgets/executions_controller.rb +++ b/app/controllers/budgets/executions_controller.rb @@ -14,8 +14,7 @@ module Budgets def investments_by_heading if params[:status].present? @budget.investments.winners - .joins(:milestones).includes(:milestones) - .select { |i| i.milestone_status_id == params[:status].to_i } + .with_milestone_status_id(params[:status]) .uniq .group_by(&:heading) else diff --git a/app/helpers/budget_executions_helper.rb b/app/helpers/budget_executions_helper.rb index 45175e806..90d0744b0 100644 --- a/app/helpers/budget_executions_helper.rb +++ b/app/helpers/budget_executions_helper.rb @@ -1,9 +1,7 @@ module BudgetExecutionsHelper def filters_select_counts(status) - @budget.investments.winners.with_milestones.select do |investment| - investment.milestone_status_id == status - end.count + @budget.investments.winners.with_milestone_status_id(status).count end def first_milestone_with_image(investment) diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 91a9f2219..110da8732 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -342,6 +342,12 @@ class Budget self.valuator_groups.collect(&:name).compact.join(', ').presence end + def self.with_milestone_status_id(status_id) + joins(:milestones).includes(:milestones).select do |investment| + investment.milestone_status_id == status_id.to_i + end + end + def milestone_status_id milestones.published.with_status.order_by_publication_date.last&.status_id end