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?
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.