Need advice on controller / view

Need some advice. I've got two modules: artists and paintings.

In my home page, I've got a 2nd page that says "Browse paintings",
which displays ALL paintings of ALL artists. This is being rendered by
paintings#index.

In another page, I should show all paintings of a specific artist
ONLY. Meaning, paintings from ONE artist. I cant use paintings#index
for this anymore since it is already being used by the homepage. Also
take note, both pages are rendered differently (meaning, data and
display are not the same). They don't look alike. So I can't use index
for both pages.

What approach would you take?

I am thinking of creating a method in the artists module called:
list_paintings. And then in /app/views/list_paintings.html.erb

What are your thoughts?

I was also thinking. Both "actions" should still call artists#index.
In artist#index, I'll have an if condition that checks which template/
view to render.

Is this a sensible approach?

The main page should belong to a controller called welcome, home or main, you can use models from within any controller so there is no problem populating this view with everything you need for the home page, then have the index for artist in the artist controllers and an index for paintings in the paintings controller.

I prefer this approach:

Have the following routes setup:

map.resources :artists do |artists|
  artists.resources :paintings
end
map.resources :paintings

Or in Rails 3

resources :artists do
  resources :paintings
end
resources :paintings

In your PaintingsController:

def index
  @artist = Artist.find(params[:artist_id])
  @paintings = @artist.paintings.all rescue Painting.all

  render 'index_by_artist' if params[:artist_id]
end

If an Artist is specified, via the URL /artists/1/paintings, it will
load all Paintings from the specified Artist and render the view named
'index_by_artist'.
If no Artist is given, using just the URL /paintings, it will load all
Paintings and use the default view 'index'.

Hi Christian,

I was also thinking. Both "actions" should still call artists#index.
In artist#index, I'll have an if condition that checks which template/
view to render.

Is this a sensible approach?

Need some advice. I've got two modules: artists and paintings.

Do you mean controllers? Or do you mean modules? For purposes of
this reply I'm assuming you mean controllers.

In my home page, I've got a 2nd page that says "Browse paintings",
which displays ALL paintings of ALL artists. This is being rendered by
paintings#index.

In another page, I should show all paintings of a specific artist
ONLY. Meaning, paintings from ONE artist. I cant use paintings#index
for this anymore since it is already being used by the homepage. Also
take note, both pages are rendered differently (meaning, data and
display are not the same). They don't look alike. So I can't use index
for both pages.

What approach would you take?

I am thinking of creating a method in the artists module called:
list_paintings. And then in /app/views/list_paintings.html.erb

What are your thoughts?

The typical approach would be to add a list method to your controller.
The default action of the index method is typically to list all
records, while a list method typically takes parameters used to filter
the returned records. Assuming that you want to remain as RESTful as
possible, use list to return your 'list by artist' page. List is a
RESTful method. Index is not. It's simply a method that's 'required'
in terms of normal web page naming.

HTH,
Bill

Hi Erol,

Your routing suggestion is good, however this

@artist = Artist.find(params[:artist_id])

will throw an exception if params[:artist_id] is not supplied. You
could avoid that using Artist.find_by_id which should return nil
rather than throwing a RecordNotFound exception. At the same time, it
is not typical for an index method to be called with parameters. More
typical to simply supply a list method for that purpose and include it
in your routes.

Best regards,
Bill

Yeah, it should have been a #find_by_id instead of #find. With regards
to params[:artist_id], it's not from an actual querystring parameter
but rather from a nested route:

/artists/1/paintings

Which IMHO is more RESTful.

Cheers,

I prefer this approach:

Have the following routes setup:

map.resources :artists do |artists|
artists.resources :paintings
end
map.resources :paintings

[...]

If an Artist is specified, via the URL /artists/1/paintings, it will
load all Paintings from the specified Artist and render the view named
'index_by_artist'.
If no Artist is given, using just the URL /paintings, it will load all
Paintings and use the default view 'index'.

This is certainly how I would do it. You've got a classic nested-
resources situation here.

Best,

Thanks all for the feedback. Very informative replies. At current I do
have it as a nested route as specified by Erol.

Bill has also indicated a very interesting fact. Being that index
should display ALL records, while list can be used for more specific
requests (i.e. to filter out results). I'll definitely give this a
shot. Which I believe is also a RESTful approach

I guess at this point, all suggestions made so far are valid. I don't
think there is a single "right way" to doing this. Both approaches
will work.

Either, I provide an "if condition", and based on that, render a
template. Or, create a list method and view. I guess it just comes
down to personal preference?

The former is arguably more RESTful.

Best,

Hi Christian,

Bill has also indicated a very interesting fact. Being that index
should display ALL records, while list can be used for more specific
requests (i.e. to filter out results). I'll definitely give this a
shot. Which I believe is also a RESTful approach

The index should display ALL records, but then again it depends on the
context; if it is being accessed in a context of a parent-child
relationship (artist-painting), it certainly can be used to display
ALL records based on the parent (artist).

It is a common "best-practice" in REST to define parent-child
relationships in the URI by slashes ("/"), rather than using a
querystring filter. This:

/artists/DaVinci/paintings

Is more RESTful than this:

/paintings/list?artist=DaVinci

Cheers,

Hmmm, good point there Erol