ruby-on-railsrubyruby-on-rails-7pagy

Pagy gem: best practice to paginate a collection that comes from the model's has_many association


In my Rails 7.1 app I have two models: Movie and Director coupled together with the many-to-many association, so I can call e.g. Director.all[index].movies to get all movies associated with this director.

In the directors_controller.rb I had a very basic show method:

# app/controllers/directors_controller.rb
class DirectorsController < ApplicationController
  def show
    @director = Director.find(params[:id])
  end
end

On a director's page I list all movies associated with this director:

<!-- app/views/directors/show.html.erb -->
<h1><%= @director.name =></h1>
<div>
  <%= render "shared/movies_list", movies: @director.movies %>
</div>

Now I'm going to add pagination to the shared/movies_list partial, using the Pagy gem. I won’t be able to pass the @director.movies anymore, since the partial now expects a collection wrapped in the pagy() method. The collection should be provided by the controller with the include Pagy::Backend mixin.

So I came up with this approach:

# app/controllers/directors_controller.rb
class DirectorsController < ApplicationController
  include Pagy::Backend

  def show
    @director = Director.find(params[:id])
    @pagy, @movies = pagy(@director.movies)
  end
end

Now I can pass the @movies collection to the partial that uses a Pagy helper method:

<!-- app/views/directors/show.html.erb -->
<h1><%= @director.name =></h1>
<div>
  <%= render "shared/movies_list", movies: @movies %>
</div>

I wonder, if there's a better way to achieve this rather than creating the @movies instance variable in the DirectorsController#show action?


Solution

  • Moving the database query into the controller is actually far better. The view should simply take the data it gets from the controller and turn it into HTML in the simplest way possible.

    This makes it much easier to maintain an overview of the queries and views by themselves are by their nature messy as its a mix of markup and logic so KISS.

    However it's somewhat debateable if you have chosen the correct controller/route. If the intent is to show information about a director and maybe a few movies that they have created than /directors/1 is perfectly fine. But it gets a bit wierd when that controller becomes responsible for paginating a completely different resource.

    What does /directors/1?page=5 mean and what at this point is the responsiblity of DirectorsController#show method?

    If the intent is to display a paginated list it would arguably be better to treat this as a nested resource:

    resources :directors do
      resources :movies, only: :index, module: :directors
    end
    
    # Nesting this in a module is optional 
    module Directors 
      # Displays the movies of a director
      class MoviesController
        # GET /directors/1/movies
        # GET /directors/1/movies?page=5
        def index 
          @director = Director.find(params[:director_id])
          @pagy, @movies = pagy(@director.movies)
        end
      end
    end
    

    This gives the route /directors/1/movies?page=5 which is self explainatory and a controller with a single purpose which is to display the movies of a given director.