Merge pull request #4443 from consul/improve-management

Avoid exceptions in Management section

app/controllers/management/base_controller.rb

@@ -32,9 +32,10 @@ class Management::BaseController < ActionController::Base
     end
 
     def check_verified_user(alert_msg)
-      unless managed_user.level_two_or_three_verified?
-        redirect_to management_document_verifications_path, alert: alert_msg
-      end
+      return if managed_user.persisted? && managed_user.level_two_or_three_verified?
+
+      message = managed_user.persisted? ? alert_msg : t("management.sessions.need_managed_user")
+      redirect_to management_document_verifications_path, alert: message
     end
 
     def set_locale

config/locales/en/management.yml

@@ -106,6 +106,7 @@ en:
         one: " containing the term '%{search_term}'"
         other: " containing the term '%{search_term}'"
     sessions:
+      need_managed_user: To perform this action you must select a user
       signed_out: Signed out successfully.
       signed_out_managed_user: User session signed out successfully.
     username_label: Username

config/locales/es/management.yml

@@ -106,6 +106,7 @@ es:
         one: " que contiene '%{search_term}'"
         other: " que contienen '%{search_term}'"
     sessions:
+      need_managed_user: Para realizar esta acción debes seleccionar un usuario.
       signed_out: Has cerrado la sesión correctamente.
       signed_out_managed_user: Se ha cerrado correctamente la sesión del usuario.
     username_label: Nombre de usuario

spec/system/management/account_spec.rb

@@ -98,4 +98,28 @@ describe "Account" do
     expect(page).to have_css("a[href='javascript:window.print();']", text: "Print password")
     expect(page).to have_css("div.for-print-only", text: "another_new_password", visible: :hidden)
   end
+
+  describe "When a user has not been selected" do
+    before do
+      Setting["feature.user.skip_verification"] = "true"
+    end
+
+    scenario "we can't reset password via email" do
+      login_as_manager
+
+      click_link "Reset password via email"
+
+      expect(page).to have_content "To perform this action you must select a user"
+      expect(page).to have_current_path management_document_verifications_path
+    end
+
+    scenario "we can't reset password manually" do
+      login_as_manager
+
+      click_link "Reset password manually"
+
+      expect(page).to have_content "To perform this action you must select a user"
+      expect(page).to have_current_path management_document_verifications_path
+    end
+  end
 end

spec/system/management/budget_investments_spec.rb

@@ -7,8 +7,6 @@ describe "Budget Investments" do
   let(:heading) { create(:budget_heading, group: group, name: "Health") }
   let(:user)    { create(:user, :level_two) }
 
-  before { login_managed_user(user) }
-
   it_behaves_like "mappable",
                   "budget_investment",
                   "investment",
@@ -22,6 +20,7 @@ describe "Budget Investments" do
     let(:investment) { create(:budget_investment, budget: budget) }
 
     scenario "finds investment using budget slug" do
+      login_managed_user(user)
       login_as_manager(manager)
       visit management_budget_investment_path("budget_slug", investment)
 
@@ -33,6 +32,7 @@ describe "Budget Investments" do
     before { heading.budget.update(phase: "accepting") }
 
     scenario "Creating budget investments on behalf of someone, selecting a budget" do
+      login_managed_user(user)
       login_as_manager(manager)
       click_link "Create budget investment"
       within "#budget_#{budget.id}" do
@@ -77,6 +77,8 @@ describe "Budget Investments" do
     end
 
     scenario "Shows suggestions to unverified managers" do
+      login_managed_user(user)
+
       expect(manager.user.level_two_or_three_verified?).to be false
 
       create(:budget_investment, budget: budget, title: "More parks")
@@ -98,6 +100,16 @@ describe "Budget Investments" do
         expect(page).not_to have_content "Plant trees"
       end
     end
+
+    scenario "when user has not been selected we can't create a budget investment" do
+      Setting["feature.user.skip_verification"] = "true"
+      login_as_manager(manager)
+
+      click_link "Create budget investment"
+
+      expect(page).to have_content "To perform this action you must select a user"
+      expect(page).to have_current_path management_document_verifications_path
+    end
   end
 
   context "Searching" do
@@ -105,6 +117,7 @@ describe "Budget Investments" do
       budget_investment1 = create(:budget_investment, budget: budget, title: "Show me what you got")
       budget_investment2 = create(:budget_investment, budget: budget, title: "Get Schwifty")
 
+      login_managed_user(user)
       login_as_manager(manager)
       click_link "Support budget investments"
       expect(page).to have_content(budget.name)
@@ -131,6 +144,7 @@ describe "Budget Investments" do
       budget_investment2 = create(:budget_investment, budget: budget, title: "Let's go",
                                                       heading: create(:budget_heading, name: "Area 52"))
 
+      login_managed_user(user)
       login_as_manager(manager)
       click_link "Support budget investments"
       expect(page).to have_content(budget.name)
@@ -156,6 +170,7 @@ describe "Budget Investments" do
     budget_investment1 = create(:budget_investment, budget: budget, title: "Show me what you got")
     budget_investment2 = create(:budget_investment, budget: budget, title: "Get Schwifty")
 
+    login_managed_user(user)
     login_as_manager(manager)
     click_link "Support budget investments"
     expect(page).to have_content(budget.name)
