diff --git a/app/jobs/scheduled/enqueue_reminders.rb b/app/jobs/scheduled/enqueue_reminders.rb index c5cb59e..13aea39 100644 --- a/app/jobs/scheduled/enqueue_reminders.rb +++ b/app/jobs/scheduled/enqueue_reminders.rb @@ -20,6 +20,8 @@ module Jobs Group.assign_allowed_groups.pluck(:id).join(",") end + REMINDER_BUFFER_MINUTES = 10 + def user_ids global_frequency = SiteSetting.remind_assigns_frequency frequency = @@ -46,7 +48,7 @@ module Jobs AND #{frequency} > 0 AND ( last_reminder.value IS NULL OR - last_reminder.value::TIMESTAMP <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * #{frequency}) + last_reminder.value::TIMESTAMP <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * #{frequency}) + ('1 MINUTE'::INTERVAL * #{REMINDER_BUFFER_MINUTES}) ) AND assignments.updated_at::TIMESTAMP <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * #{frequency}) AND assignments.assigned_to_type = 'User' diff --git a/spec/jobs/scheduled/enqueue_reminders_spec.rb b/spec/jobs/scheduled/enqueue_reminders_spec.rb index 4733101..5dec108 100644 --- a/spec/jobs/scheduled/enqueue_reminders_spec.rb +++ b/spec/jobs/scheduled/enqueue_reminders_spec.rb @@ -3,12 +3,13 @@ require "rails_helper" RSpec.describe Jobs::EnqueueReminders do - let(:assign_allowed_group) { Group.find_by(name: "staff") } - let(:user) { Fabricate(:user, groups: [assign_allowed_group]) } + fab!(:assign_allowed_group) { Fabricate(:group) } + fab!(:user) { Fabricate(:user, groups: [assign_allowed_group]) } before do SiteSetting.remind_assigns_frequency = RemindAssignsFrequencySiteSettings::MONTHLY_MINUTES SiteSetting.assign_enabled = true + SiteSetting.assign_allowed_on_groups = "#{assign_allowed_group.id}" end describe "#execute" do @@ -36,47 +37,6 @@ RSpec.describe Jobs::EnqueueReminders do assert_reminders_enqueued(0) end - it "does not enqueue a reminder if it's too soon" do - user.upsert_custom_fields(PendingAssignsReminder::REMINDED_AT => 2.days.ago) - assign_multiple_tasks_to(user) - - assert_reminders_enqueued(0) - end - - it "enqueues a reminder if the user was reminded more than a month ago" do - user.upsert_custom_fields(PendingAssignsReminder::REMINDED_AT => 31.days.ago) - assign_multiple_tasks_to(user) - - assert_reminders_enqueued(1) - end - - it "does not enqueue reminders if the remind frequency is set to never" do - SiteSetting.remind_assigns_frequency = 0 - assign_multiple_tasks_to(user) - - assert_reminders_enqueued(0) - end - - it "does not enqueue reminders if the topic was just assigned to the user" do - just_assigned = DateTime.now - assign_multiple_tasks_to(user, assigned_on: just_assigned) - - assert_reminders_enqueued(0) - end - - it "enqueues a reminder when the user overrides the global frequency" do - SiteSetting.remind_assigns_frequency = 0 - user.custom_fields.merge!( - PendingAssignsReminder::REMINDERS_FREQUENCY => - RemindAssignsFrequencySiteSettings::DAILY_MINUTES, - ) - user.save_custom_fields - - assign_multiple_tasks_to(user) - - assert_reminders_enqueued(1) - end - it "doesn't count assigns from deleted topics" do deleted_post = Fabricate(:post) assign_one_task_to(user, post: deleted_post) @@ -87,6 +47,61 @@ RSpec.describe Jobs::EnqueueReminders do assert_reminders_enqueued(0) end + describe "assignment frequency" do + it "enqueues a reminder if the user reminder frequency is 1 day and the last reminded at is almost 1 day" do + user.custom_fields[ + PendingAssignsReminder::REMINDERS_FREQUENCY + ] = RemindAssignsFrequencySiteSettings::DAILY_MINUTES + user.custom_fields[PendingAssignsReminder::REMINDED_AT] = 1.days.ago + user.save + + assign_multiple_tasks_to(user, assigned_on: 1.day.ago - 1.minute) + + assert_reminders_enqueued(1) + end + + it "does not enqueue a reminder if it's too soon" do + user.upsert_custom_fields(PendingAssignsReminder::REMINDED_AT => 1.days.ago) + assign_multiple_tasks_to(user) + + assert_reminders_enqueued(0) + end + + it "enqueues a reminder if the user was reminded more than a month ago" do + user.upsert_custom_fields(PendingAssignsReminder::REMINDED_AT => 31.days.ago) + assign_multiple_tasks_to(user) + + assert_reminders_enqueued(1) + end + + it "does not enqueue reminders if the remind frequency is set to never" do + SiteSetting.remind_assigns_frequency = 0 + assign_multiple_tasks_to(user) + + assert_reminders_enqueued(0) + end + + it "does not enqueue reminders if the topic was just assigned to the user" do + just_assigned = DateTime.now + assign_multiple_tasks_to(user, assigned_on: just_assigned) + + assert_reminders_enqueued(0) + end + + it "enqueues a reminder when the user overrides the global frequency" do + SiteSetting.remind_assigns_frequency = 0 + user.custom_fields.merge!( + PendingAssignsReminder::REMINDERS_FREQUENCY => + RemindAssignsFrequencySiteSettings::DAILY_MINUTES, + ) + user.save_custom_fields + + assign_multiple_tasks_to(user) + + assert_reminders_enqueued(1) + end + end + def assert_reminders_enqueued(expected_amount) expect { subject.execute({}) }.to change(Jobs::RemindUser.jobs, :size).by(expected_amount) end