diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 35ccb2c7d8..179d50d0f5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,6 +49,11 @@ jobs: - name: Checkout code uses: actions/checkout@v6 + # poppler-utils provides pdftoppm/pdfinfo, used by the PDF previewer and the + # PDF page-count analyzer (mirrors the production runtime dependency). + - name: Install poppler-utils + run: sudo apt-get update && sudo apt-get install -y poppler-utils + - name: Set up Ruby uses: ruby/setup-ruby@v1 with: diff --git a/AGENTS.md b/AGENTS.md index 0443af703b..9b5919396c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -420,8 +420,9 @@ RuboCop linting on PRs and pushes to main. ## Rake Tasks -Located in `lib/tasks/` (4 files): +Located in `lib/tasks/` (5 files): - `dev.rake` — Development database seeding from XML/CSV - `rhino_migrator.rake` — Rich text editor migration - `attachment_report.rake` — Attachment reporting - `migrate_internal_id_to_filemaker_code.rake` — FileMaker code migration +- `pdfs.rake` — Backfill page-count metadata for existing PDF attachments (`pdfs:backfill_page_counts`) diff --git a/app/analyzers/pdf_page_count_analyzer.rb b/app/analyzers/pdf_page_count_analyzer.rb new file mode 100644 index 0000000000..a45c70f13d --- /dev/null +++ b/app/analyzers/pdf_page_count_analyzer.rb @@ -0,0 +1,41 @@ +require "open3" + +# Records the page count of PDF attachments in blob metadata (metadata["pages"]), +# so views can show a "N pages" indicator and decide whether to offer a scrollable +# multi-page preview without shelling out per request. Uses poppler's `pdfinfo` +# (already a runtime dependency via HighResPopplerPdfPreviewer); silently skipped +# when poppler isn't installed. +class PdfPageCountAnalyzer < ActiveStorage::Analyzer + class << self + def accept?(blob) + blob.content_type == "application/pdf" && pdfinfo_exists? + end + + def pdfinfo_path + ActiveStorage.paths[:pdfinfo] || "pdfinfo" + end + + def pdfinfo_exists? + return @pdfinfo_exists if defined?(@pdfinfo_exists) + + @pdfinfo_exists = system(pdfinfo_path, "-v", out: File::NULL, err: File::NULL) + end + end + + def metadata + download_blob_to_tempfile do |file| + pages = page_count(file) + pages ? { pages: pages } : {} + end + end + + private + + def page_count(file) + output, status = Open3.capture2(self.class.pdfinfo_path, file.path) + return unless status.success? + + match = output.match(/^Pages:\s+(\d+)/) + match && match[1].to_i + end +end diff --git a/app/views/events/callouts/_resource_body.html.erb b/app/views/events/callouts/_resource_body.html.erb index 4b22d0e80e..6f8546d1f1 100644 --- a/app/views/events/callouts/_resource_body.html.erb +++ b/app/views/events/callouts/_resource_body.html.erb @@ -1,11 +1,21 @@ <%# 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. %> -<% if resource.downloadable_asset&.file&.attached? %> + downloadable file is attached) with a "N pages" badge for multi-page PDFs. A + PDF with a known page count > 1 embeds inline as a scrollable viewer, falling + back to the first-page preview where inline PDFs aren't supported (e.g. some + mobile browsers). When the page count isn't known yet (e.g. before the + backfill runs) a resource we still recognize as multi-page keeps the + "download for all pages" prompt. Single-page PDFs and everything else render + via the shared asset pipeline. `resource` is a decorated Resource. Shared by + the registrant resource viewer and admin callouts that link a resource. %> +<% file = resource.downloadable_asset&.file %> +<% pages = file&.attached? && file.content_type == "application/pdf" ? file.metadata["pages"].to_i : 0 %> +<% if file&.attached? %>
- <% unless resource.single_page_pdf? %> + <% if pages > 1 %> + + <%= pages %> pages + + <% elsif !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 %> @@ -16,6 +26,16 @@

The preview below may take a moment to load.

