refactor nudge mail jobs

.rubocop_todo.yml

@@ -6,6 +6,13 @@
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
+# Offense count: 1
+# This cop supports safe autocorrection (--autocorrect).
+# Configuration parameters: AllowInHeredoc.
+Layout/TrailingWhitespace:
+  Exclude:
+    - 'app/jobs/application_job.rb'
+
 # Offense count: 1
 Lint/MixedRegexpCaptureTypes:
   Exclude:
@@ -44,8 +51,8 @@ Rails/CreateTableWithTimestamps:
 # Include: app/**/*.rb, config/**/*.rb, db/**/*.rb, lib/**/*.rb
 Rails/Output:
   Exclude:
+    - 'app/jobs/application_job.rb'
     - 'app/jobs/content_check_job.rb'
-    - 'app/jobs/dashboard_job.rb'
     - 'app/jobs/fill_page_views_job.rb'
     - 'app/models/concerns/to_csv.rb'
     - 'app/services/contentful_data_integrity.rb'

app/controllers/hook_controller.rb

@@ -10,13 +10,13 @@ class HookController < ApplicationController
   #     - Release (execute)
   #
   def release
-    Release.create!(
+    new_release = Release.create!(
       name: payload.dig('sys', 'id'),
       time: payload.dig('sys', 'completedAt'),
       properties: payload,
     )
 
-    NewModuleMailJob.enqueue
+    NewModuleMailJob.enqueue(new_release.id)
 
     # Potentially useful but is a LONG running task and concurrent runs must be avoided
     # TODO: consider que-locks if webhooks are to trigger the worker

app/jobs/application_job.rb

@@ -1,9 +1,41 @@
-# :nocov:
-class ApplicationJob < ActiveJob::Base
-  # Automatically retry jobs that encountered a deadlock
-  # retry_on ActiveRecord::Deadlocked
+class ApplicationJob < Que::Job
+  class DuplicateJobError < StandardError
+  end
 
-  # Most jobs are safe to ignore if the underlying records are no longer available
-  # discard_on ActiveJob::DeserializationError
+  # @return [void]
+  # We are checking for duplicate jobs to prevent undesirable mulitple actions in the event of blocked or slow jobs
+  def run(*_args)
+    start_time = Time.zone.now
+    log "#{self.class.name} running"
+    if duplicate_job_queued?
+      raise DuplicateJobError, "#{self.class.name} already queued"
+    elsif block_given?
+      yield
+    end
+
+    log "#{self.class.name} finished in #{(Time.zone.now - start_time).round(2)} seconds"
+  end
+
+private
+
+  # @return [Boolean]
+  def duplicate_job_queued?
+    Que.job_stats.any? { |job| job[:job_class] == self.class.name && job[:count] > 1 }
+  end
+
+  # @return [void]
+  def handle_error(error)
+    message = "#{self.class.name} failed with '#{error.message}'"
+    log(message)
+    Sentry.capture_message(message, level: :error) if Rails.application.live?
+  end
+
+  # @return [String]
+  def log(message)
+    if ENV['RAILS_LOG_TO_STDOUT'].present?
+      Rails.logger.info(message)
+    else
+      puts message
+    end
+  end
 end
-# :nocov:

app/jobs/complete_registration_mail_job.rb

@@ -1,5 +1,15 @@
-class CompleteRegistrationMailJob < ScheduledJob
+class CompleteRegistrationMailJob < ApplicationJob
+  # @return [void]
   def run
-    NudgeMail.new.complete_registration
+    super do
+      notify_users
+    end
+  end
+
+private
+
+  # @return [void]
+  def notify_users
+    User.complete_registration_recipients.each { |recipient| NotifyMailer.complete_registration(recipient) }
   end
 end

app/jobs/continue_training_mail_job.rb

@@ -1,5 +1,18 @@
-class ContinueTrainingMailJob < ScheduledJob
+class ContinueTrainingMailJob < ApplicationJob
+  # @return [void]
   def run
-    NudgeMail.new.continue_training
+    super do
+      notify_users
+    end
+  end
+
+private
+
+  # @return [void]
+  def notify_users
+    User.continue_training_recipients.each do |recipient|
+      progress = recipient.course
+      NotifyMailer.continue_training(recipient, progress.current_modules.first)
+    end
   end
 end

app/jobs/dashboard_job.rb

@@ -1,6 +1,6 @@
 # DashboardJob.enqueue
 #
-class DashboardJob < ScheduledJob
+class DashboardJob < ApplicationJob
   # Jobs will default to priority 100 and run immediately
   # a lower number is more important
   #
@@ -10,9 +10,11 @@ class DashboardJob < ScheduledJob
 
   # @param upload [Boolean] defaults to true in production or if $DASHBOARD_UPDATE exists
   def run(upload: Rails.application.dashboard?)
-    log "DashboardJob: Running upload=#{upload}"
+    super do
+      log "DashboardJob: Running upload=#{upload}"
 
-    Dashboard.new(path: build_dir).call(upload: upload, clean: true)
+      Dashboard.new(path: build_dir).call(upload: upload, clean: true)
+    end
   end
 
 private

app/jobs/new_module_mail_job.rb

@@ -1,28 +1,37 @@
-class NewModuleMailJob < ScheduledJob
-  def run
-    return if new_module.nil?
+class NewModuleMailJob < ApplicationJob
+  # @param release_id [Integer]
+  # @return [void]
+  def run(release_id)
+    super do
+      return if new_module.nil?
 
-    notify_users(new_module)
-    create_published_record(new_module)
+      notify_users(new_module)
+      create_published_record(new_module, Release.find(release_id))
+    end
   end
 
 private
 
-  def create_published_record(mod)
-    PreviouslyPublishedModule.create!(module_position: mod.position, name: mod.name, first_published_at: mod.published_at)
+  # @param mod [Training::Module]
+  # @param release [Release]
+  # @return [ModuleRelease]
+  def create_published_record(mod, release)
+    ModuleRelease.create!(release_id: release.id, module_position: mod.position, name: mod.name, first_published_at: release.time)
   end
 
+  # @param mod [Training::Module]
+  # @return [void]
   def notify_users(mod)
-    mail_service = NudgeMail.new
-    mail_service.new_module(mod)
+    User.completed_available_modules.each { |recipient| NotifyMailer.new_module(recipient, mod) }
   end
 
+  # @return [Training::Module, nil]
   def new_module
     latest_published = Training::Module.ordered.reject(&:draft?).last
-    if latest_published.position == PreviouslyPublishedModule.ordered.last.module_position
+    if latest_published.position == ModuleRelease.ordered.last.module_position
       nil
     else
-      Training::Module.ordered.reject(&:draft?).last
+      latest_published
     end
   end
 end

app/jobs/start_training_mail_job.rb

@@ -1,5 +1,15 @@
-class StartTrainingMailJob < ScheduledJob
+class StartTrainingMailJob < ApplicationJob
+  # @return [void]
   def run
-    NudgeMail.new.start_training
+    super do
+      notify_users
+    end
+  end
+
+private
+
+  # @return [void]
+  def notify_users
+    User.start_training_recipients.each { |recipient| NotifyMailer.start_training(recipient) }
   end
 end

app/models/previously_published_module.rb → app/models/module_release.rb

@@ -1,8 +1,10 @@
-class PreviouslyPublishedModule < ApplicationRecord
+class ModuleRelease < ApplicationRecord
   validates :name, presence: true, uniqueness: true
   validates :module_position, presence: true, uniqueness: true
 
-  # @return [ActiveRecord::Relation<PreviouslyPublishedModule>]
+  belongs_to :release
+
+  # @return [ActiveRecord::Relation<ModuleRelease>]
   def self.ordered
     order(:module_position)
   end

app/models/user.rb

@@ -86,6 +86,11 @@ def self.dashboard_headers
   scope :with_assessments, -> { joins(:user_assessments) }
   scope :with_passing_assessments, -> { with_assessments.merge(UserAssessment.passes) }
 
+  scope :start_training_recipients, -> { training_email_recipients.month_old_confirmation.registration_complete.not_started_training }
+  scope :complete_registration_recipients, -> { training_email_recipients.month_old_confirmation.registration_incomplete }
+  scope :continue_training_recipients, -> { training_email_recipients.select(&:continue_training_recipient?) }
+  scope :completed_available_modules, -> { training_email_recipients.select(&:completed_available_modules?) }
+
   scope :dashboard, -> { not_closed }
 
   validates :first_name, :last_name, :setting_type_id,
@@ -216,6 +221,7 @@ def course_started?
 
   # @return [Boolean]
   def course_in_progress?
+    # course_started? && !module_time_to_completion.values.all?(&:positive?)
     course.current_modules.present?
   end
 
@@ -331,6 +337,21 @@ def dashboard_row
     data_attributes.dup.merge(module_ttc)
   end
 
+  # @return [Boolean]
+  def completed_available_modules?
+    available_modules = ModuleRelease.pluck(:name)
+    available_modules.all? { |mod| module_completed?(mod) }
+  end
+
+  # @return [Boolean]
+  def continue_training_recipient?
+    return unless course_in_progress?
+
+    recent_visits = Ahoy::Visit.last_4_weeks
+    old_visits = Ahoy::Visit.month_old.reject { |visit| recent_visits.pluck(:user_id).include?(visit.user_id) }
+    old_visits.pluck(:user_id).include?(id)
+  end
+
 private
 
   # @return [Hash]

db/migrate/20230727102149_create_module_releases.rb

@@ -0,0 +1,13 @@
+class CreateModuleReleases < ActiveRecord::Migration[7.0]
+  def change
+    create_table :module_releases do |t|
+      t.references :release, null: false, foreign_key: true
+      t.integer :module_position, null: false
+      t.string :name, null: false
+      t.datetime :first_published_at, null: false
+      t.timestamps
+    end
+    add_index :module_releases, :name, unique: true
+    add_index :module_releases, :module_position, unique: true
+  end
+end