fixes #1198 - Send mail to question's author on new answer create and test this

[saurabh-sikchi]

Original Issue #1198

I have made changes to answer model's answer_notify method. Earlier, the mail was not being sent to the question's author. After this commit, the mail is being sent and the tests are passing.

Also see, pull #1237 (not by me) which implements the proposed solution but the tests fail.

Thank you.

• all tests pass -- rake test:all
• code is in uniquely-named feature branch, and has been rebased on top of latest master (especially if you've been asked to make additional changes)
• pull request is descriptively named with #number reference back to original issue

[PublicLabBot]

	3 Messages
📖	@khamba Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution!
📖	Your pull request is on the master branch. Please make a separate feature branch) with a descriptive name like new-blog-design while making PRs in the future.
📖	This pull request doesn’t link to a issue number. Please refer to the issue it fixes (if any) in the format: Fixes #123.

Generated by 🚫 Danger

[saurabh-sikchi]

I am new to open source contributions. I did make a new feature branch and then merged it with master. Should I re-submit the request?

[jywarren]

Here I'm not sure this is correct, @ananyo2012, can you take a look? I think we do want to notify original node (question) author when an answer is posted, but not the current user (that is, the author of the answer)

[saurabh-sikchi]

Earlier, the if conditional current_user.uid != self.author.uid was always false since the current_user was always the the answer's author. I have changed that to the node's author.

I think we do want to notify original node (question) author when an answer is posted

Agreed. Line 30 sends the mail to the node's author.

Thank you very much for reviewing.

[ananyo2012]

Yes this look good to me

[jywarren]

No need to resubmit - you should be able to add more commits to this same branch and they'll show up in the PR. Thanks!

…

On Mar 8, 2017 10:44 AM, "Saurabh Sikchi" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app/models/answer.rb <#1321 (comment)>: > @@ -26,17 +26,17 @@ def likers def answer_notify(current_user) # notify question author - if current_user.uid != self.author.uid - AnswerMailer.notify_question_author(self.author, self).deliver + if current_user.uid != self.node.author.uid Earlier, the if conditional current_user.uid != self.author.uid was always false since the current_user was always the the answer's author. I have changed that to the node's author. I think we do want to notify original node (question) author when an answer is posted Agreed. Line 30 sends the mail to the node's author. Thank you very much for reviewing. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1321 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJxSOd84uanFj7OLmcLugnYfdMCw8ks5rjtrsgaJpZM4MVCWm> .

[ananyo2012]

@khamba You didn't need to merge the feature branch with the master. Just make the PR with the feature branch. Also whenever you solve an issue it is better to include the issue no like Fixes #<issue_no> in the commit message itself. It autocloses the issue when merged.

[jywarren]

Merged, thank you! Please consider picking another issue from our help wanted listings! Thanks!!!!!