From 49808195288dd94f23ea42391325699361e51659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 25 Feb 2019 12:18:24 +0100 Subject: [PATCH] Fix valuation tags being overwritten When params[:budget_investment][:valuation_tag_list] was not present, which is the case when updating an investment using the "mark as visible to valuators" checkbox, we were removing all valuation tags. Using a virtual attribute to assign the tags only if the parameter is present simplifies the code in the controller and avoids the issue. --- .../admin/budget_investments_controller.rb | 6 ------ app/models/budget/investment.rb | 4 ++++ spec/features/admin/budget_investments_spec.rb | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/budget_investments_controller.rb b/app/controllers/admin/budget_investments_controller.rb index 71c92aee9..7bf3a4bb4 100644 --- a/app/controllers/admin/budget_investments_controller.rb +++ b/app/controllers/admin/budget_investments_controller.rb @@ -38,7 +38,6 @@ class Admin::BudgetInvestmentsController < Admin::BaseController end def update - set_valuation_tags if @investment.update(budget_investment_params) redirect_to admin_budget_budget_investment_path(@budget, @investment, @@ -118,11 +117,6 @@ class Admin::BudgetInvestmentsController < Admin::BaseController @ballot = @budget.balloting? ? query.first_or_create : query.first_or_initialize end - def set_valuation_tags - @investment.set_tag_list_on(:valuation, budget_investment_params[:valuation_tag_list]) - params[:budget_investment] = params[:budget_investment].except(:valuation_tag_list) - end - def parse_valuation_filters if params[:valuator_or_group_id] model, id = params[:valuator_or_group_id].split("_") diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index a4d3be921..a040e48e9 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -349,6 +349,10 @@ class Budget self.valuator_groups.collect(&:name).compact.join(', ').presence end + def valuation_tag_list=(tags) + set_tag_list_on(:valuation, tags) + end + def self.with_milestone_status_id(status_id) joins(:milestones).includes(:milestones).select do |investment| investment.milestone_status_id == status_id.to_i diff --git a/spec/features/admin/budget_investments_spec.rb b/spec/features/admin/budget_investments_spec.rb index 4f7e56c3c..fc9d51ce1 100644 --- a/spec/features/admin/budget_investments_spec.rb +++ b/spec/features/admin/budget_investments_spec.rb @@ -1376,6 +1376,22 @@ feature "Admin budget investments" do expect(valuating_checkbox).not_to be_checked end end + + scenario "Keeps the valuation tags", :js do + investment1.set_tag_list_on(:valuation, %w[Possimpible Truthiness]) + investment1.save + + visit admin_budget_budget_investments_path(budget) + + within("#budget_investment_#{investment1.id}") do + check "budget_investment_visible_to_valuators" + end + + visit edit_admin_budget_budget_investment_path(budget, investment1) + + expect(page).to have_content "Possimpible" + expect(page).to have_content "Truthiness" + end end context "Selecting csv" do