Automatically eager load the blobs when the `files` field is used

[MrJoy]

Describe the bug

When displaying a :files field, Avo doesn't do .includes(:blob), and so there's an N+1 that occurs. (Super annoying if you use the Bullet gem!)

Steps to Reproduce

1. Create a model with a has_many_attached association.
2. Create a resource that has a :files field to represent the association.
3. Upload multiple files in association with one record of the model.
4. View the Show page for the record
5. Observe multiple ActiveStorage::Blob Load lines in the log, each of the form:

   SELECT "active_storage_blobs".* FROM "active_storage_blobs" WHERE "active_storage_blobs"."id" = ... LIMIT 1

Expected behavior & Actual behavior

Files field should use .includes(:blob) before iterating a has_many_attached field.

Models and resource files

System configuration

Avo version: 3.0.1.beta9

Rails version: 7.0.8

Ruby version: 3.2.2

License type:

• [x] Community
• [ ] Pro

Are you using Avo monkey patches, overriding views or view components?

• [x] Yes. If so, please post code samples.
• [ ] No

My only monkey-patches are to add helper methods exposed in various places, and aren't in the flow of execution for this bug.

class Avo::Resources::Items::TabGroup::Builder
  def comments_tab!
    tab("Comments") do
      panel do
        field(:comments,
              name:                "Comments",
              as:                  :has_many,
              discreet_pagination: true,
              scope:               -> { query.unscope(:order).order(created_at: :desc, id: :desc) },
              use_resource:        Avo::Resources::CommentsOn)
      end
    end
  end

  def versions_tab!
    tab("Versions") do
      panel do
        field(:versions,
              as:                  :has_many,
              discreet_pagination: true,
              scope:               -> { query.unscope(:order).order(created_at: :desc, id: :desc) })
      end
    end
  end
end

class Avo::BaseResource
  def timestamp_fields!(skip_index: false)
    discardable = model_class.columns.find { |col| col.name == "discarded_at" }.present?

    field(:created_at,   as: :date_time, hide_on: %i[show new edit], sortable: true) unless skip_index # rubocop:disable Layout/LineLength
    field(:discarded_at, as: :date_time, hide_on: %i[new edit],      sortable: true) if discardable && !skip_index # rubocop:disable Layout/LineLength

    panel("Timestamps", hide_on: %i[new edit]) do
      field(:created_at,   as: :date_time, hide_on: %i[new edit])
      field(:updated_at,   as: :date_time, hide_on: %i[new edit])
      field(:discarded_at, as: :date_time, hide_on: %i[new edit]) if discardable
    end
  end
end

Screenshots or screen recordings

Additional context

Impact

