Use separate actions to select/deselect investments

This is consistent to what we usually do. Also, we're applying the same
criteria mentioned in commit 72704d776:

> We're also making these actions idempotent, so sending many requests
> to the same action will get the same result, which wasn't the case
> with the `toggle` action. Although it's a low probability case, the
> `toggle` action could result in [selecting an investment] when trying
> to [deselect] it if someone else has [deselected it] it between the
> time the page loaded and the time the admin clicked on the
> "[Selected]" button.

app/components/admin/budget_investments/toggle_selection_component.html.erb

@@ -1,4 +1,4 @@
-<% if can?(:toggle_selection, investment) %>
+<% if can?(action, investment) %>
   <%= render Admin::ToggleSwitchComponent.new(action, investment, pressed: selected?, **options) %>
 <% elsif selected? %>
   <%= selected_text %>

app/components/admin/budget_investments/toggle_selection_component.rb

@@ -14,20 +14,26 @@ class Admin::BudgetInvestments::ToggleSelectionComponent < ApplicationComponent
     end
 
     def action
-      :toggle_selection
+      if selected?
+        :deselect
+      else
+        :select
+      end
     end
 
     def path
-      toggle_selection_admin_budget_budget_investment_path(
+      url_for({
-        investment.budget,
+        controller: "admin/budget_investments",
-        investment,
+        action: action,
+        budget_id: investment.budget,
+        id: investment,
         filter: params[:filter],
         sort_by: params[:sort_by],
         min_total_supports: params[:min_total_supports],
         max_total_supports: params[:max_total_supports],
         advanced_filters: params[:advanced_filters],
         page: params[:page]
-      )
+      })
     end
 
     def options

app/controllers/admin/budget_investments_controller.rb

@@ -9,7 +9,7 @@ class Admin::BudgetInvestmentsController < Admin::BaseController
   has_filters %w[all], only: :index
 
   before_action :load_budget
-  before_action :load_investment, only: [:show, :edit, :update, :toggle_selection]
+  before_action :load_investment, except: [:index]
   before_action :load_ballot, only: [:show, :index]
   before_action :parse_valuation_filters
   before_action :load_investments, only: :index
@@ -60,10 +60,18 @@ class Admin::BudgetInvestmentsController < Admin::BaseController
     end
   end
 
-  def toggle_selection
+  def select
-    authorize! :toggle_selection, @investment
+    authorize! :select, @investment
-    @investment.toggle :selected
+    @investment.update!(selected: true)
-    @investment.save!
+
+    render :toggle_selection
+  end
+
+  def deselect
+    authorize! :deselect, @investment
+    @investment.update!(selected: false)
+
+    render :toggle_selection
   end
 
   private

app/models/abilities/administrator.rb

@@ -75,7 +75,7 @@ module Abilities
       can [:valuate, :comment_valuation], Budget::Investment
       cannot [:admin_update, :valuate, :comment_valuation],
              Budget::Investment, budget: { phase: "finished" }
-      can :toggle_selection, Budget::Investment do |investment|
+      can [:select, :deselect], Budget::Investment do |investment|
         investment.feasible? && investment.valuation_finished? && !investment.budget.finished?
       end
 

config/routes/admin.rb

@@ -66,7 +66,10 @@ namespace :admin do
     end
 
     resources :budget_investments, only: [:index, :show, :edit, :update] do
-      member { patch :toggle_selection }
+      member do
+        patch :select
+        patch :deselect
+      end
 
       resources :audits, only: :show, controller: "budget_investment_audits"
       resources :milestones, controller: "budget_investment_milestones"

spec/controllers/admin/budget_investments_controller_spec.rb

@@ -38,18 +38,63 @@ describe Admin::BudgetInvestmentsController, :admin do
     end
   end
 
-  describe "PATCH toggle selection" do
+  describe "PATCH select" do
-    it "uses the toggle_selection authorization rules" do
+    let(:investment) { create(:budget_investment, :feasible, :finished) }
-      investment = create(:budget_investment)
 
-      patch :toggle_selection, xhr: true, params: {
+    it "selects the investment" do
-        id: investment,
+      expect do
-        budget_id: investment.budget,
+        patch :select, xhr: true, params: { id: investment, budget_id: investment.budget }
-      }
+      end.to change { investment.reload.selected? }.from(false).to(true)
+
+      expect(response).to be_successful
+    end
+
+    it "does not modify already selected investments" do
+      investment.update!(selected: true)
+
+      expect do
+        patch :select, xhr: true, params: { id: investment, budget_id: investment.budget }
+      end.not_to change { investment.reload.selected? }
+    end
+
+    it "uses the select/deselect authorization rules" do
+      investment.update!(valuation_finished: false)
+
+      patch :select, xhr: true, params: { id: investment, budget_id: investment.budget }
 
       expect(flash[:alert]).to eq "You do not have permission to carry out the action " \
-                                  "'toggle_selection' on Investment."
+                                  "'select' on Investment."
       expect(investment).not_to be_selected
     end
   end
+
+  describe "PATCH deselect" do
+    let(:investment) { create(:budget_investment, :feasible, :finished, :selected) }
+
+    it "deselects the investment" do
+      expect do
+        patch :deselect, xhr: true, params: { id: investment, budget_id: investment.budget }
+      end.to change { investment.reload.selected? }.from(true).to(false)
+
+      expect(response).to be_successful
+    end
+
+    it "does not modify non-selected investments" do
+      investment.update!(selected: false)
+
+      expect do
+        patch :deselect, xhr: true, params: { id: investment, budget_id: investment.budget }
+      end.not_to change { investment.reload.selected? }
+    end
+
+    it "uses the select/deselect authorization rules" do
+      investment.update!(valuation_finished: false)
+
+      patch :deselect, xhr: true, params: { id: investment, budget_id: investment.budget }
+
+      expect(flash[:alert]).to eq "You do not have permission to carry out the action " \
+                                  "'deselect' on Investment."
+      expect(investment).to be_selected
+    end
+  end
 end

spec/models/abilities/administrator_spec.rb

@@ -116,10 +116,10 @@ describe Abilities::Administrator do
   it { should_not be_able_to(:valuate, finished_investment) }
   it { should_not be_able_to(:comment_valuation, finished_investment) }
 
-  it { should be_able_to(:toggle_selection, create(:budget_investment, :feasible, :finished)) }
+  it { should be_able_to([:select, :deselect], create(:budget_investment, :feasible, :finished)) }
-  it { should_not be_able_to(:toggle_selection, create(:budget_investment, :feasible, :open)) }
+  it { should_not be_able_to([:select, :deselect], create(:budget_investment, :feasible, :open)) }
-  it { should_not be_able_to(:toggle_selection, create(:budget_investment, :unfeasible, :finished)) }
+  it { should_not be_able_to([:select, :deselect], create(:budget_investment, :unfeasible, :finished)) }
-  it { should_not be_able_to(:toggle_selection, finished_investment) }
+  it { should_not be_able_to([:select, :deselect], finished_investment) }
 
   it { should be_able_to(:destroy, proposal_image) }
   it { should be_able_to(:destroy, proposal_document) }