From 1b1109eea31de3a1547f7cfe65bf55d760b9b6bb Mon Sep 17 00:00:00 2001 From: jack-coggin <119428483+jack-coggin@users.noreply.github.com> Date: Wed, 6 Sep 2023 15:34:39 +0100 Subject: [PATCH] Er 661 new module email (#747) * wip: add complete registration reminder email * wip: update spec * add start training email notification * add training emails condition to email recipients * update recipient selector spec and user scope * rubocop * wip: add nudge email for new module * wip: new module alert email * add nudge emails for new module * update recipient selector and spec * fix syntax error * remove new module nudge email * refactor nudge mails * update dashboard spec update time * refactor nudge mails * wip: new module nudge email * add new module release nudge emails * add sandbox contentful env * revert changes to app_config * check new modules for duplicate names for contentful sandbox * add email send to test release webhook * add new modules check to change method in hook controller * remove new modules check on change method * add new modules check to change method in hook controller * remove unnecessary mail jobs from hook controller * check new modules for repeated names for sandbox * update spec and remove package-lock.json * rubocop and revert yarn.lock changes * revert schema changes * use cms training modules * use course progress service for continue module nudge emails * make modules_in_progress a method on user model * update interval time on spec * fix typo * rubocop * rubocop * use user course method for modules progress * revert dashboard interval change * use que-locks to prevent duplicate concurrent mailjobs * add que-locks gem * revert change to que version * change que version * protect against duplicate mail jobs in queue * rubocop * add comment to training module record migration * add yard annotation * add job helper to guard against duplicate queued jobs * refactor nudge mail service and spec * refactor new module check * wip: create base job to guard against duplicate jobs * refactor spec * refactor scheduled que jobs * update email preferences logic on account page * refactor nudge mail jobs * change contentful env to sandbox * add changes to make testing easier * rubocop * fix merge conflicts * fix merge conflicts * make completed method public in course progress * fix bug for user account page * remove typo from course progress * change new module mail job from run to enqueue for testing * in the event of no modulerelease records, populate with training module data * rubocop * change contentful env to test * remove draft new module check for testing * remove duplicate check for testing * update timestamps * trigger new module email even if no released module records for testing * rubocop * rubocop * update spec for testing * comment out new module mail spec for testing * comment out new module mail spec for testing * skip new module spec for testing * comment out new module mail spec for testing * add draft check back to new module job * rubocop * rubocop * send email to more users for testing and add sentry output * send latest module in new mod mailer to all users for testing * rubocop * add filtered recipients back * add filtered recipients back * select first module for nudge email * remove create new module release record, for testing * send mail to all users * send new module email to all users when there is a release * rubocop * trigger email taken mail when release for testing notify * remove commented test code * remove application job super to simplify testing * ruboocop * log to sentry when job runs * return recipients to contentful response for debugging * more sentry logging * send email directly in release hook * more sentry logging * call notify properly * fix email sending for nudge mail jobs * update base job and spec * rubocop * update mail job specs * add check for empty ModuleRelease table * rubocop * update for qa * fix merge conflicts * fix merge conflicts * scenario 2 qa * scenario 3 qa * fix typo * update continuetraining mail job * fix typo in notify mailer * add live scope to modules for email job * fix continue training mail job * revert changes made for qa testing * refactor mail jobs * use with progress context in mail specs * add nudge mail recipients to user overview data export * remove unused matcher * add mail job shared context * Rework mail prompt shared context to assert without using log output and that specific users are messaged * refactor: use dynamic user scope to select recipients base mail job * fix user overview data model to reference correct recipients scopes * remove unused error from mail job * rubocop * remove rogue comments * remove completed todo * remove missed comments * add comments to document mail job spec setup * remove missed comment * rubocop --------- Co-authored-by: Peter Hamilton --- .rubocop_todo.yml | 14 ++++- app/controllers/application_controller.rb | 2 + app/controllers/hook_controller.rb | 3 +- app/jobs/application_job.rb | 44 ++++++++++++++ app/jobs/complete_registration_mail_job.rb | 7 +++ app/jobs/content_check_job.rb | 22 +------ app/jobs/continue_training_mail_job.rb | 12 ++++ app/jobs/dashboard_job.rb | 24 ++------ app/jobs/fill_page_views_job.rb | 20 +------ app/jobs/mail_job.rb | 13 +++++ app/jobs/new_module_mail_job.rb | 42 ++++++++++++++ app/jobs/start_training_mail_job.rb | 7 +++ app/mailers/notify_mailer.rb | 47 +++++++++++++++ app/models/ahoy/visit.rb | 2 + app/models/data/user_overview.rb | 8 +++ app/models/module_release.rb | 8 +++ app/models/user.rb | 58 ++++++++++++++++++- app/services/course_progress.rb | 16 ++--- app/views/user/show.html.slim | 2 +- config/application.rb | 1 + config/initializers/que.rb | 3 + .../20230823163600_create_module_releases.rb | 13 +++++ db/schema.rb | 15 ++++- spec/factories/module_releases.rb | 8 +++ spec/factories/releases.rb | 6 ++ spec/factories/training_module.rb | 5 ++ .../complete_registration_mail_job_spec.rb | 19 ++++++ spec/jobs/continue_training_mail_job_spec.rb | 48 +++++++++++++++ spec/jobs/new_module_mail_job_spec.rb | 29 ++++++++++ spec/jobs/start_training_mail_job_spec.rb | 19 ++++++ spec/mailers/notify_mailer_spec.rb | 34 +++++++++++ spec/models/data/user_overview_spec.rb | 48 ++++++++++----- spec/support/shared/email_prompt.rb | 26 +++++++++ 33 files changed, 539 insertions(+), 86 deletions(-) create mode 100644 app/jobs/application_job.rb create mode 100644 app/jobs/complete_registration_mail_job.rb create mode 100644 app/jobs/continue_training_mail_job.rb create mode 100644 app/jobs/mail_job.rb create mode 100644 app/jobs/new_module_mail_job.rb create mode 100644 app/jobs/start_training_mail_job.rb create mode 100644 app/models/module_release.rb create mode 100644 db/migrate/20230823163600_create_module_releases.rb create mode 100644 spec/factories/module_releases.rb create mode 100644 spec/factories/releases.rb create mode 100644 spec/factories/training_module.rb create mode 100644 spec/jobs/complete_registration_mail_job_spec.rb create mode 100644 spec/jobs/continue_training_mail_job_spec.rb create mode 100644 spec/jobs/new_module_mail_job_spec.rb create mode 100644 spec/jobs/start_training_mail_job_spec.rb create mode 100644 spec/support/shared/email_prompt.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 485081b68..0f7d8d9ef 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -6,6 +6,18 @@ # 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: + - 'lib/seed_course_entries.rb' + # Offense count: 1 Lint/ShadowedException: Exclude: @@ -39,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/content_integrity.rb' diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e2ee89718..fe2167f50 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -38,8 +38,10 @@ def clear_flash def prepare_cms # ensure correct API for each request ContentfulModel.use_preview_api = Rails.application.preview? + # memoise the latest release timestamp Training::Module.reset_cache_key! + :done end diff --git a/app/controllers/hook_controller.rb b/app/controllers/hook_controller.rb index e87a58c11..14cd30d59 100644 --- a/app/controllers/hook_controller.rb +++ b/app/controllers/hook_controller.rb @@ -10,12 +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(new_release.id) render json: { status: 'content release received' }, status: :ok end diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb new file mode 100644 index 000000000..eae9806ce --- /dev/null +++ b/app/jobs/application_job.rb @@ -0,0 +1,44 @@ +class ApplicationJob < Que::Job + class DuplicateJobError < StandardError + end + + # We are checking for duplicate jobs to prevent undesirable mulitple actions in the event of blocked or slow jobs + # + # @return [void] + def run(*) + 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 + + # @param error [Error] + # @return [void] + def handle_error(error) + log("#{self.class.name} failed with '#{error.message}'") + end + + # @return [String] + def log(message) + Sentry.capture_message(message, level: :info) if Rails.application.live? + + if ENV['RAILS_LOG_TO_STDOUT'].present? + Rails.logger.info(message) + elsif ENV['VERBOSE'].present? + puts message + end + end +end diff --git a/app/jobs/complete_registration_mail_job.rb b/app/jobs/complete_registration_mail_job.rb new file mode 100644 index 000000000..452e2ba85 --- /dev/null +++ b/app/jobs/complete_registration_mail_job.rb @@ -0,0 +1,7 @@ +class CompleteRegistrationMailJob < MailJob + def run + super do + self.class.recipients.each(&:send_complete_registration_notification) + end + end +end diff --git a/app/jobs/content_check_job.rb b/app/jobs/content_check_job.rb index ff2bb9f80..bbdec6811 100644 --- a/app/jobs/content_check_job.rb +++ b/app/jobs/content_check_job.rb @@ -1,5 +1,5 @@ # :nocov: -class ContentCheckJob < Que::Job +class ContentCheckJob < ApplicationJob # @return [Boolean] def run(*) Training::Module.cache.clear @@ -7,12 +7,6 @@ def run(*) valid? end - def handle_error(error) - message = "#{self.class.name}: Failed with '#{error.message}'" - log(message) - Sentry.capture_message(message) - end - private # @return [Boolean] are all modules valid @@ -23,9 +17,7 @@ def valid? log Training::Module.cache.size # should be larger than 0 unless check.valid? - message = "ContentCheckJob: #{mod.name} in '#{env}' via '#{api}' is not valid" - log(message) - Sentry.capture_message(message) + log "ContentCheckJob: #{mod.name} in '#{env}' via '#{api}' is not valid" end check.valid? @@ -41,15 +33,5 @@ def env def api ContentfulModel.use_preview_api ? 'preview' : 'delivery' end - - # @param message [String] - # @return [String, nil] - def log(message) - if ENV['RAILS_LOG_TO_STDOUT'].present? - Rails.logger.info(message) - elsif ENV['VERBOSE'].present? - puts message - end - end end # :nocov: diff --git a/app/jobs/continue_training_mail_job.rb b/app/jobs/continue_training_mail_job.rb new file mode 100644 index 000000000..9d61bf64b --- /dev/null +++ b/app/jobs/continue_training_mail_job.rb @@ -0,0 +1,12 @@ +class ContinueTrainingMailJob < MailJob + def run + super do + self.class.recipients.each do |recipient| + recipient.modules_in_progress.each do |mod_name| + mod = Training::Module.by_name(mod_name) + recipient.send_continue_training_notification(mod) + end + end + end + end +end diff --git a/app/jobs/dashboard_job.rb b/app/jobs/dashboard_job.rb index fa32d5ecf..19773a2f0 100644 --- a/app/jobs/dashboard_job.rb +++ b/app/jobs/dashboard_job.rb @@ -1,5 +1,5 @@ # :nocov: -class DashboardJob < Que::Job +class DashboardJob < ApplicationJob # Jobs will default to priority 100 and run immediately # a lower number is more important # @@ -9,15 +9,11 @@ class DashboardJob < Que::Job # @param upload [Boolean] defaults to true in production or if $DASHBOARD_UPDATE exists def run(upload: Rails.application.dashboard?) - log "#{self.class.name}: Running upload=#{upload}" + super do + log "DashboardJob: Running upload=#{upload}" - Dashboard.new(path: build_dir).call(upload: upload, clean: true) - end - - def handle_error(error) - message = "#{self.class.name}: Failed with '#{error.message}'" - log(message) - Sentry.capture_message(message) if Rails.application.live? + Dashboard.new(path: build_dir).call(upload: upload, clean: true) + end end private @@ -26,15 +22,5 @@ def handle_error(error) def build_dir Rails.root.join('tmp') end - - # @param message [String] - # @return [String, nil] - def log(message) - if ENV['RAILS_LOG_TO_STDOUT'].present? - Rails.logger.info(message) - elsif ENV['VERBOSE'].present? - puts message - end - end end # :nocov: diff --git a/app/jobs/fill_page_views_job.rb b/app/jobs/fill_page_views_job.rb index c39d4208e..77f40567d 100644 --- a/app/jobs/fill_page_views_job.rb +++ b/app/jobs/fill_page_views_job.rb @@ -1,5 +1,5 @@ # :nocov: -class FillPageViewsJob < Que::Job +class FillPageViewsJob < ApplicationJob self.queue = 'default' self.priority = 1 # self.run_at = proc { 1.minute.from_now } @@ -11,23 +11,5 @@ def run(*) log "#{self.class.name}: Running" FillPageViews.new.call end - - def handle_error(error) - message = "#{self.class.name}: Failed with '#{error.message}'" - log(message) - Sentry.capture_message(message) if Rails.application.live? - end - -private - - # @param message [String] - # @return [String, nil] - def log(message) - if ENV['RAILS_LOG_TO_STDOUT'].present? - Rails.logger.info(message) - elsif ENV['VERBOSE'].present? - puts message - end - end end # :nocov: diff --git a/app/jobs/mail_job.rb b/app/jobs/mail_job.rb new file mode 100644 index 000000000..1118372e8 --- /dev/null +++ b/app/jobs/mail_job.rb @@ -0,0 +1,13 @@ +class MailJob < ApplicationJob + # @return [Array] + def self.recipients + scope_name = "#{name.underscore}_recipients" + User.send(scope_name) + end + + def run(*) + super + + log("#{self.class.name}: #{self.class.recipients.count} recipients") + end +end diff --git a/app/jobs/new_module_mail_job.rb b/app/jobs/new_module_mail_job.rb new file mode 100644 index 000000000..c3436bcdc --- /dev/null +++ b/app/jobs/new_module_mail_job.rb @@ -0,0 +1,42 @@ +class NewModuleMailJob < MailJob + # @param release_id [Integer] + def run(release_id) + super do + return unless new_module_published? + + self.class.recipients.each do |recipient| + recipient.send_new_module_notification(latest_module) + end + + newest_release = Release.find(release_id) + + record_module_release(latest_module, newest_release) + end + end + +private + + # @return [Training::Module] + def latest_module + Training::Module.live.last + end + + # @return [Boolean] + def new_module_published? + return false unless ModuleRelease.exists? + + ModuleRelease.ordered.last.module_position < latest_module.position + end + + # @param mod [Training::Module] + # @param release [Release] + # @return [ModuleRelease] + def record_module_release(mod, release) + ModuleRelease.create!( + release_id: release.id, + module_position: mod.position, + name: mod.name, + first_published_at: release.time, + ) + end +end diff --git a/app/jobs/start_training_mail_job.rb b/app/jobs/start_training_mail_job.rb new file mode 100644 index 000000000..3d6ef649d --- /dev/null +++ b/app/jobs/start_training_mail_job.rb @@ -0,0 +1,7 @@ +class StartTrainingMailJob < MailJob + def run + super do + self.class.recipients.each(&:send_start_training_notification) + end + end +end diff --git a/app/mailers/notify_mailer.rb b/app/mailers/notify_mailer.rb index c920d7401..f699c79e2 100644 --- a/app/mailers/notify_mailer.rb +++ b/app/mailers/notify_mailer.rb @@ -8,6 +8,10 @@ class NotifyMailer < GovukNotifyRails::Mailer PASSWORD_CHANGED_TEMPLATE_ID = 'f77e1eba-3fa8-45ae-9cec-a4cc54633395'.freeze RESET_PASSWORD_TEMPLATE_ID = 'ad77aab8-d903-4f77-b074-a16c2658ca79'.freeze UNLOCK_TEMPLATE_ID = 'e18e8419-cfcc-4fcb-abdb-84f932f3cf55'.freeze + COMPLETE_REGISTRATION_TEMPLATE_ID = 'b960eb6a-d183-484b-ac3b-93ae01b3cee1'.freeze + START_TRAINING_TEMPLATE_ID = 'b3c2e4ff-da06-4672-8941-b2f50d37eadc'.freeze + CONTINUE_TRAINING_TEMPLATE_ID = '83dd3dc6-c5de-4e32-a6b4-25c76e805d87'.freeze + NEW_MODULE_TEMPLATE_ID = '2352b6ce-a098-47f0-870a-286308b9798f'.freeze include Devise::Controllers::UrlHelpers @@ -106,4 +110,47 @@ def unlock_instructions(record, token, _opts = {}) ) mail(to: record.email) end + + # @param [User] record + def complete_registration(record) + set_template(COMPLETE_REGISTRATION_TEMPLATE_ID) + set_personalisation( + url: root_url, + ) + mail(to: record.email) + end + + # @param [User] record + def start_training(record) + set_template(START_TRAINING_TEMPLATE_ID) + set_personalisation( + url: root_url, + ) + mail(to: record.email) + end + + # @param [User] record + # @param [Training::Module] mod + def continue_training(record, mod) + set_template(CONTINUE_TRAINING_TEMPLATE_ID) + set_personalisation( + mod_number: mod.position, + mod_name: mod.name, + url: root_url, + ) + mail(to: record.email) + end + + # @param [User] record + # @param [Training::Module] mod + def new_module(record, mod) + set_template(NEW_MODULE_TEMPLATE_ID) + set_personalisation( + mod_number: mod.position, + mod_name: mod.name, + mod_criteria: mod.criteria, + url: root_url, + ) + mail(to: record.email) + end end diff --git a/app/models/ahoy/visit.rb b/app/models/ahoy/visit.rb index 58ab72f21..ff785a5b7 100644 --- a/app/models/ahoy/visit.rb +++ b/app/models/ahoy/visit.rb @@ -6,5 +6,7 @@ class Ahoy::Visit < ApplicationRecord has_many :events, class_name: 'Ahoy::Event' belongs_to :user, optional: true + scope :month_old, -> { where(started_at: 4.weeks.ago.beginning_of_day..4.weeks.ago.end_of_day) } + scope :last_4_weeks, -> { where(started_at: 4.weeks.ago.end_of_day..Time.zone.now) } scope :dashboard, -> { where(started_at: Time.zone.now.beginning_of_month..Time.zone.now) } end diff --git a/app/models/data/user_overview.rb b/app/models/data/user_overview.rb index 321d48d8d..aa327f6a0 100644 --- a/app/models/data/user_overview.rb +++ b/app/models/data/user_overview.rb @@ -26,6 +26,10 @@ def column_names 'With Notes Percentage', 'Without Notes', 'Without Notes Percentage', + 'Complete Registration Mail Recipients', + 'Start Training Mail Recipients', + 'Continue Training Mail Recipients', + 'New Module Mail Recipients', ] end @@ -52,6 +56,10 @@ def dashboard with_notes_percentage: with_notes_percentage, without_notes: without_notes_count, without_notes_percentage: 1 - with_notes_percentage, + complete_registration_mail_recipients: CompleteRegistrationMailJob.recipients.count, + start_training_mail_recipients: StartTrainingMailJob.recipients.count, + continue_training_mail_recipients: ContinueTrainingMailJob.recipients.count, + new_module_mail_recipients: NewModuleMailJob.recipients.count, }] end diff --git a/app/models/module_release.rb b/app/models/module_release.rb new file mode 100644 index 000000000..12ed9e7f1 --- /dev/null +++ b/app/models/module_release.rb @@ -0,0 +1,8 @@ +class ModuleRelease < ApplicationRecord + validates :name, presence: true, uniqueness: true + validates :module_position, presence: true, uniqueness: true + + belongs_to :release + + scope :ordered, -> { order(:module_position) } +end diff --git a/app/models/user.rb b/app/models/user.rb index c45104c6d..6e83b4f78 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -68,9 +68,15 @@ def self.dashboard_headers scope :confirmed, -> { where.not(confirmed_at: nil) } scope :unconfirmed, -> { where(confirmed_at: nil) } scope :locked_out, -> { where.not(locked_at: nil) } - + scope :since_public_beta, -> { where(created_at: Rails.application.public_beta_launch_date..Time.zone.now) } + scope :month_old_confirmation, -> { where(confirmed_at: 4.weeks.ago.beginning_of_day..4.weeks.ago.end_of_day) } scope :with_local_authority, -> { where.not(local_authority: nil) } scope :with_notes, -> { joins(:notes).distinct.select(&:has_notes?) } + scope :not_started_training, -> { reject(&:course_started?) } + scope :course_in_progress, -> { select(&:course_in_progress?) } + + scope :training_email_recipients, -> { where(training_emails: [true, nil]) } + scope :early_years_email_recipients, -> { where(early_years_emails: true) } scope :without_notes, -> { where.not(id: with_notes) } scope :closed, -> { where.not(closed_at: nil) } @@ -81,6 +87,11 @@ def self.dashboard_headers scope :with_assessments, -> { joins(:user_assessments) } scope :with_passing_assessments, -> { with_assessments.merge(UserAssessment.passes) } + scope :start_training_mail_job_recipients, -> { order(:id).training_email_recipients.month_old_confirmation.registration_complete.not_started_training } + scope :complete_registration_mail_job_recipients, -> { order(:id).training_email_recipients.month_old_confirmation.registration_incomplete } + scope :continue_training_mail_job_recipients, -> { order(:id).training_email_recipients.select(&:continue_training_recipient?) } + scope :new_module_mail_job_recipients, -> { order(:id).training_email_recipients.select(&:completed_available_modules?) } + scope :dashboard, -> { not_closed } validates :first_name, :last_name, :setting_type_id, @@ -165,6 +176,24 @@ def send_account_closed_internal_notification(user_account_email) send_devise_notification(:account_closed_internal, user_account_email) end + def send_complete_registration_notification + send_devise_notification(:complete_registration) + end + + def send_start_training_notification + send_devise_notification(:start_training) + end + + # @param mod [Training::Module] + def send_continue_training_notification(mod) + send_devise_notification(:continue_training, mod) + end + + # @param mod [Training::Module] + def send_new_module_notification(mod) + send_devise_notification(:new_module, mod) + end + # @return [String] def name [first_name, last_name].compact.join(' ') @@ -191,10 +220,20 @@ def course_started? !module_time_to_completion.empty? end + # @return [Boolean] + def course_in_progress? + course_started? && !module_time_to_completion.values.all?(&:positive?) + end + + # @return [Array] + def modules_in_progress + module_time_to_completion.select { |_k, v| v.zero? }.keys + end + # @param module_name [String] # @return [Boolean] def module_completed?(module_name) - module_time_to_completion[module_name]&.positive? + module_time_to_completion.key?(module_name) && module_time_to_completion[module_name].positive? end # @return [Integer] @@ -318,6 +357,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_name| module_completed?(mod_name) } + end + + # @return [Boolean] + def continue_training_recipient? + return false 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] diff --git a/app/services/course_progress.rb b/app/services/course_progress.rb index e95db8b8b..19416e2c0 100644 --- a/app/services/course_progress.rb +++ b/app/services/course_progress.rb @@ -51,22 +51,22 @@ def debug_summary end end -private - # @param mod [Training::Module] - # @return [Boolean] module content has been viewed - def started?(mod) + # @return [Boolean] + def completed?(mod) return false if mod.draft? - module_progress(mod).started? + module_progress(mod).completed? end +private + # @param mod [Training::Module] - # @return [Boolean] - def completed?(mod) + # @return [Boolean] module content has been viewed + def started?(mod) return false if mod.draft? - module_progress(mod).completed? + module_progress(mod).started? end # @param mod [Training::Module] diff --git a/app/views/user/show.html.slim b/app/views/user/show.html.slim index 8cfdc8054..004be36a6 100644 --- a/app/views/user/show.html.slim +++ b/app/views/user/show.html.slim @@ -44,7 +44,7 @@ - row.with_value { nil } - email_preferences.with_row do |row| - row.with_key { 'Email updates about this training course' } - - row.with_value(text: current_user.training_emails ? t('my_account.training_emails_true') : t('my_account.training_emails_false'), classes: ['data-hj-suppress']) + - row.with_value(text: current_user.training_emails.nil? || current_user.training_emails ? t('.training_emails_true') : t('.training_emails_false'), classes: ['data-hj-suppress']) - row.with_action(text: t('links.change'), href: edit_training_emails_user_path, visually_hidden_text: 'training_emails', html_attributes: { id: :edit_training_emails_user }) = content_resource 'my_account.closing.information' diff --git a/config/application.rb b/config/application.rb index 8f97d8a1e..09d264f13 100644 --- a/config/application.rb +++ b/config/application.rb @@ -47,6 +47,7 @@ class Application < Rails::Application config.google_cloud_bucket = ENV.fetch('GOOGLE_CLOUD_BUCKET', '#GOOGLE_CLOUD_BUCKET_env_var_missing') config.dashboard_update_interval = ENV.fetch('DASHBOARD_UPDATE_INTERVAL', '0 0 * * *') # Midnight daily + config.mail_job_interval = ENV.fetch('MAIL_JOB_INTERVAL', '0 12 * * *') # Noon daily config.bot_token = ENV['BOT_TOKEN'] config.feedback_url = ENV.fetch('FEEDBACK_URL', '#FEEDBACK_URL_env_var_missing') diff --git a/config/initializers/que.rb b/config/initializers/que.rb index 0b5d9644f..0dd849f91 100644 --- a/config/initializers/que.rb +++ b/config/initializers/que.rb @@ -1,6 +1,9 @@ Que::Scheduler.configure do |config| config.schedule = { DashboardJob: { cron: Rails.application.config.dashboard_update_interval }, + CompleteRegistrationMailJob: { cron: Rails.application.config.mail_job_interval }, + StartTrainingMailJob: { cron: Rails.application.config.mail_job_interval }, + ContinueTrainingMailJob: { cron: Rails.application.config.mail_job_interval }, } end diff --git a/db/migrate/20230823163600_create_module_releases.rb b/db/migrate/20230823163600_create_module_releases.rb new file mode 100644 index 000000000..f19a058cc --- /dev/null +++ b/db/migrate/20230823163600_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 diff --git a/db/schema.rb b/db/schema.rb index 24e8b4753..787825696 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_06_06_133651) do +ActiveRecord::Schema[7.0].define(version: 2023_08_23_163600) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -56,6 +56,18 @@ t.index ["visit_token"], name: "index_ahoy_visits_on_visit_token", unique: true end + create_table "module_releases", force: :cascade do |t| + t.bigint "release_id", null: false + t.integer "module_position", null: false + t.string "name", null: false + t.datetime "first_published_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["module_position"], name: "index_module_releases_on_module_position", unique: true + t.index ["name"], name: "index_module_releases_on_name", unique: true + t.index ["release_id"], name: "index_module_releases_on_release_id" + end + create_table "notes", force: :cascade do |t| t.bigint "user_id", null: false t.string "title" @@ -222,6 +234,7 @@ t.index ["unlock_token"], name: "index_users_on_unlock_token" end + add_foreign_key "module_releases", "releases" add_foreign_key "notes", "users" add_foreign_key "que_scheduler_audit_enqueued", "que_scheduler_audit", column: "scheduler_job_id", primary_key: "scheduler_job_id", name: "que_scheduler_audit_enqueued_scheduler_job_id_fkey" add_foreign_key "responses", "user_assessments" diff --git a/spec/factories/module_releases.rb b/spec/factories/module_releases.rb new file mode 100644 index 000000000..75fa7630d --- /dev/null +++ b/spec/factories/module_releases.rb @@ -0,0 +1,8 @@ +FactoryBot.define do + factory :module_release do + release + module_position { 1 } + name { 'alpha' } + first_published_at { Time.zone.now } + end +end diff --git a/spec/factories/releases.rb b/spec/factories/releases.rb new file mode 100644 index 000000000..6fae0834b --- /dev/null +++ b/spec/factories/releases.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :release do + name { 'alpha' } + time { Time.zone.now } + end +end diff --git a/spec/factories/training_module.rb b/spec/factories/training_module.rb new file mode 100644 index 000000000..d839fda04 --- /dev/null +++ b/spec/factories/training_module.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :training_module do + name { 'delta' } + end +end diff --git a/spec/jobs/complete_registration_mail_job_spec.rb b/spec/jobs/complete_registration_mail_job_spec.rb new file mode 100644 index 000000000..13574e506 --- /dev/null +++ b/spec/jobs/complete_registration_mail_job_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +RSpec.describe CompleteRegistrationMailJob do + let(:template) { :complete_registration } + + # Must have confirmed email 4 weeks ago and not have completed registration + let(:included) do + create_list :user, 3, confirmed_at: 4.weeks.ago + end + + let(:excluded) do + [ + create(:user, :registered, confirmed_at: 4.weeks.ago), + create(:user, confirmed_at: 2.months.ago), + ] + end + + it_behaves_like 'an email prompt' +end diff --git a/spec/jobs/continue_training_mail_job_spec.rb b/spec/jobs/continue_training_mail_job_spec.rb new file mode 100644 index 000000000..376f8c1fb --- /dev/null +++ b/spec/jobs/continue_training_mail_job_spec.rb @@ -0,0 +1,48 @@ +require 'rails_helper' + +RSpec.describe ContinueTrainingMailJob do + include_context 'with progress' + + let(:template) { :continue_training } + + let(:user) { create(:user, :registered, confirmed_at: 4.weeks.ago) } + + let(:user_2) do + create :user, :registered, + confirmed_at: 4.weeks.ago, + module_time_to_completion: { alpha: 0 } + end + + let(:included) { [user] } + let(:excluded) { [user_2] } + + # Must have started, but not completed training. + # Must be 4 weeks since their last visit + # `user_2` won't be included because they have a visit from 2 weeks ago + before do + create :visit, + id: 9, + visitor_token: '345', + user_id: user.id, + started_at: 4.weeks.ago + + create :visit, + id: 7, + visitor_token: '123', + user_id: user_2.id, + started_at: 4.weeks.ago + + create :visit, + id: 8, + visitor_token: '234', + user_id: user_2.id, + started_at: 2.weeks.ago + + # Travel to 4 weeks ago so that the module start event won't count as a recent visit + travel_to 4.weeks.ago + start_module(alpha) + travel_back + end + + it_behaves_like 'an email prompt', nil, Training::Module.by_name(:alpha) +end diff --git a/spec/jobs/new_module_mail_job_spec.rb b/spec/jobs/new_module_mail_job_spec.rb new file mode 100644 index 000000000..a95903005 --- /dev/null +++ b/spec/jobs/new_module_mail_job_spec.rb @@ -0,0 +1,29 @@ +require 'rails_helper' + +RSpec.describe NewModuleMailJob do + include_context 'with progress' + + let(:template) { :new_module } + + let(:included) { [user] } + + let(:excluded) do + create_list :user, 2, :registered, confirmed_at: 4.weeks.ago + end + + # Recipients must have completed all available modules - in this case, alpha and bravo + before do + # Create records for the previously released modules completed by the recipients + # Each `module_release` must have a corresponding `release` record + create(:release, id: 1) + create(:module_release, release_id: 1, module_position: 1, name: 'alpha') + create(:release, id: 2) + create(:module_release, release_id: 1, module_position: 2, name: 'bravo') + + # This will complete the alpha and bravo modules for `user` + complete_module(alpha, 1.minute) + complete_module(bravo, 1.minute) + end + + it_behaves_like 'an email prompt', 2, Training::Module.by_name(:charlie) +end diff --git a/spec/jobs/start_training_mail_job_spec.rb b/spec/jobs/start_training_mail_job_spec.rb new file mode 100644 index 000000000..b6ec56559 --- /dev/null +++ b/spec/jobs/start_training_mail_job_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +RSpec.describe StartTrainingMailJob do + let(:template) { :start_training } + + # Must have confirmed email 4 weeks ago, completed registration and not have started training + let(:included) do + create_list :user, 3, :registered, confirmed_at: 4.weeks.ago + end + + let(:excluded) do + [ + create(:user, :registered, confirmed_at: 2.months.ago), + create(:user, :registered, confirmed_at: 4.weeks.ago, module_time_to_completion: { alpha: 0 }), + ] + end + + it_behaves_like 'an email prompt' +end diff --git a/spec/mailers/notify_mailer_spec.rb b/spec/mailers/notify_mailer_spec.rb index fb133b25e..baa5ed040 100644 --- a/spec/mailers/notify_mailer_spec.rb +++ b/spec/mailers/notify_mailer_spec.rb @@ -87,4 +87,38 @@ end end end + + describe 'users without email preferences' do + context 'when user not completed registration and not opted out of emails' do + it 'sends email to user to remind them to complete registration' do + mail = described_class.complete_registration(user) + expect(mail.to).to contain_exactly(user.email) + expect(mail.subject).to eq 'Complete registration' + end + end + + context 'when user completed registration and not started training' do + it 'sends email to user to remind them to start' do + mail = described_class.start_training(user) + expect(mail.to).to contain_exactly(user.email) + expect(mail.subject).to eq 'Start training' + end + end + + context 'when user started training and not completed training' do + it 'sends email to user to remind them to complete training' do + mail = described_class.continue_training(user, Training::Module.first) + expect(mail.to).to contain_exactly(user.email) + expect(mail.subject).to eq 'Continue training' + end + end + + context 'when user has completed all available modules and a new module is released' do + it 'sends email to user to inform them of new module' do + mail = described_class.new_module(user, Training::Module.first) + expect(mail.to).to contain_exactly(user.email) + expect(mail.subject).to eq 'New module' + end + end + end end diff --git a/spec/models/data/user_overview_spec.rb b/spec/models/data/user_overview_spec.rb index 6aa80ca81..c398bdc7d 100644 --- a/spec/models/data/user_overview_spec.rb +++ b/spec/models/data/user_overview_spec.rb @@ -23,32 +23,40 @@ 'With Notes Percentage', 'Without Notes', 'Without Notes Percentage', + 'Complete Registration Mail Recipients', + 'Start Training Mail Recipients', + 'Continue Training Mail Recipients', + 'New Module Mail Recipients', ] end let(:rows) do [ { - registration_complete: 4, - registration_incomplete: 0, + registration_complete: 5, + registration_incomplete: 1, reregistered: 0, - registered_since_private_beta: 4, + registered_since_private_beta: 6, private_beta_only_registration_incomplete: 0, private_beta_only_registration_complete: 0, registration_events: 0, private_beta_registration_events: 0, public_beta_registration_events: 0, - total: 4, + total: 6, locked_out: 0, - confirmed: 4, + confirmed: 6, unconfirmed: 0, - user_defined_roles: 1, + user_defined_roles: 2, started_learning: 4, - not_started_learning: 0, + not_started_learning: 2, with_notes: 3, - with_notes_percentage: 0.75, - without_notes: 1, - without_notes_percentage: 0.25, + with_notes_percentage: 0.5, + without_notes: 3, + without_notes_percentage: 0.5, + complete_registration_mail_recipients: 1, + start_training_mail_recipients: 1, + continue_training_mail_recipients: 0, + new_module_mail_recipients: 2, }, ] end @@ -65,11 +73,23 @@ create :user, :registered, module_time_to_completion: { alpha: 2, bravo: 0, charlie: 1 } end + let(:release_1) { create(:release) } + before do - create :note, user: user_1 - create :note, user: user_2 - create :note, user: user_3 - create :user, :registered, module_time_to_completion: { alpha: 2, charlie: 1, bravo: 3 } + # create records for the previously released modules completed by the `new_module_mail_recipients` + create(:module_release, release_id: release_1.id, module_position: 1, name: 'alpha') + create(:module_release, release_id: release_1.id, module_position: 2, name: 'bravo') + create(:module_release, release_id: release_1.id, module_position: 3, name: 'charlie') + # create notes for the `with_notes` and `without_notes` users + create(:note, user: user_1) + create(:note, user: user_2) + create(:note, user: user_3) + # A user who has completed all available modules and will receive the new module mail + create(:user, :registered, module_time_to_completion: { alpha: 2, charlie: 1, bravo: 3 }) + # A user who confirmed their email 4 weeks ago will receive the complete registration mail + create(:user, :confirmed, confirmed_at: 4.weeks.ago) + # A registered user who will receive the start training mail + create(:user, :registered, confirmed_at: 4.weeks.ago) end it_behaves_like 'a data export model' diff --git a/spec/support/shared/email_prompt.rb b/spec/support/shared/email_prompt.rb new file mode 100644 index 000000000..aef55538d --- /dev/null +++ b/spec/support/shared/email_prompt.rb @@ -0,0 +1,26 @@ +RSpec.shared_examples 'an email prompt' do |job_vars, mailer_vars| + # Context for verifying that the correct users are messaged with the correct template. + # + # Before using this shared context, make sure to define the following variables in your spec: + # - `template`: The email template being sent. + # - `included`: An array of users who should receive the email. + # - `excluded`: An array of users who should not receive the email. + # + # This context checks if the recipients match `included` and not `excluded`, + # and if the job uses the specified email `template` correctly. + + it 'messages the correct users' do + expect(included).to eq described_class.recipients + expect(excluded).not_to eq described_class.recipients + end + + it 'uses the correct template' do + email = instance_double(Mail::Message, deliver: nil) + + described_class.recipients.each do |recipient| + allow(NotifyMailer).to receive(template).with(recipient, *mailer_vars).and_return(email) + end + + described_class.run(*job_vars) + end +end