Ensure demo-seeded invoice PDFs are present on storage - #174
Merged
Conversation
Active Storage uploads wait for after_commit, and a reload inside the seeder transaction can drop the pending upload (Rails #57222). The resulting attached-but-missing blob still looks pdf_current?, so admin invoice preview hits FileNotFoundError (AppSignal #370). Co-authored-by: Thibaud Guillaume-Gentil <thibaud@thibaud.gg>
Skip invoices with no pdf_file attachment so the ensure step repairs orphans without generating PDFs for unrelated records. Co-authored-by: Thibaud Guillaume-Gentil <thibaud@thibaud.gg>
thibaudgg
marked this pull request as ready for review
September 4, 2026 05:47
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Production AppSignal exception incident #370 is an
ActiveStorage::FileNotFoundErroron tenantdemo-de(blob id 131). Admin invoice show preview (_invoice_preview.html.erb) callsinvoice.pdf_file.previewwheninvoice.pdf_current? && !invoice.processing?. The attachment row existed; the S3 object key did not.This is separate from:
pdf_stale/ Send-button fixes (6cf1408,b8a4a3a,1811e08) — those keep Other invoices sendable around Active Storageupdated_attouches.pdf_current?is!pdf_stale?when attached, so an orphan blob withpdf_stale: falsestill offers the preview. Timing alone would not have prevented #370.ChoresJob#purge_unattached_active_storage_blobs!only purges unattached blobs older than a week, so this attached orphan stays.What
Demo seed wraps
invoice.process!→attach_pdfin a transaction (create_other_invoices!,seed_historical_shop_orders!). On Rails 8.1, Active Storage persists blob/attachment rows inafter_saveand uploads inafter_commit. Areloadon the same invoice instance before commit (shop historical orders do this) clearsattachment_changesand skips the upload — DB row, missing object key (Rails #57222).After the seed transaction:
pdf_filewithblob.service.exist?(blob.key)(works fortenant_object_store/ TenantS3 in production and Disk in test).Invoice#attach_pdfoutside the transaction when the key is missing, so the upload is synchronous andpdf_stalestays false.mark_deliveries_delivered!still rescuesFileNotFoundErroras a safety net (same class of race, added in066e96c). ChoresJob purge rules are unchanged.Tests
pdf_current?/ sendability stay true.Do not merge or deploy from this PR. Leave the AppSignal incident open.