From 14231093905f227086af5366c2d211b5d34eb06e Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 21 Jul 2026 22:22:09 -0400 Subject: [PATCH 1/2] Track invoice page views so admins can see if it's been opened The registrant invoice and bulk-payment invoice pages had no view tracking, so there was no way to tell whether a payer had opened their invoice. Emit an Ahoy view event on each and expose queryable helpers and scopes on EventRegistration. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/controllers/events/invoices_controller.rb | 10 +++++ .../events/registrations_controller.rb | 10 +++++ app/models/event_registration.rb | 35 ++++++++++++++++++ spec/models/event_registration_spec.rb | 37 +++++++++++++++++++ spec/requests/events/invoices_spec.rb | 14 +++++++ spec/requests/events/registrations_spec.rb | 11 ++++++ 6 files changed, 117 insertions(+) diff --git a/app/controllers/events/invoices_controller.rb b/app/controllers/events/invoices_controller.rb index 8b94a5245c..5b5f840519 100644 --- a/app/controllers/events/invoices_controller.rb +++ b/app/controllers/events/invoices_controller.rb @@ -3,6 +3,8 @@ module Events # event's content (line item + cost); when a `submission_id` is supplied it # autofills the bill-to/attention from that bulk-payment submission. class InvoicesController < ApplicationController + include AhoyTracking + # Bulk-payment payers have no account; authorization (below) gates access. skip_before_action :authenticate_user!, only: [ :show ] before_action :set_event @@ -14,6 +16,14 @@ def show @submission = FormSubmission.find(params[:submission_id]) authorize! @submission, to: :show_invoice? @invoice = EventInvoice.from_bulk_payment(@submission) + + # Record that the payer opened their bulk-payment invoice. The blank + # admin template (else branch) is an internal tool, so it isn't tracked. + track_view("form_submission_invoice", { + resource_type: "FormSubmission", + resource_id: @submission.id, + event_id: @event.id + }) else # The blank template is an admin tool. authorize! @event, to: :invoice? diff --git a/app/controllers/events/registrations_controller.rb b/app/controllers/events/registrations_controller.rb index 082ddf4779..69adac4885 100644 --- a/app/controllers/events/registrations_controller.rb +++ b/app/controllers/events/registrations_controller.rb @@ -1,5 +1,7 @@ module Events class RegistrationsController < ApplicationController + include AhoyTracking + before_action :authenticate_user!, only: [ :create ] before_action :set_event, only: [ :create ] before_action :set_registrant, only: [ :create ] @@ -33,6 +35,14 @@ def invoice @event = @event_registration.event @invoice = EventInvoice.from_registration(@event_registration) + + # Record that the registrant (or whoever holds the slug) opened their + # invoice, so admins can tell whether it's been seen yet. + track_view("event_registration_invoice", { + resource_type: "EventRegistration", + resource_id: @event_registration.id, + event_id: @event.id + }) end def receipt diff --git a/app/models/event_registration.rb b/app/models/event_registration.rb index f48e5693b2..c2ef9e620a 100644 --- a/app/models/event_registration.rb +++ b/app/models/event_registration.rb @@ -372,6 +372,41 @@ def invoice_available? event.cost_cents.to_i.positive? end + # Ahoy event name emitted by Events::RegistrationsController#invoice each time + # the invoice page is opened. Kept distinct from a plain "view.event_registration" + # so it tracks the invoice specifically, not the ticket page. + INVOICE_VIEW_EVENT = "view.event_registration_invoice".freeze + + # Registrations whose invoice page has (or hasn't) been opened at least once — + # so admins can search/filter for people who haven't looked at their invoice. + scope :invoice_viewed, -> { + where(id: invoice_view_events_scope.select(:resource_id)) + } + scope :invoice_not_viewed, -> { + where.not(id: invoice_view_events_scope.select(:resource_id)) + } + + def self.invoice_view_events_scope + Ahoy::Event.where(name: INVOICE_VIEW_EVENT, resource_type: name) + end + + # Invoice page-view tracking for this registration (see INVOICE_VIEW_EVENT). + def invoice_view_events + self.class.invoice_view_events_scope.where(resource_id: id) + end + + def invoice_viewed? + invoice_view_events.exists? + end + + def invoice_first_viewed_at + invoice_view_events.minimum(:time) + end + + def invoice_last_viewed_at + invoice_view_events.maximum(:time) + end + # A receipt is proof money changed hands, so it's available only once a paid # event is settled in full AND at least some of that settlement was an actual # payment. A balance cleared purely by scholarship or discount (no money diff --git a/spec/models/event_registration_spec.rb b/spec/models/event_registration_spec.rb index 1560821739..15a1e680ee 100644 --- a/spec/models/event_registration_spec.rb +++ b/spec/models/event_registration_spec.rb @@ -1126,4 +1126,41 @@ def registration_for(person) expect(preloaded.paid_in_full?).to be(true) end end + + describe "invoice view tracking" do + let(:registration) { create(:event_registration) } + + def track_invoice_view(reg) + create( + :ahoy_event, + name: EventRegistration::INVOICE_VIEW_EVENT, + properties: { resource_type: "EventRegistration", resource_id: reg.id } + ) + end + + it "reports whether the invoice has been viewed" do + expect(registration.invoice_viewed?).to be(false) + track_invoice_view(registration) + expect(registration.invoice_viewed?).to be(true) + end + + it "exposes first and last view timestamps" do + travel_to(2.days.ago) { track_invoice_view(registration) } + travel_to(1.hour.ago) { track_invoice_view(registration) } + + expect(registration.invoice_first_viewed_at).to be_within(1.second).of(2.days.ago) + expect(registration.invoice_last_viewed_at).to be_within(1.second).of(1.hour.ago) + end + + it "scopes registrations by whether their invoice was viewed" do + viewed = registration + unviewed = create(:event_registration) + track_invoice_view(viewed) + + expect(EventRegistration.invoice_viewed).to include(viewed) + expect(EventRegistration.invoice_viewed).not_to include(unviewed) + expect(EventRegistration.invoice_not_viewed).to include(unviewed) + expect(EventRegistration.invoice_not_viewed).not_to include(viewed) + end + end end diff --git a/spec/requests/events/invoices_spec.rb b/spec/requests/events/invoices_spec.rb index 01755557ea..1ea0154ff1 100644 --- a/spec/requests/events/invoices_spec.rb +++ b/spec/requests/events/invoices_spec.rb @@ -43,6 +43,20 @@ def add_answer(identifier, value) # 8 attendees × $1,500 = $12,000 expect(response.body).to include("$12,000") end + + it "records an Ahoy view event for the bulk-payment invoice" do + # A real browser User-Agent so Ahoy doesn't skip the request as a bot. + browser = { "User-Agent" => "Mozilla/5.0 (Macintosh) AppleWebKit/537.36 Chrome/120 Safari/537.36" } + + expect { get event_invoice_path(event, submission_id: submission.id), headers: browser } + .to change { + Ahoy::Event.where( + name: "view.form_submission_invoice", + resource_type: "FormSubmission", + resource_id: submission.id + ).count + }.by(1) + end end end diff --git a/spec/requests/events/registrations_spec.rb b/spec/requests/events/registrations_spec.rb index a2b85ea598..0b04a09da8 100644 --- a/spec/requests/events/registrations_spec.rb +++ b/spec/requests/events/registrations_spec.rb @@ -91,6 +91,17 @@ expect(response.body).to include("$1,500") end + it "records an Ahoy view event so admins can tell the invoice was opened" do + # A real browser User-Agent so Ahoy doesn't skip the request as a bot. + browser = { "User-Agent" => "Mozilla/5.0 (Macintosh) AppleWebKit/537.36 Chrome/120 Safari/537.36" } + + expect { get registration_invoice_path(registration.slug), headers: browser } + .to change { registration.reload.invoice_view_events.count }.by(1) + + expect(registration.invoice_viewed?).to be(true) + expect(EventRegistration.invoice_viewed).to include(registration) + end + it "shows the balance due once a scholarship or payment is applied" do scholarship = create(:scholarship, recipient: registration.registrant, amount_cents: 60_000) create(:allocation, source: scholarship, allocatable: registration, amount: 60_000) From 13cbaba1f6abdfeddda6c140b4879d39389264fc Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 22 Jul 2026 07:17:55 -0400 Subject: [PATCH 2/2] Show admins when a recipient opened their invoice Track every invoice open, tagging each with a viewer_role so an admin previewing an invoice never counts as the recipient having seen it. An admin-only badge on the invoice shows when the recipient first opened it (or 'Not opened yet'), with a CSS-only hover tooltip listing all opens. Timestamps render in the viewer's time zone. Renames the bulk-payment event to view.event_bulk_payment_invoice and extracts the shared read logic into an InvoiceViewTrackable concern. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/copilot-instructions.md | 20 +++++- AGENTS.md | 3 +- CLAUDE.md | 20 +++++- app/controllers/concerns/ahoy_tracking.rb | 6 ++ app/controllers/events/invoices_controller.rb | 7 +- .../events/registrations_controller.rb | 5 +- .../event_registration_decorator.rb | 5 ++ app/decorators/form_submission_decorator.rb | 5 ++ app/models/concerns/invoice_view_trackable.rb | 35 ++++++++++ app/models/event_registration.rb | 38 +---------- app/models/form_submission.rb | 4 ++ app/services/invoice_viewed_label.rb | 9 +++ app/views/events/invoices/_invoice.html.erb | 24 +++++++ app/views/events/invoices/show.html.erb | 3 +- .../events/registrations/invoice.html.erb | 3 +- spec/models/event_registration_spec.rb | 27 +++++--- spec/models/form_submission_spec.rb | 32 +++++++++ spec/requests/events/invoices_spec.rb | 48 +++++++++---- spec/requests/events/registrations_spec.rb | 67 +++++++++++++++++-- spec/services/invoice_viewed_label_spec.rb | 15 +++++ 20 files changed, 302 insertions(+), 74 deletions(-) create mode 100644 app/models/concerns/invoice_view_trackable.rb create mode 100644 app/services/invoice_viewed_label.rb create mode 100644 spec/services/invoice_viewed_label_spec.rb diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 3b0108839c..8f15146464 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -56,7 +56,7 @@ When changing a model or controller, check whether these related files need upda - Prefer POROs over concerns when possible - **Prefer decorators (Draper, app/decorators/) over view helpers for model-specific presentation** — when display logic is "about a record" (labels, badges, formatted attributes, status pills), put it on that model's decorator and call `record.decorate.thing`. Reserve `app/helpers/` for generic, cross-model view utilities that aren't tied to one model. Decorators keep presentation testable and out of ERB. - Use `after_commit` instead of `after_save` for side effects -- **Comment only when the reason isn't obvious from the code** — don't restate what the code already says. When a comment is warranted (a non-obvious why, a gotcha), keep it brief and clear. +- **Write expressive, idiomatic Ruby that explains itself, then comment only what it can't.** Optimize first for clear names, small methods, guard clauses, and standard idioms so the code reads without narration. **Add a comment only when the code genuinely can't be made clear enough on its own** — a non-obvious *why*, a gotcha, a subtle constraint or ordering dependency. Don't restate what the code already says or narrate the obvious. Prefer rewriting unclear code over annotating it; when a comment is warranted, keep it brief. ## RuboCop (rubocop-rails-omakase) @@ -198,6 +198,23 @@ it needs a `page_bg_class` and register it:** - **Match neighboring pages.** Use the same marker as sibling views with the same authorization level rather than inventing a new value. +## Admin-only content styling + +Some pages are public or shared (registrant/payer-facing) yet surface extra info only admins +should see (e.g. the invoice "First opened …" badge, the org program-status boxes). Two +established markers, both keyed on the same `admin-only bg-blue-100` convention: + +- **Whole page:** `content_for(:page_bg_class, "admin-only bg-blue-100")` — tints an entire + admin-only page (this is one of the `page_bg_class` policy markers above). +- **Inline element:** add `admin-only bg-blue-100` to the element — the blue tint signals + "admin-only" info embedded in an otherwise-shared page. + +**`admin-only` is a semantic marker only — there is NO CSS rule that hides it.** It documents +intent and carries the blue tint; it does not gate visibility. You MUST still gate the content +in ERB by the viewer's role — `current_user&.super_user?`, `allowed_to?(:manage?, …)`, or the +relevant policy — so non-admins never receive it in the response. The class only tints what +admins already see. Match how sibling admin-only elements are gated rather than inventing a check. + ## Lazy index/filter frames Filterable index pages load their rows lazily in a Turbo frame so changing a @@ -225,6 +242,7 @@ this). Match the existing pattern: ## JavaScript +- **Reach for JavaScript last — prefer a no-JS solution first.** Before writing any Stimulus/JS, check whether the behavior can be done declaratively with: semantic HTML and native elements (`
`/`` for disclosure, ``, real form controls), Tailwind/CSS state (`hover:`, `focus:`, `group-hover:`, `peer-*`, `:has()`, transitions/animations — e.g. a hover tooltip or a toggle), or Turbo (frames/streams) for navigation and partial updates. Only add Stimulus/JS when the behavior genuinely can't be expressed that way, and keep it minimal when you do. Reference existing no-JS patterns before introducing a controller. - ES6+ syntax, ESM imports/exports, `const`/`let` (no `var`) - Use `const` for fixed values — not `SCREAMING_SNAKE_CASE` constants (e.g., `const styleId = "foo"` not `const STYLE_ID = "foo"`) - **Strongly prefer Stimulus** for JavaScript behavior — do not write raw/inline JS or jQuery diff --git a/AGENTS.md b/AGENTS.md index a3ea19ff71..93db470413 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,7 +51,7 @@ This codebase (Rails 8.1) | `app/models/` | ActiveRecord models | ~80 files | | `app/services/` | Service objects and POROs (e.g. `MoneyFormatter` for currency display) | ~30 files | | `app/jobs/` | SolidQueue background jobs | 4 files | -| `app/models/concerns/` | Shared model modules | 16 concerns | +| `app/models/concerns/` | Shared model modules | 17 concerns | ### Presentation @@ -133,6 +133,7 @@ This codebase (Rails 8.1) | `AhoyTrackable` | Event tracking integration | | `AuthorCreditable` | Author attribution | | `Featureable` | `featured`, `publicly_featured` scopes | +| `InvoiceViewTrackable` | Recipient-only invoice open tracking via Ahoy `viewer_role`; `invoice_views`/`invoice_viewed?`/`invoice_view_times` + `invoice_viewed`/`invoice_not_viewed` scopes (EventRegistration, FormSubmission) | | `Mentioner` | ActionText @mention extraction and grouping | | `NameFilterable` | Name-based filtering | | `Publishable` | `published`, `publicly_visible` scopes | diff --git a/CLAUDE.md b/CLAUDE.md index 8a2d2f9683..92e1a7bc5c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,7 +56,7 @@ When changing a model or controller, check whether these related files need upda - Prefer POROs over concerns when possible - **Prefer decorators (Draper, app/decorators/) over view helpers for model-specific presentation** — when display logic is "about a record" (labels, badges, formatted attributes, status pills), put it on that model's decorator and call `record.decorate.thing`. Reserve `app/helpers/` for generic, cross-model view utilities that aren't tied to one model. Decorators keep presentation testable and out of ERB. - Use `after_commit` instead of `after_save` for side effects -- **Comment only when the reason isn't obvious from the code** — don't restate what the code already says. When a comment is warranted (a non-obvious why, a gotcha), keep it brief and clear. +- **Write expressive, idiomatic Ruby that explains itself, then comment only what it can't.** Optimize first for clear names, small methods, guard clauses, and standard idioms so the code reads without narration. **Add a comment only when the code genuinely can't be made clear enough on its own** — a non-obvious *why*, a gotcha, a subtle constraint or ordering dependency. Don't restate what the code already says or narrate the obvious. Prefer rewriting unclear code over annotating it; when a comment is warranted, keep it brief. ## RuboCop (rubocop-rails-omakase) @@ -198,6 +198,23 @@ it needs a `page_bg_class` and register it:** - **Match neighboring pages.** Use the same marker as sibling views with the same authorization level rather than inventing a new value. +## Admin-only content styling + +Some pages are public or shared (registrant/payer-facing) yet surface extra info only admins +should see (e.g. the invoice "First opened …" badge, the org program-status boxes). Two +established markers, both keyed on the same `admin-only bg-blue-100` convention: + +- **Whole page:** `content_for(:page_bg_class, "admin-only bg-blue-100")` — tints an entire + admin-only page (this is one of the `page_bg_class` policy markers above). +- **Inline element:** add `admin-only bg-blue-100` to the element — the blue tint signals + "admin-only" info embedded in an otherwise-shared page. + +**`admin-only` is a semantic marker only — there is NO CSS rule that hides it.** It documents +intent and carries the blue tint; it does not gate visibility. You MUST still gate the content +in ERB by the viewer's role — `current_user&.super_user?`, `allowed_to?(:manage?, …)`, or the +relevant policy — so non-admins never receive it in the response. The class only tints what +admins already see. Match how sibling admin-only elements are gated rather than inventing a check. + ## Lazy index/filter frames Filterable index pages load their rows lazily in a Turbo frame so changing a @@ -225,6 +242,7 @@ this). Match the existing pattern: ## JavaScript +- **Reach for JavaScript last — prefer a no-JS solution first.** Before writing any Stimulus/JS, check whether the behavior can be done declaratively with: semantic HTML and native elements (`
`/`` for disclosure, ``, real form controls), Tailwind/CSS state (`hover:`, `focus:`, `group-hover:`, `peer-*`, `:has()`, transitions/animations — e.g. a hover tooltip or a toggle), or Turbo (frames/streams) for navigation and partial updates. Only add Stimulus/JS when the behavior genuinely can't be expressed that way, and keep it minimal when you do. Reference existing no-JS patterns before introducing a controller. - ES6+ syntax, ESM imports/exports, `const`/`let` (no `var`) - Use `const` for fixed values — not `SCREAMING_SNAKE_CASE` constants (e.g., `const styleId = "foo"` not `const STYLE_ID = "foo"`) - **Strongly prefer Stimulus** for JavaScript behavior — do not write raw/inline JS or jQuery diff --git a/app/controllers/concerns/ahoy_tracking.rb b/app/controllers/concerns/ahoy_tracking.rb index 6b2f12096d..7b2671a7c4 100644 --- a/app/controllers/concerns/ahoy_tracking.rb +++ b/app/controllers/concerns/ahoy_tracking.rb @@ -59,6 +59,12 @@ def track_view(resource, properties = {}) def track_print(resource) = track(:print, resource) def track_download(resource)= track(:download, resource) + # Marks tracked views as an admin preview vs the actual recipient opening a + # page, so recipient-only view counts can exclude admins. + def viewer_role + current_user&.super_user? ? "admin" : "recipient" + end + def track_create(resource) = track(:create, resource) def track_update(resource) = track(:update, resource) def track_destroy(resource) = track(:destroy, resource) diff --git a/app/controllers/events/invoices_controller.rb b/app/controllers/events/invoices_controller.rb index 5b5f840519..7992fe9990 100644 --- a/app/controllers/events/invoices_controller.rb +++ b/app/controllers/events/invoices_controller.rb @@ -17,12 +17,11 @@ def show authorize! @submission, to: :show_invoice? @invoice = EventInvoice.from_bulk_payment(@submission) - # Record that the payer opened their bulk-payment invoice. The blank - # admin template (else branch) is an internal tool, so it isn't tracked. - track_view("form_submission_invoice", { + track_view("event_bulk_payment_invoice", { resource_type: "FormSubmission", resource_id: @submission.id, - event_id: @event.id + event_id: @event.id, + viewer_role: viewer_role }) else # The blank template is an admin tool. diff --git a/app/controllers/events/registrations_controller.rb b/app/controllers/events/registrations_controller.rb index 69adac4885..034d5fc556 100644 --- a/app/controllers/events/registrations_controller.rb +++ b/app/controllers/events/registrations_controller.rb @@ -36,12 +36,11 @@ def invoice @event = @event_registration.event @invoice = EventInvoice.from_registration(@event_registration) - # Record that the registrant (or whoever holds the slug) opened their - # invoice, so admins can tell whether it's been seen yet. track_view("event_registration_invoice", { resource_type: "EventRegistration", resource_id: @event_registration.id, - event_id: @event.id + event_id: @event.id, + viewer_role: viewer_role }) end diff --git a/app/decorators/event_registration_decorator.rb b/app/decorators/event_registration_decorator.rb index 82374fdae6..b0caca63a6 100644 --- a/app/decorators/event_registration_decorator.rb +++ b/app/decorators/event_registration_decorator.rb @@ -11,6 +11,11 @@ class EventRegistrationDecorator < ApplicationDecorator amber: "bg-amber-50 text-amber-700 border-amber-200" }.freeze + # Formatted recipient invoice opens, oldest first, for the admin-only badge. + def invoice_view_labels + invoice_view_times.map { |time| InvoiceViewedLabel.for(time) } + end + # Nil when CE isn't in play (so the index can show a "Create" affordance instead). # `simulate_paid:` lets the CE callout's ?admin=true preview the post-payment state # without recording a payment. diff --git a/app/decorators/form_submission_decorator.rb b/app/decorators/form_submission_decorator.rb index 487ca067fb..995a0d66c2 100644 --- a/app/decorators/form_submission_decorator.rb +++ b/app/decorators/form_submission_decorator.rb @@ -1,6 +1,11 @@ class FormSubmissionDecorator < ApplicationDecorator delegate_all + # Formatted recipient invoice opens, oldest first, for the admin-only badge. + def invoice_view_labels + invoice_view_times.map { |time| InvoiceViewedLabel.for(time) } + end + def matched_attendees(event_registrations) object.bulk_payment_attendees.map do |attendee| first = attendee["first_name"]&.strip diff --git a/app/models/concerns/invoice_view_trackable.rb b/app/models/concerns/invoice_view_trackable.rb new file mode 100644 index 0000000000..30913d8207 --- /dev/null +++ b/app/models/concerns/invoice_view_trackable.rb @@ -0,0 +1,35 @@ +# Reads invoice page-view Ahoy events for a record, counting only recipient +# opens (the viewer_role stamped when the event is tracked), so an admin +# previewing an invoice never registers as the recipient having seen it. Hosts +# define INVOICE_VIEW_EVENT with their Ahoy event name. +module InvoiceViewTrackable + extend ActiveSupport::Concern + + RECIPIENT_VIEWS = "JSON_UNQUOTE(JSON_EXTRACT(ahoy_events.properties, '$.viewer_role')) = 'recipient'".freeze + + class_methods do + def recipient_invoice_views + Ahoy::Event.where(name: self::INVOICE_VIEW_EVENT, resource_type: name).where(RECIPIENT_VIEWS) + end + + def invoice_viewed + where(id: recipient_invoice_views.select(:resource_id)) + end + + def invoice_not_viewed + where.not(id: recipient_invoice_views.select(:resource_id)) + end + end + + def invoice_views + self.class.recipient_invoice_views.where(resource_id: id).order(:time) + end + + def invoice_view_times + invoice_views.pluck(:time) + end + + def invoice_viewed? + invoice_views.exists? + end +end diff --git a/app/models/event_registration.rb b/app/models/event_registration.rb index c2ef9e620a..fa8a10726b 100644 --- a/app/models/event_registration.rb +++ b/app/models/event_registration.rb @@ -1,6 +1,9 @@ class EventRegistration < ApplicationRecord include RemoteSearchable include Registerable + include InvoiceViewTrackable + + INVOICE_VIEW_EVENT = "view.event_registration_invoice".freeze belongs_to :registrant, class_name: "Person" belongs_to :event @@ -372,41 +375,6 @@ def invoice_available? event.cost_cents.to_i.positive? end - # Ahoy event name emitted by Events::RegistrationsController#invoice each time - # the invoice page is opened. Kept distinct from a plain "view.event_registration" - # so it tracks the invoice specifically, not the ticket page. - INVOICE_VIEW_EVENT = "view.event_registration_invoice".freeze - - # Registrations whose invoice page has (or hasn't) been opened at least once — - # so admins can search/filter for people who haven't looked at their invoice. - scope :invoice_viewed, -> { - where(id: invoice_view_events_scope.select(:resource_id)) - } - scope :invoice_not_viewed, -> { - where.not(id: invoice_view_events_scope.select(:resource_id)) - } - - def self.invoice_view_events_scope - Ahoy::Event.where(name: INVOICE_VIEW_EVENT, resource_type: name) - end - - # Invoice page-view tracking for this registration (see INVOICE_VIEW_EVENT). - def invoice_view_events - self.class.invoice_view_events_scope.where(resource_id: id) - end - - def invoice_viewed? - invoice_view_events.exists? - end - - def invoice_first_viewed_at - invoice_view_events.minimum(:time) - end - - def invoice_last_viewed_at - invoice_view_events.maximum(:time) - end - # A receipt is proof money changed hands, so it's available only once a paid # event is settled in full AND at least some of that settlement was an actual # payment. A balance cleared purely by scholarship or discount (no money diff --git a/app/models/form_submission.rb b/app/models/form_submission.rb index 6714a1d2bf..2f029d618c 100644 --- a/app/models/form_submission.rb +++ b/app/models/form_submission.rb @@ -1,4 +1,8 @@ class FormSubmission < ApplicationRecord + include InvoiceViewTrackable + + INVOICE_VIEW_EVENT = "view.event_bulk_payment_invoice".freeze + belongs_to :person belongs_to :form belongs_to :event, optional: true diff --git a/app/services/invoice_viewed_label.rb b/app/services/invoice_viewed_label.rb new file mode 100644 index 0000000000..94c7769546 --- /dev/null +++ b/app/services/invoice_viewed_label.rb @@ -0,0 +1,9 @@ +# Formats an invoice view timestamp in the viewer's time zone, so both invoice +# surfaces render opens identically. +class InvoiceViewedLabel + FORMAT = "%b %-d, %Y at %-l:%M %p".freeze + + def self.for(time) + time&.in_time_zone&.strftime(FORMAT) + end +end diff --git a/app/views/events/invoices/_invoice.html.erb b/app/views/events/invoices/_invoice.html.erb index e724c82e38..cad5e9e3dd 100644 --- a/app/views/events/invoices/_invoice.html.erb +++ b/app/views/events/invoices/_invoice.html.erb @@ -1,6 +1,30 @@ <%# Renders one EventInvoice as a printable document. Shared by the per- registration and bulk-payment invoice pages. `invoice` is an EventInvoice. %>
+ <%# Admin-only, non-printing badge showing recipient opens. %> + <% if local_assigns[:viewed_labels] %> +
+ <% if viewed_labels.any? %> + + First opened <%= viewed_labels.first %> + <% if viewed_labels.many? %> + · +<%= viewed_labels.size - 1 %> more + + <% end %> + + <% else %> + + Not opened yet + + <% end %> +
+ <% end %> + <%# Header: issuer details (left) + brand logo (right) %>
diff --git a/app/views/events/invoices/show.html.erb b/app/views/events/invoices/show.html.erb index ada3082988..4377dda7c6 100644 --- a/app/views/events/invoices/show.html.erb +++ b/app/views/events/invoices/show.html.erb @@ -15,7 +15,8 @@ [ dashboard_event_path(@event), "Back to event" ] end %> +<% viewed_labels = @submission && current_user&.super_user? ? @submission.decorate.invoice_view_labels : nil %> <%= render "events/invoices/actions", back_path: back_path, back_label: back_label %>
- <%= render "events/invoices/invoice", invoice: @invoice %> + <%= render "events/invoices/invoice", invoice: @invoice, viewed_labels: viewed_labels %>
diff --git a/app/views/events/registrations/invoice.html.erb b/app/views/events/registrations/invoice.html.erb index 95e64e4d21..129c26322e 100644 --- a/app/views/events/registrations/invoice.html.erb +++ b/app/views/events/registrations/invoice.html.erb @@ -8,7 +8,8 @@ else [ registration_ticket_path(@event_registration.slug), "Back to ticket" ] end %> +<% viewed_labels = current_user&.super_user? ? @event_registration.decorate.invoice_view_labels : nil %> <%= render "events/invoices/actions", back_path: back_path, back_label: back_label %>
- <%= render "events/invoices/invoice", invoice: @invoice %> + <%= render "events/invoices/invoice", invoice: @invoice, viewed_labels: viewed_labels %>
diff --git a/spec/models/event_registration_spec.rb b/spec/models/event_registration_spec.rb index 15a1e680ee..308a85ecce 100644 --- a/spec/models/event_registration_spec.rb +++ b/spec/models/event_registration_spec.rb @@ -1130,36 +1130,45 @@ def registration_for(person) describe "invoice view tracking" do let(:registration) { create(:event_registration) } - def track_invoice_view(reg) + def track_invoice_view(reg, viewer_role: "recipient") create( :ahoy_event, name: EventRegistration::INVOICE_VIEW_EVENT, - properties: { resource_type: "EventRegistration", resource_id: reg.id } + properties: { resource_type: "EventRegistration", resource_id: reg.id, viewer_role: viewer_role } ) end - it "reports whether the invoice has been viewed" do + it "counts recipient opens but ignores admin previews" do expect(registration.invoice_viewed?).to be(false) + + track_invoice_view(registration, viewer_role: "admin") + expect(registration.invoice_viewed?).to be(false) + track_invoice_view(registration) expect(registration.invoice_viewed?).to be(true) end - it "exposes first and last view timestamps" do + it "returns recipient view times oldest first" do travel_to(2.days.ago) { track_invoice_view(registration) } travel_to(1.hour.ago) { track_invoice_view(registration) } + track_invoice_view(registration, viewer_role: "admin") - expect(registration.invoice_first_viewed_at).to be_within(1.second).of(2.days.ago) - expect(registration.invoice_last_viewed_at).to be_within(1.second).of(1.hour.ago) + times = registration.invoice_view_times + expect(times.size).to eq(2) + expect(times.first).to be_within(1.second).of(2.days.ago) + expect(times.last).to be_within(1.second).of(1.hour.ago) end - it "scopes registrations by whether their invoice was viewed" do + it "scopes registrations by whether a recipient opened the invoice" do viewed = registration + admin_only = create(:event_registration) unviewed = create(:event_registration) track_invoice_view(viewed) + track_invoice_view(admin_only, viewer_role: "admin") expect(EventRegistration.invoice_viewed).to include(viewed) - expect(EventRegistration.invoice_viewed).not_to include(unviewed) - expect(EventRegistration.invoice_not_viewed).to include(unviewed) + expect(EventRegistration.invoice_viewed).not_to include(admin_only, unviewed) + expect(EventRegistration.invoice_not_viewed).to include(admin_only, unviewed) expect(EventRegistration.invoice_not_viewed).not_to include(viewed) end end diff --git a/spec/models/form_submission_spec.rb b/spec/models/form_submission_spec.rb index 6d416be252..36f6f257ad 100644 --- a/spec/models/form_submission_spec.rb +++ b/spec/models/form_submission_spec.rb @@ -80,4 +80,36 @@ expect(submission.bulk_payment_amount_cents(free_event)).to eq(0) end end + + describe "invoice view tracking" do + let(:submission) { create(:form_submission, role: "bulk_payment") } + + def view_invoice(submission, viewer_role: "recipient") + create( + :ahoy_event, + name: FormSubmission::INVOICE_VIEW_EVENT, + properties: { resource_type: "FormSubmission", resource_id: submission.id, viewer_role: viewer_role } + ) + end + + it "counts recipient opens but ignores admin previews" do + expect(submission.invoice_viewed?).to be(false) + + view_invoice(submission, viewer_role: "admin") + expect(submission.invoice_viewed?).to be(false) + + view_invoice(submission) + expect(submission.invoice_viewed?).to be(true) + end + + it "returns recipient view times oldest first" do + travel_to(2.days.ago) { view_invoice(submission) } + travel_to(1.hour.ago) { view_invoice(submission) } + + times = submission.invoice_view_times + expect(times.size).to eq(2) + expect(times.first).to be_within(1.second).of(2.days.ago) + expect(times.last).to be_within(1.second).of(1.hour.ago) + end + end end diff --git a/spec/requests/events/invoices_spec.rb b/spec/requests/events/invoices_spec.rb index 1ea0154ff1..94a5b6138e 100644 --- a/spec/requests/events/invoices_spec.rb +++ b/spec/requests/events/invoices_spec.rb @@ -3,6 +3,17 @@ RSpec.describe "Events::Invoices", type: :request do let(:admin) { create(:user, :admin) } let(:event) { create(:event, title: "AWBW 2-Day Art Facilitator Training", cost_cents: 150_000) } + # A real browser User-Agent so Ahoy doesn't skip the request as a bot. + let(:browser_headers) { { "User-Agent" => "Mozilla/5.0 (Macintosh) AppleWebKit/537.36 Chrome/120 Safari/537.36" } } + + def recipient_view(submission, time: Time.current) + create( + :ahoy_event, + name: FormSubmission::INVOICE_VIEW_EVENT, + properties: { resource_type: "FormSubmission", resource_id: submission.id, viewer_role: "recipient" }, + time: time + ) + end describe "GET /events/:event_id/invoice" do context "as an admin" do @@ -44,18 +55,18 @@ def add_answer(identifier, value) expect(response.body).to include("$12,000") end - it "records an Ahoy view event for the bulk-payment invoice" do - # A real browser User-Agent so Ahoy doesn't skip the request as a bot. - browser = { "User-Agent" => "Mozilla/5.0 (Macintosh) AppleWebKit/537.36 Chrome/120 Safari/537.36" } - - expect { get event_invoice_path(event, submission_id: submission.id), headers: browser } - .to change { - Ahoy::Event.where( - name: "view.form_submission_invoice", - resource_type: "FormSubmission", - resource_id: submission.id - ).count - }.by(1) + it "records its own visit as an admin view, not a recipient open" do + expect { get event_invoice_path(event, submission_id: submission.id), headers: browser_headers } + .not_to change { submission.invoice_views.count } + end + + it "shows when the payer first opened it" do + admin.update!(time_zone: "UTC") # render the timestamp in a known zone + recipient_view(submission, time: Time.utc(2026, 11, 12, 19, 26)) + + get event_invoice_path(event, submission_id: submission.id) + + expect(response.body).to include("First opened Nov 12, 2026 at 7:26 PM") end end end @@ -79,6 +90,19 @@ def add_answer(identifier, value) expect(response.body).to include("INVOICE") end + it "records the payer's open as a recipient view" do + expect { get event_invoice_path(event, submission_id: submission.id), headers: browser_headers } + .to change { submission.invoice_views.count }.by(1) + end + + it "does not show the admin-only badge to the payer" do + recipient_view(submission) + + get event_invoice_path(event, submission_id: submission.id) + + expect(response.body).not_to include("First opened") + end + it "is denied an invoice for a non-bulk submission" do other = create(:form_submission, form: form, event: event, role: "registration") get event_invoice_path(event, submission_id: other.id) diff --git a/spec/requests/events/registrations_spec.rb b/spec/requests/events/registrations_spec.rb index 0b04a09da8..3bc15fd1a2 100644 --- a/spec/requests/events/registrations_spec.rb +++ b/spec/requests/events/registrations_spec.rb @@ -7,6 +7,8 @@ before { sign_in user } let(:turbo_headers) { { "Accept" => "text/vnd.turbo-stream.html" } } + # A real browser User-Agent so Ahoy doesn't skip the request as a bot. + let(:browser_headers) { { "User-Agent" => "Mozilla/5.0 (Macintosh) AppleWebKit/537.36 Chrome/120 Safari/537.36" } } describe "GET /registration/:slug" do let(:admin) { create(:user, :with_person, super_user: true) } @@ -91,17 +93,70 @@ expect(response.body).to include("$1,500") end - it "records an Ahoy view event so admins can tell the invoice was opened" do - # A real browser User-Agent so Ahoy doesn't skip the request as a bot. - browser = { "User-Agent" => "Mozilla/5.0 (Macintosh) AppleWebKit/537.36 Chrome/120 Safari/537.36" } - - expect { get registration_invoice_path(registration.slug), headers: browser } - .to change { registration.reload.invoice_view_events.count }.by(1) + it "records the registrant's open as a recipient view" do + expect { get registration_invoice_path(registration.slug), headers: browser_headers } + .to change { registration.invoice_views.count }.by(1) expect(registration.invoice_viewed?).to be(true) expect(EventRegistration.invoice_viewed).to include(registration) end + it "does not show the admin-only badge to the registrant" do + recipient_view(registration) + + get registration_invoice_path(registration.slug) + + expect(response.body).not_to include("First opened") + end + + def recipient_view(registration, time: Time.current) + create( + :ahoy_event, + name: EventRegistration::INVOICE_VIEW_EVENT, + properties: { resource_type: "EventRegistration", resource_id: registration.id, viewer_role: "recipient" }, + time: time + ) + end + + context "as an admin" do + let(:admin) { create(:user, :with_person, super_user: true, time_zone: "UTC") } + + before { sign_in admin } + + it "records its own visit as an admin view, not a recipient open" do + get registration_invoice_path(registration.slug), headers: browser_headers + + all_views = Ahoy::Event.where(name: EventRegistration::INVOICE_VIEW_EVENT, resource_id: registration.id) + expect(all_views.count).to eq(1) # the admin view is tracked + expect(registration.invoice_views.count).to eq(0) # but never counts as a recipient open + end + + it "shows when the registrant first opened it" do + recipient_view(registration, time: Time.utc(2026, 11, 12, 19, 26)) + + get registration_invoice_path(registration.slug) + + expect(response.body).to include("First opened Nov 12, 2026 at 7:26 PM") + end + + it "reveals every recipient open in the badge" do + recipient_view(registration, time: Time.utc(2026, 11, 12, 19, 26)) + recipient_view(registration, time: Time.utc(2026, 11, 13, 8, 0)) + + get registration_invoice_path(registration.slug) + + expect(response.body).to include("First opened Nov 12, 2026 at 7:26 PM") + expect(response.body).to include("+1 more") + expect(response.body).to include("Nov 13, 2026 at 8:00 AM") + end + + it "shows 'Not opened yet' when only admin previews exist" do + get registration_invoice_path(registration.slug) + + expect(response.body).to include("Not opened yet") + end + end + it "shows the balance due once a scholarship or payment is applied" do scholarship = create(:scholarship, recipient: registration.registrant, amount_cents: 60_000) create(:allocation, source: scholarship, allocatable: registration, amount: 60_000) diff --git a/spec/services/invoice_viewed_label_spec.rb b/spec/services/invoice_viewed_label_spec.rb new file mode 100644 index 0000000000..6697105dea --- /dev/null +++ b/spec/services/invoice_viewed_label_spec.rb @@ -0,0 +1,15 @@ +require "rails_helper" + +RSpec.describe InvoiceViewedLabel do + describe ".for" do + it "formats the timestamp in the viewer's time zone" do + Time.use_zone("UTC") do + expect(described_class.for(Time.utc(2026, 11, 12, 19, 26))).to eq("Nov 12, 2026 at 7:26 PM") + end + end + + it "returns nil when there's no timestamp" do + expect(described_class.for(nil)).to be_nil + end + end +end