New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AO3-5763 AO3-5348 Add more info to gift notification email and internationalize it #4137
Conversation
We don't want empty lines if some tag type is missing.
…n the right language
@@ -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é : ". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/AsciiComments: Use only ascii symbols in comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Va te faire enculer, Hound. It's French.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duo serait très fier de toi :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we don't have to do anything here. This check will eventually be phased out (rubocop/rubocop#9674).
<%= 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? %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(It's like this so we don't end up with extra blank lines in the email when a tag type is not present.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Counterintuitively enough, I think you can actually prevent extra blank lines in the output by adding newlines. A template like this produces three blank lines:
<% if false %><%= "Text." %><% end %>
<% if false %><%= "Text." %><% end %>
<% if false %><%= "Text." %><% end %>
Whereas a template like this produces no blank lines:
<% if false %>
<%= "Text." %>
<% end %>
<% if false %>
<%= "Text." %>
<% end %>
<% if false %>
<%= "Text." %>
<% end %>
(This is because a <% %>
tag that's alone on the line doesn't affect the output. But multiple tags in one line are trickier.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that would be much better -- thanks!
<% 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) %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(The work title/byline is separate from the other metadata because subscription emails display this info differently.)
<% unless work.summary.blank? %> | ||
|
||
<%= work_metadata_label(Work.human_attribute_name("summary")) %> | ||
<%= raw to_plain_text(sanitize_field(work, :summary)) %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be indented 2 spaces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, I wanted to match the indenting from the subscription emails (I assume we went with four spaces because it's more obviously done for a pretty formatting effect than two -- two just looks kinda like we misaligned the text):
Summary: | |
<%= raw to_plain_text(sanitize_field(this_work, :summary)) %><% @seen_summary[this_work.id] = true %><% end %><% if (index < @creations.length-1) %> |
|
||
# 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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used directly in a view? If so, we should make it private and keep the list of available helpers shorter.
Then we can have specs just for the public ones as well.
…ags), and style_work_tag_metadata_list(tags) private and do not test individually
Issue
https://otwarchive.atlassian.net/browse/AO3-5348
https://otwarchive.atlassian.net/browse/AO3-5763
Purpose
Make sure the gift notification email includes the same info we'd send for a subscription notification. Accomplishes this using a partial we can reuse across many emails. I did not update the subscription emails to use the partial, even though it's the same info and formatting -- I can add that change if we'd like.
Also makes the email translatable and fixes an issue where the subject was always in English.
Testing Instructions
Give people gifts! Check how the emails look in various languages, HTML and plain text!
Credit
Sarken, she/her