# Destroy question (best practices?)

**URL:** <https://www.ruby-forum.com/t/destroy-question-best-practices/125533>\
**Category:** Rails\
**Created:** [January 9, 2008, 11:18pm UTC](https://www.ruby-forum.com/t/destroy-question-best-practices/125533 "2008-01-09T23:18:59Z")\
**Posts on this page:** 9\
**Page:** 1

<div class="post-metadata">

**Author:** ![dotty](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/e56c9b/32.png) [@dotty](https://www.ruby-forum.com/u/dotty)\
**Post date:** [January 9, 2008, 11:18pm UTC](https://www.ruby-forum.com/t/destroy-question-best-practices/125533/1 "2008-01-09T23:18:59Z")

</div>

Hey everyone, I’m pretty new (really new) to RoR, actually rails brought  
the coder back out in me.

I’ve been learning how Rails works by creating a Project/Issue’s  
application, which I will expand as my knowledge grows. Right now I  
have 2 Model’s under one controller. Issue and Category laid out with  
Issue belongs to Category and Category has many issues. My basic  
functionality works perfect as I can created edit both models the way I  
wish.

The initial screen is my list or projects (list.rhtml), once you click  
on the project you get a list of issues that are part of that  
project(show\_issues.rhtml) which is passed and expects a project\_id.  
ie: localhost:3000/isssues/show\_issues/3 for project id #3)

I would like some idea’s our best practices, on how I can delete an  
issue and be redirected back to the show\_issues/#project\_id to list of  
issues. Right now my delete definition in my controller is just  
redirected back to the project list.

Here’s the code from my show\_issues

\<%= stylesheet\_link\_tag “style” %\>

 \<% @project.issues.each do |c| %\> \<% end %\> 

| Outstanding Tasks for \<%= @project.pj\_name %\> |
| --- |
| \<%= link\_to c.title, :action =\> "show", :id =\> c.id -%\> | \<%= link\_to 'edit', :action =\> 'edit', :id =\> c.id %\> | \<%= link\_to 'delete', :action =\> 'delete', :id =\> c.id %\> |
| \<%= link\_to 'show projects', :action =\> 'list' %\> | \<%= link\_to 'post new task', :action =\> 'new' %\> |

Thanks

---

<div class="post-metadata">

**Author:** ![dotty](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/e56c9b/32.png) [@dotty](https://www.ruby-forum.com/u/dotty)\
**Post date:** [January 9, 2008, 11:24pm UTC](https://www.ruby-forum.com/t/destroy-question-best-practices/125533/2 "2008-01-09T23:24:38Z")

</div>

> The initial screen is my list or projects (list.rhtml), once you click  
> on the project you get a list of issues that are part of that  
> project(show\_issues.rhtml) which is passed and expects a project\_id.  
> ie: localhost:3000/isssues/show\_issues/3 for project id #3)
> 
> I would like some idea’s our best practices, on how I can delete an  
> issue and be redirected back to the show\_issues/#project\_id to list of  
> issues. Right now my delete definition in my controller is just  
> redirected back to the project list.

In your controller:

def delete  
issue = Issue.find\_by\_id(params[:id])  
if issue.nil?  
#toss an error maybe?  
else  
issue.destroy  
redirect\_to :action =\> :show\_issues, :id =\> issue  
end  
end

> \<%= link\_to c.title, :action =\> "show", :id =\> c.id -%\>
> 
> It doesn’t really matter, but it’s simpler to just say :id =\> c
> 
> That way you could put this in your Issue model:
> 
> def to\_param  
> “#{id}-#{title.gsub(/[^a-z0-9]+/i, ‘-’).downcase}”  
> rescue  
> id  
> end
> 
> and you’re url for that issue would be pretty.

---

<div class="post-metadata">

**Author:** ![dotty](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/e56c9b/32.png) [@dotty](https://www.ruby-forum.com/u/dotty)\
**Post date:** [January 9, 2008, 11:42pm UTC](https://www.ruby-forum.com/t/destroy-question-best-practices/125533/3 "2008-01-09T23:42:47Z")

</div>

Instead of:

def delete  
issue = Issue.find\_by\_id(params[:id])  
if issue.nil?  
#toss an error maybe?  
else  
issue.destroy  
redirect\_to :action =\> :show\_issues, :id =\> issue  
end  
end

I would do:

def destroy  
Issue.find(params[:id]).destroy  
rescue ActiveRecord::RecordNotFound  
flash[:notice] = “The issue you were looking for could not be found.”  
ensure  
redirect\_to issues\_path  
end

Try to keep yourself from repeating words in your code, such as the word  
“issue”, you have /issues/show\_issues which may just be an index page  
(one  
that shows all the issues, right?). This URL could be shortened down to  
just  
/issues/1 using Restful Routing:

> **[frozenplague.net](http://frozenplague.net/2008/01/06/restful-routing-an-overview/)**
>
> This domain may be for sale!

---

<div class="post-metadata">

**Author:** ![dotty](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/e56c9b/32.png) [@dotty](https://www.ruby-forum.com/u/dotty)\
**Post date:** [January 9, 2008, 11:53pm UTC](https://www.ruby-forum.com/t/destroy-question-best-practices/125533/4 "2008-01-09T23:53:19Z")

</div>

And that’s what Restful Routing does for you automatically.

On Jan 10, 2008 9:19 AM, Jeremy Weiskotten  
[removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid)  
wrote:

> ## You might also want to defend against GET requests to the delete resource, just in case. (Google for why it’s a good idea.)
> 
> Posted via [http://www.ruby-forum.com/](http://www.ruby-forum.com/).
> 
> >

–  
Ryan B.

> **[frozenplague.net](http://www.frozenplague.net)**
>
> This domain may be for sale!

Feel free to add me to MSN and/or GTalk as this email.

---

<div class="post-metadata">

**Author:** ![dotty](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/e56c9b/32.png) [@dotty](https://www.ruby-forum.com/u/dotty)\
**Post date:** [January 10, 2008, 12:00am UTC](https://www.ruby-forum.com/t/destroy-question-best-practices/125533/5 "2008-01-10T00:00:40Z")

</div>

Thanks for the info on some other things to look at, I’ll be reading up  
on most of it as time goes on. The next chapter in the book I’m  
somewhat following goes in to the routing which sounds interesting.

But…

Here’s the big issue. show\_issues is expecting the project\_id from the  
issue table or also as you know the id from the projects table. A  
simple redirect\_to show\_issues will give me an nil error since no  
project id has been passed back.

show\_issues will then display back all issues thats tied to that  
project.

Maybe I designed this wrong (wouldn’t be surprised as it’s my first  
real shot at it).

---

<div class="post-metadata">

**Author:** ![dotty](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/e56c9b/32.png) [@dotty](https://www.ruby-forum.com/u/dotty)\
**Post date:** [January 10, 2008, 12:10am UTC](https://www.ruby-forum.com/t/destroy-question-best-practices/125533/6 "2008-01-10T00:10:19Z")

</div>

I’m going to sound like a broken record here, but restful routing fixes  
that  
too!

You can do redirect\_to project\_issues\_path(@project) and it will show  
you  
all those issues from that project.

On Jan 10, 2008 9:30 AM, Kevin R. [removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid)  
wrote:

> project id has been passed back.
> 
> >

–  
Ryan B.

> **[frozenplague.net](http://www.frozenplague.net)**
>
> This domain may be for sale!

Feel free to add me to MSN and/or GTalk as this email.

---

<div class="post-metadata">

**Author:** ![dotty](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/e56c9b/32.png) [@dotty](https://www.ruby-forum.com/u/dotty)\
**Post date:** [January 9, 2008, 11:49pm UTC](https://www.ruby-forum.com/t/destroy-question-best-practices/125533/7 "2008-01-09T23:49:10Z")

</div>

def delete  
Issue.destroy params[:id]  
redirect\_to :action =\> :show\_issues  
end

I’d handle the error condition of an invalid issue ID as a 500 error (or  
a 404 might be better) because you’re trying to access an invalid  
resource.

You might also want to defend against GET requests to the delete  
resource, just in case. (Google for why it’s a good idea.)

---

<div class="post-metadata">

**Author:** ![dotty](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/e56c9b/32.png) [@dotty](https://www.ruby-forum.com/u/dotty)\
**Post date:** [January 10, 2008, 12:10am UTC](https://www.ruby-forum.com/t/destroy-question-best-practices/125533/8 "2008-01-10T00:10:58Z")

</div>

Kevin R. wrote:

> Thanks for the info on some other things to look at, I’ll be reading up  
> on most of it as time goes on. The next chapter in the book I’m  
> somewhat following goes in to the routing which sounds interesting.
> 
> But…
> 
> Here’s the big issue. show\_issues is expecting the project\_id from the  
> issue table or also as you know the id from the projects table. A  
> simple redirect\_to show\_issues will give me an nil error since no  
> project id has been passed back.
> 
> show\_issues will then display back all issues thats tied to that  
> project.
> 
> Maybe I designed this wrong (wouldn’t be surprised as it’s my first  
> real shot at it).

def delete  
issue = Issue.find params[:id]  
issue.destroy  
redirect\_to :action =\> :show\_issues, :id =\> issue.project\_id  
end

That should do what you want.

However, you should consider a more RESTful design like Ryan suggested.

---

<div class="post-metadata">

**Author:** ![dotty](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/e56c9b/32.png) [@dotty](https://www.ruby-forum.com/u/dotty)\
**Post date:** [January 10, 2008, 12:24am UTC](https://www.ruby-forum.com/t/destroy-question-best-practices/125533/9 "2008-01-10T00:24:01Z")

</div>

Jeremy Weiskotten wrote:

> However, you should consider a more RESTful design like Ryan suggested.

Thanks guys, that worked, I thought I tried that before but I guess  
not. And yes I’ll check out restful routing, I just want to get the  
basics down fairly well before I bring on more on
