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-6847 Create anonymous_or_unrevealed_notification mailer preview #5040

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amecreate
Copy link
Contributor

Pull Request Checklist

Issue

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

Purpose

Add mailer preview for the anonymous_or_unrevealed_notification email.

Include cucumber test for sending a translated anonymous_or_unrevealed_notification email to a user.

Remove the redundant html_safe call in the HTML mail.

Testing Instructions

See Jira.

References

Credit

Amy Lee

@@ -63,3 +63,65 @@ Feature: Collectible items email
And I fill in "bookmark_collection_names" with "dont_bookmark_me_bro"
And I press "Create"
Then 1 email should be delivered

Scenario: Scenario: Translated email is sent when the status of a Collection item is changed to anonymous
Copy link
Member

Choose a reason for hiding this comment

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

Duplicate "Scenario"

Suggested change
Scenario: Scenario: Translated email is sent when the status of a Collection item is changed to anonymous
Scenario: Translated email is sent when the status of a Collection item is changed to anonymous

And the email should have "Your work was made anonymous" in the subject
And the email to "user1" should be translated

Scenario: Scenario: Translated email is sent when the status of a Collection item is changed to unrevealed
Copy link
Member

Choose a reason for hiding this comment

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

Duplicate "Scenario"

Suggested change
Scenario: Scenario: Translated email is sent when the status of a Collection item is changed to unrevealed
Scenario: Translated email is sent when the status of a Collection item is changed to unrevealed

And the email should have "Your work was made unrevealed" in the subject
And the email to "user1" should be translated

Scenario: Scenario: Translated email is sent when the status of a Collection item is changed to anonymous and unrevealed
Copy link
Member

Choose a reason for hiding this comment

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

Duplicate "Scenario"

Suggested change
Scenario: Scenario: Translated email is sent when the status of a Collection item is changed to anonymous and unrevealed
Scenario: Translated email is sent when the status of a Collection item is changed to anonymous and unrevealed

And I submit
Then "user1" should be emailed
And the email should have "Your work was made anonymous and unrevealed" in the subject
And the email to "user1" should be translated
Copy link
Member

Choose a reason for hiding this comment

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

Please add a trailing newline to the file https://stackoverflow.com/a/5813359

Comment on lines +62 to +63
subject: t(".subject.#{@status}",
app_name: ArchiveConfig.APP_SHORT_NAME)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
subject: t(".subject.#{@status}",
app_name: ArchiveConfig.APP_SHORT_NAME)
subject: t(".subject.#{@status}",
app_name: ArchiveConfig.APP_SHORT_NAME)

@@ -112,6 +112,42 @@ def change_email
new_email = "new_email"
UserMailer.change_email(user.id, old_email, new_email)
end

# Sends email when collection item changes status
[:anonymous_unrevealed_collection, :anonymous_collection, :unrevealed_collection].each do |status|
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this loop is doing exactly what you want. For example, should the first method be more like the following?

def anonymous_unrevealed_collection_notification
    user, collection, item = anonymous_or_unrevealed_data(:anonymous_unrevealed_collection)
    newly_anonymous = true
    newly_unrevealed = true
    UserMailer.anonymous_or_unrevealed_notification(
      user.id, item.id, collection.id,
      anonymous: newly_anonymous, unrevealed: newly_unrevealed
    )
end

Otherwise, I think the status needs to be in the method name to avoid new definitions overriding previous ones (I only see anonymous_unrevealed_collection_notification once on /rails/mailers for example)

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.

2 participants