Skip to content

Digest for Unmoderated Notes #7987

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/controllers/admin_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,11 @@ def queue
end
end

def test_digest_email_spam
DigestSpamJob.perform_async(0)
Copy link
Member

Choose a reason for hiding this comment

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

What does perform_async do here? Does it send the email in the background?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, It is just for testing. It performs Job and creates Digest on daily basis. perform_async gets pushed to the Queue right away. 0 is for daily digest and 1 is for a weekly digest

redirect_to "/spam"
end

def smtp_test
require 'socket'

Expand Down
15 changes: 15 additions & 0 deletions app/jobs/digest_spam_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class DigestSpamJob
include Sidekiq::Worker
def perform(frequency_digest)
Copy link
Member

Choose a reason for hiding this comment

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

Later on we could maybe add a way for moderators to customize the frequency of their digest so that only those who want it will receive theirs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now it just checks for tags digest:daily, digest:weekly, and if any of them is present then digest is sent. rn it tasks whatever is the settings of the moderator in the notification settings page. I will add more customization for moderators specifically for spam digest like frequency, type, and status (By tagging different tag like spam_digest:daily)
I was thinking of doing this on /spam2 and make the settings page there. But as of now, it is not merged. Should I add it to the current notification settings page for moderators??

if frequency_digest.zero?
tag_digest = 'digest:daily'
elsif frequency_digest == 1
tag_digest = 'digest:weekly'
end
users = User.where(role: %w(moderator admin))
.includes(:user_tags)
.where('user_tags.value=?', tag_digest)
.references(:user_tags)
users.each(&:send_digest_email_spam)
end
end
11 changes: 11 additions & 0 deletions app/mailers/admin_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,4 +120,15 @@ def notify_moderators_of_spam(node, moderator)
subject: subject
)
end