-<%= render "assets/display_assets", - resource: resource, file: resource.display_image, - variant: :hero, link: true %> +<% if pages > 1 %> + + <%= render "assets/display_assets", + resource: resource, file: resource.display_image, + variant: :hero, link: true %> + +<% else %> + <%= render "assets/display_assets", + resource: resource, file: resource.display_image, + variant: :hero, link: true %> +<% end %> diff --git a/config/initializers/active_storage_previewers.rb b/config/initializers/active_storage_previewers.rb index 8e6068335c..621715417f 100644 --- a/config/initializers/active_storage_previewers.rb +++ b/config/initializers/active_storage_previewers.rb @@ -1,3 +1,4 @@ Rails.application.config.after_initialize do Rails.application.config.active_storage.previewers.unshift HighResPopplerPdfPreviewer + Rails.application.config.active_storage.analyzers.unshift PdfPageCountAnalyzer end diff --git a/lib/tasks/pdfs.rake b/lib/tasks/pdfs.rake new file mode 100644 index 0000000000..e5043ec306 --- /dev/null +++ b/lib/tasks/pdfs.rake @@ -0,0 +1,17 @@ +namespace :pdfs do + desc "Backfill page-count metadata (metadata[\"pages\"]) for existing PDF attachments" + task backfill_page_counts: :environment do + scope = ActiveStorage::Blob.where(content_type: "application/pdf") + total = scope.count + puts "Analyzing #{total} PDF blob(s)…" + + scope.find_each.with_index(1) do |blob, i| + blob.analyze + puts " [#{i}/#{total}] blob ##{blob.id} (#{blob.filename}) → #{blob.reload.metadata["pages"] || "?"} pages" + rescue => e + warn " [#{i}/#{total}] blob ##{blob.id} failed: #{e.message}" + end + + puts "Done." + end +end diff --git a/spec/analyzers/pdf_page_count_analyzer_spec.rb b/spec/analyzers/pdf_page_count_analyzer_spec.rb new file mode 100644 index 0000000000..1aa5f372ea --- /dev/null +++ b/spec/analyzers/pdf_page_count_analyzer_spec.rb @@ -0,0 +1,34 @@ +require "rails_helper" + +RSpec.describe PdfPageCountAnalyzer do + before { skip "requires poppler's pdfinfo" unless described_class.pdfinfo_exists? } + + let(:multipage) do + ActiveStorage::Blob.create_and_upload!( + io: File.open(Rails.root.join("spec/fixtures/files/sample_multipage.pdf")), + filename: "sample_multipage.pdf", content_type: "application/pdf" + ) + end + let(:image) do + ActiveStorage::Blob.create_and_upload!( + io: File.open(Rails.root.join("spec/fixtures/files/sample.png")), + filename: "sample.png", content_type: "image/png" + ) + end + + describe ".accept?" do + it "accepts PDFs" do + expect(described_class.accept?(multipage)).to be(true) + end + + it "rejects non-PDFs" do + expect(described_class.accept?(image)).to be(false) + end + end + + describe "#metadata" do + it "records the page count" do + expect(described_class.new(multipage).metadata).to include(pages: 2) + end + end +end diff --git a/spec/factories/downloadable_assets.rb b/spec/factories/downloadable_assets.rb index 93209786a5..24c49cd58a 100644 --- a/spec/factories/downloadable_assets.rb +++ b/spec/factories/downloadable_assets.rb @@ -9,6 +9,16 @@ ) end + trait :multipage do + after(:build) do |asset| + asset.file.attach( + io: File.open(Rails.root.join("spec/fixtures/files/sample_multipage.pdf")), + filename: "sample_multipage.pdf", + content_type: "application/pdf" + ) + end + end + trait :with_image do after(:build) do |asset| asset.file.attach( diff --git a/spec/fixtures/files/sample_multipage.pdf b/spec/fixtures/files/sample_multipage.pdf new file mode 100644 index 0000000000..150a8aa76c Binary files /dev/null and b/spec/fixtures/files/sample_multipage.pdf differ diff --git a/spec/requests/events/registration_ticket_callouts_spec.rb b/spec/requests/events/registration_ticket_callouts_spec.rb index 037b6d8ece..20474afc40 100644 --- a/spec/requests/events/registration_ticket_callouts_spec.rb +++ b/spec/requests/events/registration_ticket_callouts_spec.rb @@ -72,6 +72,42 @@ end end + context "when linked to a multi-page PDF resource" do + before { skip "requires poppler's pdfinfo" unless PdfPageCountAnalyzer.pdfinfo_exists? } + + let(:resource) { create(:resource) } + let(:callout) { create(:registration_ticket_callout, event:, resource:, description: "") } + + before do + asset = create(:downloadable_asset, :multipage, owner: resource) + asset.file.analyze + end + + it "embeds a scrollable inline PDF and shows a page-count badge" do + get event_registration_ticket_callout_path(event, callout) + + expect(response).to have_http_status(:ok) + expect(response.body).to include("2 pages") + expect(response.body).to include(rails_blob_path(resource.downloadable_asset.file, disposition: :inline)) + expect(response.body).to include("type=\"application/pdf\"") + end + end + + context "when linked to a single-page PDF resource" do + let(:resource) { create(:resource) } + let(:callout) { create(:registration_ticket_callout, event:, resource:, description: "") } + + before { create(:downloadable_asset, owner: resource) } + + it "does not show a page-count badge or an inline embed" do + get event_registration_ticket_callout_path(event, callout) + + expect(response).to have_http_status(:ok) + expect(response.body).not_to include("fa-regular fa-copy") + 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