Disable calculating winners during balloting

Calculating winners before the balloting is over is useless (results
aren't published at that point) and can lead to the wrong results since
users are still voting and results might change.

And we were showing the button to calculate winners even when a budget
had finished. However, in this case the action to calculate winners did
nothing, which resulted in administrators seeing nothing happened after
pressing the button.
This commit is contained in:
Javi Martín
2021-08-30 15:11:02 +02:00
parent 2b709f1a36
commit 0a14337580
11 changed files with 81 additions and 36 deletions

View File

@@ -1,4 +1,4 @@
<% if display_button? %> <% if can?(:calculate_winners, budget) %>
<%= render Admin::ActionComponent.new( <%= render Admin::ActionComponent.new(
:calculate_winners, :calculate_winners,
budget, budget,
@@ -9,6 +9,7 @@
<%= t("admin.budgets.winners.calculate") %> <%= t("admin.budgets.winners.calculate") %>
</span> </span>
<div class="callout warning clear"> <div class="callout warning clear">
<%= t("admin.budget_investments.index.cannot_calculate_winners") %> <%= t("admin.budget_investments.index.cannot_calculate_winners",
phase: t("budgets.phase.reviewing_ballots")) %>
</div> </div>
<% end %> <% end %>

View File

@@ -1,5 +1,6 @@
class Admin::Budgets::CalculateWinnersButtonComponent < ApplicationComponent class Admin::Budgets::CalculateWinnersButtonComponent < ApplicationComponent
attr_reader :budget, :from_investments attr_reader :budget, :from_investments
delegate :can?, to: :helpers
def initialize(budget, from_investments: false) def initialize(budget, from_investments: false)
@budget = budget @budget = budget
@@ -8,10 +9,6 @@ class Admin::Budgets::CalculateWinnersButtonComponent < ApplicationComponent
private private
def display_button?
budget.balloting_or_later?
end
def text def text
if budget.investments.winners.empty? if budget.investments.winners.empty?
t("admin.budgets.winners.calculate") t("admin.budgets.winners.calculate")

View File

@@ -26,8 +26,6 @@ class Admin::BudgetsController < Admin::BaseController
end end
def calculate_winners def calculate_winners
return unless @budget.balloting_process?
@budget.headings.each { |heading| Budget::Result.new(@budget, heading).delay.calculate_winners } @budget.headings.each { |heading| Budget::Result.new(@budget, heading).delay.calculate_winners }
redirect_to admin_budget_budget_investments_path( redirect_to admin_budget_budget_investments_path(
budget_id: @budget.id, budget_id: @budget.id,

View File

@@ -61,8 +61,10 @@ module Abilities
can :manage, Dashboard::Action can :manage, Dashboard::Action
can [:index, :read, :new, :create, :update, :destroy, :calculate_winners], Budget can [:index, :read, :new, :create, :update, :destroy], Budget
can :publish, Budget, id: Budget.drafting.ids can :publish, Budget, id: Budget.drafting.ids
can :calculate_winners, Budget, &:reviewing_ballots?
can [:read, :create, :update, :destroy], Budget::Group can [:read, :create, :update, :destroy], Budget::Group
can [:read, :create, :update, :destroy], Budget::Heading can [:read, :create, :update, :destroy], Budget::Heading
can [:hide, :admin_update, :toggle_selection], Budget::Investment can [:hide, :admin_update, :toggle_selection], Budget::Investment

View File

@@ -154,10 +154,6 @@ class Budget < ApplicationRecord
current_phase&.publishing_prices_or_later? current_phase&.publishing_prices_or_later?
end end
def balloting_process?
balloting? || reviewing_ballots?
end
def balloting_or_later? def balloting_or_later?
current_phase&.balloting_or_later? current_phase&.balloting_or_later?
end end

View File

@@ -267,7 +267,7 @@ en:
incompatible: Incompatible incompatible: Incompatible
price: Price price: Price
author: Author author: Author
cannot_calculate_winners: The budget has to stay on phase "Balloting projects", "Reviewing Ballots" or "Finished budget" in order to calculate winners projects cannot_calculate_winners: The budget has to stay on phase "%{phase}" in order to calculate winners projects
see_results: "See results" see_results: "See results"
milestone_tags_filter_all: "All milestone tags" milestone_tags_filter_all: "All milestone tags"
show: show:

View File

@@ -267,7 +267,7 @@ es:
incompatible: Incompatible incompatible: Incompatible
price: Precio price: Precio
author: Autor author: Autor
cannot_calculate_winners: El presupuesto debe estar en las fases "Votación final", "Votación finalizada" o "Resultados" para poder calcular las propuestas ganadoras cannot_calculate_winners: El presupuesto debe estar en la fase "%{phase}" para poder calcular las propuestas ganadoras
see_results: "Ver resultados" see_results: "Ver resultados"
milestone_tags_filter_all: "Todas las etiquetas de seguimiento" milestone_tags_filter_all: "Todas las etiquetas de seguimiento"
show: show:

View File

@@ -0,0 +1,65 @@
require "rails_helper"
describe Admin::Budgets::CalculateWinnersButtonComponent, controller: Admin::BaseController do
let(:budget) { create(:budget) }
let(:component) { Admin::Budgets::CalculateWinnersButtonComponent.new(budget) }
before { sign_in(create(:administrator).user) }
it "renders when reviewing ballots" do
budget.update!(phase: "reviewing_ballots")
render_inline component
expect(page).to have_button "Calculate Winner Investments"
expect(page).not_to have_button "Recalculate Winner Investments"
end
it "does not render before balloting has finished" do
budget.update!(phase: "balloting")
render_inline component
expect(page).not_to have_button "Calculate Winner Investments"
expect(page).not_to have_button "Recalculate Winner Investments"
end
it "does not render after the budget process has finished" do
budget.update!(phase: "finished")
render_inline component
expect(page).not_to have_button "Calculate Winner Investments"
expect(page).not_to have_button "Recalculate Winner Investments"
end
context "budget with winners" do
before { create(:budget_investment, :winner, budget: budget) }
it "renders when reviewing ballots" do
budget.update!(phase: "reviewing_ballots")
render_inline component
expect(page).to have_button "Recalculate Winner Investments"
expect(page).not_to have_button "Calculate Winner Investments"
end
it "does not render before balloting has finished" do
budget.update!(phase: "balloting")
render_inline component
expect(page).not_to have_button "Calculate Winner Investments"
expect(page).not_to have_button "Recalculate Winner Investments"
end
it "does not render after the budget process has finished" do
budget.update!(phase: "finished")
render_inline component
expect(page).not_to have_button "Calculate Winner Investments"
expect(page).not_to have_button "Recalculate Winner Investments"
end
end
end

View File

@@ -78,6 +78,9 @@ describe Abilities::Administrator do
it { should be_able_to(:create, Budget) } it { should be_able_to(:create, Budget) }
it { should be_able_to(:update, Budget) } it { should be_able_to(:update, Budget) }
it { should be_able_to(:read_results, Budget) } it { should be_able_to(:read_results, Budget) }
it { should be_able_to(:calculate_winners, create(:budget, :reviewing_ballots)) }
it { should_not be_able_to(:calculate_winners, create(:budget, :balloting)) }
it { should_not be_able_to(:calculate_winners, create(:budget, :finished)) }
it { should be_able_to(:create, Budget::ValuatorAssignment) } it { should be_able_to(:create, Budget::ValuatorAssignment) }

View File

@@ -515,9 +515,8 @@ describe "Admin budget investments", :admin do
click_button "Filter" click_button "Filter"
expect(page).not_to have_button "Calculate Winner Investments" expect(page).not_to have_button "Calculate Winner Investments"
expect(page).to have_content 'The budget has to stay on phase "Balloting projects", '\ expect(page).to have_content 'The budget has to stay on phase "Reviewing voting" '\
'"Reviewing Ballots" or "Finished budget" in order '\ "in order to calculate winners projects"
"to calculate winners projects"
visit admin_budget_path(budget) visit admin_budget_path(budget)

View File

@@ -354,26 +354,10 @@ describe "Admin budgets", :admin do
expect(page).to have_link "See results" expect(page).to have_link "See results"
end end
scenario "For a finished Budget" do scenario "Recalculate for a budget in reviewing ballots" do
budget = create(:budget, :finished) budget = create(:budget, :reviewing_ballots)
allow_any_instance_of(Budget).to receive(:has_winning_investments?).and_return(true)
visit admin_budget_path(budget)
expect(page).to have_content "Calculate Winner Investments"
expect(page).to have_content "See results"
end
scenario "Recalculate for a finished Budget" do
budget = create(:budget, :finished)
create(:budget_investment, :winner, budget: budget) create(:budget_investment, :winner, budget: budget)
visit admin_budget_path(budget)
expect(page).to have_content "Recalculate Winner Investments"
expect(page).to have_content "See results"
expect(page).not_to have_content "Calculate Winner Investments"
visit admin_budget_budget_investments_path(budget) visit admin_budget_budget_investments_path(budget)
click_link "Advanced filters" click_link "Advanced filters"
check "Winners" check "Winners"