Mail event log, metrics and scopes (#1208)

* Job and AppplicationJob coverage

* Rework mailer jobs for postponed delivery with a mail event log

* Defend against callback error and report failure

* retest mail event callback update

* New Module recipient exclusion for sent and pending mail
Coverage 92.55%

* Fix yard doc

* Fix logging to Sentry upon delivery
Ensure only mail events only receive callbacks once
Document mail job scopes that require remedial work

* Silence failed delivery alerts in prod
Serialise Contentful module entries for background mail jobs

* Parent mailer job coverage

.env.example

@@ -8,6 +8,8 @@ DEV_GOV_ONE_TOKEN=urn:fdc:gov.uk:2022:23-random-alpha-numeric
 DEV_FIRST_NAME=First
 DEV_LAST_NAME=Last
 
+DELIVERY_QUEUE=false
+
 # Display console information
 VERBOSE=true
 # Display debugging information

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config --no-auto-gen-timestamp`
-# using RuboCop version 1.60.2.
+# using RuboCop version 1.63.5.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -11,12 +11,20 @@ Lint/ShadowedException:
   Exclude:
     - 'app/models/concerns/to_csv.rb'
 
-# Offense count: 2
+# Offense count: 1
 # Configuration parameters: AllowComments, AllowNil.
 Lint/SuppressedException:
   Exclude:
     - 'lib/tasks/cms.rake'
-    - 'spec/lib/migrate_training_spec.rb'
+
+# Offense count: 9
+RSpec/LetSetup:
+  Exclude:
+    - 'spec/jobs/complete_registration_mail_job_spec.rb'
+    - 'spec/jobs/continue_training_mail_job_spec.rb'
+    - 'spec/jobs/new_module_mail_job_spec.rb'
+    - 'spec/jobs/start_training_mail_job_spec.rb'
+    - 'spec/jobs/test_bulk_mail_job_spec.rb'
 
 # Offense count: 2
 # Configuration parameters: Database, Include.
@@ -46,12 +54,11 @@ Rails/Output:
     - 'app/services/content_integrity.rb'
     - 'app/services/dashboard.rb'
 
-# Offense count: 17
+# Offense count: 16
 # This cop supports unsafe autocorrection (--autocorrect-all).
 # Configuration parameters: AllowImplicitReturn, AllowedReceivers.
 Rails/SaveBang:
   Exclude:
-    - 'app/controllers/registrations_controller.rb'
     - 'app/controllers/training/responses_controller.rb'
     - 'app/forms/registration/setting_type_form.rb'
     - 'app/models/user.rb'

app/controllers/close_accounts_controller.rb

@@ -12,12 +12,10 @@ def reset_password
     redirect_to new_user_password_path
   end
 
+  # TODO: use deliver_later for closed accounts
   def close_account
-    current_user.send_account_closed_notification
-
-    # TODO: refactor this internal user mailer logic
-    User.new(email: Course.config.internal_mailbox).send_account_closed_internal_notification(current_user.email)
-
+    NotifyMailer.account_closed(current_user).deliver_now
+    NotifyMailer.account_closed_internal(current_user).deliver_now
     current_user.redact!
     sign_out current_user
     redirect_to user_close_account_path

app/controllers/notify_controller.rb

@@ -1,15 +1,31 @@
+# Notify only supports a single callback URL
+#
 class NotifyController < WebhookController
   # @see https://docs.notifications.service.gov.uk/ruby.html#delivery-receipts
   def update
-    user = User.find_by(email: payload['to'])
-    user.update!(notify_callback: payload) if user
-    render json: { status: 'callback received' }, status: :ok
+    if recipient
+      recipient.update!(notify_callback: payload)
+      mail_event.update!(callback: payload) if mail_event
+      render json: { status: 'callback received' }, status: :ok
+    else
+      render json: { status: 'callback received' }, status: :not_modified
+    end
   end
 
 private
 
-  # @return [String]
+  # @return [Boolean]
   def bot_token?
     request.headers['Authorization']&.include?(Rails.configuration.bot_token)
   end
+
+  # @return [User, nil]
+  def recipient
+    User.find_by(email: payload['to'])
+  end
+
+  # @return [MailEvent, nil]
+  def mail_event
+    recipient.mail_events.where(template: payload['template_id'], callback: nil).last
+  end
 end

app/controllers/user_controller.rb

@@ -1,6 +1,5 @@
 class UserController < ApplicationController
-  before_action :authenticate_registered_user!,
-                except: %i[check_email_confirmation check_email_password_reset]
+  before_action :authenticate_registered_user!
 
   def show
     track('profile_page')
@@ -56,20 +55,8 @@ def update_training_emails
     end
   end
 
-  def check_email_confirmation
-    return unless params[:ref]
-
-    @user = User.find_by(confirmation_token: params[:ref])
-  end
-
-  def check_email_password_reset; end
-
 private
 
-  # def user
-  #   @user ||= current_user
-  # end
-
   def user_params
     params.require(:user).permit(:first_name, :last_name, :postcode, :ofsted_number, :email, :setting_type, :setting_type_other, :training_emails)
   end

app/jobs/application_job.rb

@@ -22,7 +22,7 @@ def run(*)
 
   # @return [Boolean]
   def duplicate_job_queued?
-    Que.job_stats.any? { |job| job[:job_class] == self.class.name && job[:count] > 1 }
+    Job.by_job_class(self.class.name).count > 1
   end
 
   # @param error [Error]

app/jobs/complete_registration_mail_job.rb

@@ -1,7 +1,9 @@
 class CompleteRegistrationMailJob < MailJob
   def run
     super do
-      self.class.recipients.each(&:send_complete_registration_notification)
+      self.class.recipients.find_each do |user|
+        prepare_message(user)
+      end
     end
   end
 end

app/jobs/continue_training_mail_job.rb

@@ -1,10 +1,10 @@
 class ContinueTrainingMailJob < MailJob
   def run
     super do
-      self.class.recipients.each do |recipient|
-        recipient.modules_in_progress.each do |mod_name|
+      self.class.recipients.find_each do |user|
+        user.modules_in_progress.each do |mod_name|
           mod = Training::Module.by_name(mod_name)
-          recipient.send_continue_training_notification(mod)
+          prepare_message(user, mod)
         end
       end
     end

app/jobs/mail_job.rb

@@ -1,13 +1,40 @@
+# Base class for NotifyMailer jobs
+#
 class MailJob < ApplicationJob
   # @return [Array<User>]
   def self.recipients
     scope_name = "#{name.underscore}_recipients"
     User.send(scope_name)
   end
 
+  # @return [Symbol]
+  def self.template
+    name.underscore.delete_suffix('_mail_job').to_sym
+  end
+
+  # @return [String]
+  def self.template_id
+    NotifyMailer::TEMPLATE_IDS[template]
+  end
+
+  # @return [Boolean]
+  def self.enqueue?
+    Types::Params::Bool[ENV.fetch('DELIVERY_QUEUE', Rails.env.production?)]
+  end
+
   def run(*)
     log "#{self.class.recipients.count} recipients"
 
     super
   end
+
+private
+
+  # @note Mailer arguments must be able to be serialised for delayed delivery
+  # @param args [Array]
+  # @return [Mail::Message, ActionMailer::MailDeliveryJob]
+  def prepare_message(*args)
+    message = NotifyMailer.send(self.class.template, *args)
+    self.class.enqueue? ? message.deliver_later : message.deliver_now
+  end
 end

app/jobs/new_module_mail_job.rb

@@ -12,13 +12,19 @@ def run(release_id)
       Training::Module.reset_cache_key!
       log "cache key #{Training::Module.cache_key}"
 
+      begin
+        release = Release.find(release_id)
+      rescue ActiveRecord::RecordNotFound
+        return :no_new_module_release if release.blank?
+      end
+
       return :no_new_module unless new_module_published?
 
-      self.class.recipients.find_each do |recipient|
-        recipient.send_new_module_notification(latest_module)
+      self.class.recipients.find_each do |user|
+        prepare_message(user, latest_module)
       end
 
-      record_module_release latest_module, Release.find(release_id)
+      record_module_release latest_module, release
     end
   end
 

app/jobs/start_training_mail_job.rb

@@ -1,7 +1,9 @@
 class StartTrainingMailJob < MailJob
   def run
     super do
-      self.class.recipients.each(&:send_start_training_notification)
+      self.class.recipients.find_each do |user|
+        prepare_message(user)
+      end
     end
   end
 end

app/jobs/test_bulk_mail_job.rb

@@ -1,23 +1,14 @@
 # Simple background job using a dummy template
 class TestBulkMailJob < MailJob
+  # @note guard clause prevents testing against production data
+  #
   def run
     expire if Rails.application.live?
 
     super do
-      self.class.recipients.find_each do |recipient|
-        recipient.test_email
-
-        # TODO: user record log of mail activity?
-        # recipient.update!(notify_log:)
-        # where("notify_callback -> 'template_id' ?", template_id)
+      self.class.recipients.find_each do |user|
+        prepare_message(user)
       end
     end
-
-    private
-
-    # @return [String]
-    def template_id
-      NotifyMailer::TEMPLATE_IDS[:bulk_test]
-    end
   end
 end