@@ -190,6 +205,7 @@ describe "Budget Investments" do
     reviewing_ballots_budget = create(:budget, :reviewing_ballots)
     finished = create(:budget, :finished)
 
+    login_managed_user(user)
     login_as_manager(manager)
     click_link "Create budget investment"
 
@@ -212,6 +228,7 @@ describe "Budget Investments" do
     reviewing_ballots_budget = create(:budget, :reviewing_ballots)
     finished = create(:budget, :finished)
 
+    login_managed_user(user)
     login_as(create(:administrator).user)
 
     visit management_sign_in_path
@@ -232,6 +249,7 @@ describe "Budget Investments" do
     scenario "Supporting budget investments on behalf of someone in index view" do
       budget_investment = create(:budget_investment, heading: heading)
 
+      login_managed_user(user)
       login_as_manager(manager)
       click_link "Support budget investments"
       expect(page).to have_content(budget.name)
@@ -252,6 +270,7 @@ describe "Budget Investments" do
     xscenario "Supporting budget investments on behalf of someone in show view" do
       budget_investment = create(:budget_investment, budget: budget)
 
+      login_managed_user(user)
       login_as_manager(manager)
       click_link "Support budget investments"
       expect(page).to have_content(budget.name)
@@ -277,6 +296,16 @@ describe "Budget Investments" do
 
       expect(page).to have_content "User is not verified"
     end
+
+    scenario "when user has not been selected we can't support budget investments" do
+      Setting["feature.user.skip_verification"] = "true"
+      login_as_manager(manager)
+
+      click_link "Support budget investments"
+
+      expect(page).to have_content "To perform this action you must select a user"
+      expect(page).to have_current_path management_document_verifications_path
+    end
   end
 
   context "Printing" do

spec/system/management/proposals_spec.rb

@@ -3,12 +3,9 @@ require "rails_helper"
 describe "Proposals" do
   let(:user) { create(:user, :level_two) }
 
-  before do
-    login_managed_user(user)
-  end
-
   context "Create" do
     scenario "Creating proposals on behalf of someone" do
+      login_managed_user(user)
       login_as_manager
       click_link "Create proposal"
 
@@ -47,6 +44,16 @@ describe "Proposals" do
 
       expect(page).to have_content "User is not verified"
     end
+
+    scenario "when user has not been selected we can't create a proposal" do
+      Setting["feature.user.skip_verification"] = "true"
+      login_as_manager
+
+      click_link "Create proposal"
+
+      expect(page).to have_content "To perform this action you must select a user"
+      expect(page).to have_current_path management_document_verifications_path
+    end
   end
 
   context "Show" do
@@ -54,6 +61,7 @@ describe "Proposals" do
       proposal = create(:proposal)
 
       right_path = management_proposal_path(proposal)
+      login_managed_user(user)
       login_as_manager
       visit right_path
 
@@ -66,6 +74,7 @@ describe "Proposals" do
       right_path = management_proposal_path(proposal)
       old_path = "#{management_proposals_path}/#{proposal.id}-something-else"
 
+      login_managed_user(user)
       login_as_manager
       visit old_path
 
@@ -76,6 +85,7 @@ describe "Proposals" do
     scenario "Successful proposal" do
       proposal = create(:proposal, :successful, title: "Success!")
 
+      login_managed_user(user)
       login_as_manager
       visit management_proposal_path(proposal)
 
@@ -87,6 +97,7 @@ describe "Proposals" do
     proposal1 = create(:proposal, title: "Show me what you got")
     proposal2 = create(:proposal, title: "Get Schwifty")
 
+    login_managed_user(user)
     login_as_manager
     click_link "Support proposals"
 
@@ -108,6 +119,7 @@ describe "Proposals" do
     proposal1 = create(:proposal, title: "Show me what you got")
     proposal2 = create(:proposal, title: "Get Schwifty")
 
+    login_managed_user(user)
     login_as_manager
     click_link "Support proposals"
 
@@ -133,6 +145,7 @@ describe "Proposals" do
     let!(:proposal) { create(:proposal) }
 
     scenario "Voting proposals on behalf of someone in index view" do
+      login_managed_user(user)
       login_as_manager
       click_link "Support proposals"
 
@@ -146,6 +159,7 @@ describe "Proposals" do
     end
 
     scenario "Voting proposals on behalf of someone in show view" do
+      login_managed_user(user)
       login_as_manager
       click_link "Support proposals"
 
@@ -167,6 +181,16 @@ describe "Proposals" do
 
       expect(page).to have_content "User is not verified"
     end
+
+    scenario "when user has not been selected we can't support proposals" do
+      Setting["feature.user.skip_verification"] = "true"
+      login_as_manager
+
+      click_link "Support proposals"
+
+      expect(page).to have_content "To perform this action you must select a user"
+      expect(page).to have_current_path management_document_verifications_path
+    end
   end
 
   context "Printing" do
@@ -210,5 +234,19 @@ describe "Proposals" do
         expect(best_proposal.title).to appear_before(worst_proposal.title)
       end
     end
+
+    scenario "when user has not been selected we can't support a proposal" do
+      create(:proposal)
+      Setting["feature.user.skip_verification"] = "true"
+      login_as_manager
+
+      click_link "Print proposals"
+      within ".proposals-list" do
+        click_link "Support"
+      end
+
+      expect(page).to have_content "To perform this action you must select a user"
+      expect(page).to have_current_path management_document_verifications_path
+    end
   end
 end