From 0aa0cb625653c54849cd2857218490929b7054a2 Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 23 Jun 2026 09:01:55 -0400 Subject: [PATCH] Show PDF resources in the browser's inline viewer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Callout resource pages and the resource detail page previewed only a PDF's first page as an image, so multi-page handouts looked like one-pagers. Render PDFs in the browser's built-in inline viewer instead (scrollable, all pages — every major browser ships one), falling back to the first-page/image preview for the rare client that can't. Non-PDF resources keep the existing preview. Extracted the PDF-or-preview choice into assets/_resource_display so the callout viewer and resource show page share it. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 4 +-- AGENTS.md | 5 +++- app/models/resource.rb | 11 ------- app/views/assets/_resource_display.html.erb | 17 +++++++++++ .../events/callouts/_resource_body.html.erb | 16 ++++------ app/views/resources/show.html.erb | 2 +- spec/models/resource_spec.rb | 11 ------- .../registration_ticket_callouts_spec.rb | 29 +++++++++++++++++++ spec/requests/events/registrations_spec.rb | 18 ------------ 9 files changed, 59 insertions(+), 54 deletions(-) create mode 100644 app/views/assets/_resource_display.html.erb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 35ccb2c7d8..ed5c797138 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,7 +13,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Set up Ruby uses: ruby/setup-ruby@v1 @@ -47,7 +47,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Set up Ruby uses: ruby/setup-ruby@v1 diff --git a/AGENTS.md b/AGENTS.md index 0443af703b..ab63e0cd64 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,7 +51,7 @@ This codebase (Rails 8.1) | `app/models/` | ActiveRecord models | ~78 files | | `app/services/` | Service objects and POROs (e.g. `MoneyFormatter` for currency display) | ~29 files | | `app/jobs/` | SolidQueue background jobs | 3 files | -| `app/models/concerns/` | Shared model modules | 15 concerns | +| `app/models/concerns/` | Shared model modules | 16 concerns | ### Presentation @@ -104,6 +104,8 @@ This codebase (Rails 8.1) | `Organization` | Groups with affiliations, addresses, logos via ActiveStorage | | `Grant` | Donated funds (polymorphic `donor`: Organization or Person) with eligibility criteria, tasks, deadlines; parent of `Scholarship`. Scholarship totals cannot exceed the grant amount | | `Scholarship` | Award to a `Person`; optionally drawn from a `Grant`, syncs to event registration `Allocation` | +| `ProfessionalLicense` | A license a `Person` holds (`number`, `kind`, `issuing_state`, `expires_on`); a null `number` is a placeholder. `find_or_create_for` keeps one license per (person, number) | +| `ContinuingEducationRegistration` | A registrant's CE for one event against one `ProfessionalLicense`; billable `allocatable` (`Registerable`) with stored `hours` + `cost_cents` (default from the event). Payment is computed (no stored status); the certificate is delivered via `certificate_sent_at` and gated by its own `certificate_available?` | | `Report` | STI base class for MonthlyReport | | `WorkshopLog` | Standalone model for workshop log submissions (attendance, form fields) | @@ -133,6 +135,7 @@ This codebase (Rails 8.1) | `NameFilterable` | Name-based filtering | | `Publishable` | `published`, `publicly_visible` scopes | | `PunctuationStrippable` | Strips punctuation from strings | +| `Registerable` | Shared payment (`allocations_sum`/`paid?`/`remaining_cost`/…) + certificate (`certificate_sent?`, `mark_certificate_sent!`) interface for `EventRegistration` and `ContinuingEducationRegistration`; includers supply `cost_cents` + their own `certificate_available?` | | `RemoteSearchable` | AJAX remote search by column | | `RichTextSearchable` | Full-text search on ActionText rich_text fields | | `SectorsTaggable` | Enforces a single primary sector for sector-tagged owners | diff --git a/app/models/resource.rb b/app/models/resource.rb index 751e83897e..a3d4bd134e 100644 --- a/app/models/resource.rb +++ b/app/models/resource.rb @@ -12,11 +12,6 @@ def self.mentionable_rich_text_fields PUBLISHED_KINDS = [ "Handout", "Template", "Toolkit", "Form" ] KINDS = PUBLISHED_KINDS + [ "Resource", "Story", "LeaderSpotlight", "SectorImpact", "Theme", "Scholarship" ] - # Titles whose downloadable PDF is a single page. Every other resource PDF is - # multi-page, so its first-page preview needs a "download for all pages" note. - # Hardcoded for now — replace with real PDF introspection later. - SINGLE_PAGE_PDF_TITLES = [ "Letter to Supervisors", "W-9" ].freeze - has_rich_text :rhino_body belongs_to :created_by, class_name: "User" @@ -124,12 +119,6 @@ def story? [ "Story", "LeaderSpotlight" ].include? self.kind end - # A multi-page PDF only previews its first page, so the viewer needs a prompt - # to download for the rest. See SINGLE_PAGE_PDF_TITLES. - def single_page_pdf? - SINGLE_PAGE_PDF_TITLES.include?(title) - end - def custom_label_list "#{self.title} (#{self.kind.upcase})" unless self.kind.nil? end diff --git a/app/views/assets/_resource_display.html.erb b/app/views/assets/_resource_display.html.erb new file mode 100644 index 0000000000..a3930a740d --- /dev/null +++ b/app/views/assets/_resource_display.html.erb @@ -0,0 +1,17 @@ +<%# Renders a resource's main display. A PDF opens in the browser's built-in + inline viewer (scrollable, all pages) — every major browser ships one — with + the first-page/image preview as the fallback child for the rare client that + can't. Anything else (images, etc.) renders through the shared asset + pipeline. `resource` is a decorated Resource. %> +<% display = resource.display_image %> +<% if display.respond_to?(:attached?) && display.attached? && display.content_type == "application/pdf" %> + + <%= render "assets/display_assets", + resource: resource, file: display, variant: :hero, link: true %> + +<% else %> + <%= render "assets/display_assets", + resource: resource, file: display, variant: :hero, link: true %> +<% end %> diff --git a/app/views/events/callouts/_resource_body.html.erb b/app/views/events/callouts/_resource_body.html.erb index 4b22d0e80e..5ad87a359d 100644 --- a/app/views/events/callouts/_resource_body.html.erb +++ b/app/views/events/callouts/_resource_body.html.erb @@ -1,13 +1,11 @@ <%# Renders a resource inside a callout page: a top-right download button (when a - downloadable file is attached) plus the resource display (PDF first-page preview - etc., via the same pipeline as resources/show). `resource` is a decorated - Resource. Shared by the registrant resource viewer and admin callouts that link - a resource. %> + downloadable file is attached) plus the resource display. PDFs show in the + browser's inline viewer; everything else renders via the shared asset + pipeline (see assets/resource_display). `resource` is a decorated Resource. + Shared by the registrant resource viewer and admin callouts that link a + resource. %> <% if resource.downloadable_asset&.file&.attached? %>
- <% unless resource.single_page_pdf? %> - The preview shows the first page only — download to get all pages. - <% end %> <%= link_to resource_download_path(resource), download: true, class: "btn btn-utility" do %> Download <% end %> @@ -16,6 +14,4 @@

