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.

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("_")

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

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