Skip to content
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-6017 Add role of commenter to comment notification emails #4683

Conversation

Bilka2
Copy link
Contributor

@Bilka2 Bilka2 commented Dec 4, 2023

Issue

https://otwarchive.atlassian.net/browse/AO3-6017

Purpose

Adds the role of the commenter to comment notification emails. This marks logged-in users either as registered or official.

Add mailer previews for some comment notification emails, to cover all the different permutations of the pseud/user text (default pseud, different pseud, official, guest, anonymous creator).

Add tests for the commenter name display changes, including showing the username for comments left with pseuds and adding (Guest) for guest commenters from AO3-6548.

Testing Instructions

See Jira.

Credit

Bilka (he/him)

style_link(comment.comment_owner_name, polymorphic_url(comment.comment_owner, only_path: false))
role = comment.user.official ? t("roles.official_with_parens") : t("roles.registered_with_parens")
pseud_link = style_link(comment.pseud.byline, user_pseud_url(comment.user, comment.pseud))
t("roles.commenter_name_html", name: style_bold(pseud_link), role: style_role(role))
end
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not 100% sold myself, but might be more manageable with smaller scope if like:

if comment.by_anonymous_creator?
  return style_bold(t("roles.anonymous_creator"))
end

name =
  if comment.comment_owner.nil?
    comment.comment_owner_name
  else
    style_link(comment.pseud.byline, user_pseud_url(comment.user, comment.pseud))
  end

role =
  if comment.comment_owner.nil?
    t("roles.guest_with_parens")
  elsif comment.user.official
    t("roles.official_with_parens")
  else
    t("roles.registered_with_parens")  
  end

t("roles.commenter_name_html", name: style_bold(name), role: style_role(role))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of turning the anon comment into a guard clause like

return style_bold(t("roles.anonymous_creator")) if comment.by_anonymous_creator?

but I don't know about the rest of the conditions.

@Bilka2 Bilka2 changed the title AO3-6017 Add username and role of commenter to comment notification emails AO3-6017 Add role of commenter to comment notification emails Jan 4, 2024
@brianjaustin brianjaustin self-requested a review February 6, 2024 17:51
app/helpers/mailer_helper.rb Outdated Show resolved Hide resolved
spec/mailers/admin_mailer_spec.rb Outdated Show resolved Hide resolved
spec/mailers/admin_mailer_spec.rb Outdated Show resolved Hide resolved
spec/mailers/comment_mailer_spec.rb Show resolved Hide resolved
Co-authored-by: Brian Austin <13002992+brianjaustin@users.noreply.github.com>
Copy link
Member

@brianjaustin brianjaustin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@brianjaustin brianjaustin merged commit 3ae1433 into otwcode:master May 17, 2024
26 checks passed
@Bilka2 Bilka2 deleted the AO3-6017-notification-emails-for-pseud-formatting branch May 18, 2024 13:18
sarken added a commit that referenced this pull request May 31, 2024
sarken added a commit that referenced this pull request May 31, 2024
#4829)

Revert "AO3-6017 Add role of commenter to comment notification emails (#4683)"

This reverts commit 3ae1433.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants