Bulk emailer: general + event-scoped, with drafts & templates

[maebeale]

🤖 suggested review level: 5 Inspect 🔬 new model + audience-resolution + send fan-out; foundation of a large feature landing in slices

Builds the general bulk emailer with saved drafts and reusable templates, where event reminders become one event-scoped case of the same flow. Design + rationale: see the grilling write-up (prototype-driven).

Landing in tested slices — this PR grows as work continues.

So far

• NotificationComposition model (kind: draft|template) — the persisted composition that was missing (why drafts/templates weren't possible before).
  • Content as discrete columns; presence is the on/off flag (no _enabled).
  • Audience is a re-resolvable recipe (recipient_segments + split added/excluded overrides), never a frozen recipient list.
• Migration + factory + model spec (green).

Next

• AudienceResolver (recipe → people; general pool + event roster via existing ReminderRecipientFilter/EventRegistration scopes).
• Send fan-out → FYI parent + child Notifications (reusing NotificationMailerJob); person_id + batch FK on notifications.
• EmailContent PORO (shared styled AWBW renderer) + compose.
• Controller/policy/routes + the Recipients builder UI (Stimulus).
• Migrate the event reminder flow onto it.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Introduces a new persisted NotificationComposition model to support the upcoming bulk-emailer feature (saved drafts + reusable templates), including database schema, factory, and model specs as the foundation for later audience resolution and send fan-out.

Changes:

• Adds NotificationComposition model with kind (draft/template) and scope_type (general/event) plus basic validations and scopes.
• Persists email content blocks (subject/body/CTA/grey callout) and an audience “recipe” (recipient_segments + added/excluded overrides) in JSON columns.
• Adds migration, schema update, FactoryBot factory, and model specs covering validation/scopes/flag helpers/accessors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
app/models/notification_composition.rb	New model, validations, scopes, and helper/accessor methods for content/audience recipe.
db/migrate/20260802011450_create_notification_compositions.rb	Creates notification_compositions table to persist drafts/templates + audience recipe columns.
db/schema.rb	Schema version bump + new notification_compositions table definition.
spec/factories/notification_compositions.rb	Factory for drafts/templates and event-scoped compositions.
spec/models/notification_composition_spec.rb	Model specs for validations, scopes, and helper/accessor behavior.

Suppressed comments (1)

app/models/notification_composition.rb:56

• 🤖 From Copilot: should-fix: excluded_ids has the same casting issue as added_ids (blank strings become 0, and nil would raise); rejecting blanks first keeps the accessor safer for later audience-resolution code.

  def excluded_ids
    (recipient_excluded_ids || []).map(&:to_i)
  end

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

db/migrate/20260802011450_create_notification_compositions.rb:12

• 🤖 From Copilot: should-fix: Add foreign keys for the user/event references here to match the repo’s established migration pattern (many t.references use foreign_key: true), otherwise the new table won’t enforce referential integrity.

      t.references :user, null: false, index: true
      t.references :event, index: true

spec/factories/notification_compositions.rb:6

• 🤖 From Copilot: spelling: This subject line looks like a typo of the organization name ("Art With A Woman"), which could leak into fixtures/examples and be confusing.

    subject { "A note from Art With A Woman" }

app/models/notification_composition.rb:5

• 🤖 From Copilot: should-fix: The class header comment states drafts are deleted after send, but nothing in this PR performs that deletion (and SendComposition currently only creates notifications). This is likely to become misleading as the feature lands in slices.

# recipe. On send it fans out into Notification rows (an FYI parent + one child
# per recipient); the draft is deleted afterwards, so the composition itself is
# never the record of a sent email — that lives in `notifications`.

db/migrate/20260802011936_add_bulk_email_fields_to_notifications.rb:6

• 🤖 From Copilot: should-fix: notifications.person_id should be a proper reference with a foreign key (this schema already uses foreign keys heavily); otherwise deletions/renames can leave orphaned notification rows.

    add_column :notifications, :person_id, :bigint
    add_index :notifications, :person_id

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (3)

spec/factories/notification_compositions.rb:6

• 🤖 From Copilot: should-fix: Factory default subject says "Art With A Woman", which doesn’t match the app/brand naming used elsewhere (AWBW / A Window Between Worlds) and is likely a copy mistake that can leak into previews/spec failures.

    subject { "A note from Art With A Woman" }

app/models/notification_composition.rb:5

• 🤖 From Copilot: should-fix: The class header comment states that drafts are deleted after sending, but this PR doesn’t implement draft deletion yet, so the comment is currently misleading for readers trying to understand persistence semantics.

# recipe. On send it fans out into Notification rows (an FYI parent + one child
# per recipient); the draft is deleted afterwards, so the composition itself is
# never the record of a sent email — that lives in `notifications`.

app/services/audience_resolver.rb:43

• 🤖 From Copilot: should-fix: Person#preferred_email consults user&.email, so this will trigger an N+1 query on users when resolving larger audiences. Preloading :user avoids per-person lookups while keeping the Ruby-level preferred-email filter.

  def people
    Person.where(id: resolved_ids.to_a).select { |person| person.preferred_email.present? }
  end