Partials and a question of correct form

I'm new to Rails and trying to figure out the correct way to handle dynamic data in a partial. The partial in question will be used to display a google ad. The html for this ad will differ based upon the user (do they pay to not see ads, what the format/style of the ad will look like needs to match the look and feel they have configured for their profile, etc.).

I was looking to place an ad in a view as follows:

<%= render "shared/ad_banner", :ad_type => "120x90"%>

where the ad type basically specifies what size ad the ad_banner partial needs to render. The different html for the ads are stored in a database. My question is can/should the partial do a lookup to figure out what ad html to render based upon the ad_type passed in (along with information about the currently logged in user)? Or should the actual lookup occur in the controller and the html be passed into the partial? My first thought was it would be cleaner if I had the partial do the lookup since the controller would not have to have any knowledge about what size ad needs to be rendered, this information was basically embedded in the view.

However, when trying to figure out how to make the partial do the lookup I started wondering if this was bad form based upon the lack of examples I saw for this type of approach (all the examples seemed to be passing the data in).

Any guidance would greatly be appreciated.


My thought would be that the view code should preferably not be looking up stuff. The controller should provide (via @ variables) all the data that it needs. However, if you are talking about the size of something on display this should certainly not be defined in the controller. The controller should not say 'display this at a certain size', but should provide whatever data the view needs in order that it can decide the size. In fact generally the size would be in css with the view just setting up classes so even the view does not have to know the size or layout.


you should keep logic out of views partials.

here are a couple different ways to accomplish this

1) create a helper function

# app/helpers/application_helper.rb def render_ad_html   if not logged_in? or current_user.show_ads?     ad = Ad.find_by_controller_name( params[:controller] ) #     render :partial => "shared/ad_#{ad.dimensions}", :ad => ad   end end

then in your application layout file (or in any view) you can call <%= render_ad_html %>

2) create a before_filter (*preferred) # app/controllers/application_controller.rb class ApplicationController < ...   def setup_html_ads      if logged_in? and current_user.show_ads?         @ad = Ad.find_by_controller_name( params[:controller] )      end   end

you can load the before_filter in whatever controllers you want to show ads #app/controllers/user_controller class UserController < ApplicationController   before_filter :setup_html_ads end

and render them in your views if you have an @ad object <%= render :partial => "shared/ad" if @ad %>

either way will work. but it's best to keep your views as dumb as possible. for instance, if you want to show the adverts in your rss stream or in emails... you don't want to worry about the conditional logic contained in the partial... since you just want the html.

cheers, sean