Migrate mailers to use parametrized attributes

Author: MrSerth

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

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

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)

app/mailers/user_mailer.rb

@@ -18,24 +18,26 @@ 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',
         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(
@@ -44,22 +46,28 @@ 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
     @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)
+
+    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

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

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('[email protected]')
@@ -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('[email protected]')
@@ -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('[email protected]')

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)