Fixes for batch invitations

app/controllers/batch_invitations_controller.rb

@@ -15,24 +15,24 @@ def create
     authorize @batch_invitation
 
     unless file_uploaded?
-      flash[:alert] = "You must upload a file"
+      flash.now[:alert] = "You must upload a file"
       render :new
       return
     end
 
     begin
       csv = CSV.parse(params[:batch_invitation][:user_names_and_emails].read, headers: true)
     rescue CSV::MalformedCSVError => e
-      flash[:alert] = "Couldn't understand that file: #{e.message}"
+      flash.now[:alert] = "Couldn't understand that file: #{e.message}"
       render :new
       return
     end
     if csv.empty?
-      flash[:alert] = "CSV had no rows."
+      flash.now[:alert] = "CSV had no rows."
       render :new
       return
     elsif %w[Name Email].any? { |required_header| csv.headers.exclude?(required_header) }
-      flash[:alert] = "CSV must have headers including 'Name' and 'Email'"
+      flash.now[:alert] = "CSV must have headers including 'Name' and 'Email'"
       render :new
       return
     end
@@ -47,7 +47,7 @@ def create
       batch_user = BatchInvitationUser.new(batch_user_args)
 
       unless batch_user.valid?
-        flash[:alert] = batch_users_error_message(batch_user)
+        flash.now[:alert] = batch_users_error_message(batch_user)
         return render :new
       end
 

app/models/batch_invitation_user.rb

@@ -7,6 +7,8 @@ class BatchInvitationUser < ApplicationRecord
   validates :outcome, inclusion: { in: [nil, "success", "failed", "skipped"] }
 
   before_save :strip_whitespace_from_name
+  before_validation :strip_whitespace_from_email
+  before_save :strip_whitespace_from_organisation_slug
 
   scope :processed, -> { where.not(outcome: nil) }
   scope :unprocessed, -> { where(outcome: nil) }
@@ -99,4 +101,12 @@ def sanitise_attributes_for_inviting_user_role(raw_attributes, inviting_user)
   def strip_whitespace_from_name
     name.strip!
   end
+
+  def strip_whitespace_from_email
+    email&.strip!
+  end
+
+  def strip_whitespace_from_organisation_slug
+    organisation_slug&.strip!
+  end
 end

test/controllers/batch_invitations_controller_test.rb

@@ -89,7 +89,7 @@ def users_csv(filename = "users.csv")
         post :create, params: { batch_invitation: { user_names_and_emails: nil } }
 
         assert_template :new
-        assert_match(/You must upload a file/i, flash[:alert])
+        assert_match(/You must upload a file/i, flash.now[:alert])
       end
     end
 
@@ -98,7 +98,7 @@ def users_csv(filename = "users.csv")
         post :create, params: { batch_invitation: { user_names_and_emails: users_csv("users_with_non_valid_emails.csv") } }
 
         assert_template :new
-        assert_match(/One or more emails were invalid/i, flash[:alert])
+        assert_match(/One or more emails were invalid/i, flash.now[:alert])
       end
     end
 
@@ -117,7 +117,7 @@ def users_csv(filename = "users.csv")
         post :create, params: { batch_invitation: { user_names_and_emails: users_csv("empty_users.csv") } }
 
         assert_template :new
-        assert_match(/no rows/i, flash[:alert])
+        assert_match(/no rows/i, flash.now[:alert])
       end
     end
 
@@ -126,7 +126,7 @@ def users_csv(filename = "users.csv")
         post :create, params: { batch_invitation: { user_names_and_emails: users_csv("invalid_users.csv") } }
 
         assert_template :new
-        assert_match(/Couldn't understand that file/i, flash[:alert])
+        assert_match(/Couldn't understand that file/i, flash.now[:alert])
       end
     end
 
@@ -135,7 +135,7 @@ def users_csv(filename = "users.csv")
         post :create, params: { batch_invitation: { user_names_and_emails: users_csv("no_headers_users.csv") } }
 
         assert_template :new
-        assert_match(/must have headers/i, flash[:alert])
+        assert_match(/must have headers/i, flash.now[:alert])
       end
     end
   end

test/integration/batch_inviting_users_test.rb

@@ -25,6 +25,28 @@ class BatchInvitingUsersTest < ActionDispatch::IntegrationTest
       assert_user_created_and_invited("lara@example.com", @application, organisation: @cabinet_office)
       assert_user_not_created("emma@example.com")
     end
+
+    should "only display flash alert once on validation error" do
+      visit root_path
+      signin_with(@user)
+
+      visit new_batch_invitation_path
+      click_button "Manage permissions for new users"
+
+      assert_response_contains "You must upload a file"
+
+      path = Rails.root.join("test/fixtures/users.csv")
+      attach_file("Upload a CSV file", path)
+      click_button "Manage permissions for new users"
+
+      batch_invitation = BatchInvitation.last
+      assert batch_invitation.present?
+
+      invited_user = batch_invitation.batch_invitation_users.last
+      assert_equal "fred@example.com", invited_user.email
+
+      refute_response_contains "You must upload a file"
+    end
   end
 
   context "for admin users" do

test/models/batch_invitation_user_test.rb

@@ -7,22 +7,41 @@ class BatchInvitationUserTest < ActiveSupport::TestCase
 
       assert_equal "Ailean Millard", user.name
     end
+
+    should "strip unwanted whitespace from email before validating" do
+      user = build(:batch_invitation_user, email: "  foo@example.com ")
+      user.valid?
+
+      assert_equal "foo@example.com", user.email
+    end
+
+    should "strip unwanted whitespace from organisation_slug before persisting" do
+      user = create(:batch_invitation_user, organisation_slug: "  cabinet-office ")
+
+      assert_equal "cabinet-office", user.organisation_slug
+    end
   end
 
   context "validations" do
-    should "validate email address" do
+    should "validate presence of email address" do
+      user = build(:batch_invitation_user, email: nil)
+
+      assert_not user.valid?
+      assert_includes user.errors[:email], "can't be blank"
+    end
+
+    should "validate format of email address" do
       user = build(:batch_invitation_user, email: "@gov.uk")
 
       assert_not user.valid?
-      assert_equal ["is invalid"], user.errors[:email]
+      assert_includes user.errors[:email], "is invalid"
     end
 
     should "prevent user being created with a known non-government email address" do
       user = build(:batch_invitation_user, email: "piers.quinn@yahoo.co.uk")
 
       assert_not user.valid?
-      assert_equal ["not accepted. Please enter a workplace email to continue."],
-                   user.errors[:email]
+      assert_includes user.errors[:email], "not accepted. Please enter a workplace email to continue."
     end
 
     should "not allow user to be updated with a known non-government email address" do