internationalize default email, minor refactoring, add more UTs

app/controllers/challenge_assignments_controller.rb

@@ -219,11 +219,10 @@ def default_all
   def default
     @challenge_assignment.defaulted_at = Time.now
     @challenge_assignment.save
-    offer_byline = @challenge_assignment.offer_byline
-    request_byline = @challenge_assignment.request_byline
+
     assignments_page_url = collection_assignments_url(@challenge_assignment.collection)
 
-    @challenge_assignment.collection.notify_maintainers_challenge_default(offer_byline, request_byline, assignments_page_url)
+    @challenge_assignment.collection.notify_maintainers_challenge_default(@challenge_assignment, assignments_page_url)
 
     flash[:notice] = "We have notified the collection maintainers that you had to default on your assignment."
     redirect_to user_assignments_path(current_user)

app/models/challenge_assignment.rb

@@ -214,15 +214,15 @@ def offer_byline
     if offer_signup && offer_signup.pseud
       offer_signup.pseud.byline
     else
-      (pinch_hitter ? (pinch_hitter.byline + "* (pinch hitter)") : "- none -")
+      (pinch_hitter ? I18n.t("challenge_assignment.offer_byline.pinch_hitter", pinch_hitter_byline: pinch_hitter.byline) : I18n.t("challenge_assignment.offer_byline.none"))
     end
   end
 
   def request_byline
     if request_signup && request_signup.pseud
       request_signup.pseud.byline
     else
-      (pinch_request_signup ? (pinch_request_byline + "* (pinch recipient)") : "- None -")
+      (pinch_request_signup ? I18n.t("challenge_assignment.request_byline.pinch_recipient", pinch_request_byline: pinch_request_byline) : I18n.t("challenge_assignment.request_byline.none"))
     end
   end
 
@@ -387,10 +387,8 @@ def self.delayed_generate(collection_id)
     end
     REDIS_GENERAL.del(progress_key(collection))
 
-    if collection.email.present?
-      UserMailer.potential_match_generation_notification(collection.id, collection.email).deliver_later
-    elsif collection.parent && collection.parent.email.present?
-      UserMailer.potential_match_generation_notification(collection.id, collection.parent.email).deliver_later
+    if collection.collection_email.present?
+      UserMailer.potential_match_generation_notification(collection.id, collection.collection_email).deliver_later
     else
       collection.maintainers_list.each do |user|
         I18n.with_locale(user.preference.locale.iso) do

app/models/collection.rb

@@ -147,7 +147,7 @@ def title
                       too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.SUMMARY_MAX) }
 
   validates :header_image_url, format: { allow_blank: true, with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), message: ts("is not a valid URL.") }
-  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: ts("can only point to a gif, jpg, or png file."), multiline: true }
+  validates :header_image_url, format: { allow_blank: true, with: /\A\S+\.(png|gif|jpg)\z/, message: ts("can only point to a gif, jpg, or png file.") }
 
   validates :tags_after_saving,
             length: { maximum: ArchiveConfig.COLLECTION_TAGS_MAX,
@@ -337,13 +337,16 @@ def maintainers_list
     self.maintainers.collect(&:user).flatten.uniq
   end
 
+  def collection_email
+    return self.email if self.email.present?
+    return parent.email if parent && parent.email.present? 
+  end
+
   def notify_maintainers_assignments_sent
     subject = I18n.t("user_mailer.collection_notification.assignments_sent.subject")
     message = I18n.t("user_mailer.collection_notification.assignments_sent.complete")
-    if self.email.present?
-      UserMailer.collection_notification(self.id, subject, message, self.email).deliver_later
-    elsif self.parent && self.parent.email.present?
-      UserMailer.collection_notification(self.id, subject, message, self.parent.email).deliver_later
+    if self.collection_email.present?
+      UserMailer.collection_notification(self.id, subject, message, self.collection_email).deliver_later
     else
       # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
       self.maintainers_list.each do |user|
@@ -356,20 +359,17 @@ def notify_maintainers_assignments_sent
     end
   end
 
-  def notify_maintainers_challenge_default(offer_byline, request_byline, assignments_page_url)
-    subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: offer_byline)
-    message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: offer_byline, request_byline: request_byline, assignments_page_url: assignments_page_url)
-
-    if self.email.present?
-      UserMailer.collection_notification(self.id, subject, message, self.email).deliver_later
-    elsif self.parent && self.parent.email.present?
-      UserMailer.collection_notification(self.id, subject, message, self.parent.email).deliver_later
+  def notify_maintainers_challenge_default(challenge_assignment, assignments_page_url)
+    if self.collection_email.present?
+      subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: challenge_assignment.offer_byline)
+      message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: challenge_assignment.offer_byline, request_byline: challenge_assignment.request_byline, assignments_page_url: assignments_page_url)
+      UserMailer.collection_notification(self.id, subject, message, self.collection_email).deliver_later
     else
       # if collection email is not set and collection parent email is not set, loop through maintainers and send each a notice via email
       self.maintainers_list.each do |user|
         I18n.with_locale(user.preference.locale.iso) do
-          translated_subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: offer_byline)
-          translated_message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: offer_byline, request_byline: request_byline, assignments_page_url: assignments_page_url)
+          translated_subject = I18n.t("user_mailer.collection_notification.challenge_default.subject", offer_byline: challenge_assignment.offer_byline)
+          translated_message = I18n.t("user_mailer.collection_notification.challenge_default.complete", offer_byline: challenge_assignment.offer_byline, request_byline: challenge_assignment.request_byline, assignments_page_url: assignments_page_url)
           UserMailer.collection_notification(self.id, translated_subject, translated_message, user.email).deliver_later
         end
       end

app/models/potential_match.rb

