Skip to content

Commit

Permalink
Dan suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
brendanshean committed Dec 2, 2024
1 parent 2379694 commit f2738ad
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 54 deletions.
8 changes: 1 addition & 7 deletions services/QuillLMS/app/models/cron.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,14 @@ def self.interval_10_min
def self.interval_1_hour
CreditReferringAccountsWorker.perform_async
TeacherNotifications::EnqueueUsersForRollupEmailWorker.perform_async(TeacherInfo::HOURLY_EMAIL)
RecalculateStaggeredReleaseLocksForRecentlyCompletedActivitiesWorker.perform_async

# 7/8AM depending on daylight savings
run_at_12_hour_mark if now.hour == 12
# 12/1PM depending on daylight savings
run_at_17_hour_mark if now.hour == 17
# 4/5PM depending on daylight savings
run_at_21_hour_mark if now.hour == 21
# 6/7PM depending on daylight savings
run_at_22_hour_mark if now.hour == 22
end

# Configured in Heroku Scheduler to run every day at 07:00UTC
Expand Down Expand Up @@ -91,11 +90,6 @@ def self.run_at_21_hour_mark
AdminDiagnosticReports::PerformanceBenchmarkWorker.perform_async
end

# 6/7PM depending on daylight savings
def self.run_at_22_hour_mark
RecalculateStaggeredReleaseLocksForRecentlyCompletedActivitiesWorker.perform_async
end

def self.run_weekday
IdentifyStripeInvoicesWithoutSubscriptionsWorker.perform_async
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,17 @@

class RecalculateStaggeredReleaseLocksForRecentlyCompletedActivitiesWorker
include Sidekiq::Worker
class FailedSaveUserPackSequenceItemsWorkerError < StandardError; end

HOURS = 24

def perform
recently_completed_activity_sessions.each do |activity_session|
enqueue_worker(activity_session.user_id, activity_session.classroom_id)
SaveUserPackSequenceItemsWorker.perform_async(activity_session.user_id, activity_session.classroom_id)
end
end

private def enqueue_worker(user_id, classroom_id)
SaveUserPackSequenceItemsWorker.perform_async(user_id, classroom_id)
rescue StandardError => e
ErrorNotifier.report(
FailedSaveUserPackSequenceItemsWorkerError,
user_id:,
classroom_id:,
error: e
)
end

private def recently_completed_activity_sessions
ActivitySession
.joins(:classroom_unit)
.where(completed_at: HOURS.hours.ago..)
.where(completed_at: 2.hours.ago..1.hour.ago)
.select('DISTINCT ON (user_id, classroom_id) user_id, classroom_id')
end
end
12 changes: 5 additions & 7 deletions services/QuillLMS/spec/models/cron_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
Cron.interval_1_hour
end

it 'enqueues RecalculateStaggeredReleaseLocksForRecentlyCompletedActivitiesWorker' do
expect(RecalculateStaggeredReleaseLocksForRecentlyCompletedActivitiesWorker).to receive(:perform_async)
Cron.interval_1_hour
end

it 'enqueues TeacherNotifications::EnqueueUsersForRollupEmailWorker' do
expect(TeacherNotifications::EnqueueUsersForRollupEmailWorker)
.to receive(:perform_async)
Expand All @@ -47,13 +52,6 @@
end
end
end

it 'enqueues RecalculateStaggeredReleaseLocksForRecentlyCompletedActivitiesWorker' do
travel_to Time.current.midnight + 22.hours do
expect(RecalculateStaggeredReleaseLocksForRecentlyCompletedActivitiesWorker).to receive(:perform_async)
Cron.interval_1_hour
end
end
end

describe '#interval_1_day' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
subject { described_class.new.perform }

let(:user) { create(:user) }
let(:hours) { described_class::HOURS }
let(:hours) { 2 }
let(:classroom) { create(:classroom) }
let(:classroom_unit) { create(:classroom_unit, classroom:) }
let(:completed_at) { (hours - 1).hours.ago }
Expand Down Expand Up @@ -41,6 +41,15 @@
end
end

context 'with activity session completed after window' do
before { create(:activity_session, user:, completed_at: (hours - 2).hours.ago) }

it do
expect(SaveUserPackSequenceItemsWorker).not_to receive(:perform_async)
subject
end
end

context 'duplicate activities' do
let(:classroom_unit2) { create(:classroom_unit) }

Expand All @@ -67,27 +76,4 @@
subject
end
end

context 'error handling' do
before do
create(:activity_session, user:, classroom_unit:, completed_at:)

allow(SaveUserPackSequenceItemsWorker)
.to receive(:perform_async)
.and_raise(StandardError.new('Test error'))
end

it 'handles errors gracefully' do
expect(ErrorNotifier)
.to receive(:report)
.with(
described_class::FailedSaveUserPackSequenceItemsWorkerError,
user_id: user.id,
classroom_id: classroom.id,
error: instance_of(StandardError)
)

expect { subject }.not_to raise_error
end
end
end

0 comments on commit f2738ad

Please sign in to comment.