diff --git a/app/controllers/assessor_interface/assessment_sections_controller.rb b/app/controllers/assessor_interface/assessment_sections_controller.rb index a929800641..ff7fec0211 100644 --- a/app/controllers/assessor_interface/assessment_sections_controller.rb +++ b/app/controllers/assessor_interface/assessment_sections_controller.rb @@ -98,7 +98,11 @@ def notify_teacher return end - TeacherMailer.with(application_form:).initial_checks_passed.deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :initial_checks_passed, + ) end def unassign_assessor diff --git a/app/controllers/assessor_interface/consent_requests_controller.rb b/app/controllers/assessor_interface/consent_requests_controller.rb index 0da446a96e..a27c02d651 100644 --- a/app/controllers/assessor_interface/consent_requests_controller.rb +++ b/app/controllers/assessor_interface/consent_requests_controller.rb @@ -52,7 +52,11 @@ def update_request ) end - TeacherMailer.with(application_form:).consent_requested.deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :consent_requested, + ) end redirect_to [ diff --git a/app/controllers/teacher_interface/consent_requests_controller.rb b/app/controllers/teacher_interface/consent_requests_controller.rb index 57c2959793..c2f5452d9a 100644 --- a/app/controllers/teacher_interface/consent_requests_controller.rb +++ b/app/controllers/teacher_interface/consent_requests_controller.rb @@ -26,7 +26,11 @@ def submit end end - TeacherMailer.with(application_form:).consent_submitted.deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :consent_submitted, + ) redirect_to %i[teacher_interface application_form] end diff --git a/app/lib/application_mailer_observer.rb b/app/lib/application_mailer_observer.rb deleted file mode 100644 index 0c9e6455a9..0000000000 --- a/app/lib/application_mailer_observer.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -class ApplicationMailerObserver - def self.delivered_email(message) - mailer_class_name = message.try(:mailer_class_name) - mailer_action_name = message.try(:mailer_action_name) - application_form_id = message.try(:application_form_id) - message_subject = message.try(:subject) - - if mailer_class_name.blank? || mailer_action_name.blank? || - application_form_id.blank? || message_subject.blank? - return - end - - application_form = ApplicationForm.find(application_form_id) - - CreateTimelineEvent.call( - "email_sent", - application_form:, - user: "Mailer", - mailer_class_name:, - mailer_action_name:, - message_subject:, - ) - end -end - -ActionMailer::Base.register_observer(ApplicationMailerObserver) diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 0c95fd1a12..e894de7278 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ApplicationMailer < Mail::Notify::Mailer default from: I18n.t("service.email.enquiries") @@ -6,20 +8,4 @@ class ApplicationMailer < Mail::Notify::Mailer "GOVUK_NOTIFY_TEMPLATE_ID_APPLICATION", "95adafaf-0920-4623-bddc-340853c047af", ) - - after_action :store_observer_metadata - - def store_observer_metadata - mailer_class_name = self.class.name.demodulize - mailer_action_name = action_name - application_form_id = application_form.id - - message.instance_variable_set(:@mailer_class_name, mailer_class_name) - message.instance_variable_set(:@mailer_action_name, mailer_action_name) - message.instance_variable_set(:@application_form_id, application_form_id) - - message.class.send(:attr_reader, :mailer_class_name) - message.class.send(:attr_reader, :mailer_action_name) - message.class.send(:attr_reader, :application_form_id) - end end diff --git a/app/models/application_form.rb b/app/models/application_form.rb index d2dfd08094..c1bc10a0a4 100644 --- a/app/models/application_form.rb +++ b/app/models/application_form.rb @@ -272,22 +272,27 @@ def should_send_reminder_email?(name, number_of_reminders_sent) def send_reminder_email(name, number_of_reminders_sent) case name when "consent" - TeacherMailer.with(application_form: self).consent_reminder.deliver_later + DeliverEmail.call( + application_form: self, + mailer: TeacherMailer, + action: :consent_reminder, + ) when "expiration" - TeacherMailer - .with(application_form: self, number_of_reminders_sent:) - .application_not_submitted - .deliver_later + DeliverEmail.call( + application_form: self, + mailer: TeacherMailer, + action: :application_not_submitted, + number_of_reminders_sent:, + ) when "references" - TeacherMailer - .with( - application_form: self, - number_of_reminders_sent:, - reference_requests: - reference_requests_not_yet_received_or_rejected.to_a, - ) - .references_reminder - .deliver_later + DeliverEmail.call( + application_form: self, + mailer: TeacherMailer, + action: :references_reminder, + number_of_reminders_sent:, + reference_requests: + reference_requests_not_yet_received_or_rejected.to_a, + ) end end diff --git a/app/models/further_information_request.rb b/app/models/further_information_request.rb index 3dda340c02..fa67b342ef 100644 --- a/app/models/further_information_request.rb +++ b/app/models/further_information_request.rb @@ -47,10 +47,12 @@ def should_send_reminder_email?(_name, number_of_reminders_sent) end def send_reminder_email(_name, _number_of_reminders_sent) - TeacherMailer - .with(application_form:, further_information_request: self) - .further_information_reminder - .deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :further_information_reminder, + further_information_request: self, + ) end def expires_after @@ -63,10 +65,11 @@ def expires_after end def after_received(*) - TeacherMailer - .with(application_form:) - .further_information_received - .deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :further_information_received, + ) end def after_expired(user:) diff --git a/app/models/professional_standing_request.rb b/app/models/professional_standing_request.rb index 3886a1e9d1..fc1f5a208a 100644 --- a/app/models/professional_standing_request.rb +++ b/app/models/professional_standing_request.rb @@ -40,10 +40,11 @@ def expires_after def after_received(*) if should_send_received_email? - TeacherMailer - .with(application_form:) - .professional_standing_received - .deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :professional_standing_received, + ) end end diff --git a/app/models/reference_request.rb b/app/models/reference_request.rb index 0d61d0c917..27b72842a8 100644 --- a/app/models/reference_request.rb +++ b/app/models/reference_request.rb @@ -100,10 +100,13 @@ def should_send_reminder_email?(_name, number_of_reminders_sent) end def send_reminder_email(_name, number_of_reminders_sent) - RefereeMailer - .with(reference_request: self, number_of_reminders_sent:) - .reference_reminder - .deliver_later + DeliverEmail.call( + application_form:, + mailer: RefereeMailer, + action: :reference_reminder, + reference_request: self, + number_of_reminders_sent:, + ) end def expires_after diff --git a/app/services/award_qts.rb b/app/services/award_qts.rb index 84228fe071..a5c0c759e9 100644 --- a/app/services/award_qts.rb +++ b/app/services/award_qts.rb @@ -32,7 +32,11 @@ def call ApplicationFormStatusUpdater.call(application_form:, user:) end - TeacherMailer.with(application_form:).application_awarded.deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :application_awarded, + ) end class InvalidState < StandardError diff --git a/app/services/decline_qts.rb b/app/services/decline_qts.rb index d22007b70e..0393544ec5 100644 --- a/app/services/decline_qts.rb +++ b/app/services/decline_qts.rb @@ -17,7 +17,11 @@ def call CreateTimelineEvent.call("application_declined", application_form:, user:) end - TeacherMailer.with(application_form:).application_declined.deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :application_declined, + ) end private diff --git a/app/services/deliver_email.rb b/app/services/deliver_email.rb new file mode 100644 index 0000000000..f7f1dcddb1 --- /dev/null +++ b/app/services/deliver_email.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +class DeliverEmail + include ServicePattern + + def initialize(application_form:, mailer:, action:, **params) + @application_form = application_form + @mailer = mailer + @action = action + @params = params + end + + def call + create_job + create_timeline_event + end + + private + + attr_reader :application_form, :mailer, :action, :params + + def mail + @mail ||= mailer.with(application_form:, **params).send(action) + end + + def create_job + mail.deliver_later + end + + def create_timeline_event + CreateTimelineEvent.call( + "email_sent", + application_form:, + user: "Mailer", + mailer_class_name: mailer.name.demodulize, + mailer_action_name: action, + message_subject: mail.subject, + ) + end +end diff --git a/app/services/request_further_information.rb b/app/services/request_further_information.rb index 63b11d8ee6..5bedd96074 100644 --- a/app/services/request_further_information.rb +++ b/app/services/request_further_information.rb @@ -48,9 +48,10 @@ def create_and_request end def send_email - TeacherMailer - .with(application_form:) - .further_information_requested - .deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :further_information_requested, + ) end end diff --git a/app/services/submit_application_form.rb b/app/services/submit_application_form.rb index bfa0ff90a5..ddfb23d373 100644 --- a/app/services/submit_application_form.rb +++ b/app/services/submit_application_form.rb @@ -32,18 +32,27 @@ def call ApplicationFormStatusUpdater.call(application_form:, user:) end - TeacherMailer.with(application_form:).application_received.deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :application_received, + ) if !application_form.requires_preliminary_check && application_form.teaching_authority_provides_written_statement - TeacherMailer.with(application_form:).initial_checks_passed.deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :initial_checks_passed, + ) end if region.teaching_authority_requires_submission_email - TeachingAuthorityMailer - .with(application_form:) - .application_submitted - .deliver_later + DeliverEmail.call( + application_form:, + mailer: TeachingAuthorityMailer, + action: :application_submitted, + ) end FindApplicantInDQTJob.perform_later( diff --git a/app/services/update_work_history_contact.rb b/app/services/update_work_history_contact.rb index df2674e2ca..0e414690c1 100644 --- a/app/services/update_work_history_contact.rb +++ b/app/services/update_work_history_contact.rb @@ -29,11 +29,19 @@ def call end if email.present? && (reference_request = work_history.reference_request) - RefereeMailer.with(reference_request:).reference_requested.deliver_later - TeacherMailer - .with(application_form:, reference_requests: [reference_request]) - .references_requested - .deliver_later + DeliverEmail.call( + application_form:, + mailer: RefereeMailer, + action: :reference_requested, + reference_request:, + ) + + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :references_requested, + reference_requests: [reference_request], + ) end end diff --git a/app/services/verify_assessment.rb b/app/services/verify_assessment.rb index 5971b62185..5ff476d2ec 100644 --- a/app/services/verify_assessment.rb +++ b/app/services/verify_assessment.rb @@ -82,12 +82,19 @@ def create_reference_requests def send_reference_request_emails(reference_requests) reference_requests.each do |reference_request| - RefereeMailer.with(reference_request:).reference_requested.deliver_later + DeliverEmail.call( + application_form:, + mailer: RefereeMailer, + action: :reference_requested, + reference_request:, + ) end - TeacherMailer - .with(application_form:, reference_requests:) - .references_requested - .deliver_later + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :references_requested, + reference_requests:, + ) end end diff --git a/lib/tasks/application_forms.rake b/lib/tasks/application_forms.rake index 996bf7b182..567e681b44 100644 --- a/lib/tasks/application_forms.rake +++ b/lib/tasks/application_forms.rake @@ -28,10 +28,11 @@ namespace :application_forms do teacher = application_form.teacher next unless teacher.application_form == application_form - TeacherMailer - .with(application_form:) - .application_from_ineligible_country - .deliver_now + DeliverEmail.call( + application_form:, + mailer: TeacherMailer, + action: :application_from_ineligible_country, + ) end end diff --git a/spec/lib/application_mailer_observer_spec.rb b/spec/lib/application_mailer_observer_spec.rb deleted file mode 100644 index 384b453a7c..0000000000 --- a/spec/lib/application_mailer_observer_spec.rb +++ /dev/null @@ -1,57 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -RSpec.describe ApplicationMailerObserver do - shared_examples "observes mail" do - subject(:delivered_email) { described_class.delivered_email(message) } - - it "records a timeline event" do - expect { delivered_email }.to have_recorded_timeline_event( - :email_sent, - application_form:, - ) - end - - it "is called when an email is sent" do - expect(ApplicationMailerObserver).to receive(:delivered_email).with( - message, - ) - message.deliver_now - end - end - - context "with a referee mailer" do - let(:reference_request) { create(:requested_reference_request) } - let(:application_form) { reference_request.assessment.application_form } - let(:message) { RefereeMailer.with(reference_request:).reference_requested } - - include_examples "observes mail" - end - - context "with a teacher mailer" do - let(:application_form) { create(:application_form) } - let(:message) { TeacherMailer.with(application_form:).application_received } - - include_examples "observes mail" - end - - context "with a teaching authority mailer" do - let(:region) do - create(:region, teaching_authority_emails: ["authority@region.com"]) - end - let(:application_form) do - create( - :application_form, - :with_personal_information, - :with_teaching_qualification, - region:, - ) - end - let(:message) do - TeachingAuthorityMailer.with(application_form:).application_submitted - end - - include_examples "observes mail" - end -end diff --git a/spec/mailers/referee_mailer_spec.rb b/spec/mailers/referee_mailer_spec.rb index 01e845bb6f..de5aaf233e 100644 --- a/spec/mailers/referee_mailer_spec.rb +++ b/spec/mailers/referee_mailer_spec.rb @@ -60,8 +60,6 @@ ) end end - - it_behaves_like "an observable mailer", "reference_reminder" end describe "#reference_requested" do @@ -100,7 +98,5 @@ ) end end - - it_behaves_like "an observable mailer", "reference_requested" end end diff --git a/spec/mailers/teacher_mailer_spec.rb b/spec/mailers/teacher_mailer_spec.rb index edb51b76ef..cb8d9fd830 100644 --- a/spec/mailers/teacher_mailer_spec.rb +++ b/spec/mailers/teacher_mailer_spec.rb @@ -53,8 +53,6 @@ it { is_expected.to include("ABCDEF") } it { is_expected.to include("https://aytq.com") } end - - it_behaves_like "an observable mailer", "application_awarded" end describe "#application_declined" do @@ -81,8 +79,6 @@ it { is_expected.to include("abc") } it { is_expected.to include("Reason for decline") } end - - it_behaves_like "an observable mailer", "application_declined" end describe "#application_from_ineligible_country" do @@ -119,9 +115,6 @@ ) end end - - it_behaves_like "an observable mailer", - "application_from_ineligible_country" end describe "#application_not_submitted" do @@ -202,8 +195,6 @@ end end end - - it_behaves_like "an observable mailer", "application_not_submitted" end describe "#application_received" do @@ -254,8 +245,6 @@ end end end - - it_behaves_like "an observable mailer", "application_received" end describe "#consent_reminder" do @@ -300,8 +289,6 @@ is_expected.to include("If you do not send them by 12 February 2020") end end - - it_behaves_like "an observable mailer", "consent_reminder" end describe "#consent_requested" do @@ -345,8 +332,6 @@ is_expected.to include("upload these documents by 12 February 2020") end end - - it_behaves_like "an observable mailer", "consent_requested" end describe "#consent_submitted" do @@ -373,8 +358,6 @@ end it { is_expected.to include("Application reference number: abc") } end - - it_behaves_like "an observable mailer", "consent_submitted" end describe "#further_information_received" do @@ -404,8 +387,6 @@ it { is_expected.to include("Dear First Last") } it { is_expected.to include("abc") } end - - it_behaves_like "an observable mailer", "further_information_received" end describe "#further_information_requested" do @@ -447,8 +428,6 @@ end it { is_expected.to include("http://localhost:3000/teacher/sign_in") } end - - it_behaves_like "an observable mailer", "further_information_requested" end describe "#further_information_reminder" do @@ -496,8 +475,6 @@ it { is_expected.to include("abc") } it { is_expected.to include("http://localhost:3000/teacher/sign_in") } end - - it_behaves_like "an observable mailer", "further_information_reminder" end describe "#professional_standing_received" do @@ -535,8 +512,6 @@ ) end end - - it_behaves_like "an observable mailer", "professional_standing_received" end describe "#references_reminder" do @@ -611,8 +586,6 @@ end end end - - it_behaves_like "an observable mailer", "references_reminder" end describe "#references_requested" do @@ -647,7 +620,5 @@ ) end end - - it_behaves_like "an observable mailer", "references_requested" end end diff --git a/spec/mailers/teaching_authority_mailer_spec.rb b/spec/mailers/teaching_authority_mailer_spec.rb index ab79ca8dc6..1fcbedd73d 100644 --- a/spec/mailers/teaching_authority_mailer_spec.rb +++ b/spec/mailers/teaching_authority_mailer_spec.rb @@ -91,7 +91,5 @@ TRA is an executive agency, sponsored by the Department for Education. We have responsibility for the regulation of the teaching profession in England. EMAIL end - - it_behaves_like "an observable mailer", "application_submitted" end end diff --git a/spec/support/shared_examples/observable_mailer.rb b/spec/support/shared_examples/observable_mailer.rb deleted file mode 100644 index 1b9d3a0ce1..0000000000 --- a/spec/support/shared_examples/observable_mailer.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -RSpec.shared_examples_for "an observable mailer" do |expected_action_name| - describe "#application_form_id" do - subject(:application_form_id) { mail.application_form_id } - - it { is_expected.to eq(application_form.id) } - end - - describe "#mailer_action_name" do - subject(:mailer_action_name) { mail.mailer_action_name } - - it { is_expected.to eq(expected_action_name) } - end - - describe "#mailer_class_name" do - subject(:mailer_class_name) { mail.mailer_class_name } - - it { is_expected.to eq(described_class.name) } - end - - describe "#subject" do - subject(:subject) { mail.subject } - - it { is_expected.to be_present } - end -end