Using update to either update or create new object

Rails
1

I’m using form_remote_for to create a new object in the beginning, and
then if the object already exists, update it. I’m using the REST form
so my form looks like this:

<% form_remote_for(@spec) do |form| %>
<% end %>

with the controller defining the @spec object like so:
@spec = (@user.spec ||= Spec.new)

I’m not sure if this is the best way, but my main question lies in the
implementation of the update action in the spec controller which is
called when the form is submitted. This definitely needs to be
refactored and I could use some help with the best way:
I also created a pastie, which is much easier to read:
http://pastie.org/370302

def update
respond_to do |format|
if logged_in_user.spec.nil?
@spec = Spec.new(params[:spec])
@spec[“user_id”] = logged_in_user.id
if @spec.save
format.js do
render :update do |page|
page.form.reset ‘login_form’
page.redirect_to user_path(logged_in_user)
end
end
else
format.js do
render :update do |page|
page << “$(‘message_popup’).popup.show();”
end
end
end
else
if logged_in_user.spec.update_attributes(params[:spec])
format.js do
render :update do |page|
page.redirect_to user_path(logged_in_user)
end
end
else
format.js do
render :update do |page|
page << “$(‘message_popup’).popup.show();”
end
end
end
end
end
end

2

David wrote:

with the controller defining the @spec object like so:
@spec = (@user.spec ||= Spec.new)

That’s just || not ||=. Why didn’t your unit tests catch that?

   @spec["user_id"] = logged_in_user.id

Why not logged_in_user.spec.build(params[:spec])

   if @spec.save
      format.js do

You can move the format.js do line up to just after respond_to. It can
wrap all
of this…

        render :update do |page|
          page.form.reset 'login_form'
          page.redirect_to user_path(logged_in_user)

Why are you clearing the form and immediately going to another page?

(Firefox has a “feature” where they preserve form contents. Are you
wisely
thwarting that?)

        end
      end
    else
      format.js do
        render :update do |page|
          page << "$('message_popup').popup.show();"

Shouldn’t that push the errors into the popup?

        end
      end
   end
 else
  if logged_in_user.spec.update_attributes(params[:spec])

Can’t this just be part of the new and save bit above? Otherwise all the
code
below it is the same.

      end
  end
 end

end
end

Your unit tests should make that very easy to refactor. Just change one
line at
a time and pass all the tests after each change!

–
Phlip