From 2781e4fee1affd0e1df14487c741919d7977b299 Mon Sep 17 00:00:00 2001 From: sarken Date: Wed, 1 Sep 2021 05:37:40 -0400 Subject: [PATCH 01/12] Initial pass at putting more work info into recipient email --- app/helpers/mailer_helper.rb | 50 +++++ app/views/user_mailer/_work_info.html.erb | 24 +++ app/views/user_mailer/_work_info.text.erb | 14 ++ .../recipient_notification.html.erb | 18 +- .../recipient_notification.text.erb | 7 +- config/locales/mailers/en.yml | 1 + config/locales/models/en.yml | 12 +- spec/helpers/mailer_helper_spec.rb | 182 ++++++++++++++++++ spec/mailers/user_mailer_spec.rb | 8 +- 9 files changed, 294 insertions(+), 22 deletions(-) create mode 100644 app/views/user_mailer/_work_info.html.erb create mode 100644 app/views/user_mailer/_work_info.text.erb diff --git a/app/helpers/mailer_helper.rb b/app/helpers/mailer_helper.rb index 8917a84ed6..3dfcd49b1c 100644 --- a/app/helpers/mailer_helper.rb +++ b/app/helpers/mailer_helper.rb @@ -127,4 +127,54 @@ def creator_text(work) work.pseuds.map { |p| text_pseud(p) }.to_sentence.html_safe end end + + def work_metadata_label(text) + text.html_safe + t("mailer.general.metadata_label_indicator") + end + + def work_tag_metadata_label(tags) + return if tags.empty? + type = tags.first.type + # Rating has no pluralization. + if type == "Rating" + t("activerecord.models.rating") + t("mailer.general.metadata_label_indicator") + else + t("activerecord.models.#{type.underscore}", count: tags.count) + t("mailer.general.metadata_label_indicator") + end + end + + # 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. + def work_tag_metadata_list(tags) + return if tags.empty? + tags.pluck(:name).join(t("support.array.words_connector")) + end + + # Spacing is dealt with in locale files, e.g. " : " for French. + def work_tag_metadata(tags) + return if tags.empty? + "#{work_tag_metadata_label(tags)}#{work_tag_metadata_list(tags)}" + end + + def style_work_metadata_label(text) + style_bold(work_metadata_label(text)) + end + + def style_work_tag_metadata_list(tags) + return if tags.empty? + type = tags.first.type + # Fandom tags are linked and to_sentence'd. + if type == "Fandom" + tags.map { |f| style_link(f.name, fandom_url(f)) }.to_sentence.html_safe + else + work_tag_metadata_list(tags) + end + end + + # Spacing is dealt with in locale files, e.g. " : " for French. + def style_work_tag_metadata(tags) + return if tags.empty? + label = style_bold(work_tag_metadata_label(tags)) + "#{label}#{style_work_tag_metadata_list(tags)}".html_safe + end end # end of MailerHelper diff --git a/app/views/user_mailer/_work_info.html.erb b/app/views/user_mailer/_work_info.html.erb new file mode 100644 index 0000000000..877faa3421 --- /dev/null +++ b/app/views/user_mailer/_work_info.html.erb @@ -0,0 +1,24 @@ +<% # expects work %> +

+ <%= style_work_metadata_label(Work.human_attribute_name("chapter_total_display")) %><%= work.chapter_total_display %> +
<%= style_work_tag_metadata(work.fandoms) %> +
<%= style_work_tag_metadata(work.ratings) %> +
<%= style_work_tag_metadata(work.archive_warnings) %> + <% unless work.relationship_string.blank? %> +
<%= style_work_tag_metadata(work.relationships) %> + <% end %> + <% unless work.character_string.blank? %> +
<%= style_work_tag_metadata(work.characters) %> + <% end %> + <% unless work.freeform_string.blank? %> +
<%= style_work_tag_metadata(work.freeforms) %> + <% end %> + <% unless work.series.count.zero? %> +
<%= style_work_metadata_label(Series.model_name.human(count: work.series.count)) %><%= series_list_for_feeds(work).html_safe %> + <% end %> +

+ +<% unless work.summary.blank? %> +

<%= style_work_metadata_label(Work.human_attribute_name("summary")) %>

+ <%= style_quote(raw sanitize_field(work, :summary)) %> +<% end %> diff --git a/app/views/user_mailer/_work_info.text.erb b/app/views/user_mailer/_work_info.text.erb new file mode 100644 index 0000000000..fc25f5546d --- /dev/null +++ b/app/views/user_mailer/_work_info.text.erb @@ -0,0 +1,14 @@ +<% # expects work %> +<%= work_metadata_label(Work.human_attribute_name("chapter_total_display"))%><%= work.chapter_total_display %> +<%= work_tag_metadata(work.fandoms) %> +<%= work_tag_metadata(work.ratings) %> +<%= work_tag_metadata(work.archive_warnings) %> +<% unless work.relationship_string.blank? %><%= work_tag_metadata(work.relationships) %><% end %> +<% unless work.character_string.blank? %><%= work_tag_metadata(work.characters) %><% end %> +<% unless work.freeform_string.blank? %><%= work_tag_metadata(work.freeforms) %><% end %> +<% unless work.series.count.zero? %><%= Series.model_name.human(count: work.series.count) %><%= raw to_plain_text(series_list_for_feeds(work)) %><% end %> + +<% unless work.summary.blank? %> +<%= work_metadata_label(Work.human_attribute_name("summary")) %> + <%= raw to_plain_text(sanitize_field(work, :summary)) %> +<% end %> diff --git a/app/views/user_mailer/recipient_notification.html.erb b/app/views/user_mailer/recipient_notification.html.erb index dd85d467e6..bd4cecf817 100644 --- a/app/views/user_mailer/recipient_notification.html.erb +++ b/app/views/user_mailer/recipient_notification.html.erb @@ -7,23 +7,13 @@

