From c8073d7b4613113413b55a3be3f93435a0caa1ac Mon Sep 17 00:00:00 2001 From: Cory Musick Date: Fri, 21 Aug 2026 20:48:31 +0000 Subject: [PATCH] Fix check-in reminder scheduling on profile update PUT /api/profiles/:id crashed whenever the onboarding reminder step was completed. The frontend always sets onboarding_reminder, so the scheduling branch ran for both choices on that step: - Opting out left checkin_reminder_at nil, and get_reminder_time called strftime on it. - Picking a time passed an ActiveSupport::TimeWithZone to perform_in, which Sidekiq 7 rejects as a non-JSON-native argument. Extract scheduling into schedule_reminder, which returns nil when there is no time to schedule against and passes an iso8601 string to the job. Opting out now cancels the existing job and clears reminder_job_id instead of leaving a stale one. CheckinReminderJob is unchanged; it never reads that argument, it only passes it through when re-enqueueing itself. Add specs for both branches; all four fail against the previous controller with the errors above. --- .../controllers/api/v1/profiles_controller.rb | 17 ++++- .../api/v1/profiles_controller_spec.rb | 63 +++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/backend/app/controllers/api/v1/profiles_controller.rb b/backend/app/controllers/api/v1/profiles_controller.rb index 5d8ba86a7..41add4518 100644 --- a/backend/app/controllers/api/v1/profiles_controller.rb +++ b/backend/app/controllers/api/v1/profiles_controller.rb @@ -31,9 +31,7 @@ def update if time_changed || initial_onboarding_reminder delete_old_job(@profile.reminder_job_id) - - job_id = CheckinReminderJob.perform_in(get_reminder_time.minutes, @profile.id, @profile.checkin_reminder_at) - @profile.update_column(:reminder_job_id, job_id) + @profile.update_column(:reminder_job_id, schedule_reminder) end current_user.profile.reload @@ -59,6 +57,19 @@ def transform_hash_time {checkin_reminder_at: user_time.try(:to_time, :utc)} end + # Returns the id of the newly scheduled job, or nil when there is no time to + # remind at (e.g. the user opted out of reminders during onboarding). + def schedule_reminder + return if @profile.checkin_reminder_at.blank? + + CheckinReminderJob.perform_in( + get_reminder_time.minutes, + @profile.id, + # Sidekiq only accepts native JSON types as job arguments. + @profile.checkin_reminder_at.iso8601 + ) + end + def get_reminder_time time_zone_name = @profile.time_zone_name checkin_at_timezone = @profile.checkin_reminder_at.strftime("%H:%M").in_time_zone(time_zone_name) diff --git a/backend/spec/controllers/api/v1/profiles_controller_spec.rb b/backend/spec/controllers/api/v1/profiles_controller_spec.rb index 808329be3..1e947a6ba 100644 --- a/backend/spec/controllers/api/v1/profiles_controller_spec.rb +++ b/backend/spec/controllers/api/v1/profiles_controller_spec.rb @@ -1,4 +1,9 @@ require "rails_helper" +require "sidekiq/testing" + +# Requiring sidekiq/testing switches Sidekiq into fake mode for the whole suite; +# keep the default as-is and opt in per example group instead. +Sidekiq::Testing.disable! RSpec.describe Api::V1::ProfilesController do let(:user) { create(:user) } @@ -81,5 +86,63 @@ end end end + + context "reminder scheduling" do + before { sign_in user } + + around do |example| + Sidekiq::Testing.fake! do + CheckinReminderJob.clear + example.run + end + end + + context "when the user opts out of reminders" do + let(:opt_out_params) { {checkin_reminder: false, onboarding_reminder: true} } + + it "responds successfully without scheduling a reminder" do + put :update, params: {id: profile.id, profile: opt_out_params} + + expect(response.status).to eq 200 + expect(CheckinReminderJob.jobs).to be_empty + end + + it "clears a previously scheduled job id" do + profile.update_column(:reminder_job_id, "stale-job-id") + + put :update, params: {id: profile.id, profile: opt_out_params} + + expect(profile.reload.reminder_job_id).to be_nil + end + end + + context "when the user picks a reminder time" do + let(:reminder_params) do + { + checkin_reminder: true, + onboarding_reminder: true, + time_zone_name: "America/New_York", + checkin_reminder_at: {hours: 20, minutes: 30} + } + end + + it "schedules a reminder and stores its job id" do + put :update, params: {id: profile.id, profile: reminder_params} + + expect(response.status).to eq 200 + expect(CheckinReminderJob.jobs.size).to eq 1 + expect(profile.reload.reminder_job_id).to eq CheckinReminderJob.jobs.first["jid"] + end + + it "passes only native JSON types to the job" do + put :update, params: {id: profile.id, profile: reminder_params} + + profile_id, reminder_at = CheckinReminderJob.jobs.first["args"] + expect(profile_id).to eq profile.id + expect(reminder_at).to be_a String + expect(Time.parse(reminder_at).utc.strftime("%H:%M")).to eq "20:30" + end + end + end end end