From 4f57c62098c2c13526d3053a9f4486e7ed75c8a4 Mon Sep 17 00:00:00 2001 From: James Mead Date: Thu, 21 Sep 2023 16:25:51 +0100 Subject: [PATCH] Fix normalization of BatchInvitationUser#email Trello: https://trello.com/c/aO4nVCZT Having implemented strip_whitespace_from_organisation_slug I realised I'd probably missed a scenario for strip_whitespace_from_email. And sure enough there was no test coverage for the case when BatchInvitationUser#email is nil. In this commit I've improved the test coverage for email validation which for one thing explicitly forces the presence validation to be enabled for the email attribute, and for another, it forces me to use the safe navigation operator in strip_whitespace_from_email. This is because the latter is called from a before_validation callback and so email could be nil at this point. --- app/models/batch_invitation_user.rb | 2 +- test/models/batch_invitation_user_test.rb | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/models/batch_invitation_user.rb b/app/models/batch_invitation_user.rb index c25a00755..ad88f349e 100644 --- a/app/models/batch_invitation_user.rb +++ b/app/models/batch_invitation_user.rb @@ -103,7 +103,7 @@ def strip_whitespace_from_name end def strip_whitespace_from_email - email.strip! + email&.strip! end def strip_whitespace_from_organisation_slug diff --git a/test/models/batch_invitation_user_test.rb b/test/models/batch_invitation_user_test.rb index 36f9b3f3a..8f6604899 100644 --- a/test/models/batch_invitation_user_test.rb +++ b/test/models/batch_invitation_user_test.rb @@ -23,19 +23,25 @@ class BatchInvitationUserTest < ActiveSupport::TestCase 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