AR::Base#attributes = nil

Published February 14, 2007

I just noticed a quirk of ActiveRecord::Base#attributes= when writing some functional tests.

Suppose I have a SitesController with the following code:


  def create
    @site = Site.new(params[:site])
    @site.save!
    flash[:notice] = "Site created"
    redirect_to site_path(@site)
  rescue ActiveRecord::RecordInvalid
    flash[:notice] = "Please correct invalid fields"
    render :action => "new"
  end
def update
@site.attributes = params[:site]
@site.save!
flash[:notice] = “Site udpated”
redirect_to site_path(@site)
rescue ActiveRecord::RecordInvalid
flash[:notice] = “Please correct invalid fields”
render :action => “edit”
end

When you call create w/o specifying the site param (i.e. params[:site] == nil), you’ll get a validation error back.

When you call update w/o specifying the site param it succeeds in saving and redirects you back to the site.

It turns out calling @site.attributes = nil does nothing to the site, and you end up just resaving it, even though no attributes were updated. Makes sense, but not really the behaviour I want.

Anybody know an easy way to handle this nil param case whilst still keeping the controller nice and lean?

Also, what’s the story with having both attributes= and update_attributes? The only difference I see is that update_attributes returns self.

Archived comments

Comments were previously allowed on articles. Though no new comments are being accepted you can see the old comments below.

Shalev

update_attributes is like create in that it updates AND saves the record in one go. With attributes= you need to manually save it.

That should simplify your calls to attributes= and save.
Ben Askins

Shalev’s hit the nail on the head with the differences between
updates_attributes and attributes=

Also, IIRC, update_attributes doesn’t raise exceptions but returns nil
(or false) if the update failed. Personally I prefer Tim’s method of
using attributes= and save!, then rescuing the exception.

Tim – the validate_request plugin (http://tinyurl.com/33ewqq) might
help you out with verifying the contents of parms[:site] before
assigning the attributes of the model.
Tim Lucas

Ah… dunno how I missed that. Cheer Shalev!

Ben: there’s also an update_attributes! that does raise the RecordInvalid exception.

Thanks for the link, though it’s not really much cleaner than verify :params => :site, :only => :update.
Tim Lucas
In practice the cleanest way is probably to defined a ParamMissingError in ApplicationController, and then have all update actions have this at the start:
```
def update
  raise ParamMissingError unless params[:site]
  ...
  rescue ActiveRecord::RecordInvalid, ParamMissingError
  ..
end
```
Ben Askins

Didn’t know about update_attributes! Thanks for the tip, although I think I’ll stay with attributes= and save! as it seems a little cleaner.

Raising the exception is nice and clean, keeps all your error handling in one place.
K. Adam Christensen

If you want to treat the missing params[:site] the same as improperly saving a record, then the exception raising is the way to go. But, I wouldn’t give up on verify just yet.

The reason I say this is because you have to ask the question why first. Why did a user try to save a record without sending site information?

It could be that someone is trying to hack with you, so does it matter if we don’t give feedback to someone who isn’t playing by the rules? Would it be better to perhaps log that user’s information? Or it could be someone who is trying to use an API? While your code example is not using respond_to, what if it did? You could use verify to intercept the bad calls and return help for your API: verify :params => 'site', :only => [:update, :create], :redirect_to => :site_help.

The beauty and frustration of all this is that there are a million ways to do anything.
Tim Lucas

Cheers Mr K.

Sure it’s possible that somebody’s “hacking” (i.e. performing bogey requests), but I’m not overly concerned about that. Though verify works, it doesn’t feel natural the way it handles things when verification fails, and you can’t add any more complex param checking than “does this param exist?”

Whilst I love repond_to, does anybody out there actually provide their public API by just pointing the world at their controllers? Can you provide useful, robust and human error handling just using this, and is there a perfect 1:1 mapping between browser request params or do you end up having to mung params and litter if request.api? around various actions?

Previously: Getting up-and-running with RSpec

Next up: A new jobfutures.com.au and my BDD/RSpec preso

AR::Base#attributes = nil

Archived comments

Thoughts

toolmantim