Refactor to queue messages

app/controllers/locations_controller.rb

@@ -52,12 +52,13 @@ class LocationsController < ApplicationController
       logs = reset_location_and_scheme_for_logs!
 
       flash[:notice] = deactivate_success_notice
-      LocationOrSchemeDeactivationMailer.new.send_deactivation_mails(
+      logs.group_by(&:created_by).transform_values(&:count).compact.each do |user, count|
-        logs,
+        LocationOrSchemeDeactivationMailer.send_deactivation_mail(user,
+                                                                  count,
                                                                   url_for(controller: "lettings_logs", action: "update_logs"),
                                                                   @location.scheme.service_name,
-        @location.postcode,
+                                                                  @location.postcode).deliver_later
-      )
+      end
     end
     redirect_to scheme_location_path(@scheme, @location)
   end

app/controllers/schemes_controller.rb

@@ -53,11 +53,12 @@ class SchemesController < ApplicationController
       logs = reset_location_and_scheme_for_logs!
 
       flash[:notice] = deactivate_success_notice
-      LocationOrSchemeDeactivationMailer.new.send_deactivation_mails(
+      logs.group_by(&:created_by).transform_values(&:count).compact.each do |user, count|
-        logs,
+        LocationOrSchemeDeactivationMailer.send_deactivation_mail(user,
+                                                                  count,
                                                                   url_for(controller: "lettings_logs", action: "update_logs"),
-        @scheme.service_name,
+                                                                  @scheme.service_name).deliver_later
-      )
+      end
     end
     redirect_to scheme_details_path(@scheme)
   end

app/mailers/location_or_scheme_deactivation_mailer.rb

@@ -14,12 +14,6 @@ class LocationOrSchemeDeactivationMailer < NotifyMailer
     )
   end
 
-  def send_deactivation_mails(logs, update_logs_url, scheme_name, postcode = nil)
-    logs.group_by(&:created_by).transform_values(&:count).compact.each do |user, count|
-      send_deactivation_mail(user, count, update_logs_url, scheme_name, postcode)
-    end
-  end
-
 private
 
   def description(scheme_name, postcode)

app/mailers/notify_mailer.rb

@@ -1,4 +1,4 @@
-class NotifyMailer
+class NotifyMailer < ApplicationMailer
   require "notifications/client"
 
   def notify_client

spec/requests/locations_controller_spec.rb

@@ -1338,19 +1338,27 @@ RSpec.describe LocationsController, type: :request do
         let(:params) { { deactivation_date:, confirm: true, deactivation_date_type: "other" } }
         let(:mailer) { instance_double(LocationOrSchemeDeactivationMailer) }
 
+        let(:user_a) { FactoryBot.create(:user, email: "user_a@example.com") }
+        let(:user_b) { FactoryBot.create(:user, email: "user_b@example.com") }
+
         before do
-          allow(LocationOrSchemeDeactivationMailer).to receive(:new).and_return(mailer)
+          FactoryBot.create_list(:lettings_log, 1, :sh, location:, scheme:, startdate:, created_by: user_a)
-          allow(mailer).to receive(:send_deactivation_mails)
+          FactoryBot.create_list(:lettings_log, 3, :sh, location:, scheme:, startdate:, created_by: user_b)
+          allow(LocationOrSchemeDeactivationMailer).to receive_message_chain(:send_deactivation_mail, :deliver_later).and_return(true)
 
           Timecop.freeze(Time.utc(2022, 10, 10))
           sign_in user
-          patch "/schemes/#{scheme.id}/locations/#{location.id}/deactivate", params:
         end
 
         after do
           Timecop.unfreeze
         end
 
+        context "and a log startdate is after location deactivation date" do
+          before do
+            patch "/schemes/#{scheme.id}/locations/#{location.id}/deactivate", params:
+          end
+
           it "updates existing location with valid deactivation date and renders location page" do
             follow_redirect!
             expect(response).to have_http_status(:ok)
@@ -1360,7 +1368,6 @@ RSpec.describe LocationsController, type: :request do
             expect(location.location_deactivation_periods.first.deactivation_date).to eq(deactivation_date)
           end
 
-        context "and a log startdate is after location deactivation date" do
           it "clears the location and scheme answers" do
             expect(lettings_log.location).to eq(location)
             expect(lettings_log.scheme).to eq(scheme)
@@ -1374,9 +1381,23 @@ RSpec.describe LocationsController, type: :request do
             lettings_log.reload
             expect(lettings_log.unresolved).to eq(true)
           end