def send_digest_spam(nodes, frequency_digest)
if frequency_digest == User::Frequency::DAILY
@subject = 'Your daily digest for moderation'
elsif frequency_digest == User::Frequency::WEEKLY
@subject = 'Your weekly digest for moderation'
end
moderators = User.where(role: %w(moderator admin)).collect(&:email)
@nodes = nodes
mail(to: moderators, subject: @subject)
end
end
30 changes: 30 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,23 @@ def content_followed_in_period(start_time, end_time, node_type = 'note', include
.distinct
end

def unmoderated_in_period(start_time, end_time)
tag_following = TagSelection.where(following: true, user_id: uid)
ids = []
tag_following.each do |tagname|
ids += NodeTag.where(tid: tagname.tid).collect(&:nid)
end
range = "(created >= #{start_time.to_i} AND created <= #{end_time.to_i})"
Node.where(nid: ids)
.includes(:revision, :tag)
.references(:node_revision)
.where('node.status = 4')
.where(type: 'note')
.where(range)
.order('node_revisions.timestamp DESC')
.distinct
end

def social_link(site)
return nil unless has_power_tag(site)

Expand Down Expand Up @@ -356,6 +373,19 @@ def send_digest_email
end
end

def send_digest_email_spam
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add tests for these model methods?
Everything else looks good...great job 🎉

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I will add it soon. Thanks!!

if has_tag('digest:weekly')
@frequency_digest = Frequency::WEEKLY
@nodes_unmoderated = unmoderated_in_period(1.week.ago, Time.current)
else
@frequency_digest = Frequency::DAILY
@nodes_unmoderated = unmoderated_in_period(1.day.ago, Time.current)
end
if @nodes_unmoderated.size.positive?
AdminMailer.send_digest_spam(@nodes_unmoderated, @frequency_digest).deliver_now
end
end

def tag_counts
tags = {}
Node.order('nid DESC').where(type: 'note', status: 1, uid: id).limit(20).each do |node|
Expand Down
39 changes: 39 additions & 0 deletions app/views/admin_mailer/send_digest_spam.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<header style="margin:0 auto; text-align:center; width:100%;">
<img src="https://avatars2.githubusercontent.com/u/4621650?s=200&v=4">
<p style="font-size:1.5em; color:blue;">[Public Lab] <%= @subject %></p>
<p style="font-size:1em; color:blue;"><%= Time.now.strftime("%B %-d %Y") %></p>
<p style="font-size:1em; font-family:sans-serif; margin-bottom:0; color:grey;">Unmoderated Notes at <a href="https://publiclab.org" style="color:green; text-decoration: none;">PublicLab.org</a></p>
<hr style="height:1px; background-color:black; width:50%; background-color:green;">
</header>

<ul style="list-style-type:none; height:auto; font-family:sans-serif;width:auto;">
<% @nodes.each do |n| %>
<li style="position:relative; height:auto; width:90vw;">
<div style="padding:3vh 0 0 5vw ;">
<a style="text-decoration:none; color:black; font-weight:500; font-size:1.5em; " href="<%= ActionMailer::Base.default_url_options[:host] + n.path %>"><%= n.title %></a>
</div>
<table style="padding-left:5vw; font-size:1em;width:70vw">
<tbody> <tr><td style="width:8vh;">
<% if n.author.photo? %>
<img style="width:7vh; height:7vh; border-radius:50%; padding-top:2vh" src="https://<%= ActionMailer::Base.default_url_options[:host] %><%= n.author.photo_path(:thumb) %>"/>
<% else %>
<img style="width:7vh; height:7vh;border-radius:50%;padding-top:2vh;" src="https://www.gravatar.com/avatar/1aedb8d9dc4751e229a335e371db8058"/>
<% end %>
</td>
<td style= "font-size: 2vh;">
<p style="font-weight:500; "><%= n.author.username.capitalize %> </p>
<span style="color:#999;font-weight:500;">created at <%= n.created_at.strftime("%B %-d %Y") %></span>
</td></tr>
</table>
<div style="padding-left:5vw;">
<p style="color:black; font-size:1.0em; line-height: 1.6;"><%= n.body.truncate(300) %></p>
</div>
<a style="color:green; text-decoration:none; padding-left:5vw" href="https://<%= ActionMailer::Base.default_url_options[:host] %>/moderate/publish/<%= n.id %>">Approve</a> <a style="color:red; text-decoration:none;"href="https://<%= ActionMailer::Base.default_url_options[:host] %>/moderate/spam/<%= n.id %>">Spam</a></div>
<hr style="border:none; height:1px; width: 45%; background-color:grey; margin-top:2%;">
</li>
<% end %>
</ul>
<div style="margin:5% 0 0 8vw; font-size:1em;">
<p style=" text-decoration:none;">Click <a style="color:blue; text-decoration:none;"href="https://<%= ActionMailer::Base.default_url_options[:host] %>/subscriptions">here</a> to choose your followed topics</p>
<p style=" text-decoration:none;">Click <a style="color:blue; text-decoration:none;" href="https://<%= ActionMailer::Base.default_url_options[:host] %>/settings">here</a> to change your subscription settings</p>
</div>
5 changes: 4 additions & 1 deletion app/views/users/profile.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,11 @@
<%= form_tag "/users/test_digest_email", method: :post do %>
<%= submit_tag "Test Digest Email", class: "btn btn-info", style: "width: 100%;" %>
<% end %>
<%= form_tag "/admin/test_digest_email_spam", method: :post do %>
<%= submit_tag "Test Spam Digest Email", class: "btn btn-sm btn-primary mt-2", style: "width: 100%;" %>
<% end %>
<% end %>

<% end %>

<hr />
Expand Down
3 changes: 2 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,8 @@
get 'questions/liked(/:tagnames)' => 'questions#liked'

post 'users/test_digest_email' => 'users#test_digest_email'

post 'admin/test_digest_email_spam' => 'admin#test_digest_email_spam'

get 'comment/delete/:id' => 'comment#delete'
get 'comment/update/:id' => 'comment#update'
post 'comment/update/:id' => 'comment#update'
Expand Down
2 changes: 2 additions & 0 deletions config/schedule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@

every 1.day do
runner "DigestMailJob.perform_async(0)"
runner "DigestSpamJob.perform_async(0)"
end

every 1.week do
runner "DigestMailJob.perform_async(1)"
runner "DigestSpamJob.perform_async(1)"
end
6 changes: 6 additions & 0 deletions test/functional/admin_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -638,4 +638,10 @@ def teardown
assert_response :success
assert_not_nil assigns(:users)
end

test "test digest emails to moderators" do
UserSession.create(users(:moderator))
post :test_digest_email_spam
assert_redirected_to '/spam'
end
end
3 changes: 3 additions & 0 deletions test/unit/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -305,4 +305,7 @@ class UserTest < ActiveSupport::TestCase
end
end

test 'Send digest email for unmoderated posts' do
assert users(:bob).send_digest_email_spam
Copy link
Member

Choose a reason for hiding this comment

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

Here, or in our functional tests, i wonder if we can check the content of the email itself to confirm? There are some other examples of this in our tests. It would help secure this feature against future issues!

end
end