AO3-3997 Fix for email address validation otwcode#4390 (otwcode#4565)

* AO3-3997 Catch email validation failure in import

* AO3-3997 Add missing error message in invitation page

* AO3-3997 more fixes for invite

* AO3-3997 more fixes for import

* AO3-3997 more fixes for import (2)

---------

Co-authored-by: tickinginstant <[email protected]>

app/models/story_parser.rb

@@ -416,23 +416,25 @@ def parse_author_from_unknown(_location)
   end
 
   def parse_author_common(email, name)
-    if name.present? && email.present?
-      # convert to ASCII and strip out invalid characters (everything except alphanumeric characters, _, @ and -)
-      name = name.to_ascii.gsub(/[^\w[ \-@.]]/u, "")
+    errors = []
+
+    errors << "No author name specified" if name.blank?
+
+    if email.present?
       external_author = ExternalAuthor.find_or_create_by(email: email)
-      external_author_name = external_author.default_name
-      unless name.blank?
-        external_author_name = ExternalAuthorName.where(name: name, external_author_id: external_author.id).first ||
-                               ExternalAuthorName.new(name: name)
-        external_author.external_author_names << external_author_name
-        external_author.save
-      end
-      external_author_name
+      errors += external_author.errors.full_messages
+    else
+      errors << "No author email specified"
+    end
+
+    raise Error, errors.join("\n") if errors.present?
+
+    # convert to ASCII and strip out invalid characters (everything except alphanumeric characters, _, @ and -)
+    redacted_name = name.to_ascii.gsub(/[^\w[ \-@.]]/u, "")
+    if redacted_name.present?
+      external_author.names.find_or_create_by(name: redacted_name)
     else
-      messages = []
-      messages << "No author name specified" if name.blank?
-      messages << "No author email specified" if email.blank?
-      raise Error, messages.join("\n")
+      external_author.default_name
     end
   end
 

app/views/invitations/_invitation.html.erb

@@ -11,7 +11,8 @@
     <% else %>
     <dd>
       <%= form_for(invitation) do |f| %>
-        <p><label for="invitee_email">Enter an email address:</label> <%= f.text_field :invitee_email %></p>
+        <%= error_messages_for invitation %>
+        <p><%= f.label :invitee_email, t(".email_address_label") %> <%= f.text_field :invitee_email %></p>
         <p><%= hidden_field_tag :user_id, @user.try(:login) %></p>
         <p class="submit actions"><%= f.submit %></p>
       <% end %>

config/locales/views/en.yml

@@ -428,6 +428,9 @@ en:
   home:
     donate:
       page_title: Donate or Volunteer
+  invitations:
+    invitation:
+      email_address_label: Enter an email address
   invite_requests:
     invite_request:
       date: 'At our current rate, you should receive an invitation on or around: %{date}.'

features/other_a/invite_request.feature

@@ -62,6 +62,16 @@ Feature: Invite requests
       And I should not see "Sorry, you have no unsent invitations right now."
       And I should see "You have 2 open invitations and 0 that have been sent but not yet used."
 
+  Scenario: User can see an error after trying to invite an invalid email address
+
+    Given I am logged in as "user1"
+      And "user1" has "1" invitation
+      And I am on user1's manage invitations page
+    When I follow the link for "user1" first invite
+      And I fill in "Enter an email address" with "test@"
+      And I press "Update Invitation"
+    Then I should see "Invitee email should look like an email address"
+
   Scenario: User can send out invites they have been granted, and the recipient can sign up
 
     Given invitations are required

spec/models/story_parser_spec.rb

@@ -193,21 +193,67 @@ class StoryParser
     end
 
     it "raises an exception when the external author name is not provided" do
-      expect {
+      expect do
         @sp.parse_author("", nil, "[email protected]")
-      }.to raise_exception(StoryParser::Error) { |e| expect(e.message).to eq("No author name specified") }
+      end.to raise_exception(StoryParser::Error, "No author name specified")
     end
 
     it "raises an exception when the external author email is not provided" do
-      expect {
+      expect do
         @sp.parse_author("", "Author Name", nil)
-      }.to raise_exception(StoryParser::Error) { |e| expect(e.message).to eq("No author email specified") }
+      end.to raise_exception(StoryParser::Error, "No author email specified")
     end
 
     it "raises an exception when neither the external author name nor email is provided" do
-      expect {
+      expect do
         @sp.parse_author("", nil, nil)
-      }.to raise_exception(StoryParser::Error) { |e| expect(e.message).to eq("No author name specified\nNo author email specified") }
+      end.to raise_exception(StoryParser::Error, "No author name specified\nNo author email specified")
+    end
+
+    it "gives the same external author object for the same email" do
+      res1 = @sp.parse_author("", "Author Name", "[email protected]")
+      res2 = @sp.parse_author("", "Author Name Second", "[email protected]")
+      res3 = @sp.parse_author("", "Author!! Name!!", "[email protected]")
+      expect(res2.external_author.id).to eq(res1.external_author.id)
+      expect(res3.external_author.id).to eq(res1.external_author.id)
+      expect(res1.name).to eq("Author Name")
+      expect(res2.name).to eq("Author Name Second")
+    end
+
+    it "ignores the external author name when it is invalid" do
+      results = @sp.parse_author("", "!!!!", "[email protected]")
+      expect(results.name).to eq("[email protected]")
+      expect(results.external_author.email).to eq("[email protected]")
+    end
+
+    it "ignores invalid letters in the external author name" do
+      results = @sp.parse_author("", "Author!! Name!!", "[email protected]")
+      expect(results.name).to eq("Author Name")
+      expect(results.external_author.email).to eq("[email protected]")
+    end
+
+    it "raises an exception when the external author email is invalid" do
+      expect do
+        @sp.parse_author("", "Author Name", "not_email")
+      end.to raise_exception(StoryParser::Error, "Email should look like an email address.")
+    end
+
+    it "raises an exception when the external author name and email are invalid" do
+      expect do
+        @sp.parse_author("", "!!!!", "not_email")
+      end.to raise_exception(StoryParser::Error, "Email should look like an email address.")
+    end
+
+    it "raises an exception when the external author name is blank and email is invalid" do
+      expect do
+        @sp.parse_author("", "", "not_email")
+      end.to raise_exception(StoryParser::Error, "No author name specified\nEmail should look like an email address.")
+    end
+
+    it "raises an exception when the external author name is invalid and email is blank" do
+      expect do
+        @sp.parse_author("", "!!!!", "")
+      end.to raise_exception(StoryParser::Error, "No author email specified")
     end
   end