Implementing review before save -- Controller or Model?

Is there a way to do a "save" to an existing record instead of using update_attributes?

Half of the values I'm saving are calculated from user inputs and so are not present in the params hash. Also, there can be multiple user review cycles, which means that :id drops out of the params hash (I'm saving it in the sessions hash).

To me it seems conceptually simpler to "save" on the final version of @post. I ended up saving the final version of @post in an attributes hash, then going back to the database to get the original record, then doing update_attributes. Hopefully that 2nd retrieval of the record is cached? Actually now it's easy to generate a list of changes in a given edit, so maybe I'll stay with this if it works, but it seems awkward.

What is the "Rails Way?" I'm new around here and want to fit in.

<form is submitted>
@post = Post.new(params[:post])
<lots of calculations and validity checking>
finalattrhash = @post.attributes
@post = nil
@post = post.find(session[:postid])

respond_to do |format|
  if @post.update_attributes(finalattrhash)
    se开发者_运维百科ssion[:postid] = nil
    format.html { redirect_to(@post, :notice => 'post was successfully updated.') }
  else ... end
end

Your complex handling and validations should be done inside the model. A fat controller is almost always worse than a fat model.

To make it clear, let's make an example.

Suppose you have a model Post. You have title and content stored in the database. And suppose you don't want the use to directly input those fields, but other four fields: trip_name, trip_date, visited and with_who. (Of course it is not very 'real case' :D)

class Post < ActiveRecord::Base
  # DB fields: title:string, content:text
  attr_accessor :trip_name, :trip_date, :visited, :with_who
  attr_protected :title, :content # This line could be ignored, but to only protected these two fields from mass assignment.

  before_save :complex_handles

  validates :title, :presence => true, :uniqueness => true
  validates :content, :presence => true

  private

  def complex_handles
    @title = @trip_name + " " + @trip_date
    @content = @visited + " " + @with_who
  end

end

So your form would give you the following hash:

params[:post] = {
  :trip_name => "some trip",
  :trip_date => "2010-01-29",
  :visited => "Hong Kong",
  :with_who => "with my GF"
}

This should be a normal simple form and the controller would be the simplest form too:

respond_to do |format|
  if @post.update_attributes(params[:post])
    format.html { redirect_to(@post, :notice => 'post was successfully updated.') }
  else
    ...
  end
end

As all those complex logics and validations are related to the model itself, it is best to keep inside the model. In this way, your controller and view would be very clean.

===== UPDATED =====

If my understanding is correct, I would guess you have a form with two buttons (one is review and one is finalize). And for review, you just update the fields, without saving to the database.

So this would be easier to give the two submit buttons a different name attribute. And in your controller:

respond_to do |format|
  if (params[:action] == "finalize" && @post.update_attributes(params[:post]))
    || (params[:action] == "review" && @post.attributes = params[:post] && @post.valid?)
    format.html { redirect_to(@post, :notice => 'post was successfully updated.') }
  else
    ...
  end
end

This part seems a little bit tricky. As the params[:action] could be one value in one time, so you won't mess up the checking.

So if it is finalize, you just call the update_attributes to save it. Else, you assign those attributes without saving, and then check if it is valid.

However, you have to update the Post model's code a little bit:

# Before, I have used:
before_save :complex_handles

# Now, I change to:
before_validation :complex_handles

So that the handling would be done before you call @post.valid? in review part. (This should not affect the finalize part)

What is the "Rails Way?" I'm new around here and want to fit in

No, it should be the other way around. Ask "How can Rails simplify my life, help me write good, maintainable code, and add value to my client's business"

It's a common mistake to try and force your business requirements to fit the code shown in the examples, or to always ask about "best practices". Or in other words: ask what your framework can do for you, not what you can do for your framework!

That being said, you should strive to put as much logic in the model as you can, and try to simplify the controller. This topic has been discussed to death -- just google "Fat model skinny controller" for a raft of relevant posts.

In particular, keep in mind that you don't need to limit yourself to the built-in methods save and update_attributes. You've called out two actions: Review and Finalize. So add these to your model:

class Post << AR::Base
  def review
    # do the validations, calculations, etc for the review step
  end
  def finalize
    # do whatever you need for the finalize step
    save # save the model instance
  end
end

You don't have to accept the standard RESTful action names either. Need an action called 'review'? Call it that:

resources :posts do
  member do
    post 'review' # maps to PostsController#review
  end
end

BDD/TDD is a good way of driving out these methods and help you to meet your specific requirements. It also helps enforce good separation of concerns, since you end up testing methods in isolation, which helps write maintainable code.