How do I make this controller RESTful?

Rails
1

Hello,

I’m trying to make this controller RESTful, but its post method uses
two other methods – select_category and show_form – see them in
action at http://demo.chuckslist.org/ads/post. How do I reduce all of
this to just one method? Other advice on making this controller as
RESTful and beautiful as possible would ofcourse be much obliged.
Here’s the controller:

class AdsController < ApplicationController
def show
@ad = Ad.find_by_id(params[:id])
if (@ad.nil?)
flash[:warning] = “ad doesn’t exist”
redirect_to root_path
end
end
def destroy
if request.post?
@ad = Ad.find_by_activation_code(params[:id])
if (@ad.nil?)
flash[:warning] = “ad doesn’t exist”
redirect_to root_path
else
@ad.destroy
flash[:notice] = “ad removed”
redirect_to root_path
end
end
end
def list
@category = Category.find_by_slug(params[:slug])
if @category
@ads = @category.ads.all_active
@requested_category = @category.name + ’ in ’ +
@category.parent_category.name
else
@category = ParentCategory.find_by_slug(params[:slug])
if @category
@ads = @category.all_ads
@requested_category = @category.name
else
flash[:warning] = “invalid request”
redirect_to root_path
end
end
end
def post
@parents = ParentCategory.find :all, :order => ‘name ASC’
end
def select_category
@parent_category = ParentCategory.find_by_id(params[:id])
end
def show_form
@category = Category.find_by_id(params[:id])
end
def new
if (params[:email] != params[:email_confirmation])
flash[:warning] = “e-mails don’t match”
redirect_to :controller => ‘ads’, :action => ‘post’
else
@author = Author.find_by_email(params[:email])
if @author.blank?
@author = Author.new
@author.email = params[:email]
@author.ip = request.env[‘REMOTE_ADDR’]
@author.save
end
@ad = Category.find_by_id(params[:category]).ads.new
@ad.title = params[:title]
@ad.ad = params[:ad].gsub(“\n”, “
”)
@ad.expiration = Time.now + 30.days
@ad.author = @author
@ad.author_ip = request.env[‘REMOTE_ADDR’]
@ad.save
@ad.handle_images(params[“image_attachments”])
Mail.deliver_activation(@ad, @author.email)
flash[:notice] = “ad pending activation”
end
end
def activate_ad
@ad = Ad.find_by_activation_code(params[:activation_code])
if (@ad.nil?)
flash[:warning] = “error activating ad”
redirect_to root_path
else
if @ad.activate_ad(params[:activation_code])
flash[:notice] = “ad activated”
Mail.deliver_activated(@ad, @ad.author.email)
redirect_to :action => ‘edit’, :activation_code =>
@ad.activation_code
else
flash[:warning] = “error activating ad”
redirect_to root_path
end
end
end
def manage
@ad = Ad.find_by_activation_code(params[:activation_code])
if (@ad.nil?)
flash[:warning] = “ad doesn’t exist”
redirect_to root_path
else
end
end
def edit
@ad = Ad.find_by_activation_code(params[:activation_code])
if (@ad.nil?)
flash[:warning] = “ad doesn’t exist”
redirect_to root_path
else
end
end
def update
@ad = Ad.find_by_activation_code(params[:activation_code])
if (@ad.nil?)
flash[:warning] = “ad doesn’t exist”
redirect_to root_path
else
@ad.ad = params[:ad].gsub(“\n”, “
”)
@ad.title = params[:title]
if @ad.save
@ad.handle_images(params[“image_attachments”])
flash[:notice] = “ad updated”
else
flash[:warning] = “error updating ad”
end
redirect_to :controller => ‘ads’, :action =>
‘manage’, :activation_code => @ad.activation_code
end
end
def feed
@ads = Ad.all_active
respond_to do |format|
format.rss { render :layout => false }
format.atom # index.atom.builder
end
end
end

Take care!

– Rubienubie

2

Can somebody please help me?

So far I’ve moved show_category and edit_form into my ad model, and
edited ads_controller.rb to look like this:

