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

Add "reply-to" field in user messages #1545

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Conversation

hesslau
Copy link
Member

@hesslau hesslau commented Feb 10, 2020

No description provided.

@hesslau hesslau linked an issue Feb 10, 2020 that may be closed by this pull request
Copy link
Contributor

@andreasknoepfle andreasknoepfle left a comment

Choose a reason for hiding this comment

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

Looks good already. Have some tiny change requests.

@@ -18,7 +18,7 @@ class UserMailerTest < ActiveSupport::TestCase
mail = UserMailer.contact(sender: sender, resource_id: receiver.id, text: text)

mail.must deliver_to receiver.email
mail.subject.must_equal('[Fairmondo] ein/e Nutzer*in hat eine Frage an Dich')
mail.subject.must_equal('[Fairmondo] #{sender.nickname} hat eine Frage an Dich')
Copy link
Contributor

Choose a reason for hiding this comment

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

For interpolation in ruby you need double quotes "" instead of single quotes '' that is the reason for the failed CI as well.

@@ -13,7 +13,7 @@ def contact(sender:, resource_id:, text:)
@sender = sender
@receiver = User.find resource_id
@text = text
@subject = I18n.t('email.user.contact.subject')
mail to: @receiver.email, subject: @subject
@subject = I18n.t('email.user.contact.subject', sender_name: @sender.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

in the tests it is sender.name, while here we go for sender.nickname. We should use the same value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add "reply-to" field in user messages
2 participants