From e9f5c93f07e627ac2afe1def543faccd469972e8 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 28 Nov 2024 21:29:43 +0300 Subject: [PATCH] perform time comparison in unix epoch for consistency --- app/services/reminders/update_service.rb | 2 +- spec/services/reminders/update_service_spec.rb | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/app/services/reminders/update_service.rb b/app/services/reminders/update_service.rb index 8ad833d7c102..795fbacfc427 100644 --- a/app/services/reminders/update_service.rb +++ b/app/services/reminders/update_service.rb @@ -47,7 +47,7 @@ def after_perform(service_call) def remind_at_changed?(reminder) # For some reason reminder.remind_at_changed? returns false # so we assume a change if remind_at is present in the params (would have passed contract validation) - params.key?(:remind_at) && reminder.remind_at == params[:remind_at] + params.key?(:remind_at) && reminder.remind_at.to_i == params[:remind_at].to_i end def destroy_scheduled_reminder_job(job_id) diff --git a/spec/services/reminders/update_service_spec.rb b/spec/services/reminders/update_service_spec.rb index 311088074782..71662c72d22d 100644 --- a/spec/services/reminders/update_service_spec.rb +++ b/spec/services/reminders/update_service_spec.rb @@ -58,12 +58,11 @@ end it "reschedules the reminder" do - subject + expect { subject }.to change(model_instance, :job_id).from("1").to("2") aggregate_failures "destroy existing job" do expect(GoodJob::Job).to have_received(:find_by).with(id: "1") expect(job).to have_received(:destroy) - expect(model_instance.reload.job_id).to eq("2") end aggregate_failures "marks unread notifications as read" do @@ -72,6 +71,7 @@ end aggregate_failures "schedule new job" do + expect(model_instance.remind_at).to eq(call_attributes[:remind_at]) expect(Reminders::ScheduleReminderJob).to have_received(:schedule).with(model_instance) end end @@ -85,15 +85,15 @@ end it "schedules a new job" do - subject + expect { subject }.to change(model_instance, :job_id).from("1").to("2") aggregate_failures "does NOT destroy existing job" do expect(GoodJob::Job).to have_received(:find_by).with(id: "1") expect(job).not_to have_received(:destroy) - expect(model_instance.reload.job_id).to eq("2") end aggregate_failures "schedule new job" do + expect(model_instance.remind_at).to eq(call_attributes[:remind_at]) expect(Reminders::ScheduleReminderJob).to have_received(:schedule).with(model_instance) end end @@ -102,9 +102,13 @@ context "with remind_at attribute in non-utc timezone" do let(:call_attributes) { { remind_at: 2.days.from_now.in_time_zone("Africa/Nairobi") } } - it "schedules the reminder" do - subject - expect(model_instance.reload.job_id).to eq("2") + it "reschedules the reminder" do + expect { subject }.to change(model_instance, :job_id).from("1").to("2") + + aggregate_failures "schedule new job" do + expect(model_instance.remind_at).to eq(call_attributes[:remind_at]) + expect(Reminders::ScheduleReminderJob).to have_received(:schedule).with(model_instance) + end end end end