Rails - Jan 2009 - Using update to either update or create new object

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
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

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?
> 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
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.
>         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
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


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---