- <% if @collection.nil? %> - <%= style_creation_link(@work.title, work_url(@work)) %> <%= "(#{@work.word_count} words)" %> - <% else %> - <%= style_creation_link(@work.title, collection_work_url(@collection, @work)) %> <%= "(#{@work.word_count} words)" %> - <% end %> + <% url = @collection ? collection_work_url(@collection, @work) : work_url(@work) %> + <%= style_creation_link(@work.title, url) %> <%= "(#{@work.word_count} words)" %>
- by <%= @work.anonymous? ? "an anonymous responder" : (@work.pseuds.map{|p| style_pseud_link(p)}.to_sentence.html_safe) %> -
- <% unless @work.fandom_string.blank? %> - <%= style_bold("Fandom:") %> <%= @work.fandom_string %> - <% end %> + by <%= creator_links(@work) %>

- <% unless @work.summary.blank? %> - <%= style_bold("Summary:") %> - <%= style_quote(raw sanitize_field(@work, :summary)) %> - <% end %> + <%= render "work_info", work: @work %> <% if @collection && !@collection.gift_notification.blank? %>

<%= @collection.gift_notification %>

diff --git a/app/views/user_mailer/recipient_notification.text.erb b/app/views/user_mailer/recipient_notification.text.erb index a4b3bc7c3f..3a92e0ee85 100644 --- a/app/views/user_mailer/recipient_notification.text.erb +++ b/app/views/user_mailer/recipient_notification.text.erb @@ -6,11 +6,8 @@ A gift work has been posted for you<% if @collection %> in the "<%= @collection. "<%= @work.title.html_safe %>" (<%= @work.word_count %> words) <%= @collection ? collection_work_url(@collection, @work) : work_url(@work) %> -by <%= @work.anonymous? ? "an anonymous responder" : (@work.pseuds.map{|p| text_pseud(p)}.to_sentence) %> +by <%= creator_text(@work) %> -<% unless @work.fandom_string.blank? %>Fandom: <%= @work.fandom_string %><% end %> - -<% unless @work.summary.blank? %>Summary: -<%= to_plain_text(raw sanitize_field(@work, :summary)) %><% end %> +<%= render "work_info", work: @work %> <% if @collection && !@collection.gift_notification.blank? %><%= @collection.gift_notification %><% end %><% end %> diff --git a/config/locales/mailers/en.yml b/config/locales/mailers/en.yml index 907700b651..8f554b205c 100644 --- a/config/locales/mailers/en.yml +++ b/config/locales/mailers/en.yml @@ -10,6 +10,7 @@ en: addressed: "Hi, %{name}!" unaddressed: "Hi!" introductory: "Hello from the Archive of Our Own!" + metadata_label_indicator: ": " signature: app_short_name: AO3 open_doors: The Open Doors team diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml index 11649342f1..2f26d85079 100644 --- a/config/locales/models/en.yml +++ b/config/locales/models/en.yml @@ -23,12 +23,14 @@ en: models: archive_warning: one: "Warning" - other: "Warnings" + other: "Warnings" bookmark: "Bookmark" category: one: "Category" other: "Categories" - chapter: "Chapter" + chapter: + one: "Chapter" + other: "Chapters" character: one: "Character" other: "Characters" @@ -44,6 +46,9 @@ en: relationship: one: "Relationship" other: "Relationships" + series: + one: "Series" + other: "Series" tag: one: "Tag" other: "Tags" @@ -108,4 +113,7 @@ en: external_work: user_defined_tags_count: "Fandom, relationship, and character tags" work: + chapter_total_display: "Chapters" + summary: "Summary" user_defined_tags_count: "Fandom, relationship, character, and additional tags" + diff --git a/spec/helpers/mailer_helper_spec.rb b/spec/helpers/mailer_helper_spec.rb index 9bbe2e0dc8..bc7dc80c61 100644 --- a/spec/helpers/mailer_helper_spec.rb +++ b/spec/helpers/mailer_helper_spec.rb @@ -7,4 +7,186 @@ expect(style_creation_link(work.title, work_url(work))).to eq("#{work.title}") end end + + describe "#work_tag_metadata" do + { + "Fandom" => ["Fandom: ", "Fandoms: "], + "Rating" => ["Rating: ", "Rating: "], + "ArchiveWarning" => ["Warning: ", "Warnings: "], + "Relationship" => ["Relationship: ", "Relationships: "], + "Character" => ["Character: ", "Characters: "], + "Freeform" => ["Additional Tag: ", "Additional Tags: "] + }.each_pair do |klass, labels| + context "when given one #{klass.underscore.humanize.downcase.singularize}" do + let(:tag) { create(:tag, type: klass) } + let(:tags) { [tag] } + + it "returns \"#{labels[0]}\" followed by the tag name" do + expect(work_tag_metadata(tags)).to eq("#{labels[0]}#{tag.name}") + end + end + + context "when given multiple #{klass.underscore.humanize.downcase.pluralize}" do + let(:tag1) { create(:tag, type: klass) } + let(:tag2) { create(:tag, type: klass) } + let(:tags) { [tag1, tag2] } + + it "returns \"#{labels[1]}\" followed by a comma-separated list of tag names" do + expect(work_tag_metadata(tags)).to eq("#{labels[1]}#{tag1.name}, #{tag2.name}") + end + end + end + end + + describe "#work_tag_metadata_label" do + { + "Fandom" => ["Fandom: ", "Fandoms: "], + "Rating" => ["Rating: ", "Rating: "], + "ArchiveWarning" => ["Warning: ", "Warnings: "], + "Relationship" => ["Relationship: ", "Relationships: "], + "Character" => ["Character: ", "Characters: "], + "Freeform" => ["Additional Tag: ", "Additional Tags: "] + }.each_pair do |klass, labels| + context "when given one #{klass.underscore.humanize.downcase.singularize}" do + let(:tags) { build_list(:tag, 1, type: klass) } + + it "returns \"#{labels[0]}\"" do + expect(work_tag_metadata_label(tags)).to eq(labels[0]) + end + end + + context "when given multiple #{klass.underscore.humanize.downcase.pluralize}" do + let(:tags) { build_list(:tag, 2, type: klass) } + + it "returns \"#{labels[1]}\"" do + expect(work_tag_metadata_label(tags)).to eq(labels[1]) + end + end + end + end + + describe "#work_tag_metadata_list" do + %w[Fandom Rating ArchiveWarning Relationship Character Freeform].each do |klass| + context "when given one #{klass.underscore.humanize.downcase.singularize}" do + let(:tag) { create(:tag, type: klass) } + let(:tags) { [tag] } + + it "returns a string with the tag name" do + expect(work_tag_metadata_list(tags)).to eq(tag.name) + end + end + + context "when given multiple #{klass.underscore.humanize.downcase.pluralize}" do + let(:tag1) { create(:tag, type: klass) } + let(:tag2) { create(:tag, type: klass) } + let(:tags) { [tag1, tag2] } + + it "returns a string of tag names joined by a comma and a space" do + expect(work_tag_metadata_list(tags)).to eq("#{tag1.name}, #{tag2.name}") + end + end + end + end + + describe "#style_work_tag_metadata_list" do + context "when given one fandom" do + let(:fandom) { create(:fandom) } + let(:fandoms) { [fandom] } + + it "returns a red link to the fandom" do + link = link_to(fandom.name, fandom_url(fandom), style: "color:#990000") + expect(style_work_tag_metadata_list(fandoms)).to eq(link) + end + end + + context "when given multiple fandoms" do + let(:fandom1) { create(:fandom) } + let(:fandom2) { create(:fandom) } + let(:fandoms) { [fandom1, fandom2] } + + it "returns red links to the fandoms combined using to_sentence" do + link1 = link_to(fandom1.name, fandom_url(fandom1), style: "color:#990000") + link2 = link_to(fandom2.name, fandom_url(fandom2), style: "color:#990000") + expect(style_work_tag_metadata_list(fandoms)).to eq("#{link1} and #{link2}") + end + end + + %w[Rating ArchiveWarning Relationship Character Freeform].each do |klass| + context "when given one #{klass.underscore.humanize.downcase.singularize}" do + let(:tag) { create(:tag, type: klass) } + let(:tags) { [tag] } + + it "returns a string with the tag name" do + expect(style_work_tag_metadata_list(tags)).to eq(tag.name) + end + end + + context "when given multiple #{klass.underscore.humanize.downcase.pluralize}" do + let(:tag1) { create(:tag, type: klass) } + let(:tag2) { create(:tag, type: klass) } + let(:tags) { [tag1, tag2] } + + it "returns a string of tag names joined by a comma and a space" do + expect(style_work_tag_metadata_list(tags)).to eq("#{tag1.name}, #{tag2.name}") + end + end + end + end + + describe "#style_work_tag_metadata" do + context "when given one fandom" do + let(:fandom) { create(:fandom) } + let(:fandoms) { [fandom] } + + it "returns \"Fandom: \" styled bold and red followed by a link to the fandom" do + label = "Fandom: " + link = link_to(fandom.name, fandom_url(fandom), style: "color:#990000") + expect(style_work_tag_metadata(fandoms)).to eq("#{label}#{link}") + end + end + + context "when given multiple fandoms" do + let(:fandom1) { create(:fandom) } + let(:fandom2) { create(:fandom) } + let(:fandoms) { [fandom1, fandom2] } + + it "returns \"Fandoms: \" styled bold and red followed by links to the fandoms combined using to_sentence" do + label = "Fandoms: " + link1 = link_to(fandom1.name, fandom_url(fandom1), style: "color:#990000") + link2 = link_to(fandom2.name, fandom_url(fandom2), style: "color:#990000") + list = "#{link1} and #{link2}" + expect(style_work_tag_metadata(fandoms)).to eq("#{label}#{list}") + end + end + + { + "Rating" => ["Rating:", "Rating:"], + "ArchiveWarning" => ["Warning:", "Warnings:"], + "Relationship" => ["Relationship:", "Relationships:"], + "Character" => ["Character:", "Characters:"], + "Freeform" => ["Additional Tag:", "Additional Tags:"] + }.each_pair do |klass, labels| + context "when given one #{klass.underscore.humanize.downcase.singularize}" do + let(:tag) { create(:tag, type: klass) } + let(:tags) { [tag] } + + it "returns \"#{labels[0]} \" styled bold and red followed by the tag name" do + label = "#{labels[0]} " + expect(style_work_tag_metadata(tags)).to eq("#{label}#{tag.name}") + end + end + + context "when given multiple #{klass.underscore.humanize.downcase.pluralize}" do + let(:tag1) { create(:tag, type: klass) } + let(:tag2) { create(:tag, type: klass) } + let(:tags) { [tag1, tag2] } + + it "returns \"#{labels[1]} \" styled bold and red followed by a comma-separated list of tag names" do + label = "#{labels[1]} " + list = "#{tag1.name}, #{tag2.name}" + expect(style_work_tag_metadata(tags)).to eq("#{label}#{list}") + end + end + end + end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 8ce5869323..ff61f00ec9 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -675,7 +675,7 @@ subject(:email) { UserMailer.recipient_notification(user.id, work.id, collection.id) } let(:user) { create(:user) } - let(:work) { create(:work) } + let(:work) { create(:work, fandom_string: "Fandom 1, Fandom 2", character_string: "A, B") } let(:collection) { create(:collection) } # Test the headers @@ -689,16 +689,20 @@ # Test both body contents it_behaves_like "a multipart email" + it_behaves_like "a translated email" + describe "HTML version" do it "has the correct content" do expect(email).to have_html_part_content("Hi, ") + expect(email).to have_html_part_content("FAIL") end end describe "text version" do it "has the correct content" do expect(email).to have_text_part_content("Hi, #{user.login}!") + expect(email).to have_text_part_content("FAIL") end end end @@ -720,6 +724,8 @@ # Test both body contents it_behaves_like "a multipart email" + it_behaves_like "a translated email" + describe "HTML version" do it "has the correct content" do expect(email).to have_html_part_content("Hi, Date: Sat, 11 Sep 2021 22:37:11 -0400 Subject: [PATCH 02/12] Shifting otters --- app/helpers/mailer_helper.rb | 7 +------ app/helpers/translation_helper.rb | 7 +++++++ config/locales/models/en.yml | 4 +++- spec/helpers/mailer_helper_spec.rb | 6 +++--- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/helpers/mailer_helper.rb b/app/helpers/mailer_helper.rb index 3dfcd49b1c..bc2b48e8d1 100644 --- a/app/helpers/mailer_helper.rb +++ b/app/helpers/mailer_helper.rb @@ -135,12 +135,7 @@ def work_metadata_label(text) def work_tag_metadata_label(tags) return if tags.empty? type = tags.first.type - # Rating has no pluralization. - if type == "Rating" - t("activerecord.models.rating") + t("mailer.general.metadata_label_indicator") - else - t("activerecord.models.#{type.underscore}", count: tags.count) + t("mailer.general.metadata_label_indicator") - end + t("activerecord.models.#{type.underscore}", count: tags.count) + t("mailer.general.metadata_label_indicator") end # We don't use .to_sentence because these aren't links and we risk making any diff --git a/app/helpers/translation_helper.rb b/app/helpers/translation_helper.rb index ea3fe70c79..b3e26d4d17 100644 --- a/app/helpers/translation_helper.rb +++ b/app/helpers/translation_helper.rb @@ -60,4 +60,11 @@ def time_ago_in_words(from_time, include_seconds = false) alias distance_of_time_in_words_to_now time_ago_in_words + # Take some text and add whatever punctuation, symbols, and/or spacing + # we use to separate a metadata property from its value, e.g., "Property: ", + # "Propriété : ". + def metadata_property(text) + text.html_safe + t("mailer.general.metadata_label_indicator") + end + end diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml index 2f26d85079..21fd44a1f3 100644 --- a/config/locales/models/en.yml +++ b/config/locales/models/en.yml @@ -42,7 +42,9 @@ en: one: "Additional Tag" other: "Additional Tags" pseud: "Pseud" - rating: "Rating" + rating: + one: "Rating" + other: "Ratings" relationship: one: "Relationship" other: "Relationships" diff --git a/spec/helpers/mailer_helper_spec.rb b/spec/helpers/mailer_helper_spec.rb index bc7dc80c61..e5012fbfd0 100644 --- a/spec/helpers/mailer_helper_spec.rb +++ b/spec/helpers/mailer_helper_spec.rb @@ -11,7 +11,7 @@ describe "#work_tag_metadata" do { "Fandom" => ["Fandom: ", "Fandoms: "], - "Rating" => ["Rating: ", "Rating: "], + "Rating" => ["Rating: ", "Ratings: "], "ArchiveWarning" => ["Warning: ", "Warnings: "], "Relationship" => ["Relationship: ", "Relationships: "], "Character" => ["Character: ", "Characters: "], @@ -41,7 +41,7 @@ describe "#work_tag_metadata_label" do { "Fandom" => ["Fandom: ", "Fandoms: "], - "Rating" => ["Rating: ", "Rating: "], + "Rating" => ["Rating: ", "Ratings: "], "ArchiveWarning" => ["Warning: ", "Warnings: "], "Relationship" => ["Relationship: ", "Relationships: "], "Character" => ["Character: ", "Characters: "], @@ -160,7 +160,7 @@ end { - "Rating" => ["Rating:", "Rating:"], + "Rating" => ["Rating:", "Ratings:"], "ArchiveWarning" => ["Warning:", "Warnings:"], "Relationship" => ["Relationship:", "Relationships:"], "Character" => ["Character:", "Characters:"], From bf72efb19ed260137aeafba2206f8541dc3533d0 Mon Sep 17 00:00:00 2001 From: Sarken Date: Tue, 28 Dec 2021 19:41:46 -0500 Subject: [PATCH 03/12] AO3-5763 Fix line breaks in plain text email We don't want empty lines if some tag type is missing. --- app/views/user_mailer/_work_info.text.erb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/app/views/user_mailer/_work_info.text.erb b/app/views/user_mailer/_work_info.text.erb index fc25f5546d..727b137833 100644 --- a/app/views/user_mailer/_work_info.text.erb +++ b/app/views/user_mailer/_work_info.text.erb @@ -2,13 +2,11 @@ <%= work_metadata_label(Work.human_attribute_name("chapter_total_display"))%><%= work.chapter_total_display %> <%= work_tag_metadata(work.fandoms) %> <%= work_tag_metadata(work.ratings) %> -<%= work_tag_metadata(work.archive_warnings) %> -<% unless work.relationship_string.blank? %><%= work_tag_metadata(work.relationships) %><% end %> -<% unless work.character_string.blank? %><%= work_tag_metadata(work.characters) %><% end %> -<% unless work.freeform_string.blank? %><%= work_tag_metadata(work.freeforms) %><% end %> -<% unless work.series.count.zero? %><%= Series.model_name.human(count: work.series.count) %><%= raw to_plain_text(series_list_for_feeds(work)) %><% end %> +<%= work_tag_metadata(work.archive_warnings) %><% unless work.relationship_string.blank? %> +<%= work_tag_metadata(work.relationships) %><% end %><% unless work.character_string.blank? %> +<%= work_tag_metadata(work.characters) %><% end %><% unless work.freeform_string.blank? %> +<%= work_tag_metadata(work.freeforms) %><% end %><% unless work.series.count.zero? %> +<%= Series.model_name.human(count: work.series.count) %><%= raw to_plain_text(series_list_for_feeds(work)) %><% end %><% unless work.summary.blank? %> -<% unless work.summary.blank? %> <%= work_metadata_label(Work.human_attribute_name("summary")) %> - <%= raw to_plain_text(sanitize_field(work, :summary)) %> -<% end %> + <%= raw to_plain_text(sanitize_field(work, :summary)) %><% end %> From 89d0280cdaf3fc9edd0bdce0db4645eca6c70959 Mon Sep 17 00:00:00 2001 From: Sarken Date: Tue, 28 Dec 2021 19:53:37 -0500 Subject: [PATCH 04/12] Add missing test and rearrange to match order of helper --- spec/helpers/mailer_helper_spec.rb | 62 ++++++++++++++++-------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/spec/helpers/mailer_helper_spec.rb b/spec/helpers/mailer_helper_spec.rb index e5012fbfd0..0827af2aa3 100644 --- a/spec/helpers/mailer_helper_spec.rb +++ b/spec/helpers/mailer_helper_spec.rb @@ -8,33 +8,9 @@ end end - describe "#work_tag_metadata" do - { - "Fandom" => ["Fandom: ", "Fandoms: "], - "Rating" => ["Rating: ", "Ratings: "], - "ArchiveWarning" => ["Warning: ", "Warnings: "], - "Relationship" => ["Relationship: ", "Relationships: "], - "Character" => ["Character: ", "Characters: "], - "Freeform" => ["Additional Tag: ", "Additional Tags: "] - }.each_pair do |klass, labels| - context "when given one #{klass.underscore.humanize.downcase.singularize}" do - let(:tag) { create(:tag, type: klass) } - let(:tags) { [tag] } - - it "returns \"#{labels[0]}\" followed by the tag name" do - expect(work_tag_metadata(tags)).to eq("#{labels[0]}#{tag.name}") - end - end - - context "when given multiple #{klass.underscore.humanize.downcase.pluralize}" do - let(:tag1) { create(:tag, type: klass) } - let(:tag2) { create(:tag, type: klass) } - let(:tags) { [tag1, tag2] } - - it "returns \"#{labels[1]}\" followed by a comma-separated list of tag names" do - expect(work_tag_metadata(tags)).to eq("#{labels[1]}#{tag1.name}, #{tag2.name}") - end - end + describe "#work_metadata_label" do + it "appends the metadata indicator to a string" do + expect(work_metadata_label("Text")).to eq("Text: ") end end @@ -88,7 +64,37 @@ end end - describe "#style_work_tag_metadata_list" do + describe "#work_tag_metadata" do + { + "Fandom" => ["Fandom: ", "Fandoms: "], + "Rating" => ["Rating: ", "Ratings: "], + "ArchiveWarning" => ["Warning: ", "Warnings: "], + "Relationship" => ["Relationship: ", "Relationships: "], + "Character" => ["Character: ", "Characters: "], + "Freeform" => ["Additional Tag: ", "Additional Tags: "] + }.each_pair do |klass, labels| + context "when given one #{klass.underscore.humanize.downcase.singularize}" do + let(:tag) { create(:tag, type: klass) } + let(:tags) { [tag] } + + it "returns \"#{labels[0]}\" followed by the tag name" do + expect(work_tag_metadata(tags)).to eq("#{labels[0]}#{tag.name}") + end + end + + context "when given multiple #{klass.underscore.humanize.downcase.pluralize}" do + let(:tag1) { create(:tag, type: klass) } + let(:tag2) { create(:tag, type: klass) } + let(:tags) { [tag1, tag2] } + + it "returns \"#{labels[1]}\" followed by a comma-separated list of tag names" do + expect(work_tag_metadata(tags)).to eq("#{labels[1]}#{tag1.name}, #{tag2.name}") + end + end + end + end + + describe "#style_work_tag_metadata_list" do context "when given one fandom" do let(:fandom) { create(:fandom) } let(:fandoms) { [fandom] } From 4ba98a988fe31ab016066855452789d579f9b85e Mon Sep 17 00:00:00 2001 From: Sarken Date: Tue, 28 Dec 2021 19:53:52 -0500 Subject: [PATCH 05/12] Stop intentionally failing tests --- spec/mailers/user_mailer_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index ff61f00ec9..37390dffe3 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -695,14 +695,12 @@ it "has the correct content" do expect(email).to have_html_part_content("Hi, ") - expect(email).to have_html_part_content("FAIL") end end describe "text version" do it "has the correct content" do expect(email).to have_text_part_content("Hi, #{user.login}!") - expect(email).to have_text_part_content("FAIL") end end end From 90301b97c75ee88e4183c703fff3063066f1c35d Mon Sep 17 00:00:00 2001 From: Sarken Date: Tue, 28 Dec 2021 20:03:44 -0500 Subject: [PATCH 06/12] Update test because we're now using the standard anonymous byline, whee --- features/gift_exchanges/challenge_yuletide.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/gift_exchanges/challenge_yuletide.feature b/features/gift_exchanges/challenge_yuletide.feature index 364f97daea..1d60b5c17e 100644 --- a/features/gift_exchanges/challenge_yuletide.feature +++ b/features/gift_exchanges/challenge_yuletide.feature @@ -574,7 +574,7 @@ Feature: Collection And the email should contain "A gift work has been posted for you in the" And the email should contain "Yuletide" And the email should contain "at the Archive of Our Own" - And the email should contain "by an anonymous responder" + And the email should contain "by Anonymous" And the email should not contain "by myname1" And the email should not contain "by myname2" And the email should not contain "by myname3" From bd1bd2159a5719b9460584f541719540ff584f01 Mon Sep 17 00:00:00 2001 From: Sarken Date: Tue, 28 Dec 2021 20:06:06 -0500 Subject: [PATCH 07/12] Subject needs to be determined in the local block to make sure it's in the right language --- app/mailers/user_mailer.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 99cbdc6c5c..b0c06bcb71 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -316,15 +316,15 @@ def recipient_notification(user_id, work_id, collection_id = nil) @user = User.find(user_id) @work = Work.find(work_id) @collection = Collection.find(collection_id) if collection_id - subject = if @collection - t("user_mailer.recipient_notification.subject.collection", - app_name: ArchiveConfig.APP_SHORT_NAME, - collection_title: @collection.title) - else - t("user_mailer.recipient_notification.subject.no_collection", - app_name: ArchiveConfig.APP_SHORT_NAME) - end I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do + subject = if @collection + t("user_mailer.recipient_notification.subject.collection", + app_name: ArchiveConfig.APP_SHORT_NAME, + collection_title: @collection.title) + else + t("user_mailer.recipient_notification.subject.no_collection", + app_name: ArchiveConfig.APP_SHORT_NAME) + end mail( to: @user.email, subject: subject From 11b0c3b4f7bb1310f166669fddb53da7282df428 Mon Sep 17 00:00:00 2001 From: Sarken Date: Tue, 28 Dec 2021 20:06:48 -0500 Subject: [PATCH 08/12] Fix indenting --- app/mailers/user_mailer.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index b0c06bcb71..bdb61dd869 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -318,12 +318,12 @@ def recipient_notification(user_id, work_id, collection_id = nil) @collection = Collection.find(collection_id) if collection_id I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do subject = if @collection - t("user_mailer.recipient_notification.subject.collection", - app_name: ArchiveConfig.APP_SHORT_NAME, - collection_title: @collection.title) + t("user_mailer.recipient_notification.subject.collection", + app_name: ArchiveConfig.APP_SHORT_NAME, + collection_title: @collection.title) else - t("user_mailer.recipient_notification.subject.no_collection", - app_name: ArchiveConfig.APP_SHORT_NAME) + t("user_mailer.recipient_notification.subject.no_collection", + app_name: ArchiveConfig.APP_SHORT_NAME) end mail( to: @user.email, From d76cfb37c0075f7338dc6b40bdf6853989d9bcbc Mon Sep 17 00:00:00 2001 From: Sarken Date: Mon, 17 Jan 2022 18:36:39 -0500 Subject: [PATCH 09/12] Style fixes --- app/helpers/mailer_helper.rb | 5 +++++ app/helpers/translation_helper.rb | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/helpers/mailer_helper.rb b/app/helpers/mailer_helper.rb index bc2b48e8d1..4fa0c9c4e9 100644 --- a/app/helpers/mailer_helper.rb +++ b/app/helpers/mailer_helper.rb @@ -134,6 +134,7 @@ def work_metadata_label(text) def work_tag_metadata_label(tags) return if tags.empty? + type = tags.first.type t("activerecord.models.#{type.underscore}", count: tags.count) + t("mailer.general.metadata_label_indicator") end @@ -142,12 +143,14 @@ def work_tag_metadata_label(tags) # connector word (e.g., "and") look like part of the final tag. def work_tag_metadata_list(tags) return if tags.empty? + tags.pluck(:name).join(t("support.array.words_connector")) end # Spacing is dealt with in locale files, e.g. " : " for French. def work_tag_metadata(tags) return if tags.empty? + "#{work_tag_metadata_label(tags)}#{work_tag_metadata_list(tags)}" end @@ -157,6 +160,7 @@ def style_work_metadata_label(text) def style_work_tag_metadata_list(tags) return if tags.empty? + type = tags.first.type # Fandom tags are linked and to_sentence'd. if type == "Fandom" @@ -169,6 +173,7 @@ def style_work_tag_metadata_list(tags) # Spacing is dealt with in locale files, e.g. " : " for French. def style_work_tag_metadata(tags) return if tags.empty? + label = style_bold(work_tag_metadata_label(tags)) "#{label}#{style_work_tag_metadata_list(tags)}".html_safe end diff --git a/app/helpers/translation_helper.rb b/app/helpers/translation_helper.rb index b3e26d4d17..f86f4936e3 100644 --- a/app/helpers/translation_helper.rb +++ b/app/helpers/translation_helper.rb @@ -66,5 +66,4 @@ def time_ago_in_words(from_time, include_seconds = false) def metadata_property(text) text.html_safe + t("mailer.general.metadata_label_indicator") end - end From 870baa2285842925aeaced67d88d7c7be86da735 Mon Sep 17 00:00:00 2001 From: Sarken Date: Mon, 17 Jan 2022 18:44:03 -0500 Subject: [PATCH 10/12] One more code style fix --- app/mailers/user_mailer.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index bdb61dd869..b0c06bcb71 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -318,12 +318,12 @@ def recipient_notification(user_id, work_id, collection_id = nil) @collection = Collection.find(collection_id) if collection_id I18n.with_locale(Locale.find(@user.preference.preferred_locale).iso) do subject = if @collection - t("user_mailer.recipient_notification.subject.collection", - app_name: ArchiveConfig.APP_SHORT_NAME, - collection_title: @collection.title) + t("user_mailer.recipient_notification.subject.collection", + app_name: ArchiveConfig.APP_SHORT_NAME, + collection_title: @collection.title) else - t("user_mailer.recipient_notification.subject.no_collection", - app_name: ArchiveConfig.APP_SHORT_NAME) + t("user_mailer.recipient_notification.subject.no_collection", + app_name: ArchiveConfig.APP_SHORT_NAME) end mail( to: @user.email, From 5a317ad70613cd8c35456fc35dc2395f3bd396e5 Mon Sep 17 00:00:00 2001 From: Sarken Date: Mon, 17 Jan 2022 22:13:39 -0500 Subject: [PATCH 11/12] More lines, more readable --- app/views/user_mailer/_work_info.text.erb | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/app/views/user_mailer/_work_info.text.erb b/app/views/user_mailer/_work_info.text.erb index 727b137833..a50823cd0b 100644 --- a/app/views/user_mailer/_work_info.text.erb +++ b/app/views/user_mailer/_work_info.text.erb @@ -2,11 +2,21 @@ <%= work_metadata_label(Work.human_attribute_name("chapter_total_display"))%><%= work.chapter_total_display %> <%= work_tag_metadata(work.fandoms) %> <%= work_tag_metadata(work.ratings) %> -<%= work_tag_metadata(work.archive_warnings) %><% unless work.relationship_string.blank? %> -<%= work_tag_metadata(work.relationships) %><% end %><% unless work.character_string.blank? %> -<%= work_tag_metadata(work.characters) %><% end %><% unless work.freeform_string.blank? %> -<%= work_tag_metadata(work.freeforms) %><% end %><% unless work.series.count.zero? %> -<%= Series.model_name.human(count: work.series.count) %><%= raw to_plain_text(series_list_for_feeds(work)) %><% end %><% unless work.summary.blank? %> +<%= work_tag_metadata(work.archive_warnings) %> +<% unless work.relationship_string.blank? %> +<%= work_tag_metadata(work.relationships) %> +<% end %> +<% unless work.character_string.blank? %> +<%= work_tag_metadata(work.characters) %> +<% end %> +<% unless work.freeform_string.blank? %> +<%= work_tag_metadata(work.freeforms) %> +<% end %> +<% unless work.series.count.zero? %> +<%= Series.model_name.human(count: work.series.count) %><%= raw to_plain_text(series_list_for_feeds(work)) %> +<% end %> +<% unless work.summary.blank? %> <%= work_metadata_label(Work.human_attribute_name("summary")) %> - <%= raw to_plain_text(sanitize_field(work, :summary)) %><% end %> + <%= raw to_plain_text(sanitize_field(work, :summary)) %> +<% end %> From 0a99542f8f08a48f727f18b94946367e1347d2ac Mon Sep 17 00:00:00 2001 From: Sarken Date: Mon, 14 Mar 2022 23:00:39 -0400 Subject: [PATCH 12/12] AO3-5763 Make work_tag_metadata_label(tags), work_tag_metadata_list(tags), and style_work_tag_metadata_list(tags) private and do not test individually --- app/helpers/mailer_helper.rb | 40 +++++++------ spec/helpers/mailer_helper_spec.rb | 95 ------------------------------ 2 files changed, 21 insertions(+), 114 deletions(-) diff --git a/app/helpers/mailer_helper.rb b/app/helpers/mailer_helper.rb index 4fa0c9c4e9..f29b2ff59a 100644 --- a/app/helpers/mailer_helper.rb +++ b/app/helpers/mailer_helper.rb @@ -132,6 +132,27 @@ def work_metadata_label(text) text.html_safe + t("mailer.general.metadata_label_indicator") end + # Spacing is dealt with in locale files, e.g. " : " for French. + def work_tag_metadata(tags) + return if tags.empty? + + "#{work_tag_metadata_label(tags)}#{work_tag_metadata_list(tags)}" + end + + def style_work_metadata_label(text) + style_bold(work_metadata_label(text)) + end + + # Spacing is dealt with in locale files, e.g. " : " for French. + def style_work_tag_metadata(tags) + return if tags.empty? + + label = style_bold(work_tag_metadata_label(tags)) + "#{label}#{style_work_tag_metadata_list(tags)}".html_safe + end + + private + def work_tag_metadata_label(tags) return if tags.empty? @@ -147,17 +168,6 @@ def work_tag_metadata_list(tags) tags.pluck(:name).join(t("support.array.words_connector")) end - # Spacing is dealt with in locale files, e.g. " : " for French. - def work_tag_metadata(tags) - return if tags.empty? - - "#{work_tag_metadata_label(tags)}#{work_tag_metadata_list(tags)}" - end - - def style_work_metadata_label(text) - style_bold(work_metadata_label(text)) - end - def style_work_tag_metadata_list(tags) return if tags.empty? @@ -169,12 +179,4 @@ def style_work_tag_metadata_list(tags) work_tag_metadata_list(tags) end end - - # Spacing is dealt with in locale files, e.g. " : " for French. - def style_work_tag_metadata(tags) - return if tags.empty? - - label = style_bold(work_tag_metadata_label(tags)) - "#{label}#{style_work_tag_metadata_list(tags)}".html_safe - end end # end of MailerHelper diff --git a/spec/helpers/mailer_helper_spec.rb b/spec/helpers/mailer_helper_spec.rb index 0827af2aa3..02a5b9ca5e 100644 --- a/spec/helpers/mailer_helper_spec.rb +++ b/spec/helpers/mailer_helper_spec.rb @@ -14,56 +14,6 @@ end end - describe "#work_tag_metadata_label" do - { - "Fandom" => ["Fandom: ", "Fandoms: "], - "Rating" => ["Rating: ", "Ratings: "], - "ArchiveWarning" => ["Warning: ", "Warnings: "], - "Relationship" => ["Relationship: ", "Relationships: "], - "Character" => ["Character: ", "Characters: "], - "Freeform" => ["Additional Tag: ", "Additional Tags: "] - }.each_pair do |klass, labels| - context "when given one #{klass.underscore.humanize.downcase.singularize}" do - let(:tags) { build_list(:tag, 1, type: klass) } - - it "returns \"#{labels[0]}\"" do - expect(work_tag_metadata_label(tags)).to eq(labels[0]) - end - end - - context "when given multiple #{klass.underscore.humanize.downcase.pluralize}" do - let(:tags) { build_list(:tag, 2, type: klass) } - - it "returns \"#{labels[1]}\"" do - expect(work_tag_metadata_label(tags)).to eq(labels[1]) - end - end - end - end - - describe "#work_tag_metadata_list" do - %w[Fandom Rating ArchiveWarning Relationship Character Freeform].each do |klass| - context "when given one #{klass.underscore.humanize.downcase.singularize}" do - let(:tag) { create(:tag, type: klass) } - let(:tags) { [tag] } - - it "returns a string with the tag name" do - expect(work_tag_metadata_list(tags)).to eq(tag.name) - end - end - - context "when given multiple #{klass.underscore.humanize.downcase.pluralize}" do - let(:tag1) { create(:tag, type: klass) } - let(:tag2) { create(:tag, type: klass) } - let(:tags) { [tag1, tag2] } - - it "returns a string of tag names joined by a comma and a space" do - expect(work_tag_metadata_list(tags)).to eq("#{tag1.name}, #{tag2.name}") - end - end - end - end - describe "#work_tag_metadata" do { "Fandom" => ["Fandom: ", "Fandoms: "], @@ -94,51 +44,6 @@ end end - describe "#style_work_tag_metadata_list" do - context "when given one fandom" do - let(:fandom) { create(:fandom) } - let(:fandoms) { [fandom] } - - it "returns a red link to the fandom" do - link = link_to(fandom.name, fandom_url(fandom), style: "color:#990000") - expect(style_work_tag_metadata_list(fandoms)).to eq(link) - end - end - - context "when given multiple fandoms" do - let(:fandom1) { create(:fandom) } - let(:fandom2) { create(:fandom) } - let(:fandoms) { [fandom1, fandom2] } - - it "returns red links to the fandoms combined using to_sentence" do - link1 = link_to(fandom1.name, fandom_url(fandom1), style: "color:#990000") - link2 = link_to(fandom2.name, fandom_url(fandom2), style: "color:#990000") - expect(style_work_tag_metadata_list(fandoms)).to eq("#{link1} and #{link2}") - end - end - - %w[Rating ArchiveWarning Relationship Character Freeform].each do |klass| - context "when given one #{klass.underscore.humanize.downcase.singularize}" do - let(:tag) { create(:tag, type: klass) } - let(:tags) { [tag] } - - it "returns a string with the tag name" do - expect(style_work_tag_metadata_list(tags)).to eq(tag.name) - end - end - - context "when given multiple #{klass.underscore.humanize.downcase.pluralize}" do - let(:tag1) { create(:tag, type: klass) } - let(:tag2) { create(:tag, type: klass) } - let(:tags) { [tag1, tag2] } - - it "returns a string of tag names joined by a comma and a space" do - expect(style_work_tag_metadata_list(tags)).to eq("#{tag1.name}, #{tag2.name}") - end - end - end - end - describe "#style_work_tag_metadata" do context "when given one fandom" do let(:fandom) { create(:fandom) }