The preview below may take a moment to load.

-<%= render "assets/display_assets", - resource: resource, file: resource.display_image, - variant: :hero, link: true %> +<%= render "assets/resource_display", resource: resource %> diff --git a/app/views/resources/show.html.erb b/app/views/resources/show.html.erb index a0f45a6d2c..5d6077bb14 100644 --- a/app/views/resources/show.html.erb +++ b/app/views/resources/show.html.erb @@ -47,7 +47,7 @@
- <%= render "assets/display_assets", resource: @resource, file: @resource.display_image, variant: :hero, link: true %> + <%= render "assets/resource_display", resource: @resource %>
diff --git a/spec/models/resource_spec.rb b/spec/models/resource_spec.rb index cdd166248e..3e4b0a1671 100644 --- a/spec/models/resource_spec.rb +++ b/spec/models/resource_spec.rb @@ -56,15 +56,4 @@ end end end - - describe "#single_page_pdf?" do - it "is true for the known single-page titles" do - expect(build(:resource, title: "W-9").single_page_pdf?).to be(true) - expect(build(:resource, title: "Letter to Supervisors").single_page_pdf?).to be(true) - end - - it "is false for any other title" do - expect(build(:resource, title: "AHA Moments").single_page_pdf?).to be(false) - end - end end diff --git a/spec/requests/events/registration_ticket_callouts_spec.rb b/spec/requests/events/registration_ticket_callouts_spec.rb index 037b6d8ece..794dddc78a 100644 --- a/spec/requests/events/registration_ticket_callouts_spec.rb +++ b/spec/requests/events/registration_ticket_callouts_spec.rb @@ -72,6 +72,35 @@ end end + context "when linked to a PDF resource" do + let(:resource) { create(:resource) } + let(:callout) { create(:registration_ticket_callout, event:, resource:, description: "") } + + before { create(:downloadable_asset, owner: resource) } + + it "shows the PDF in the browser's inline viewer" do + get event_registration_ticket_callout_path(event, callout) + + expect(response).to have_http_status(:ok) + expect(response.body).to include("type=\"application/pdf\"") + expect(response.body).to include(rails_blob_path(resource.downloadable_asset.file, disposition: :inline)) + end + end + + context "when linked to a non-PDF resource" do + let(:resource) { create(:resource) } + let(:callout) { create(:registration_ticket_callout, event:, resource:, description: "") } + + before { create(:downloadable_asset, :with_image, owner: resource) } + + it "renders the preview instead of an inline PDF viewer" do + get event_registration_ticket_callout_path(event, callout) + + expect(response).to have_http_status(:ok) + expect(response.body).not_to include("type=\"application/pdf\"") + end + end + context "when linked to a resource without a downloadable file" do let(:resource) { create(:resource) } let(:callout) do diff --git a/spec/requests/events/registrations_spec.rb b/spec/requests/events/registrations_spec.rb index abab55caf9..3775bcc4bc 100644 --- a/spec/requests/events/registrations_spec.rb +++ b/spec/requests/events/registrations_spec.rb @@ -292,24 +292,6 @@ expect(response.body).to include(resource_download_path(resource)) end - it "prompts to download for all pages on a multi-page resource" do - resource = create(:resource, title: "AHA Moments", kind: "Handout") - create(:downloadable_asset, owner: resource) - - get registration_resource_path(registration.slug, resource) - - expect(response.body).to include("download to get all pages") - end - - it "omits the all-pages prompt on a known single-page resource" do - resource = create(:resource, title: "W-9", kind: "Form") - create(:downloadable_asset, owner: resource) - - get registration_resource_path(registration.slug, resource) - - expect(response.body).not_to include("download to get all pages") - end - it "is reachable by slug without logging in" do resource = create(:resource, title: "AHA Moments", kind: "Handout")