Skip to content
Merged
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
4 changes: 2 additions & 2 deletions app/controllers/comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@

def send_mail_to_author(comment, request_for_comment)
if current_user != request_for_comment.user
UserMailer.got_new_comment(comment, request_for_comment, current_user).deliver_later
UserMailer.with(comment:, request_for_comment:, commenting_user: current_user).got_new_comment.deliver_later

Check warning on line 91 in app/controllers/comments_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/comments_controller.rb#L91

Added line #L91 was not covered by tests
end
end

Expand All @@ -101,7 +101,7 @@
)
subscriptions.each do |subscription|
if (((subscription.subscription_type == 'author') && (current_user == request_for_comment.user)) || (subscription.subscription_type == 'all')) && !((subscription.user == current_user) || already_sent_mail)
UserMailer.got_new_comment_for_subscription(comment, subscription, current_user).deliver_later
UserMailer.with(comment:, subscription:, from_user: current_user).got_new_comment_for_subscription.deliver_later

Check warning on line 104 in app/controllers/comments_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/comments_controller.rb#L104

Added line #L104 was not covered by tests
already_sent_mail = true
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/request_for_comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@
@request_for_comment.thank_you_note = params[:note]

commenters = @request_for_comment.commenters
commenters.each {|commenter| UserMailer.send_thank_you_note(@request_for_comment, commenter).deliver_later }
commenters.each {|commenter| UserMailer.with(request_for_comment: @request_for_comment, receiver: commenter).send_thank_you_note.deliver_later }

Check warning on line 107 in app/controllers/request_for_comments_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/request_for_comments_controller.rb#L107

Added line #L107 was not covered by tests

respond_to do |format|
if @request_for_comment.save
Expand Down
4 changes: 2 additions & 2 deletions app/jobs/detect_exercise_anomalies_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@

def notify_collection_author(collection, anomalies)
log("Sending E-Mail to author (#{collection.user.displayname} <#{collection.user.email}>)...", 2)
UserMailer.exercise_anomaly_detected(collection, anomalies).deliver_later
UserMailer.with(exercise_collection: collection, anomalies:).exercise_anomaly_detected.deliver_later

Check warning on line 106 in app/jobs/detect_exercise_anomalies_job.rb

View check run for this annotation

Codecov / codecov/patch

app/jobs/detect_exercise_anomalies_job.rb#L106

Added line #L106 was not covered by tests
end

def notify_contributors(collection, anomalies)
Expand Down Expand Up @@ -145,7 +145,7 @@
token = AuthenticationToken.generate!(user, last_submission.study_group).shared_secret
feedback_link = Rails.application.routes.url_helpers.url_for(action: :new,
controller: :user_exercise_feedbacks, exercise_id: exercise.id, host:, token:)
UserMailer.exercise_anomaly_needs_feedback(user, exercise, feedback_link).deliver
UserMailer.with(user:, exercise:, link: feedback_link).exercise_anomaly_needs_feedback.deliver_later

Check warning on line 148 in app/jobs/detect_exercise_anomalies_job.rb

View check run for this annotation

Codecov / codecov/patch

app/jobs/detect_exercise_anomalies_job.rb#L148

Added line #L148 was not covered by tests
end
end
log("Asked #{contributors_to_notify.size} contributors for feedback.", 2)
Expand Down
41 changes: 24 additions & 17 deletions app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,26 @@
mail(subject: t('mailers.user_mailer.reset_password.subject'), to: user.email)
end

def got_new_comment(comment, request_for_comment, commenting_user)
def got_new_comment
# TODO: check whether we can take the last known locale of the receiver?
request_for_comment = params.fetch(:request_for_comment)
token = AuthenticationToken.generate!(request_for_comment.user, request_for_comment.submission.study_group)
@receiver_displayname = request_for_comment.user.displayname
@commenting_user_displayname = commenting_user.displayname
@comment_text = ERB::Util.html_escape comment.text
@commenting_user_displayname = params.fetch(:commenting_user).displayname
@comment_text = ERB::Util.html_escape params.fetch(:comment).text
@rfc_link = request_for_comment_url(request_for_comment, token: token.shared_secret)
mail(
subject: t('mailers.user_mailer.got_new_comment.subject',
commenting_user_displayname: @commenting_user_displayname), to: request_for_comment.user.email
)
end

