Forum: Ruby on Rails Using update to either update or create new object

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
David (Guest)
on 2009-01-25 19:51
(Received via mailing list)
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
Phlip (Guest)
on 2009-01-25 20:41
(Received via mailing list)
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
This topic is locked and can not be replied to.