+        end
 
-          it "sends update E-mails for affected logs" do
+        context "and the users need to be notified" do
-            expect(mailer).to have_received(:send_deactivation_mails).with([lettings_log], "http://www.example.com/lettings-logs/update-logs", scheme.service_name, location.postcode)
+          it "sends E-mails to the creators of affected logs with counts" do
+            expect(LocationOrSchemeDeactivationMailer).to receive(:send_deactivation_mail).with(user_a,
+                                                                                                1,
+                                                                                                url_for(controller: "lettings_logs", action: "update_logs"),
+                                                                                                location.scheme.service_name,
+                                                                                                location.postcode)
+
+            expect(LocationOrSchemeDeactivationMailer).to receive(:send_deactivation_mail).with(user_b,
+                                                                                                3,
+                                                                                                url_for(controller: "lettings_logs", action: "update_logs"),
+                                                                                                location.scheme.service_name,
+                                                                                                location.postcode)
+
+            patch "/schemes/#{scheme.id}/locations/#{location.id}/deactivate", params:
           end
         end
 

spec/requests/schemes_controller_spec.rb

@@ -1791,7 +1791,7 @@ RSpec.describe SchemesController, type: :request do
       let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation, created_at: Time.zone.today) }
       let!(:location) { FactoryBot.create(:location, scheme:) }
       let(:deactivation_date) { Time.utc(2022, 10, 10) }
-      let!(:lettings_log) { FactoryBot.create(:lettings_log, :sh, location:, scheme:, startdate:, owning_organisation: user.organisation) }
+      let!(:lettings_log) { FactoryBot.create(:lettings_log, :sh, location:, scheme:, startdate:, owning_organisation: user.organisation, created_by: user) }
       let(:startdate) { Time.utc(2022, 10, 11) }
       let(:setup_schemes) { nil }
 
@@ -1865,18 +1865,21 @@ RSpec.describe SchemesController, type: :request do
         let(:mailer) { instance_double(LocationOrSchemeDeactivationMailer) }
 
         before do
-          allow(LocationOrSchemeDeactivationMailer).to receive(:new).and_return(mailer)
+          allow(LocationOrSchemeDeactivationMailer).to receive_message_chain(:send_deactivation_mail, :deliver_later).and_return(true)
-          allow(mailer).to receive(:send_deactivation_mails)
 
           Timecop.freeze(Time.utc(2022, 10, 10))
           sign_in user
-          patch "/schemes/#{scheme.id}/deactivate", params:
         end
 
         after do
           Timecop.unfreeze
         end
 
+        context "and a log startdate is after scheme deactivation date" do
+          before do
+            patch "/schemes/#{scheme.id}/deactivate", params:
+          end
+
           it "updates existing scheme with valid deactivation date and renders scheme page" do
             follow_redirect!
             follow_redirect!
@@ -1887,7 +1890,6 @@ RSpec.describe SchemesController, type: :request do
             expect(scheme.scheme_deactivation_periods.first.deactivation_date).to eq(deactivation_date)
           end
 
-        context "and a log startdate is after scheme deactivation date" do
           it "clears the scheme and scheme answers" do
             expect(lettings_log.scheme).to eq(scheme)
             expect(lettings_log.scheme).to eq(scheme)
@@ -1901,10 +1903,6 @@ RSpec.describe SchemesController, type: :request do
             lettings_log.reload
             expect(lettings_log.unresolved).to eq(true)
           end
-
-          it "sends update E-mails for affected logs" do
-            expect(mailer).to have_received(:send_deactivation_mails).with([lettings_log], "http://www.example.com/lettings-logs/update-logs", scheme.service_name)
-          end
         end
 
         context "and a log startdate is before scheme deactivation date" do
@@ -1924,6 +1922,17 @@ RSpec.describe SchemesController, type: :request do
             expect(lettings_log.unresolved).to eq(nil)
           end
         end
+
+        context "and the users need to be notified" do
+          it "sends E-mails to the creators of affected logs with counts" do
+            expect(LocationOrSchemeDeactivationMailer).to receive(:send_deactivation_mail).with(user,
+                                                                                                1,
+                                                                                                url_for(controller: "lettings_logs", action: "update_logs"),
+                                                                                                scheme.service_name)
+
+            patch "/schemes/#{scheme.id}/deactivate", params:
+          end
+        end
       end
 
       context "when the date is not selected" do