Skip to content

5563 - Replace notification spec stubs with real tests#7072

Open
fuentesjr wants to merge 4 commits into
rubyforgood:mainfrom
fuentesjr:5563-notification-specs
Open

5563 - Replace notification spec stubs with real tests#7072
fuentesjr wants to merge 4 commits into
rubyforgood:mainfrom
fuentesjr:5563-notification-specs

Conversation

@fuentesjr

Copy link
Copy Markdown
Contributor

What github issue is this PR for, if any?

Part of #5563, intentionally not Resolves, since the umbrella has more stubs remaining.

What changed, and why?

Replaces the four pending "add some tests for X" stubs in spec/notifications/ with real specs (one commit per file), following the patterns in volunteer_birthday_notifier_spec.rb, reimbursement_complete_notifier_spec.rb, and emancipation_checklist_reminder_notifier_spec.rb:

  • BaseNotifier carries the bulk of the coverage: title/message/url each raise NotImplementedError on the base, plus the utility methods (read?, created_at, updated_at, created_by) that delegate to record/params, including both branches of created_by (the params[:created_by] path and the legacy params[:created_by_name] fallback)
  • FollowupNotifier covers title, url, and both message branches (with a note, joined on newlines; without, joined on spaces)
  • FollowupResolvedNotifier covers title, message, and url in both the persisted and unsaved states
  • YouthBirthdayNotifier covers title, message, and url

These characterize current behavior; no app code changed. Three # NOTE: comments flag oddities left as-is for a separate change: the NotImplementedError message string in base_notifier.rb is missing its closing quote; FollowupResolvedNotifier#message calls the private created_by_name directly while FollowupNotifier uses the public created_by; and FollowupResolvedNotifier#url's notification_id param silently drops when the notifier is unsaved (Rails omits nil query params).

One thing worth knowing for reviewers: these specs use create, not build, for any record passed into .with(...). Noticed serializes params through a jsonb column via GlobalID, and that round-trips on the first read of params, so an unsaved record raises ActiveJob::SerializationError. Only assertions that never touch params (for example a bare title) can use build.

How is this tested? (please write rspec and jest tests!) 💖💪

  • bundle exec rspec spec/notifications/base_notifier_spec.rb spec/notifications/followup_notifier_spec.rb spec/notifications/followup_resolved_notifier_spec.rb spec/notifications/youth_birthday_notifier_spec.rb gives 19 examples, 0 failures, 0 pending
  • bundle exec standardrb spec/notifications/ reports no offenses
  • No pending / skip / xit remains in the four files

Screenshots please :)

N/A, test-only change, no UI.

@github-actions github-actions Bot added 🧪 Tests Tests ruby Touches Ruby code labels Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ruby Touches Ruby code 🧪 Tests Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant