Skip to content

Fix check-in reminder scheduling on profile creation - #878

Open
CoryMCodes wants to merge 1 commit into
rubyforgood:masterfrom
CoryMCodes:877-onboarding-checkin-controller-bug
Open

Fix check-in reminder scheduling on profile creation#878
CoryMCodes wants to merge 1 commit into
rubyforgood:masterfrom
CoryMCodes:877-onboarding-checkin-controller-bug

Conversation

@CoryMCodes

Copy link
Copy Markdown
Collaborator

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.

fixes #877

  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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Onboarding reminder step fails with 422, blocking account setup (PUT /api/profiles/:id)

1 participant