From ff9f334bb93d69f2f1c38d39fe95cf97a0afec57 Mon Sep 17 00:00:00 2001 From: potpotkettle <40988246+potpotkettle@users.noreply.github.com> Date: Tue, 11 Jul 2023 16:08:47 +0900 Subject: [PATCH] AO3-3997 Fix for email address validation #4390 (#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 --- app/models/story_parser.rb | 32 ++++++------ app/views/invitations/_invitation.html.erb | 3 +- config/locales/views/en.yml | 3 ++ features/other_a/invite_request.feature | 10 ++++ spec/models/story_parser_spec.rb | 58 +++++++++++++++++++--- 5 files changed, 84 insertions(+), 22 deletions(-) diff --git a/app/models/story_parser.rb b/app/models/story_parser.rb index 2206b2166f6..65364dc4725 100644 --- a/app/models/story_parser.rb +++ b/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 diff --git a/app/views/invitations/_invitation.html.erb b/app/views/invitations/_invitation.html.erb index 680dacf6a49..f2cefdfb5c9 100644 --- a/app/views/invitations/_invitation.html.erb +++ b/app/views/invitations/_invitation.html.erb @@ -11,7 +11,8 @@ <% else %>
<%= form_for(invitation) do |f| %> -

<%= f.text_field :invitee_email %>

+ <%= error_messages_for invitation %> +

<%= f.label :invitee_email, t(".email_address_label") %> <%= f.text_field :invitee_email %>

<%= hidden_field_tag :user_id, @user.try(:login) %>

<%= f.submit %>

<% end %> diff --git a/config/locales/views/en.yml b/config/locales/views/en.yml index 507c541b3d8..e0becc48b3e 100644 --- a/config/locales/views/en.yml +++ b/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}.' diff --git a/features/other_a/invite_request.feature b/features/other_a/invite_request.feature index d7d9fb4bada..d6f02ac12f1 100644 --- a/features/other_a/invite_request.feature +++ b/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 diff --git a/spec/models/story_parser_spec.rb b/spec/models/story_parser_spec.rb index ec23ee1ea2f..15ddfbe03e1 100644 --- a/spec/models/story_parser_spec.rb +++ b/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, "author@example.com") - }.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", "author@example.com") + res2 = @sp.parse_author("", "Author Name Second", "author@example.com") + res3 = @sp.parse_author("", "Author!! Name!!", "author@example.com") + 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("", "!!!!", "author@example.com") + expect(results.name).to eq("author@example.com") + expect(results.external_author.email).to eq("author@example.com") + end + + it "ignores invalid letters in the external author name" do + results = @sp.parse_author("", "Author!! Name!!", "author@example.com") + expect(results.name).to eq("Author Name") + expect(results.external_author.email).to eq("author@example.com") + 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