From 8376efce3f2c4466b58938754354e70d79fa42f3 Mon Sep 17 00:00:00 2001 From: Marko Lovic Date: Fri, 20 Jul 2018 12:43:36 +0200 Subject: [PATCH] Hide headings with no investments The page should not show any headings which don't have any winning investments. The "no content" message should only be shown when there are no headings with investments to avoid an otherwise blank page. __Note:__ in the main @headings query, _both_ #includes and #joins are needed to: 1. eager load all necessary data (#includes) and 2. to perform an INNER JOIN on milestones to filter out investments with no milestones (#joins). --- .../budgets/executions_controller.rb | 17 ++++++++++++- app/helpers/budget_executions_helper.rb | 25 ------------------- .../budgets/executions/_investments.html.erb | 14 +++-------- app/views/budgets/executions/show.html.erb | 8 +++++- config/locales/en/budgets.yml | 2 +- config/locales/es/budgets.yml | 2 +- spec/features/budgets/executions_spec.rb | 23 ++++++++++++++--- 7 files changed, 48 insertions(+), 43 deletions(-) delete mode 100644 app/helpers/budget_executions_helper.rb diff --git a/app/controllers/budgets/executions_controller.rb b/app/controllers/budgets/executions_controller.rb index 3d5373391..dc899c3a7 100644 --- a/app/controllers/budgets/executions_controller.rb +++ b/app/controllers/budgets/executions_controller.rb @@ -7,7 +7,16 @@ module Budgets def show authorize! :read_executions, @budget @statuses = ::Budget::Investment::Status.all - @headings = @budget.headings.order(id: :asc) + @headings = @budget.headings + .includes(investments: :milestones) + .joins(investments: :milestones) + .where(budget_investments: {winner: true}) + .distinct + .order(id: :asc) + + if params[:status].present? + @headings = @headings.where(filter_investment_by_latest_milestone, params[:status]) + end end private @@ -16,5 +25,11 @@ module Budgets @budget = Budget.find_by(slug: params[:id]) || Budget.find_by(id: params[:id]) end + def filter_investment_by_latest_milestone + <<-SQL + (SELECT status_id FROM budget_investment_milestones + WHERE investment_id = budget_investments.id ORDER BY publication_date DESC LIMIT 1) = ? + SQL + end end end diff --git a/app/helpers/budget_executions_helper.rb b/app/helpers/budget_executions_helper.rb deleted file mode 100644 index 64caa0c18..000000000 --- a/app/helpers/budget_executions_helper.rb +++ /dev/null @@ -1,25 +0,0 @@ -module BudgetExecutionsHelper - - def winner_investments(heading) - if params[:status].present? - heading.investments - .winners - .joins(:milestones) - .distinct - .where(filter_investment_by_latest_milestone, params[:status]) - else - heading.investments - .winners - .joins(:milestones) - .distinct - end - end - - def filter_investment_by_latest_milestone - <<-SQL - (SELECT status_id FROM budget_investment_milestones - WHERE investment_id = budget_investments.id ORDER BY publication_date DESC LIMIT 1) = ? - SQL - end - -end diff --git a/app/views/budgets/executions/_investments.html.erb b/app/views/budgets/executions/_investments.html.erb index b04639a50..0808b354a 100644 --- a/app/views/budgets/executions/_investments.html.erb +++ b/app/views/budgets/executions/_investments.html.erb @@ -1,10 +1,9 @@ <% @headings.each do |heading| %> -

- <%= heading.name %> -

- <% if winner_investments(heading).any? %> +

+ <%= heading.name %> +

- <% winner_investments(heading).each do |investment| %> + <% heading.investments.each do |investment| %>
<%= link_to budget_investment_path(@budget, investment, anchor: "tab-milestones"), data: { 'equalizer-watch': true } do %> @@ -31,9 +30,4 @@
<% end %>
- <% else %> -
- <%= t("budgets.executions.no_winner_investments") %> -
- <% end %> <% end %> diff --git a/app/views/budgets/executions/show.html.erb b/app/views/budgets/executions/show.html.erb index 1d793ea80..dc18d2d08 100644 --- a/app/views/budgets/executions/show.html.erb +++ b/app/views/budgets/executions/show.html.erb @@ -62,6 +62,12 @@
<% end %> - <%= render 'budgets/executions/investments' %> + <% if @headings.any? %> + <%= render 'budgets/executions/investments' %> + <% else %> +
+ <%= t("budgets.executions.no_winner_investments") %> +
+ <% end %> diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml index b342aaf86..658ac429d 100644 --- a/config/locales/en/budgets.yml +++ b/config/locales/en/budgets.yml @@ -179,7 +179,7 @@ en: page_title: "%{budget} - Milestones" heading: "Participatory budget Milestones" heading_selection_title: "By district" - no_winner_investments: "No winner investments for this heading" + no_winner_investments: "No winner investments in this state" filters: label: "Project's current state" all: "All" diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml index 7f1e03914..60a02a6eb 100644 --- a/config/locales/es/budgets.yml +++ b/config/locales/es/budgets.yml @@ -179,7 +179,7 @@ es: page_title: "%{budget} - Seguimiento de proyectos" heading: "Seguimiento de proyectos" heading_selection_title: "Ámbito de actuación" - no_winner_investments: "No hay proyectos de gasto ganadores para esta partida" + no_winner_investments: "No hay proyectos de gasto ganadores en este estado" filters: label: "Estado actual del proyecto" all: "Todos" diff --git a/spec/features/budgets/executions_spec.rb b/spec/features/budgets/executions_spec.rb index 0f8a31a8d..78b8ab4d0 100644 --- a/spec/features/budgets/executions_spec.rb +++ b/spec/features/budgets/executions_spec.rb @@ -27,7 +27,9 @@ feature 'Executions' do expect(page).not_to have_content(investment4.title) end - scenario 'render a message for headings without winner investments' do + scenario "Do not display headings with no winning investments for selected status" do + create(:budget_investment_milestone, investment: investment1) + empty_group = create(:budget_group, budget: budget) empty_heading = create(:budget_heading, group: empty_group, price: 1000) @@ -38,11 +40,25 @@ feature 'Executions' do expect(page).to have_content(empty_heading.name) click_link 'Milestones' - click_link "#{empty_heading.name}" - expect(page).to have_content('No winner investments for this heading') + expect(page).to have_content(heading.name) + expect(page).not_to have_content(empty_heading.name) end + scenario "Show message when there are no winning investments with the selected status", :js do + create(:budget_investment_status, name: I18n.t('seeds.budgets.statuses.executed')) + + visit budget_path(budget) + + click_link 'See results' + click_link 'Milestones' + + expect(page).not_to have_content('No winner investments in this state') + + select 'Executed', from: 'status' + + expect(page).to have_content('No winner investments in this state') + end context 'Images' do scenario 'renders milestone image if available' do @@ -157,7 +173,6 @@ feature 'Executions' do select 'Studying the project', from: 'status' expect(page).not_to have_content(investment1.title) - expect(page).to have_content('No winner investments for this heading') select 'Bidding', from: 'status'