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", "<br />")
@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", "<br />")
@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
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---
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", "<br />")
@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", "<br />")
@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
On Apr 23, 12:45 pm, Rubienubie
<rubienu...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:> 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 athttp://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", "<br />")
> @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", "<br />")
> @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
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---
I guess I''m on my own with this one. On Apr 24, 12:54 pm, Rubienubie <rubienu...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> 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", "<br />") > @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", "<br />") > @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 > > On Apr 23, 12:45 pm, Rubienubie <rubienu...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > > 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 athttp://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", "<br />") > > @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", "<br />") > > @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--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
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. -- Posted via http://www.ruby-forum.com/. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
@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. :)
@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 Mueller <rails-mailing-l...@andreas-
s.net> wrote:> 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.
> --
> Posted viahttp://www.ruby-forum.com/.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---
@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. -- Posted via http://www.ruby-forum.com/. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---