Refactor people edit page, now using form_tag

app/controllers/people_controller.rb

@@ -78,23 +78,23 @@ class PeopleController < ApplicationController
   end
 
   def update
-
     # upload and set new profile photo
-    params[:person][:profile] ||= {}
+    params[:profile] ||= {}
-    if params[:person][:profile][:image].present?
+    params[:profile][:searchable] ||= false
-      raw_image = params[:person][:profile].delete(:image)
+    if params[:profile][:image].present?
+      raw_image = params[:profile].delete(:image)
       params[:profile_image_hash] = { :user_file => raw_image, :to => "all" }
 
       photo = current_user.build_post(:photo, params[:profile_image_hash])
       if photo.save!
 
-        params[:person][:profile][:image_url] = photo.url(:thumb_large)
+        params[:profile][:image_url] = photo.url(:thumb_large)
-        params[:person][:profile][:image_url_medium] = photo.url(:thumb_medium)
+        params[:profile][:image_url_medium] = photo.url(:thumb_medium)
-        params[:person][:profile][:image_url_small] = photo.url(:thumb_small)
+        params[:profile][:image_url_small] = photo.url(:thumb_small)
       end
     end
 
-    if current_user.update_profile params[:person][:profile]
+    if current_user.update_profile params[:profile]
       flash[:notice] = I18n.t 'people.update.updated'
     else
       flash[:error] = I18n.t 'people.update.failed'

app/views/people/_edit.html.haml

@@ -0,0 +1,48 @@
+-#   Copyright (c) 2010, Diaspora Inc.  This file is
+-#   licensed under the Affero General Public License version 3 or later.  See
+-#   the COPYRIGHT file.
+
+
+= form_tag person_path(@person), :method => :put, :multipart => true do
+  %h3
+    = t('people.edit.your_profile')
+    .description
+      = t('people.edit.info_available_to')
+
+  = error_messages_for @person
+
+  %h4
+    = t('people.edit.your_name')
+    = text_field_tag 'profile[first_name]', @profile.first_name, :placeholder => t('people.edit.first_name')
+    = text_field_tag 'profile[last_name]', @profile.last_name, :placeholder => t('people.edit.last_name')
+
+  %h4
+    = t('people.edit.your_gender')
+    %br
+    = text_field_tag 'profile[gender]', @profile.gender, :placeholder => t("fill_me_out")
+
+  %h4
+    = t('people.edit.your_birthday')
+    %br
+    = select_date @person.profile.birthday, :prompt => true,
+      :default => true, :order => t('date.order'), :start_year => 2000, :end_year => 1930, :prefix => 'profile[date]'
+
+  %h4
+    = t('people.edit.your_bio')
+    = text_area_tag 'profile[bio]', @profile.bio, :rows => 5, :placeholder => t('fill_me_out')
+
+  %h4
+    = t('people.edit.your_photo')
+    = render 'people/profile_photo_upload'
+
+  %h4
+    = t('search')
+    %p{:class=>"checkbox_select"}
+      = label_tag 'profile[searchable]', t('people.edit.allow_search')
+      = check_box_tag 'profile[searchable]', true, @person.profile.searchable
+
+  = hidden_field_tag :getting_started, @step if @step
+
+  .submit_block
+    =yield(:submit_block)
+

app/views/people/_profile_photo_upload.html.haml

@@ -5,7 +5,7 @@
 
 #profile_photo_upload
   = owner_image_tag(:thumb_medium)
-  = form.file_field :image
+  = file_field_tag 'profile[image]'
 
   -if !@aspect.nil? && @aspect != :getting_started 
     %p

app/views/people/edit.html.haml

@@ -12,46 +12,9 @@
     %li=link_to t('_services'), services_path
 
 .span-19.prepend-5.last
-  = form_for @person, :html => { :multipart => true } do |person|
+  - content_for :submit_block do
-    %h3
+    = link_to t('cancel'), edit_user_path(current_user)
-      = t('.your_profile')
+    = t('or')
-      .description
+    = submit_tag t('.update_profile')
-        =t('.info_available_to')
+  = render :partial => 'people/edit'
 
-    = person.error_messages
-
-    = person.fields_for :profile do |profile|
-      %h4
-        = t('.your_name')
-        = profile.text_field :first_name, :value => @profile.first_name, :placeholder => t('.first_name')
-        = profile.text_field :last_name, :value => @profile.last_name, :placeholder => t('.last_name')
-
-      %h4
-        = t('.your_gender')
-        %br
-        = profile.text_field :gender, :value => @profile.gender, :placeholder => t('fill_me_out')
-
-      %h4
-        = t('.your_birthday')
-        %br
-        = select_date @person.profile.birthday, :prompt => true, :prefix => 'person[profile][date]',
-        :default => true, :order => t('date.order'), :start_year => 2000, :end_year => 1904
-
-      %h4
-        = t('.your_bio')
-        = profile.text_area :bio, :value => @profile.bio, :rows => 5, :placeholder => t('fill_me_out')
-
-      %h4
-        = t('.your_photo')
-        = render 'people/profile_photo_upload', :form => profile
-
-      %h4
-        = t('search')
-        %p{:class=>"checkbox_select"}
-          = profile.label :searchable, t('.allow_search')
-          = profile.check_box :searchable, {:checked => @person.profile.searchable}, true, false
-
-    .submit_block
-      = link_to t('cancel'), edit_user_path(current_user)
-      = t('or')
-      = person.submit t('.update_profile')

