Skip to content

Commit

Permalink
Improve BlockedUserAlert emails
Browse files Browse the repository at this point in the history
- a new mailer + new template
- a new method in BlockUserAlert to mail to a set of concerned people
  which replace the current one in blocked_user_monitor.rb
- the corresponding spec
- new line in preview mailer
  • Loading branch information
cyrillefr committed May 2, 2024
1 parent fa24255 commit df6c6f8
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 11 deletions.
28 changes: 28 additions & 0 deletions app/mailers/blocked_user_alert_mailer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# frozen_string_literal: true

class BlockedUserAlertMailer < ApplicationMailer
def self.send_mails_to_concerned(alert)
return unless Features.email?
email(alert).deliver_now
end

def email(alert)
@alert = alert
set_recipients
return if @recipients.empty?
params = { to: @recipients,
subject: @alert.main_subject }
params[:reply_to] = @alert.reply_to unless @alert.reply_to.nil?
mail(params)
end

private

def set_recipients
@course = @alert.course
@user = @alert.user
@recipients = (@course.instructors.pluck(:email) +
@course.nonstudents.where(greeter: true).pluck(:email)) <<
@user.email
end
end
6 changes: 6 additions & 0 deletions app/models/alert_types/blocked_user_alert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,10 @@ def main_subject
def url
"https://en.wikipedia.org/wiki/Special:Log?type=block&user=&page=User%3A#{user.url_encoded_username}&wpdate=&tagfilter=&subtype="
end

def send_mails_to_concerned
BlockedUserAlertMailer.send_mails_to_concerned(self)
return if emails_disabled?
update(email_sent_at: Time.zone.now)
end
end
35 changes: 35 additions & 0 deletions app/views/blocked_user_alert_mailer/email.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
%link{rel: 'stylesheet', href:'/mailer.css'}
%p.paragraph
Please investigate:
%a.link{ href: @alert.url }= @alert.main_subject

- if @alert.user_contributions_url
%p.paragraph
%a.link{href: @alert.user_contributions_url}= "#{@alert.user.username} contributions"

- if @alert.message
%p.paragraph.preserve-whitespace
= "Message regarding #{@alert.user.username}:"
= @alert.message

%p.paragraph
User email:
= @alert.user.email

%p.paragraph
User real name:
= @alert.user.real_name

- if @alert.article
%p.paragraph
Article:
%a.link{href: @alert.article.url}= @alert.article.title

- if @alert.revision
%p.paragraph
%a.link{href: @alert.revision.url} diff

- if @alert.details
%p.paragraph.preserve-whitespace
Alert details:
= @alert.details
5 changes: 2 additions & 3 deletions lib/alerts/blocked_user_monitor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ def alert_exists?(block, user)
end

def create_alert_and_send_email(block, user)
BlockedUserAlert
.create(user:, details: block, course: user.courses.last)
.email_content_expert
alert = BlockedUserAlert.create(user:, details: block, course: user.courses.last)
alert.send_mails_to_concerned
end
end
39 changes: 31 additions & 8 deletions spec/lib/alerts/blocked_user_monitor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,48 @@

describe BlockedUserMonitor do
describe '.create_alerts_for_recently_blocked_users' do
let(:user) { create(:user, username: 'Verdantpowerinc') }
let(:course) { create(:course) }
let(:user) { create(:user, username: 'Verdantpowerinc', email: '[email protected]') }
let(:instructor_1) { create(:user, username: 'Instructor1', email: '[email protected]') }
let(:instructor_2) { create(:user, username: 'Instructor2', email: '[email protected]') }
let(:staff) { create(:user, username: 'staff', email: '[email protected]', greeter: true) }
let(:course) do
now = Time.zone.now
create(:course, start: now.days_ago(7), end: now.days_since(7))
end

before do
create(:courses_user, user:, course:)
create(:courses_user, course:, user: instructor_1,
role: CoursesUsers::Roles::INSTRUCTOR_ROLE)
create(:courses_user, course:, user: instructor_2,
role: CoursesUsers::Roles::INSTRUCTOR_ROLE)
create(:courses_user, course:, user: staff,
role: CoursesUsers::Roles::WIKI_ED_STAFF_ROLE)
stub_block_log_query
end

it 'creates an Alert record for a blocked user' do
expect(Alert.count).to eq(0)
described_class.create_alerts_for_recently_blocked_users
expect(Alert.count).to eq(1)
expect { described_class.create_alerts_for_recently_blocked_users }
.to change(BlockedUserAlert, :count).by(1)
end

it 'does not create multiple alerts for the same block' do
expect(Alert.count).to eq(0)
described_class.create_alerts_for_recently_blocked_users
expect do
2.times { described_class.create_alerts_for_recently_blocked_users }
end.to change(BlockedUserAlert, :count).by(1)
end

it 'uses the proper mailer' do
expect(BlockedUserAlertMailer).to receive(:send_mails_to_concerned)
described_class.create_alerts_for_recently_blocked_users
expect(Alert.count).to eq(1)
end

it 'sends a mail to staff, instructors & student' do
expect do
described_class.create_alerts_for_recently_blocked_users
end.to change { BlockedUserAlertMailer.deliveries.count }.by(1)
msg = BlockedUserAlertMailer.deliveries.first
expect(msg.to).to match_array([instructor_1, instructor_2, user, staff].map(&:email))
end
end
end
17 changes: 17 additions & 0 deletions spec/mailers/previews/alert_mailer_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ def blocked_edits_alert
AlertMailer.alert(example_blocked_edits_alert, example_user)
end

def blocked_student_alert
BlockedUserAlertMailer.email(example_user_blocked_alert)
end

def check_timeline_alert
AlertMailer.alert(example_alert(type: 'CheckTimelineAlert'), example_user)
end
Expand Down Expand Up @@ -136,4 +140,17 @@ def example_blocked_edits_alert
user: example_user,
details:)
end

def example_user_blocked_alert
user = example_user
message = "Student #{user.username} have been blocked on Wikipedia.
This mail to inform staff, student as well as instructors."
alert = Alert.new(type: 'BlockedUserAlert', user:,
course: example_course, message:)
alert.tap do |alrt|
alrt.define_singleton_method(:main_subject) do
"User #{user.username} have been blocked on Wikipedia"
end
end
end
end

0 comments on commit df6c6f8

Please sign in to comment.