AO3-6848 Create challenge_assignment_notification mailer preview (#4971)

* AO3-6848 Create challenge_assignment_notification mailer preview * AO3-6848 Use sent_at from assignment * AO3-6848 Update mailer templates to current i18n standard * AO3-6848 Remove unused locale keys * AO3-6848 Reviewdog * AO3-6848 Fix test * AO3-6848 Add test for sending translated email * AO3-6848 Use translatable word connector for warnings etc * AO3-6848 Fix interpolation variable name * AO3-6848 Reformat ifs in erb
otwcode · Jan 22, 2025 · f545651 · f545651
1 parent c5aa49b
commit f545651
Show file tree

Hide file tree

Showing 10 changed files with 187 additions and 76 deletions.
diff --git a/app/helpers/tags_helper.rb b/app/helpers/tags_helper.rb
@@ -240,7 +240,9 @@ def show_hidden_tag_link_list_item(item, category, options = {})
 
   def get_title_string(tags, category_name = "")
     if tags && tags.size > 0
-      tags.collect(&:name).join(", ")
+      # We don't use .to_sentence because these aren't links and we risk making any
+      # connector word (e.g., "and") look like part of the final tag.
+      tags.pluck(:name).join(t("support.array.words_connector"))
     elsif tags.blank? && category_name.blank?
      "Choose Not To Use Archive Warnings"
     else

diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
@@ -197,14 +197,12 @@ def potential_match_generation_notification(collection_id, email)
   def challenge_assignment_notification(collection_id, assigned_user_id, assignment_id)
     @collection = Collection.find(collection_id)
     @assigned_user = User.find(assigned_user_id)
-    assignment = ChallengeAssignment.find(assignment_id)
-    @request = (assignment.request_signup || assignment.pinch_request_signup)
-    I18n.with_locale(@assigned_user.preference.locale.iso) do
-      mail(
-        to: @assigned_user.email,
-        subject: default_i18n_subject(app_name: ArchiveConfig.APP_SHORT_NAME, collection_title: @collection.title)
-      )
-    end
+    @assignment = ChallengeAssignment.find(assignment_id)
+    @request = (@assignment.request_signup || @assignment.pinch_request_signup)
+    mail(
+      to: @assigned_user.email,
+      subject: default_i18n_subject(app_name: ArchiveConfig.APP_SHORT_NAME, collection_title: @collection.title)
+    )
   end
 
   # Asks a user to validate and activate their new account

diff --git a/app/models/challenge_assignment.rb b/app/models/challenge_assignment.rb
@@ -271,7 +271,11 @@ def send_out
                       (self.pinch_hitter ? self.pinch_hitter.user : nil)
                     end
       request = self.request_signup || self.pinch_request_signup
-      UserMailer.challenge_assignment_notification(collection.id, assigned_to.id, self.id).deliver_later if assigned_to && request
+      if assigned_to && request
+        I18n.with_locale(assigned_to.preference.locale.iso) do
+          UserMailer.challenge_assignment_notification(collection.id, assigned_to.id, self.id).deliver_later
+        end
+      end
     end
   end
 

diff --git a/app/views/user_mailer/challenge_assignment_notification.html.erb b/app/views/user_mailer/challenge_assignment_notification.html.erb
@@ -12,7 +12,7 @@
 
     <% def styled_tag_list(tags) %>
       <% return nil if !tags || tags.empty? %>
-      <% tags.map { |tag| style_link(tag.name, tag_works_url(tag)) }.to_sentence.html_safe %>
+      <% to_sentence(tags.map { |tag| style_link(tag.name, tag_works_url(tag)) }) %>
     <% end %>
 
     <% fandoms = prompt.any_fandom ? t(".any") : styled_tag_list(tag_groups["Fandom"]) %>
@@ -28,15 +28,51 @@
     <%= index + 1 %>. <%= style_bold(prompt.title) %>
 
     <p>
-      <% if fandoms %><%= style_bold(Fandom.human_attribute_name("name_with_colon", count: prompt.any_fandom ? 1 : tag_groups["Fandom"].count)) %> <%= fandoms %><br><% end %>
-      <% if chars %><%= style_bold(Character.human_attribute_name("name_with_colon", count: prompt.any_character ? 1 : tag_groups["Character"].count)) %> <%= chars %><br><% end %>
-      <% if ships %><%= style_bold(Relationship.human_attribute_name("name_with_colon", count: prompt.any_relationship ? 1 : tag_groups["Relationship"].count)) %> <%= ships %><br><% end %>
-      <% if ratings %><%= style_bold(Rating.human_attribute_name("name_with_colon")) %> <%= ratings %><br><% end %>
-      <% if warnings %><%= style_bold(ArchiveWarning.human_attribute_name("name_with_colon", count: prompt.any_archive_warning ? 1 : tag_groups["ArchiveWarning"].count)) %> <%= warnings %><br><% end %>
-      <% if categories %><%= style_bold(Category.human_attribute_name("name_with_colon", count: prompt.any_category ? 1 : tag_groups["Category"].count)) %> <%= categories %><br><% end %>
-      <% if atags %><%= style_bold(Freeform.human_attribute_name("name_with_colon", count: prompt.any_freeform ? 1 : tag_groups["Freeform"].count)) %> <%= atags %><br><% end %>
-      <% if otags %><%= style_bold("#{t(".optional_tags")}") %> <%= otags %><br><% end %>
-      <% if prompt.url && !prompt.url.blank? %><%= style_bold("#{t(".prompt_url")}") %> <%= style_link(prompt.url, prompt.url) %><br><% end %>
+      <% if fandoms %>
+        <%= style_bold(t("activerecord.models.fandom", count: prompt.any_fandom ? 1 : tag_groups["Fandom"].count) + t("mailer.general.metadata_label_indicator")) %>
+        <%= fandoms %>
+        <br />
+      <% end %>
+      <% if chars %>
+        <%= style_bold(t("activerecord.models.character", count: prompt.any_character ? 1 : tag_groups["Character"].count) + t("mailer.general.metadata_label_indicator")) %>
+        <%= chars %>
+        <br />
+      <% end %>
+      <% if ships %>
+        <%= style_bold(t("activerecord.models.relationship", count: prompt.any_relationship ? 1 : tag_groups["Relationship"].count) + t("mailer.general.metadata_label_indicator")) %>
+        <%= ships %>
+        <br />
+      <% end %>
+      <% if ratings %>
+        <%= style_bold(t("activerecord.models.rating", count: 1) + t("mailer.general.metadata_label_indicator")) %>
+        <%= ratings %>
+        <br />
+      <% end %>
+      <% if warnings %>
+        <%= style_bold(t("activerecord.models.archive_warning", count: prompt.any_archive_warning ? 1 : tag_groups["ArchiveWarning"].count) + t("mailer.general.metadata_label_indicator")) %>
+        <%= warnings %>
+        <br />
+      <% end %>
+      <% if categories %>
+        <%= style_bold(t("activerecord.models.category", count: prompt.any_category ? 1 : tag_groups["Category"].count) + t("mailer.general.metadata_label_indicator")) %>
+        <%= categories %>
+        <br />
+      <% end %>
+      <% if atags %>
+        <%= style_bold(t("activerecord.models.freeform", count: prompt.any_freeform ? 1 : tag_groups["Freeform"].count) + t("mailer.general.metadata_label_indicator")) %>
+        <%= atags %>
+        <br />
+      <% end %>
+      <% if otags %>
+        <%= style_bold(t(".optional_tags")) %>
+        <%= otags %>
+        <br />
+      <% end %>
+      <% if prompt.url && !prompt.url.blank? %>
+        <%= style_bold(t(".prompt_url")) %>
+        <%= style_link(prompt.url, prompt.url) %>
+        <br />
+      <% end %>
       <% if prompt.description && !prompt.description.blank? %>
         <%= style_bold(t(".description")) %>
         <%= style_quote(prompt.description) %>
@@ -50,17 +86,17 @@
     <%= style_bold(t(".due")) %> <%= time_in_zone(@collection.challenge.assignments_due_at, (@collection.challenge.time_zone || Time.zone.name), @assigned_user) %>.
   </p>
 
-  <p><%= t(".html.look_up", link: style_link(t(".html.look_up_link"), user_assignments_url(@assigned_user))).html_safe %></p>
+  <p><%= t(".look_up.html", your_assignments_link: style_link(t(".look_up.your_assignments"), user_assignments_url(@assigned_user))) %></p>
 
   <% if @collection && !@collection.assignment_notification.blank? %>
     <p><%= escape_html_and_create_linebreaks(@collection.assignment_notification) %></p>
   <% end %>
 <% end %>
 
 <% content_for :footer_note do %>
-  <%= t(".html.footer", title: style_footer_link(@collection.title, collection_url(@collection)), footer_link: style_footer_link(t(".html.footer_link"), collection_profile_url(@collection))).html_safe %>
+  <%= t(".footer.html", title: style_footer_link(@collection.title, collection_url(@collection)), challenge_profile_link: style_footer_link(t(".footer.challenge_profile"), collection_profile_url(@collection))) %>
 <% end %>
 
 <% content_for :sent_at do %>
-  <%= l(Time.now) %>
+  <%= l(@assignment.sent_at) %>
 <% end %>
diff --git a/app/views/user_mailer/challenge_assignment_notification.text.erb b/app/views/user_mailer/challenge_assignment_notification.text.erb
@@ -1,5 +1,5 @@
 <% content_for :message do %>
-<%= t ".text.assignment", collection_title: @collection.title, collection_url: collection_url(@collection) %>
+<%= t ".assignment.text", collection_title: @collection.title, collection_url: collection_url(@collection) %>
 
 <%= t ".recipient" %> <%= @request.nil? ? t(".recipient_missing") : text_pseud(@request.pseud) %>
 
@@ -22,26 +22,50 @@
 
 <%= index + 1 %>. <%= prompt.title %>
 
-<% if fandoms %><%= Fandom.human_attribute_name("name_with_colon", count: prompt.any_fandom ? 1 : tag_groups["Fandom"].count) %> <%= fandoms %><% end %><% if chars %>
-<%= Character.human_attribute_name("name_with_colon", count: prompt.any_character ? 1 : tag_groups["Character"].count) %> <%= chars %><% end %><% if ships %>
-<%= Relationship.human_attribute_name("name_with_colon", count: prompt.any_relationship ? 1 : tag_groups["Relationship"].count) %> <%= ships %><% end %><% if ratings %>
-<%= Rating.human_attribute_name("name_with_colon") %> <%= ratings %><% end %><% if warnings %>
-<%= ArchiveWarning.human_attribute_name("name_with_colon", count: prompt.any_archive_warning ? 1 : tag_groups["ArchiveWarning"].count) %> <%= warnings %><% end %><% if categories %>
-<%= Category.human_attribute_name("name_with_colon", count: prompt.any_category ? 1 : tag_groups["Category"].count) %> <%= categories %><% end %><% if atags %>
-<%= Freeform.human_attribute_name("name_with_colon", count: prompt.any_freeform ? 1 : tag_groups["Freeform"].count) %> <%= atags %><% end %><% if otags %>
-<%= t ".optional_tags" %> <%= otags %><% end %><% if prompt.url && !prompt.url.blank? %>
-<%= t ".prompt_url" %> <%= prompt.url %><% end %><% if prompt.description && !prompt.description.blank? %>
-<%= t ".description" %>
-    <%= to_plain_text(prompt.description) %><% end %>
+<% if fandoms %>
+<%= t("activerecord.models.fandom", count: prompt.any_fandom ? 1 : tag_groups["Fandom"].count) + t("mailer.general.metadata_label_indicator") %><%= fandoms %>
+<% end %>
+<% if chars %>
+<%= t("activerecord.models.character", count: prompt.any_character ? 1 : tag_groups["Character"].count) + t("mailer.general.metadata_label_indicator") %><%= chars %>
+<% end %>
+<% if ships %>
+<%= t("activerecord.models.relationship", count: prompt.any_relationship ? 1 : tag_groups["Relationship"].count) + t("mailer.general.metadata_label_indicator") %><%= ships %>
+<% end %>
+<% if ratings %>
+<%= t("activerecord.models.rating", count: 1) + t("mailer.general.metadata_label_indicator") %><%= ratings %>
+<% end %>
+<% if warnings %>
+<%= t("activerecord.models.archive_warning", count: prompt.any_archive_warning ? 1 : tag_groups["ArchiveWarning"].count) + t("mailer.general.metadata_label_indicator") %><%= warnings %>
+<% end %>
+<% if categories %>
+<%= t("activerecord.models.category", count: prompt.any_category ? 1 : tag_groups["Category"].count) + t("mailer.general.metadata_label_indicator") %><%= categories %>
+<% end %>
+<% if atags %>
+<%= t("activerecord.models.freeform", count: prompt.any_freeform ? 1 : tag_groups["Freeform"].count) + t("mailer.general.metadata_label_indicator") %><%= atags %>
+<% end %>
+<% if otags %>
+<%= t(".optional_tags") %> <%= otags %>
+<% end %>
+<% if prompt.url && !prompt.url.blank? %>
+<%= t(".prompt_url") %> <%= prompt.url %>
+<% end %>
+<% if prompt.description && !prompt.description.blank? %>
+<%= t(".description") %>
+    <%= to_plain_text(prompt.description) %>
+<% end %>
 
 <% end %><%= text_divider %>
 
-<%= t '.due' %> <%= to_plain_text(time_in_zone(@collection.challenge.assignments_due_at, (@collection.challenge.time_zone || Time.zone.name), @assigned_user)).gsub(/\n\s*/, "") %>.
+<%= t(".due") %> <%= to_plain_text(time_in_zone(@collection.challenge.assignments_due_at, (@collection.challenge.time_zone || Time.zone.name), @assigned_user)).gsub(/\n\s*/, "") %>.
 
-<%= t ".text.look_up", link: user_assignments_url(@assigned_user) %>
+<%= t(".look_up.text", your_assignments_url: user_assignments_url(@assigned_user)) %>
 <% if @collection && !@collection.assignment_notification.blank? %>
 
 
 <%= @collection.assignment_notification %><% end %><% end %>
-<% content_for :footer_note do %><%= t ".text.footer", title: @collection.title, url: collection_url(@collection), profile_url: collection_profile_url(@collection) %><% end %>
-<% content_for :sent_at do %><%= l(Time.now) %><% end %>
+<% content_for :footer_note do %>
+<%= t(".footer.text", title: @collection.title, url: collection_url(@collection), challenge_profile_url: collection_profile_url(@collection)) -%>
+<% end %>
+<% content_for :sent_at do %>
+<%= l(@assignment.sent_at) -%>
+<% end %>
diff --git a/config/locales/mailers/en.yml b/config/locales/mailers/en.yml
@@ -181,23 +181,23 @@ en:
       any: Any
       assignment:
         html: You have been assigned the following request in the %{link} challenge at the Archive of Our Own!
+        text: You have been assigned the following request in the "%{collection_title}" challenge (%{collection_url}) at the Archive of Our Own!
       description: 'Description:'
       due: 'This assignment is due at:'
-      html:
-        footer: You're receiving this email because you signed up for the %{title} challenge. For more information about this challenge and contact information for the moderators, please visit %{footer_link}.
-        footer_link: the challenge profile page
-        look_up: You can look up this assignment from %{link}.
-        look_up_link: your Assignments page
+      footer:
+        challenge_profile: the challenge profile page
+        html: You're receiving this email because you signed up for the %{title} challenge. For more information about this challenge and contact information for the moderators, please visit %{challenge_profile_link}.
+        text: You're receiving this email because you signed up for the %{title} challenge (%{url}). For more information about this challenge and contact information for the moderators, please visit %{challenge_profile_url}.
+      look_up:
+        html: You can look up this assignment from %{your_assignments_link}.
+        text: You can look up this assignment from your Assignments page at %{your_assignments_url}.
+        your_assignments: your Assignments page
       optional_tags: 'Optional Tags:'
       prompt_url: 'Prompt URL:'
       prompts: 'Prompts:'
       recipient: 'Recipient:'
       recipient_missing: 'None: contact a moderator for help!'
       subject: "[%{app_name}][%{collection_title}] Your assignment!"
-      text:
-        assignment: You have been assigned the following request in the "%{collection_title}" challenge (%{collection_url}) at the Archive of Our Own!
-        footer: You're receiving this email because you signed up for the %{title} challenge (%{url}). For more information about this challenge and contact information for the moderators, please visit %{profile_url}.
-        look_up: You can look up this assignment from your Assignments page at %{link}.
     change_email:
       changed:
         html: "%{login}, the email associated with your account has been changed to %{email}"

diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml
@@ -16,35 +16,15 @@ en:
         support: Support
         tag_wrangling: Tag Wrangling
         translation: Translation
-      archive_warning:
-        name_with_colon:
-          one: 'Warning:'
-          other: 'Warnings:'
-      category:
-        name_with_colon:
-          one: 'Category:'
-          other: 'Categories:'
       chapters/creatorships:
         base: 'Invalid creator:'
         pseud_id: Pseud
-      character:
-        name_with_colon:
-          one: 'Character:'
-          other: 'Characters:'
       creatorships:
         base: 'Invalid creator:'
         pseud_id: Pseud
       external_work:
         author: Creator
         user_defined_tags_count: Fandom, relationship, and character tags
-      fandom:
-        name_with_colon:
-          one: 'Fandom:'
-          other: 'Fandoms:'
-      freeform:
-        name_with_colon:
-          one: 'Additional Tag:'
-          other: 'Additional Tags:'
       gift_exchange:
         offers_num_allowed: Number of offers allowed per sign-up
         offers_num_required: Number of offers required per sign-up
@@ -55,12 +35,6 @@ en:
         meta_tag_id: Metatag
         sub_tag: Subtag
         sub_tag_id: Subtag
-      rating:
-        name_with_colon: 'Rating:'
-      relationship:
-        name_with_colon:
-          one: 'Relationship:'
-          other: 'Relationships:'
       role:
         archivist: Archivist
         no_resets: No Resets

diff --git a/features/gift_exchanges/notification_emails.feature b/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 when assignments are generated
+  Scenario: Assignment sent 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"
 
@@ -148,3 +148,17 @@ Feature: Gift Exchange Notification Emails
 
     Then "participant1" should receive 1 email
       And the notification message to "participant1" should contain the no archive warnings tag
+
+  Scenario: Assignment notifications should be sent to participants in their respective locales
+    Given the gift exchange "Holiday Swap" is ready for matching
+      And a locale with translated emails
+      And the user "myname1" enables translated emails
+    When I close signups for "Holiday Swap"
+      And I have generated matches for "Holiday Swap"
+      And I have sent assignments for "Holiday Swap"
+    Then "myname1" should receive 1 email
+      And the email should have "Your assignment!" in the subject
+      And the email to "myname1" should be translated
+    And "myname2" should receive 1 email
+      And the email should have "Your assignment!" in the subject
+      And the email to "myname2" should be non-translated
diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb
@@ -377,7 +377,7 @@
     let!(:collection) { create(:collection, challenge: gift_exchange, challenge_type: "GiftExchange") }
     let!(:otheruser) { create(:user) }
     let!(:offer) { create(:challenge_signup, collection: collection, pseud: otheruser.default_pseud) }
-    let!(:open_assignment) { create(:challenge_assignment, collection: collection, offer_signup: offer) }
+    let!(:open_assignment) { create(:challenge_assignment, collection: collection, offer_signup: offer, sent_at: Time.current) }
 
     # Test the headers
     it_behaves_like "an email with a valid sender"