Skip to content

Commit

Permalink
refactor scheduled que jobs
Browse files Browse the repository at this point in the history
  • Loading branch information
jack-coggin committed Aug 17, 2023
1 parent 79fa9fb commit 9bdc15c
Show file tree
Hide file tree
Showing 15 changed files with 81 additions and 76 deletions.
2 changes: 0 additions & 2 deletions app/controllers/hook_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ def release
properties: payload,
)

# TODO: WHAT IF THIS TAKES A LONG TIME? ENQUEUE IT? OR HAVE IT AS A SCHEDULED JOB?
# check_new_modules
NewModuleMailJob.enqueue

# Potentially useful but is a LONG running task and concurrent runs must be avoided
Expand Down
3 changes: 1 addition & 2 deletions app/jobs/complete_registration_mail_job.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
class CompleteRegistrationMailJob < DuplicateJobChecker
class CompleteRegistrationMailJob < ScheduledJob
def run
Rails.logger.info('CompleteRegistrationMailJob running')
NudgeMail.new.complete_registration
end
end
3 changes: 1 addition & 2 deletions app/jobs/continue_training_mail_job.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
class ContinueTrainingMailJob < DuplicateJobChecker
class ContinueTrainingMailJob < ScheduledJob
def run
Rails.logger.info('ContinueTrainingMailJob running')
NudgeMail.new.continue_training
end
end
17 changes: 1 addition & 16 deletions app/jobs/dashboard_job.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# DashboardJob.enqueue
#
class DashboardJob < DuplicateJobChecker
class DashboardJob < ScheduledJob
# Jobs will default to priority 100 and run immediately
# a lower number is more important
#
Expand All @@ -15,25 +15,10 @@ def run(upload: Rails.application.dashboard?)
Dashboard.new(path: build_dir).call(upload: upload, clean: true)
end

def handle_error(error)
message = "DashboardJob: Failed with '#{error.message}'"
log(message)
Sentry.capture_message(message) if Rails.application.live?
end

private

# @return [Pathname]
def build_dir
Rails.root.join('tmp')
end

# @return [String]
def log(message)
if ENV['RAILS_LOG_TO_STDOUT'].present?
Rails.logger.info(message)
else
puts message
end
end
end
19 changes: 0 additions & 19 deletions app/jobs/duplicate_job_checker.rb

This file was deleted.

3 changes: 1 addition & 2 deletions app/jobs/new_module_mail_job.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
class NewModuleMailJob < DuplicateJobChecker
class NewModuleMailJob < ScheduledJob
def run
return if new_module.nil?

Rails.logger.info('NewModuleMailJob running')
notify_users(new_module)
create_published_record(new_module)
end
Expand Down
41 changes: 41 additions & 0 deletions app/jobs/scheduled_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
class ScheduledJob < Que::Job
def run
log "#{self.class.name} running"
if duplicate_job_queued?
log_duplicate
return
end

super
end

private

def duplicate_job_queued?
Que.job_stats.any? { |job| job[:job_class] == self.class.name && job[:count] > 1 }
end

# @return [String]
def log_duplicate
message = "Duplicate #{self.class.name} job queued - skipping"
Sentry.capture_message(message) if Rails.application.live?
log(message)
end

def handle_error(error)
message = "#{self.class.name} failed with '#{error.message}'"
log(message)
Sentry.capture_message(message) if Rails.application.live?
end

# @return [String]
def log(message)
if ENV['RAILS_LOG_TO_STDOUT'].present?
Rails.logger.info(message)
else
# rubocop:disable Rails/Output
puts message
# rubocop:enable Rails/Output
end
end
end
3 changes: 1 addition & 2 deletions app/jobs/start_training_mail_job.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
class StartTrainingMailJob < DuplicateJobChecker
class StartTrainingMailJob < ScheduledJob
def run
Rails.logger.info('StartTrainingMailJob running')
NudgeMail.new.start_training
end
end
1 change: 1 addition & 0 deletions app/models/ahoy/visit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ class Ahoy::Visit < ApplicationRecord
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.not(referrer: nil) }
end
4 changes: 2 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,13 @@ def course_started?

# @return [Boolean]
def course_in_progress?
course_started? && !module_time_to_completion.values.all?(&:positive?)
course.current_modules.present?
end

# @param module_name [String]
# @return [Boolean]
def module_completed?(module_name)
module_time_to_completion[module_name]&.positive?
course.completed?(Training::Module.by_name(module_name))
end

# @return [Integer]
Expand Down
16 changes: 8 additions & 8 deletions app/services/contentful_course_progress.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ def course_completed?
published_modules.all? { |mod| completed?(mod) }
end

