centralize abstraction for thumbnail generation

[pgwillia]

Context

Similar logic appeared in Depositable, DraftProperties, Community and the thumbnail/feature_image partials. By refactoring the thumbnail_path into a helper and being able to call thumbnail_file on each instance we can centralize the logic and have each class only deal with the things that concern them.

Related to #1343

What's New

Refactor so no new functionality. Added #thumbnail_path to page_layout_helper and updated anything that called the old way. Also added tests for the logic in the helper.

In investigating this I learned a few new things. ImageMagick is required for variant on images, but a pdf utility like Poppler is required to preview PDFs and ffmpeg is required to preview video. All three are requirements for our thumbnails to work correctly.

I tried a few different things in order to test the helper. I tried creating a dummy @logo which implemented only the surface area that the thumbnail_path touched. With this approach I got stuck on the rails_representation_path with the pdf and images case. Then I tried creating fixtures (turns out that active_storage fixtures are supported out of the box) for these objects. I ran into a similar road block because the fixtures wouldn't respond to variant and preview. Finally I tried a hybrid approach using real objects for the pdf and image and fixtures for the error cases. Success!

I would have liked to also test that a useful warning was logged in the error cases but couldn't find a approach that worked for me short of using the minitest-logger gem. Maybe that is the best way forward.

[pgwillia]

The failing test is because Poppler is not installed in the CI test environment

[murny]

Could probably clean up some of this by using an Item fixture. E.g:

    @item = items(:fancy)
    File.open(file_fixture('pdf-sample.pdf'), 'r') do |file|
      @item.add_and_ingest_files([file])
    end

[pgwillia]

Yeah, I was hoping that would work too. Maybe I'm using it wrong

test 'thumbnail_path should return preview for pdf (Invariable but Previewable)' do
    @item = items(:fancy)
    File.open(file_fixture('pdf-sample.pdf'), 'r') do |file|
      @item.add_and_ingest_files([file])
    end

    logo = @item.files.first
    expected = Rails.application.routes.url_helpers.rails_representation_path(
      logo.preview(resize: '100x100', auto_orient: true).processed
    )
    assert_equal expected, thumbnail_path(logo)
  end

gives me

Error:
PageLayoutHelperTest#test_thumbnail_path_should_return_preview_for_pdf_(Invariable_but_Previewable):
NoMethodError: undefined method `preview' for nil:NilClass
    test/helpers/page_layout_helper_test.rb:108:in `block in <class:PageLayoutHelperTest>'

[murny]

Hmm, my guess is you might have to .reload the item?

Take a peak at downloads controller test. Heres the relevant snippet:

   item = items(:fancy)

    File.open(file_fixture('text-sample.txt'), 'r') do |file|
      item.add_and_ingest_files([file])
    end

    @file = item.reload.files.first

[pgwillia]

That was exactly what I was missing. Thanks!

[murny]

Same here, just use an item fixture and add an attachment to it

(dont even think you need the Sidekiq::Testing.inline! anymore. Can look at admin items controller and else where for examples)

    @item = items(:fancy)
    File.open(file_fixture('text-sample.txt'), 'r') do |file|
      @item.add_and_ingest_files([file])
    end

[murny]

Is installing poppler easy on CI/Dev machines? If not then maybe we should keep it as an "optional" dependency (if not installed we just fallback to file icon image which already happens as it returns nil)? Maybe this test only runs if Poppler dependency is defined 🤔 ?

[pgwillia]

It's pretty easy in Ubuntu sudo apt-get install poppler-utils I'll see if I can get this to work like Clamby.

jupiter/app/models/concerns/draft_properties.rb

Line 153 in be07bda

return unless defined?(Clamby)

[pgwillia]

jupiter/.github/workflows/push.yml

Line 6 in b667a36

runs-on: ubuntu-latest

https://github.com/actions/virtual-environments/blob/master/images/linux/Ubuntu2004-README.md

Has ImageMagick installed but not the PDF dependencies.

[murny]

Looks really great so far 🎉. Just couple small comments on reducing setup boilerplate in tests and we need to figure out poppler on CI/Dev machines 🤔

[pgwillia]

Some information about the dependencies here https://guides.rubyonrails.org/active_storage_overview.html#previewing-files

[mbarnett]