def got_new_comment_for_subscription(comment, subscription, from_user)
def got_new_comment_for_subscription
subscription = params.fetch(:subscription)
token = AuthenticationToken.generate!(subscription.user, subscription.study_group)
@receiver_displayname = subscription.user.displayname
@author_displayname = from_user.displayname
@comment_text = ERB::Util.html_escape comment.text
@author_displayname = params.fetch(:from_user).displayname
@comment_text = ERB::Util.html_escape params.fetch(:comment).text
@rfc_link = request_for_comment_url(subscription.request_for_comment, token: token.shared_secret)
@unsubscribe_link = unsubscribe_subscription_url(subscription)
mail(
Expand All @@ -44,22 +46,28 @@
)
end

def send_thank_you_note(request_for_comment, receiver)
def send_thank_you_note
request_for_comment = params.fetch(:request_for_comment)
receiver = params.fetch(:receiver)
token = AuthenticationToken.generate!(receiver, request_for_comment.submission.study_group)

@receiver_displayname = receiver.displayname
@author = request_for_comment.user.displayname
@thank_you_note = ERB::Util.html_escape request_for_comment.thank_you_note
@rfc_link = request_for_comment_url(request_for_comment, token: token.shared_secret)
mail(subject: t('mailers.user_mailer.send_thank_you_note.subject', author: @author), to: receiver.email)
end

def exercise_anomaly_detected(exercise_collection, anomalies)
def exercise_anomaly_detected
# First, we try to find the best matching study group for the user being notified.
# The study group is relevant, since it determines the access rights to the exercise within the collection.
# The best matching study group is the one that grants access to the most exercises in the collection.
# This approach might look complicated, but since it is called from a Rake task and no active user session, we need it.
@relevant_exercises = Exercise.where(id: anomalies.keys)
potential_study_groups = exercise_collection.user.study_groups.where(study_group_memberships: {role: StudyGroupMembership.roles[:teacher]})
@collection = params.fetch(:exercise_collection)
@anomalies = params.fetch(:anomalies)
@relevant_exercises = Exercise.where(id: @anomalies.keys)

Check warning on line 68 in app/mailers/user_mailer.rb

View check run for this annotation

Codecov / codecov/patch

app/mailers/user_mailer.rb#L66-L68

Added lines #L66 - L68 were not covered by tests

potential_study_groups = @collection.user.study_groups.where(study_group_memberships: {role: StudyGroupMembership.roles[:teacher]})

Check warning on line 70 in app/mailers/user_mailer.rb

View check run for this annotation

Codecov / codecov/patch

app/mailers/user_mailer.rb#L70

Added line #L70 was not covered by tests
potential_study_groups_with_expected_access = potential_study_groups.to_h do |study_group|
exercises_granting_access = @relevant_exercises.count do |exercise|
author_study_groups = exercise.author.study_groups.where(study_group_memberships: {role: StudyGroupMembership.roles[:teacher]})
Expand All @@ -70,18 +78,17 @@
best_matching_study_group = potential_study_groups_with_expected_access.max_by {|_study_group, exercises_granting_access| exercises_granting_access }.first

# Second, all relevant values are passed to the view
@user = exercise_collection.user
@user = @collection.user

Check warning on line 81 in app/mailers/user_mailer.rb

View check run for this annotation

Codecov / codecov/patch

app/mailers/user_mailer.rb#L81

Added line #L81 was not covered by tests
@receiver_displayname = @user.displayname
@token = AuthenticationToken.generate!(@user, best_matching_study_group).shared_secret
@collection = exercise_collection
@anomalies = anomalies
mail(subject: t('mailers.user_mailer.exercise_anomaly_detected.subject'), to: exercise_collection.user.email)
mail(subject: t('mailers.user_mailer.exercise_anomaly_detected.subject'), to: @user.email)

Check warning on line 84 in app/mailers/user_mailer.rb

View check run for this annotation

Codecov / codecov/patch

app/mailers/user_mailer.rb#L84

Added line #L84 was not covered by tests
end

def exercise_anomaly_needs_feedback(user, exercise, link)
def exercise_anomaly_needs_feedback
user = params.fetch(:user)

Check warning on line 88 in app/mailers/user_mailer.rb

View check run for this annotation

Codecov / codecov/patch

app/mailers/user_mailer.rb#L88

Added line #L88 was not covered by tests
@receiver_displayname = user.displayname
@exercise_title = exercise.title
@link = link
@exercise_title = params.fetch(:exercise).title
@link = params.fetch(:link)

Check warning on line 91 in app/mailers/user_mailer.rb

View check run for this annotation

Codecov / codecov/patch

app/mailers/user_mailer.rb#L90-L91

