Skip to content

fix(tasks): pre-generate thumbnails for active_storage - #4037

Merged
tvdeyen merged 5 commits into
mainfrom
ingredient-thumbnail-crops
Aug 1, 2026
Merged

fix(tasks): pre-generate thumbnails for active_storage#4037
tvdeyen merged 5 commits into
mainfrom
ingredient-thumbnail-crops

Conversation

@tvdeyen

@tvdeyen tvdeyen commented Jul 2, 2026

Copy link
Copy Markdown
Member

What is this pull request for?

The alchemy:generate:thumbnails rake task was written for Dragonfly: it called PictureThumb.generate_thumbs! directly and warmed ingredient variants by requesting their urls — both of which only persist a thumbnail as a Dragonfly side effect. Under the now-default active_storage adapter it generated nothing, and its :thumbs preload references a Dragonfly-only association that does not exist there, so the ingredient task broke outright.

The logic now lives in an Alchemy::GenerateThumbnails service that branches on the configured storage adapter and materializes each variant, so existing pictures and their ingredient crop and srcset variants are warmed on both adapters. The service is pure and reports progress through a block, so the rake task can log progress without the service depending on the shell.

Notable changes

  • Background generation (ASYNC=true). Warming every thumbnail inline can take a long time on large sites. Passing ASYNC=true to the tasks enqueues the active_storage variants as native ActiveStorage::TransformJobs instead of processing them inline. Dragonfly has no background processor, so ASYNC is a no-op there and generation runs inline.
  • srcset fix. The old task merged a size string into the render options hash, which raises TypeError; it was never hit because no element configured srcset. It now builds the variant the same way the frontend does (picture_url(size: ...)), so the pre-generated variant matches what the browser requests.

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

@tvdeyen tvdeyen added this to the 8.4 milestone Jul 2, 2026
@tvdeyen
tvdeyen requested a review from a team as a code owner July 2, 2026 12:57
@tvdeyen tvdeyen added the enhancement New feature or enhancement label Jul 2, 2026
@tvdeyen
tvdeyen force-pushed the ingredient-thumbnail-crops branch from eb81251 to 516b415 Compare July 2, 2026 12:58
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.27%. Comparing base (e9e13f8) to head (3fc2622).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4037   +/-   ##
=======================================
  Coverage   98.26%   98.27%           
=======================================
  Files         350      351    +1     
  Lines        9184     9219   +35     
=======================================
+ Hits         9025     9060   +35     
  Misses        159      159           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tvdeyen
tvdeyen force-pushed the ingredient-thumbnail-crops branch 2 times, most recently from 0ff3cfc to 1bf2c67 Compare July 2, 2026 13:32
@tvdeyen

tvdeyen commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Let's use ActiveJob and add a async argument to the rake task, so if needed we can background the thumbnail generation

@tvdeyen
tvdeyen marked this pull request as draft July 6, 2026 10:20
@tvdeyen tvdeyen self-assigned this Jul 6, 2026
@tvdeyen
tvdeyen force-pushed the ingredient-thumbnail-crops branch from 9477d81 to 732429e Compare July 29, 2026 17:08
@tvdeyen
tvdeyen marked this pull request as ready for review July 29, 2026 17:09
tvdeyen added 2 commits August 1, 2026 10:07
The alchemy:generate:thumbnails task was written for Dragonfly: it called
PictureThumb.generate_thumbs! directly and warmed ingredient variants by
requesting their urls, both of which only persist a thumbnail as a Dragonfly
side effect. Under the now-default active_storage adapter it generated nothing
and its :thumbs preload referenced an association that does not exist there.
Move the logic into an Alchemy::GenerateThumbnails service that branches on the
configured storage adapter and materializes each variant inline, so existing
pictures and their ingredient crop and srcset variants are warmed on both
adapters. The service yields each processed record so the rake task can report
progress during a long running generation run.
Generating thumbnails for every picture and ingredient can take a long time on
large sites, blocking the process that runs the rake task. Add an ASYNC=true
option to the alchemy:generate:thumbnails tasks that enqueues the active_storage
variants as ActiveStorage::TransformJobs instead of processing them inline, so
the work can be spread across background workers. The dragonfly adapter has no
background processor, so ASYNC is a no-op there and generation runs inline.
@tvdeyen
tvdeyen force-pushed the ingredient-thumbnail-crops branch from 732429e to dbbd3d6 Compare August 1, 2026 08:07
tvdeyen added 3 commits August 1, 2026 10:17
Alchemy::GenerateThumbnails is only used by the alchemy:generate:thumbnails
rake task, so it does not belong in the eager/autoloaded app directory. Move
it to lib/alchemy/tasks and require it explicitly from the task and its spec,
matching the existing Alchemy::Tidy task helper.
The active_storage variant blob content_type is derived from Marcel and turned
out to be an unreliable, environment-dependent signal for the rendered format
(a webp-encoded variant can be labelled image/png). Assert on the output
filename extension instead, which directly reflects the format the variant was
rendered in.
Address review feedback: replace the bare change matchers with explicit
expected counts. Use a realistically sized fixture so the ingredient's
thumbnail crop is valid on every libvips build - the 1x1 factory image makes
cropping fail on stricter builds such as CI - and reset the persisted Dragonfly
archive thumbnails only in the srcset context, where they would otherwise make
the count ambiguous.
@tvdeyen
tvdeyen force-pushed the ingredient-thumbnail-crops branch from d961037 to 3fc2622 Compare August 1, 2026 09:04
@tvdeyen
tvdeyen merged commit d787b4b into main Aug 1, 2026
28 checks passed
@tvdeyen
tvdeyen deleted the ingredient-thumbnail-crops branch August 1, 2026 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant