diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index fcd6032cb..8835a8dee 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -88,7 +88,7 @@ def comment_params 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 end end @@ -101,7 +101,7 @@ def send_mail_to_subscribers(comment, request_for_comment) ) 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 already_sent_mail = true end end diff --git a/app/controllers/request_for_comments_controller.rb b/app/controllers/request_for_comments_controller.rb index 514f42ebc..19f88e567 100644 --- a/app/controllers/request_for_comments_controller.rb +++ b/app/controllers/request_for_comments_controller.rb @@ -104,7 +104,7 @@ def set_thank_you_note @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 } respond_to do |format| if @request_for_comment.save diff --git a/app/jobs/detect_exercise_anomalies_job.rb b/app/jobs/detect_exercise_anomalies_job.rb index 3adc40ab6..8120ef8dc 100644 --- a/app/jobs/detect_exercise_anomalies_job.rb +++ b/app/jobs/detect_exercise_anomalies_job.rb @@ -103,7 +103,7 @@ def get_contributor_working_times(exercise) 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 end def notify_contributors(collection, anomalies) @@ -145,7 +145,7 @@ def notify_contributors(collection, anomalies) 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 end end log("Asked #{contributors_to_notify.size} contributors for feedback.", 2) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index b988bc2ed..212cbebe7 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -18,12 +18,13 @@ def reset_password_email(user) 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', @@ -31,11 +32,12 @@ def got_new_comment(comment, request_for_comment, commenting_user) ) 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( @@ -44,8 +46,11 @@ def got_new_comment_for_subscription(comment, subscription, from_user) ) 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 @@ -53,13 +58,16 @@ def send_thank_you_note(request_for_comment, receiver) 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) + + potential_study_groups = @collection.user.study_groups.where(study_group_memberships: {role: StudyGroupMembership.roles[:teacher]}) 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]}) @@ -70,18 +78,17 @@ def exercise_anomaly_detected(exercise_collection, anomalies) 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 @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) end - def exercise_anomaly_needs_feedback(user, exercise, link) + def exercise_anomaly_needs_feedback + user = params.fetch(:user) @receiver_displayname = user.displayname - @exercise_title = exercise.title - @link = link + @exercise_title = params.fetch(:exercise).title + @link = params.fetch(:link) mail(subject: t('mailers.user_mailer.exercise_anomaly_needs_feedback.subject'), to: user.email) end end diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index be0ac48fc..dbd5f6f58 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -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 diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index b3b37f1cb..041baaa6e 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -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') @@ -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') @@ -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') diff --git a/spec/system/authentication_system_spec.rb b/spec/system/authentication_system_spec.rb index 2ab18c575..04e99f595 100644 --- a/spec/system/authentication_system_spec.rb +++ b/spec/system/authentication_system_spec.rb @@ -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 } @@ -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)