Added lines #L90 - L91 were not covered by tests
mail(subject: t('mailers.user_mailer.exercise_anomaly_needs_feedback.subject'), to: user.email)
end
end
2 changes: 1 addition & 1 deletion spec/mailers/previews/user_mailer_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ class UserMailerPreview < ActionMailer::Preview
def exercise_anomaly_detected
collection = ExerciseCollection.new(name: 'Hello World', user: FactoryBot.build(:admin))
anomalies = {49 => 879.325828, 51 => 924.870057, 31 => 1031.21233, 69 => 2159.182116}
UserMailer.exercise_anomaly_detected(collection, anomalies)
UserMailer.with(exercise_collection: collection, anomalies:).exercise_anomaly_detected.deliver_later
end
end
6 changes: 3 additions & 3 deletions spec/mailers/user_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
let(:token) { AuthenticationToken.find_by(user:) }
let(:request_for_comment) { create(:rfc_with_comment, user:) }
let(:commenting_user) { InternalUser.create(attributes_for(:teacher)) }
let(:mail) { described_class.got_new_comment(request_for_comment.comments.first, request_for_comment, commenting_user).deliver_now }
let(:mail) { described_class.with(comment: request_for_comment.comments.first, request_for_comment:, commenting_user:).got_new_comment.deliver_now }

it 'sets the correct sender' do
expect(mail.from).to include('codeocean@openhpi.de')
Expand Down Expand Up @@ -120,7 +120,7 @@
let(:request_for_comment) { create(:rfc_with_comment, user:) }
let(:subscription) { Subscription.create(request_for_comment:, user:, study_group_id: user.current_study_group_id) }
let(:from_user) { InternalUser.create(attributes_for(:teacher)) }
let(:mail) { described_class.got_new_comment_for_subscription(request_for_comment.comments.first, subscription, from_user).deliver_now }
let(:mail) { described_class.with(comment: request_for_comment.comments.first, subscription:, from_user:).got_new_comment_for_subscription.deliver_now }

it 'sets the correct sender' do
expect(mail.from).to include('codeocean@openhpi.de')
Expand Down Expand Up @@ -173,7 +173,7 @@
let(:receiver) { create(:teacher) }
let(:token) { AuthenticationToken.find_by(user: receiver) }
let(:request_for_comment) { create(:rfc_with_comment, user:) }
let(:mail) { described_class.send_thank_you_note(request_for_comment, receiver).deliver_now }
let(:mail) { described_class.with(request_for_comment:, receiver:).send_thank_you_note.deliver_now }

it 'sets the correct sender' do
expect(mail.from).to include('codeocean@openhpi.de')
Expand Down
6 changes: 3 additions & 3 deletions spec/system/authentication_system_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
let(:study_group) { request_for_comment.submission.study_group }
let(:request_for_comment) { create(:rfc_with_comment, user:) }
let(:commenting_user) { InternalUser.create(attributes_for(:teacher)) }
let(:mail) { UserMailer.got_new_comment(request_for_comment.comments.first, request_for_comment, commenting_user) }
let(:mail) { UserMailer.with(comment: request_for_comment.comments.first, request_for_comment:, commenting_user:).got_new_comment }
let(:rfc_link) { request_for_comment_url(request_for_comment, token: token.shared_secret) }

before { allow(AuthenticationToken).to receive(:generate!).with(user, study_group).and_return(token).once }
Expand Down Expand Up @@ -143,12 +143,12 @@
let(:request_for_comment) { create(:rfc_with_comment, user:) }
let(:study_group) { request_for_comment.submission.study_group }
let(:commenting_user) { InternalUser.create(attributes_for(:teacher)) }
let(:mail) { UserMailer.got_new_comment(request_for_comment.comments.first, request_for_comment, commenting_user) }
let(:mail) { UserMailer.with(comment: request_for_comment.comments.first, request_for_comment:, commenting_user:).got_new_comment }
let(:rfc_link) { request_for_comment_url(request_for_comment, token: token.shared_secret) }

it 'still invalidates the token on login' do
token = create(:authentication_token, user:, study_group:)
mail = UserMailer.got_new_comment(request_for_comment.comments.first, request_for_comment, commenting_user)
mail = UserMailer.with(comment: request_for_comment.comments.first, request_for_comment:, commenting_user:).got_new_comment
mail.deliver_now
visit(request_for_comment_url(request_for_comment, token: token.shared_secret))
expect(token.reload.expire_at).to be_within(10.seconds).of(Time.zone.now)
Expand Down