app/views/users/getting_started/_step_1.html.haml

@@ -2,47 +2,7 @@
 -#   licensed under the Affero General Public License version 3 or later.  See
 -#   the COPYRIGHT file.
 
-
+- content_for :submit_block do
-= form_for @person, :html => { :multipart => true } do |person|
+  = submit_tag "#{t('users.getting_started.save_and_continue')} →"
-  %h3
+= render :partial => 'people/edit'
-    = t('people.edit.your_profile')
-    .description
-      = t('people.edit.info_available_to')
-
-  = person.error_messages
-
-  = person.fields_for :profile do |profile|
-    %h4
-      = t('people.edit.your_name')
-      = profile.text_field :first_name, :value => @profile.first_name, :placeholder => t('people.edit.first_name')
-      = profile.text_field :last_name, :value => @profile.last_name, :placeholder => t('people.edit.last_name')
-
-    %h4
-      = t('people.edit.your_gender')
-      %br
-      = profile.text_field :gender, :value => @profile.gender, :placeholder => t("fill_me_out")
-
-    %h4
-      = t('people.edit.your_birthday')
-      %br
-      = select_date @person.profile.birthday, :prompt => true, :default => true, :order => t('date.order'), :start_year => 2000, :end_year => 1930, :prefix => 'person[profile][date]'
-
-    %h4
-      = t('people.edit.your_bio')
-      = profile.text_area :bio, :value => @profile.bio, :rows => 5, :placeholder => t('fill_me_out')
-
-    %h4
-      = t('people.edit.your_photo')
-      = render 'people/profile_photo_upload', :form => profile
-
-    %h4
-      = t('search')
-      %p{:class=>"checkbox_select"}
-        = profile.label :searchable, t('people.edit.allow_search')
-        = profile.check_box :searchable, {:checked => @person.profile.searchable}, true, false
-
-    = hidden_field_tag :getting_started, @step
-
-  .submit_block
-    = person.submit "#{t('users.getting_started.save_and_continue')} →"
 

spec/controllers/people_controller_spec.rb

@@ -156,29 +156,29 @@ describe PeopleController do
 
   describe '#update' do
     it "sets the flash" do
-      put :update, "id" => user.person.id.to_s, "person" => {
+      put :update, :id => user.person.id,
-        "profile" => {
+        :profile => {
-          "image_url" => "",
+          :image_url  => "",
-          "last_name" => "Smith",
+          :first_name => "Will",
-          "first_name" => "Will"
+          :last_name  => "Smith"
         }
-      }
       flash[:notice].should_not be_empty
     end
 
     context 'with a profile photo set' do
       before do
-        @params = { :profile =>
+        @params = { :id => user.person.id,
-                   { :image_url => "",
+                    :profile =>
-                     :last_name  => user.person.profile.last_name,
+                     {:image_url => "",
-                     :first_name => user.person.profile.first_name }}
+                      :last_name  => user.person.profile.last_name,
+                      :first_name => user.person.profile.first_name }}
 
         user.person.profile.image_url = "http://tom.joindiaspora.com/images/user/tom.jpg"
         user.person.profile.save
       end
       it "doesn't overwrite the profile photo when an empty string is passed in" do
         image_url = user.person.profile.image_url
-        put :update, :id => user.person.id.to_s, :person => @params
+        put :update, @params
 
         user.person.reload
         user.person.profile.image_url.should == image_url
@@ -187,7 +187,7 @@ describe PeopleController do
         fixture_name = File.dirname(__FILE__) + '/../fixtures/button.png'
         photo = user.post(:photo, :user_file => File.open(fixture_name), :to => aspect.id)
         @params[:profile][:image_url] = photo.url(:thumb_medium)
-        put :update, :id => user.person.id, :person => @params
+        put :update, @params
         goal_pod_url = (APP_CONFIG[:pod_url][-1,1] == '/' ? APP_CONFIG[:pod_url].chop : APP_CONFIG[:pod_url])
         user.person.reload.profile.image_url.should ==
           "#{goal_pod_url}#{photo.url(:thumb_medium)}"
@@ -195,14 +195,13 @@ describe PeopleController do
     end
     it 'does not allow mass assignment' do
       new_user = make_user
-      put :update, :id => user.person.id, :person => {
+      put :update, :id => user.person.id, :owner_id => new_user.id
-        :owner_id => new_user.id}
       user.person.reload.owner_id.should_not == new_user.id
     end
 
     it 'does not overwrite the profile diaspora handle' do
-      handle_params = {'profile' => {'diaspora_handle' => 'abc@a.com'}}
+      handle_params = {:id => user.person.id, :profile => {:diaspora_handle => 'abc@a.com'}}
-      put :update, :id => user.person.id, :person => handle_params
+      put :update, handle_params
       user.person.reload.profile[:diaspora_handle].should_not == 'abc@a.com'
     end
   end