-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Link addition to email for marking comment as spam #3163
Conversation
Hi @jywarren @publiclab/soc anyone facing this issue Thanks! |
yes @stefannibrasil, my previous PR #3140 also failed. Let's wait for @jywarren and @icarito . |
Generated by 🚫 Danger |
@jywarren please review this PR. Thanks! |
@@ -14,6 +14,13 @@ Hi! There's been a response to a discussion you're involved in. Do NOT reply to | |||
|
|||
<hr /> | |||
|
|||
<p>Look like spam? | |||
<% if @user.can_moderate? %> |
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.
So here's what I'm worried about, are we notifying in a bulk BCC, or do we actually have unique user record each time it's sent?
I can't find where this is used to see what the @user
variable is, but let's check!
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.
Each time a new comment is created this CommentMailer.notify
send emails to users subscribed to the tags or node. So, I have added these buttons only on normal notification emails.
Here's the effective mailer https://github.com/publiclab/plots2/blob/master/app/mailers/comment_mailer.rb#L7-L12
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.
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, that's great! I just want to know where user
is specified -- like, do you know where this .notify()
is called from, so we can be sure it represents the recipient user instead of the comment author? I think we may want to rename the parameter a little bit to be sure of it, since user
is ambiguous, you know?
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.
perfect! OK, do you want to change the parameter name to `recipient` or do
you think we're OK here? Thanks!
…On Wed, Aug 1, 2018 at 11:03 AM Gaurav Sachdeva ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In app/views/comment_mailer/notify.html.erb
<#3163 (comment)>:
> @@ -14,6 +14,13 @@ Hi! There's been a response to a discussion you're involved in. Do NOT reply to
<hr />
+<p>Look like spam?
+ <% if @user.can_moderate? %>
here it is @jywarren <https://github.com/jywarren>
https://github.com/publiclab/plots2/blob/master/app/models/comment.rb#L124-L130
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3163 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ62OIakwucwQn6MDph4SzNkK9zGlks5uMcMJgaJpZM4VlDdx>
.
|
@jywarren I can change the name but the whole comment_mailer.rb file is using @ user. So, should I change it to recipient in notify_method only or everywhere. Thanks! |
ok, well we can also do that in a follow-up PR. OK, let's see how this does! |
Fixes #3137
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake test
fixes #0000
-style reference to original issue #@publiclab/reviewers
for help, in a comment belowIf tests do fail, click on the red
X
to learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays.
Thanks!