• [ ] High impact (It makes my app un-usable.)
• [ ] Medium impact (I'm annoyed, but I'll live.)
• [x] Low impact (It's really a tiny thing that I could live with.)

The bug wouldn't be especially notable except we use bullet and I've basically hit a point where a large fraction of Avo pages are throwing N+1 alerts at me, many of which Avo doesn't really put me in a position to resolve myself.

Urgency

• [ ] High urgency (I can't continue development without it.)
• [ ] Medium urgency (I found a workaround, but I'd love to have it fixed.)
• [x] Low urgency (It can wait. I just wanted you to know about it.)

For the time being, our volume of data isn't large enough for this to present a practical problem but I'd prefer to nip it in the bud before it does become a problem.

[adrianthedev]

Thanks @MrJoy. Would you be able to submit a PR with this update?

[MrJoy]

Taking a quick peek at the code, it looks like the code is designed to try and do this already.

return query.includes "#{field.id}_#{attachment}": :blob

Trying to run the tests locally, I'm running into:

The asset "avo.base.js" is not present in the asset pipeline.

Seems like that may be a missing step on the contributing section?

[MrJoy]

Also getting issues with ChromeDriver.

The tests try to hit https://chromedriver.storage.googleapis.com/LATEST_RELEASE_117.0.5938 and that... doesn't exist.

[MrJoy]

I've worked around the ChromeDriver issue, albeit in a way I really do not want to leave in place, but... seeing errors "avo.custom.js" not being available, despite the presence of it in the dummy app.

I remove the code that loads that, and I just get this error for basically everything:

      ActionView::Template::Error:
        undefined method `custom_sign_out_path' for #<Module:0x0000000107274d88>

[MrJoy]

Ok, I'm afraid this is too much of a time sink for me. If I can't even get the dev stack operational in a reasonable timeframe, the odds of me navigating the quite complicated codebase to produce a meaningful fix for this in a timeframe that justifies the opportunity cost of working on my business is basically zero.

[Paul-Bob]

Hello there @MrJoy, for some reason I didn't received the notifications on your comments. I'm sorry that you had this unpleasant experience. I just cloned avo into a new directory, configured the database.config file, run bin/init followed by bin/dev and It runs as expected, the bin/rspec also run all testes without issue. I would be glad to help if you want to try the setup with support over a discord channel.

Next week I'll get some time to analyze better the issue. Let us know if we can help you with anything.

[MrJoy]

No dice.

     ActionView::Template::Error:
       undefined method `custom_sign_out_path' for #<Module:0x0000000113a5a0a0>

And:

              Webdrivers::VersionError:
                Unable to find latest point release version for 117.0.5938. You appear to be using a non-production version of Chrome. Please set `Webdrivers::Chromedriver.required_version = <desired driver version>` to a known chromedriver version: https://chromedriver.storage.googleapis.com/index.html

I'm having to use bundle exec rspec because bin/rspec results in this error:

/Users/jonathonfrisby/.rbenv/versions/3.2.2/gemsets/avo-v3/gems/spring-4.0.0/lib/spring/configuration.rb:46:in `application_root_path': Spring::MissingApplication (Spring::MissingApplication)
    from /Users/jonathonfrisby/.rbenv/versions/3.2.2/gemsets/avo-v3/gems/spring-4.0.0/lib/spring/application.rb:284:in `loaded_application_features'
    from /Users/jonathonfrisby/.rbenv/versions/3.2.2/gemsets/avo-v3/gems/spring-4.0.0/lib/spring/application.rb:118:in `ensure in preload'
    from /Users/jonathonfrisby/.rbenv/versions/3.2.2/gemsets/avo-v3/gems/spring-4.0.0/lib/spring/application.rb:125:in `preload'
    from /Users/jonathonfrisby/.rbenv/versions/3.2.2/gemsets/avo-v3/gems/spring-4.0.0/lib/spring/application.rb:162:in `serve'
    from /Users/jonathonfrisby/.rbenv/versions/3.2.2/gemsets/avo-v3/gems/spring-4.0.0/lib/spring/application.rb:144:in `block in run'
    from /Users/jonathonfrisby/.rbenv/versions/3.2.2/gemsets/avo-v3/gems/spring-4.0.0/lib/spring/application.rb:138:in `loop'
    from /Users/jonathonfrisby/.rbenv/versions/3.2.2/gemsets/avo-v3/gems/spring-4.0.0/lib/spring/application.rb:138:in `run'
    from /Users/jonathonfrisby/.rbenv/versions/3.2.2/gemsets/avo-v3/gems/spring-4.0.0/lib/spring/application/boot.rb:19:in `<top (required)>'
    from <internal:/Users/jonathonfrisby/.rbenv/versions/3.2.2/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
    from <internal:/Users/jonathonfrisby/.rbenv/versions/3.2.2/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
    from -e:1:in `<main>'

[adrianthedev]

Yes, we need to refactor a bit the dev setup. We have a guide but it's not ready for release. We'll take care of this one soon.

[adrianthedev]

A few things you might consider. Using the self.includes option or even the self.find_record_method for now to silence the warnings.

[MrJoy]

@adrianthedev self.includes is only for the Index view, isn't it? This is an issue on the Show view.

[adrianthedev]

Ok. I see. You can use the self.find_record_method to add that includes statement. https://docs.avohq.io/3.0/customization.html#custom-find-method-for-show-and-edit-pages