Skip to content

Commit

Permalink
Validate batch invitation users one-by-one
Browse files Browse the repository at this point in the history
Until now, one invalid email address in a batch upload (CSV) would raise
an ambiguous error and give the user no information about what went
wrong.

Now, if the batch encounters a validation problem, it returns to the
original form as before but also displays a more helpful error message.

This is a small improvement which avoids some of the complexities of
what might be even more helpful error reports (e.g. listing one or all
of the invalid email addresses).
  • Loading branch information
mike29736 committed Aug 7, 2023
1 parent 72aca21 commit 789b799
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 0 deletions.
15 changes: 15 additions & 0 deletions app/controllers/batch_invitations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ def create
end
batch_user = BatchInvitationUser.new(batch_user_args)

unless batch_user.valid?
flash[:alert] = batch_users_error_message(batch_user)
return render :new
end

@batch_invitation.batch_invitation_users << batch_user
end

Expand Down Expand Up @@ -91,4 +96,14 @@ def grant_default_permissions(batch_invitation)
batch_invitation.grant_permission(default_permission)
end
end

def batch_users_error_message(batch_user)
e = batch_user.errors.first

if e.attribute == :email
"One or more emails were invalid"
else
e.full_message
end
end
end
9 changes: 9 additions & 0 deletions test/controllers/batch_invitations_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,15 @@ def users_csv(filename = "users.csv")
end
end

context "the CSV contains one or more email addresses that aren't valid" do
should "redisplay the form and show a flash message" do
post :create, params: { batch_invitation: { user_names_and_emails: users_csv("users_with_non_valid_emails.csv") }, user: { supported_permission_ids: [] } }

assert_template :new
assert_match(/One or more emails were invalid/i, flash[:alert])
end
end

context "the CSV has all the fields, but not in the expected order" do
should "process the fields by name" do
post :create, params: { batch_invitation: { user_names_and_emails: users_csv("reversed_users.csv") }, user: { supported_permission_ids: [] } }
Expand Down
5 changes: 5 additions & 0 deletions test/controllers/fixtures/users_with_non_valid_emails.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Name,Email
Raphael Gupta,[email protected]
Bolormaa Śniegowski,@bolo
Flora Gao,[email protected]
Aureliusz Clemente,aureliusz@examplegovuk

0 comments on commit 789b799

Please sign in to comment.