Rails - Nov 2008 - Ensuring controller actions have necessary params

The usual CRUD controller actions assume that certain parameters were  
sent. Let''s use a controller for books as an example:

BooksController < ApplicationController
   def create
     @book = Book.new params[:book]
     # ..etc..
   end
end

So #create assumes that params[:book] exists. This is fine, because if  
the "book" field wasn''t POSTed to #create, then params[:book]
will
simply be nil, because ''params'' is a Hash.

However, if our controller action tries to look at params[:book] 
[:page_ids] when the "book" field wasn''t POSTed to #create,
we''d get a
NoMethodError. This is because params[:book] is nil, which means that  
params[:book][:page_ids] is equivalent to nil[:page_ids] , which will  
obviously raise an error.

So the question is, do you bother to write additional checks for your  
controller actions for these edge cases?

I would, because I like to be thorough and know that I''ve covered all  
of my bases. But I can understand why people wouldn''t bother, because  
said edge cases will probably only be generated by yourself during  
testing, and by hooligans mucking with your forms.

What do you guys think?
-Nick

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Hi --

On Sun, 30 Nov 2008, Nick Hoffman wrote:
>
> The usual CRUD controller actions assume that certain parameters were
> sent. Let''s use a controller for books as an example:
>
> BooksController < ApplicationController
>   def create
>     @book = Book.new params[:book]
>     # ..etc..
>   end
> end
>
> So #create assumes that params[:book] exists. This is fine, because if
> the "book" field wasn''t POSTed to #create, then
params[:book] will
> simply be nil, because ''params'' is a Hash.
>
> However, if our controller action tries to look at params[:book]
> [:page_ids] when the "book" field wasn''t POSTed to
#create, we''d get a
> NoMethodError. This is because params[:book] is nil, which means that
> params[:book][:page_ids] is equivalent to nil[:page_ids] , which will
> obviously raise an error.
>
> So the question is, do you bother to write additional checks for your
> controller actions for these edge cases?
>
> I would, because I like to be thorough and know that I''ve covered
all
> of my bases. But I can understand why people wouldn''t bother,
because
> said edge cases will probably only be generated by yourself during
> testing, and by hooligans mucking with your forms.
>
> What do you guys think?
One good way to think about it is: if you did have an extra check,
what would it do? In other words, if create gets executed and
params[:book] is nil, what would you want to happen? And, as a
follow-up, does your current error-reporting process already do what
you would want to have done in such a case?


David

-- 
Rails training from David A. Black and Ruby Power and Light:
INTRO TO RAILS (Jan 12-15), Fort Lauderdale, FL
See http://www.rubypal.com for details
Coming in 2009: The Well-Grounded Rubyist (http://manning.com/black2)

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

On 2008-11-30, at 16:31, David A. Black wrote:
> Hi --
>
> One good way to think about it is: if you did have an extra check,
> what would it do? In other words, if create gets executed and
> params[:book] is nil, what would you want to happen?
Hi David. I was thinking about what do in this situation, and reckon  
that any of these is a good approach to take:
1) Send a redirect to an appropriate page and return from the method;
2) Render an appropriate page, and return from the method;
3) Set params[:book] to an empty Hash;
> And, as a
> follow-up, does your current error-reporting process already do what
> you would want to have done in such a case?
I haven''t setup error-reporting in the app yet, but it''s in my
issue
tracker as another thing to do before releasing a beta!

Cheers,
Nick

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

On 30 Nov 2008, at 21:47, Nick Hoffman wrote:
>
> On 2008-11-30, at 16:31, David A. Black wrote:
>> Hi --
>>
>> One good way to think about it is: if you did have an extra check,
>> what would it do? In other words, if create gets executed and
>> params[:book] is nil, what would you want to happen?
>
> Hi David. I was thinking about what do in this situation, and reckon
> that any of these is a good approach to take:
> 1) Send a redirect to an appropriate page and return from the method;
> 2) Render an appropriate page, and return from the method;
> 3) Set params[:book] to an empty Hash;
>
My take on this: say you made a mistake somewhere, for example your  
form is submitting a parameter with the wrong name (or the controller  
was using the wrong parameter) and that it wasn''t caught by your tests
(after all your tests won''t be checking that the inputs you use in  
your functional test are the same as those that would be produced by a  
browser having rendered your form)

I want:
- to know about it yourself (ie. David''s follow up)
- it be obvious to the user that something bad happened (I.e don''t  
lull in them into a false sense of security by giving the impression  
the record was created fine whereas you actually just created it off  
the back of an empty Hash. You don''t need to display the full grungey  
error page, a ''something bad happened page'' is fine usually

Fred
>> And, as a
>> follow-up, does your current error-reporting process already do what
>> you would want to have done in such a case?
>
> I haven''t setup error-reporting in the app yet, but it''s
in my issue
> tracker as another thing to do before releasing a beta!
>
> Cheers,
> Nick
>
> >

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

On 2008-11-30, at 18:31, Frederick Cheung wrote:
> On 30 Nov 2008, at 21:47, Nick Hoffman wrote:
>> On 2008-11-30, at 16:31, David A. Black wrote:
>>> Hi --
>>>
>>> One good way to think about it is: if you did have an extra check,
>>> what would it do? In other words, if create gets executed and
>>> params[:book] is nil, what would you want to happen?
>>
>> Hi David. I was thinking about what do in this situation, and reckon
>> that any of these is a good approach to take:
>> 1) Send a redirect to an appropriate page and return from the method;
>> 2) Render an appropriate page, and return from the method;
>> 3) Set params[:book] to an empty Hash;
>>
>
> My take on this: say you made a mistake somewhere, for example your
> form is submitting a parameter with the wrong name (or the controller
> was using the wrong parameter) and that it wasn''t caught by your
tests
> (after all your tests won''t be checking that the inputs you use in
> your functional test are the same as those that would be produced by a
> browser having rendered your form)
>
> I want:
> - to know about it yourself (ie. David''s follow up)
> - it be obvious to the user that something bad happened (I.e don''t
> lull in them into a false sense of security by giving the impression
> the record was created fine whereas you actually just created it off
> the back of an empty Hash. You don''t need to display the full
grungey
> error page, a ''something bad happened page'' is fine
usually
>
> Fred
That''s a good point you raise, Fred. The third solution that I  
proposed could cover up some bugs in your app that you''d want to  
actually see. Also, as you said, there''s no need to fake rainbows and  
unicorns for a user who submits bad data on purpose.
-Nick

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---