class AdsController < ApplicationController
def index
@category = Category.find_by_slug(params[:slug])
if @category
@ads = @category.ads.all_active
@requested_category = @category.name + ’ in ’ +
@category.parent_category.name
else
@category = ParentCategory.find_by_slug(params[:slug])
if @category
@ads = @category.all_ads
@requested_category = @category.name
else
flash[:warning] = “invalid request”
redirect_to root_path
end
end
end
def show
@ad = Ad.find_by_id(params[:id])
if (@ad.nil?)
flash[:warning] = “ad doesn’t exist”
redirect_to root_path
end
end
def create
@parents = ParentCategory.find :all, :order => ‘name ASC’
end
def new
if (params[:email] != params[:email_confirmation])
flash[:warning] = “e-mails don’t match”
redirect_to create_ads_path
else
@author = Author.find_by_email(params[:email])
if @author.blank?
@author = Author.new
@author.email = params[:email]
@author.ip = request.env[‘REMOTE_ADDR’]
@author.save
end
@ad = Category.find_by_id(params[:category]).ads.new
@ad.title = params[:title]
@ad.ad = params[:ad].gsub("\n", “
”)
@ad.expiration = Time.now + 30.days
@ad.author = @author
@ad.author_ip = request.env[‘REMOTE_ADDR’]
@ad.save
@ad.handle_images(params[“image_attachments”])
Mail.deliver_activation(@ad, @author.email)
flash[:notice] = “ad pending activation”
end
end
def edit
@ad = Ad.find_by_activation_code(params[:activation_code])
if (@ad.nil?)
flash[:warning] = “ad doesn’t exist”
redirect_to root_path
else
end
end
def update
@ad = Ad.find_by_activation_code(params[:activation_code])
if (@ad.nil?)
flash[:warning] = “ad doesn’t exist”
redirect_to root_path
else
@ad.ad = params[:ad].gsub("\n", “
”)
@ad.title = params[:title]
if @ad.save
@ad.handle_images(params[“image_attachments”])
flash[:notice] = “ad updated”
else
flash[:warning] = “error updating ad”
end
redirect_to edit_ads_path, :activation_code =>
@ad.activation_code
end
end
def destroy
if request.post?
@ad = Ad.find_by_activation_code(params[:id])
if (@ad.nil?)
flash[:warning] = “ad doesn’t exist”
redirect_to root_path
else
@ad.destroy
flash[:notice] = “ad removed”
redirect_to root_path
end
end
end
def feed
@ads = Ad.all_active
respond_to do |format|
format.rss { render :layout => false }
format.atom # index.atom.builder
end
end
end

3

I guess I’m on my own with this one.

4

Rubienubie wrote:

I guess I’m on my own with this one.

sorry, but you must understand, that most people answering here are
professional programmers with something like a working day. we jump in
here once in a while and answer what we can. this may depend on our
knowledge and on the question. not so many people will have the time to
analyze that much code. especially not during the week. (so since it’s
friday today tour chances are, somebody will do during weekend.
otherwise keep your questions as short as possible and don’t expect
people to jump for cosmetical cleanup stuff.

5

@Thorsten: If you feel that way then just ignore it and move on. No
reason to get ruffled by someone you’ll probably never meet.
Moreover, the community as a whole is more likely to grow and evolve
if we don’t eat our young. :slight_smile:

@rubienubie:
There seems to be a fundamental misunderstanding about REST at play
here. Your first post in this thread suggests that you are worried
about having more than one method that responds to an HTTP post but
that is not a limiting factor. For simplicity we’ll say that a
resource is an object that responds on a particular address. It’s
response can be tailored based on two things: actions and the http
verbs associated with those actions.

So, for your AdsController you’re actually dealing with two
resources. One resource is the collection of ads and the other is an
individual ad (really, many of these but one pattern to code). For
the collection, then:

…/ads #address of the ads collection

can respond in different ways based on the HTTP verbs. Without
qualification a GET maps to the index and a POST maps to create.

Similarly,

…/ads/1 # address of an individual ad

can respond in different ways. A PUT will #update it and a DELETE
will #destroy it while a GET will #show it.

But you also get another degree of movement when you add actions. For
example,

…/ads/1/do_something

Could add as many as four different responses based on whether you do
a GET, POST, PUT, or DELETE to that address. You’d map these in
routes.rb like so:

map.resources :ads, :collection=>{:do_something_else=>verb or
any}, :member=>{:do_something=>verb or any}

So technically you could keep your controller very much the way it was
and make it “RESTful”. Your refactoring is moving in a cleaner
direction, though. One thing I’d add is to consider the “fat model,
skinny controller” suggestion:
http://www.therailsway.com/2007/6/1/railsconf-recap-skinny-controllers.
You’ve really got a lot of business logic tied up in your controllers
that should not be there.

On Apr 25, 5:33 am, Thorsten M. <rails-mailing-l…@andreas-

6

@Andy: Oops, i hope nobody misunderstood my comment. it was not the
intention to keep people from asking whatever they want or how long they
want.
i just wanted to give a tip, that clear and compact questions may get
answered faster and others like this may take some time.
(i think i answered quite some questions about “best practices” or more
complicated issues myself.)

i didn’t want to criticize Rubienubie or the question.