@@ -90,10 +90,8 @@ def self.generate_in_background(collection_id)
     if invalid_signup_ids.present?
       invalid_signup_ids.each { |sid| REDIS_GENERAL.sadd invalid_signup_key(collection), sid }
 
-      if collection.email.present?
-        UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, collection.email).deliver_later
-      elsif collection.parent && collection.parent.email.present?
-        UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, collection.parent.email).deliver_later
+      if collection.collection_email.present?
+        UserMailer.invalid_signup_notification(collection.id, invalid_signup_ids, collection.collection_email).deliver_later
       else
         collection.maintainers_list.each do |user|
           I18n.with_locale(user.preference.locale.iso) do

config/locales/models/en.yml

@@ -235,6 +235,13 @@ en:
         other: Works
   attributes:
     ticket_number: Ticket ID
+  challenge_assignment:
+    offer_byline:
+      none: "- none -"
+      pinch_hitter: "%{pinch_hitter_byline}* (pinch hitter)"
+    request_byline:
+      none: "- None -"
+      pinch_recipient: "%{pinch_request_byline}* (pinch recipient)"
   errors:
     attributes:
       ticket_number:

features/gift_exchanges/notification_emails.feature

@@ -1,7 +1,7 @@
 Feature: Gift Exchange Notification Emails
   Make sure that gift exchange notification emails are formatted properly
 
-  Scenario: Assignment notification emails should be sent to two owners in their respective locales
+  Scenario: Assignment notification emails should be sent to two owners in their respective locales when assignments are generated
     Given I have created the tagless gift exchange "Holiday Swap"
       And I open signups for "Holiday Swap"
 
@@ -32,6 +32,46 @@ Feature: Gift Exchange Notification Emails
       And "participant1" should receive 1 email
       And "participant2" should receive 1 email
 
+  Scenario: If collection email is set, use the collection email instead of moderator emails
+    Given I have created the tagless gift exchange "Holiday Swap"
+      And I open signups for "Holiday Swap"
+      And I am logged in as "participant1"
+      And I start signing up for "Holiday Swap"
+      And I press "Submit"
+      And I am logged in as "participant2"
+      And I start signing up for "Holiday Swap"
+      And I press "Submit"
+      And I have added a co-moderator "mod2" to collection "Holiday Swap"
+      And I go to "Holiday Swap" collection's page
+      And I follow "Collection Settings"
+      And I fill in "Collection email" with "test@archiveofourown.org"
+      And I press "Update"
+      And I close signups for "Holiday Swap"
+      And I have generated matches for "Holiday Swap"
+      And I have sent assignments for "Holiday Swap"
+    Then 3 emails should be delivered
+      And 1 email should be delivered to test@archiveofourown.org
+      And the email should contain "You have received a message about your collection"
+
+  Scenario: Default notification emails should be sent to two owners in their respective locales when a user defaults on an assignment
+
+    Given everyone has their assignments for "Holiday Swap"
+      And I have added a co-moderator "mod2" to collection "Holiday Swap"
+      And a locale with translated emails
+      And the user "mod1" enables translated emails
+
+    When I am logged in as "myname1"
+      And I go to my assignments page
+      And I follow "Default"
+    Then I should see "We have notified the collection maintainers that you had to default on your assignment."
+      And 7 emails should be delivered
+      And "mod1" should receive 2 emails
+      And the last email to "mod1" should be translated
+      And the last email should contain "defaulted on their assignment"
+      And "mod2" should receive 1 email
+      And the email to "mod2" should be non-translated
+      And the email should contain "defaulted on their assignment"
+
   Scenario: Assignment notifications with linebreaks.
     Given I have created the tagless gift exchange "Holiday Swap"
       And I open signups for "Holiday Swap"

features/step_definitions/email_custom_steps.rb

@@ -27,6 +27,12 @@
   step(%{the email to "#{user}" should not contain "translation missing"}) # missing translations in the target language fall back to English
 end
 
+Then "the last email to {string} should be translated" do |user|
+  step(%{the last email to "#{user}" should contain "Translated footer"})
+  step(%{the last email to "#{user}" should not contain "fan-run and fan-supported archive"}) # untranslated English text
+  step(%{the last email to "#{user}" should not contain "translation missing"}) # missing translations in the target language fall back to English
+end
+
 Then "the email to {string} should be non-translated" do |user|
   step(%{the email to "#{user}" should not contain "Translated footer"})
   step(%{the email to "#{user}" should contain "fan-run and fan-supported archive"})
@@ -54,6 +60,17 @@
   end
 end
 
+Then "the last email to {string} should contain {string}" do |user, text|
+  @user = User.find_by(login: user)
+  email = emails("to: \"#{email_for(@user.email)}\"").last
+  if email.multipart?
+    expect(email.text_part.body).to match(text)
+    expect(email.html_part.body).to match(text)
+  else
+    expect(email.body).to match(text)
+  end
+end
+
 Then "the email to {string} should not contain {string}" do |user, text|
   @user = User.find_by(login: user)
   email = emails("to: \"#{email_for(@user.email)}\"").first
@@ -65,6 +82,17 @@
   end
 end
 
+Then "the last email to {string} should not contain {string}" do |user, text|
+  @user = User.find_by(login: user)
+  email = emails("to: \"#{email_for(@user.email)}\"").last
+  if email.multipart?
+    expect(email.text_part.body).not_to match(text)
+    expect(email.html_part.body).not_to match(text)
+  else
+    expect(email.body).not_to match(text)
+  end
+end
+
 Then "{string} should receive {int} email(s)" do |user, count|
   @user = User.find_by(login: user)
   expect(emails("to: \"#{email_for(@user.email)}\"").size).to eq(count.to_i)