Model aware of params hash - Rails anti-pattern?

Take the following code:

class ChallengesController < ApplicationController

  def update
    @challenge = Challenge.find(params[:id])
    @challenge.update!(params[:challenge]) # never an expected error, show error page and give hoptoad notification

    respond_to do |format|
      format.html { redirect_to :action => 'index' }
    end
  end

end

class Challenge < ActiveRecord::Base

  def update!(options)
    if options[:accept] == '1' then
      self.a开发者_开发百科ccepted = true
      self.response_at = Time.now        
      self.shots = options[:shots] unless options[:shots].blank?             
      self.challengee_msg = options[:challengee_msg] unless options[:challengee_msg].blank?
    else
      self.accepted = false
      self.response_at = Time.now
    end
  end

end

Is it considered bad practice for the model to be aware of the params hash being passed to it? If so, how would you refactor so that it follows "best practice?"

---

One thing is that if you are passing params into your model and mucking with it, adopt the practice of doing a .dup first. There is nothing more frustrating then trying to figure out why the routing is messed up, only to find a model somewhere has been deleting keys off of the params hash.

Also, if you pass a params hash into a model for any reason, be sure to have an attr_accessible on that model. You need to treat params as unsanitized user input.

---

No, that's an accepted pattern. It's normally used like this though, with built in active_record method update_attributes.

@challenge = Challenge.find(params[:id])
if @challenge.update_attributes(params[:challenge])
  flash[:success] = "Challenge updated"
  redirect_to @challenge
else
  render :action=>:edit
end

That will take in a hash of values and automatically set the attributes that you send in (unless they are protected by attr_protected).

---

if i guessed correctly, you have certain actions you want to carry out when you have different case for accept, and if accept is false, shots and challenge_msg should be nil

this can be done in a few ways

to do it in the views, probably with some javascript scripting, you can clear and hide the fields for shots and challenge_msg and submit the form accordingly

or in the controller, you'll have to set shots and challenge_msg to nil by doing something like:

if params[:challenge][:accepted] == "0"
  params[:challenge][:shots]         = nil
  params[:challenge][:challenge_msg] = nil
end

@challenge.update_attributes(params[:challenge])

or in the model, you can do it using callbacks like before_save to set shots and challenge_msg to nil before saving if accept is false

just some suggestions to improve your code, hope it helps =)