Don't redirect when toggling visible to valuators

After upgrading to Turbolinks 5, redirects are followed on AJAX
requests, so we were accidentally redirecting the user after they mark
an investment as visible to valuators.

There was already a system spec failing due to this issue ("Admin budget
investments Mark as visible to valuators Keeps the valuation tags");
however, it only failed in some cases, so we're adding additional tests.

Ideally we would write a system test to check what happens when users
click on the checkbox. However, from the user's point of view, nothing
happens when they do so, and so testing it is hard. There's a usability
issue here (no feedback is provided to the user indicating the
investment is actually updated when they click on the checkbox and so
they might look for a button to send the form), which also results in a
feature which is difficult to test.

So we're writing two tests instead: one checking the controller does not
redirect when using a JSON request, and one checking the form submits a
JSON request.

I've chosen JSON over AJAX because usually requests to the update action
come from the edit form, and we might change the edit form to send an
AJAX request (and, in this case, Turbolinks would handle the redirect as
mentioned above).

Another option would be to send an AJAX request to a different action,
like it's done for the toggle selection action. I don't have a strong
preference for either option, so I'm leaving it the way it was. At some
point we should change the user interface, though; right now in the same
row there are two actions doing basically the same thing (toggling
valuator visibility and toggling selection) but with very different user
interfaces (one is a checkbox and the other one a link changing its
style depending on the state), resulting in a confusing interface.

app/controllers/admin/budget_investments_controller.rb

@@ -39,6 +39,9 @@ class Admin::BudgetInvestmentsController < Admin::BaseController
 
   def update
     authorize! :admin_update, @investment
+
+    respond_to do |format|
+      format.html do
         if @investment.update(budget_investment_params)
           redirect_to admin_budget_budget_investment_path(@budget,
                                                           @investment,
@@ -52,6 +55,12 @@ class Admin::BudgetInvestmentsController < Admin::BaseController
         end
       end
 
+      format.json do
+        @investment.update!(budget_investment_params)
+      end
+    end
+  end
+
   def toggle_selection
     authorize! :toggle_selection, @investment
     @investment.toggle :selected

app/views/admin/budget_investments/_select_investment.html.erb

@@ -51,7 +51,7 @@
 </td>
 
 <td class="small text-center" data-field="visible_to_valuators">
-  <%= form_for [:admin, investment.budget, investment], remote: true do |f| %>
+  <%= form_for [:admin, investment.budget, investment], remote: true, format: :json do |f| %>
       <%= f.check_box :visible_to_valuators,
                       label: false,
                       class: "js-submit-on-change",

spec/controllers/admin/budget_investments_controller_spec.rb

@@ -0,0 +1,19 @@
+require "rails_helper"
+
+describe Admin::BudgetInvestmentsController do
+  describe "PATCH update" do
+    it "does not redirect on AJAX requests" do
+      investment = create(:budget_investment)
+      sign_in(create(:administrator).user)
+
+      patch :update, params: {
+        id: investment,
+        budget_id: investment.budget,
+        format: :json,
+        budget_investment: { visible_to_valuators: true }
+      }
+
+      expect(response).not_to be_redirect
+    end
+  end
+end

spec/spec_helper.rb

@@ -14,6 +14,7 @@ RSpec.configure do |config|
   config.run_all_when_everything_filtered = true
   config.include RequestSpecHelper, type: :request
   config.include Devise::Test::ControllerHelpers, type: :controller
+  config.include Devise::Test::ControllerHelpers, type: :view
   config.include FactoryBot::Syntax::Methods
   config.include(EmailSpec::Helpers)
   config.include(EmailSpec::Matchers)

spec/views/admin/budget_investments/select_investment_spec.rb

@@ -0,0 +1,13 @@
+require "rails_helper"
+
+describe "investment row" do
+  it "uses a JSON request to update visible to valuators" do
+    investment = create(:budget_investment)
+    @budget = investment.budget
+    sign_in(create(:administrator).user)
+
+    render "admin/budget_investments/select_investment", investment: investment
+
+    expect(rendered).to have_css "form[action$='json'] input[name$='[visible_to_valuators]']"
+  end
+end