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