# @param mod [Training::Module]
# @return [Boolean]
def completed?(mod)
return false if mod.draft?

module_progress(mod).completed?
end

# @return [Array<String>]
def debug_summary
training_modules.map do |mod|
Expand Down Expand Up @@ -61,14 +69,6 @@ def started?(mod)
module_progress(mod).started?
end

# @param mod [Training::Module]
# @return [Boolean]
def completed?(mod)
return false if mod.draft?

module_progress(mod).completed?
end

# @param mod [Training::Module]
# @return [Boolean]
def active?(mod)
Expand Down
16 changes: 8 additions & 8 deletions app/services/course_progress.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ def course_completed?
published_modules.all? { |mod| completed?(mod) }
end

# @param mod [TrainingModule]
# @return [Boolean]
def completed?(mod)
return false if mod.draft?

module_progress(mod).completed?
end

# @return [Array<String>]
def debug_summary
training_modules.map do |mod|
Expand Down Expand Up @@ -66,14 +74,6 @@ def started?(mod)
module_progress(mod).started?
end

# @param mod [TrainingModule]
# @return [Boolean]
def completed?(mod)
return false if mod.draft?

module_progress(mod).completed?
end

# @param mod [TrainingModule]
# @return [Boolean]
def active?(mod)
Expand Down
2 changes: 1 addition & 1 deletion app/services/nudge_mail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def start_training_recipients

# @return [Array<User>]
def continue_training_recipients
recent_visits = Ahoy::Visit.where(started_at: 4.weeks.ago.end_of_day..Time.zone.now)
recent_visits = Ahoy::Visit.last_4_weeks
old_visits = Ahoy::Visit.month_old.reject { |visit| recent_visits.pluck(:user_id).include?(visit.user_id) }
User.course_in_progress.select { |user| old_visits.pluck(:user_id).include?(user.id) }
end
Expand Down
7 changes: 4 additions & 3 deletions spec/models/data/user_module_completion_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require 'rails_helper'

RSpec.describe Data::UserModuleCompletion do
include_context 'with progress'
let(:headers) do
[
'Module Name',
Expand All @@ -14,7 +15,7 @@
{
module_name: 'alpha',
completed_count: 1,
completed_percentage: 0.3333333333333333,
completed_percentage: 0.5,
},
{
module_name: 'bravo',
Expand All @@ -31,8 +32,8 @@

before do
create :user, :registered
create :user, :registered, module_time_to_completion: { alpha: 0 }
create :user, :registered, module_time_to_completion: { alpha: 1 }
complete_module(Training::Module.by_name('alpha'))
start_module(Training::Module.by_name('bravo'))
end

it_behaves_like 'a data export model'
Expand Down
20 changes: 11 additions & 9 deletions spec/services/nudge_mail_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,15 @@
let!(:user_3) { create(:user, :registered, confirmed_at: 4.weeks.ago, module_time_to_completion: { "alpha": 1 }) }

before do
user.update!(module_time_to_completion: { "alpha": 0 }, confirmed_at: 4.weeks.ago)
user.update!(confirmed_at: 4.weeks.ago)

Ahoy::Visit.new(
id: 9,
visitor_token: '345',
user_id: user.id,
started_at: 4.weeks.ago,
).save!

Ahoy::Visit.new(
id: 7,
visitor_token: '123',
Expand All @@ -71,12 +79,6 @@
started_at: 2.weeks.ago,
).save!

Ahoy::Visit.new(
id: 9,
visitor_token: '345',
user_id: user.id,
started_at: 4.weeks.ago,
).save!
travel_to 4.weeks.ago
start_module(alpha)
travel_back
Expand All @@ -98,13 +100,13 @@

context 'when users have completed all available modules and a new module is released' do
include_context 'with progress'
let!(:user_2) { create(:user, :registered, confirmed_at: 4.weeks.ago, module_time_to_completion: { "alpha": 1, "beta": 0 }) }
let!(:user_2) { create(:user, :registered, confirmed_at: 4.weeks.ago) }

before do
complete_module(alpha)
complete_module(bravo)
user.update!(module_time_to_completion: { "alpha": 1, "beta": 1 }, confirmed_at: 4.weeks.ago)
PreviouslyPublishedModule.create!(module_position: 1, name: 'alpha', first_published_at: Time.zone.now).save!
PreviouslyPublishedModule.create!(module_position: 2, name: 'bravo', first_published_at: Time.zone.now).save!

allow(NotifyMailer).to receive(:new_module)
end
Expand Down

0 comments on commit 9bdc15c

Please sign in to comment.