views:

39

answers:

1

When I click on the delete link I created, it doesn't do anything (even the flash[:notice] part) in the controller. Am I not calling the .delete? part correctly? The POST part works as I can add tips.

Link:

<%= link_to "Delete", :controller => "/admin", :action => "tips", :id => t.id, :method => :delete, :confirm => "Are you sure?" %>

Admin Controller

def tips
  @tips = Tip.all
  if request.post?
    tip = Tip.new(params[:geek_tips])
    if tip.save
      flash[:notice] = "Saved!"
      redirect_to :action => "tips"
    else
      flash[:notice] = "Error!"
    end
  elsif request.delete?
    tip = Tip.find_by_id(params[:id])
    tip.delete!
    flash[:notice] = "Delete Message"
    redirect_to :action => "tips"
  end
end
+2  A: 

Design issues aside, I think that your :method option is being interpreted as a query param. Can you see "method" in the URL if you hover on the link?

If so, try...

<%= link_to "Delete", {:controller => "/admin", :action => "tips", :id => t.id}, :method => :delete, :confirm => "Are you sure?" %>

Note the braces around the part that defines the URL of the request.

Regarding the design: Any time you have multiple actions in one controller method there is probably a design issue. In this case, instead of using one admin controller method to do multiple tips actions I would consider making a dedicated tips_controller controller to map to your Tip model.

If you used RESTful routes, that is, in config.rb you set...

map.resources :tips

...then you could use the create and destroy methods in your tips_controller for creating and deleting your tips respectively.

Greg
Your idea would seem to create a whole bunch of controllers for every model that has a CRUD. What specifically is the "design issue" you refer to that renders your suggestion a better solution?
Kevin
Yes, exactly. I wouldn't call it my idea. It is the pattern that Rails supports. There are exceptions, but generally you want to have a controller and views that maps to each of your models. First, this is the Rails convention so following it will make your life much easier when it comes to routes and URLs. The reasons for the MVC approach in the first place are many -- dealing with complexity (e.g. multiple simple controllers better than one giant one), maintainability, readability... too many to put in this comment, but there is plenty of literature on MVC.
Greg
Did the braces solve your problem?
Greg
MVC is the answer. every model should have its own controller. otherwise you have to deal with big controllers, which is a bad thing in the long run. worth reading: http://weblog.jamisbuck.org/2006/10/18/skinny-controller-fat-model
Labuschin
Greg: Yeah, thanks! I'm honestly not sure about using RESTful routes until I read a bit more. My controller actions are generally quite small with the model handling most of the work, and I like having one url like /users/comments/2 where if it's DELETE it deletes, POST creates, and GET retrieves without "comments" having to be its own controller. Isn't that still in the spirit of RESTful routes?
Kevin
Yes, you are definitely part of the way there. Using the RESTful routes is really simple once you get the hang of it though, and it is the way Rails supports REST directly. In fact, the RESTful routes do have the same URL for multiple actions and discriminate between them using the http method. But the actions are separate and therefore much more readable and flexible (and other Rails devs will immediately know what you are up to). I would say getting your head around this would be well worth the effort. Good luck.
Greg
Thanks Greg, I appreciate it!
Kevin