settings are more restful with account settings submitting to users#update, and profile settings submitting to person#update

app/controllers/people_controller.rb

@@ -23,8 +23,6 @@ class PeopleController < ApplicationController
       @profile = @person.profile
       @aspects_with_person = current_user.aspects_with_person(@person)
       @posts = current_user.visible_posts(:person_id => @person.id).paginate :page => params[:page], :order => 'created_at DESC'
-      @latest_status_message = current_user.raw_visible_posts.find_all_by__type_and_person_id("StatusMessage", params[:id]).last
-      @post_count = @posts.count
       respond_with @person
     end
   end
@@ -34,4 +32,38 @@ class PeopleController < ApplicationController
     respond_with :location => root_url
   end
 
+  def edit
+    @aspect  = :person_edit
+    @person  = current_user.person
+    @profile = @person.profile
+    @photos  = current_user.visible_posts(:person_id => @person.id, :_type => 'Photo').paginate :page => params[:page], :order => 'created_at DESC'
+  end
+
+  def update
+    prep_image_url(params[:person])
+
+    if current_user.update_profile params[:person][:profile]
+      flash[:notice] = "Profile updated"
+    else
+      flash[:error] = "Failed to update profile"
+    end
+
+    redirect_to edit_person_path
+  end
+
+  private
+  def prep_image_url(params)
+    url = APP_CONFIG[:pod_url].dup
+    url.chop! if APP_CONFIG[:pod_url][-1,1] == '/'
+    if params[:profile][:image_url].empty?
+      params[:profile].delete(:image_url)
+    else
+      if /^http:\/\// =~ params[:profile][:image_url]
+        params[:profile][:image_url] = params[:profile][:image_url]
+      else
+        params[:profile][:image_url] = url + params[:profile][:image_url]
+      end
+    end
+  end
+
 end

app/controllers/users_controller.rb

@@ -16,9 +16,6 @@ class UsersController < ApplicationController
   def edit
     @aspect = :user_edit
     @user   = current_user
-    @person  = @user.person
-    @profile = @user.person.profile
-    @photos  = current_user.visible_posts(:person_id => current_user.person.id, :_type => 'Photo').paginate :page => params[:page], :order => 'created_at DESC'
   end
 
   def update
@@ -32,16 +29,9 @@ class UsersController < ApplicationController
       else
         flash[:error] = "Password Change Failed"
       end
-    else
-      prep_image_url(params[:user])
-      if @user.update_profile params[:user][:profile]
-        flash[:notice] = "Profile updated"
-      else
-        flash[:error] = "Failed to update profile"
     end
-    end
-    redirect_to edit_user_path(@user)
 
+    redirect_to edit_user_path(@user)
   end
 
   def destroy
@@ -100,19 +90,5 @@ class UsersController < ApplicationController
   end
 
 
-  private
-  def prep_image_url(params)
-    url = APP_CONFIG[:pod_url].dup
-    url.chop! if APP_CONFIG[:pod_url][-1,1] == '/'
-    if params[:profile][:image_url].empty?
-      params[:profile].delete(:image_url)
-    else
-      if /^http:\/\// =~ params[:profile][:image_url]
-        params[:profile][:image_url] = params[:profile][:image_url]
-      else
-        params[:profile][:image_url] = url + params[:profile][:image_url]
-      end
-    end
-  end
 
 end

app/views/people/edit.html.haml

@@ -0,0 +1,67 @@
+-#   Copyright (c) 2010, Diaspora Inc.  This file is
+-#   licensed under the Affero General Public License version 3 or later.  See
+-#   the COPYRIGHT file.
+
+
+#section_header
+  %h2
+    Settings
+  %ul#settings_nav
+    %li=link_to 'Profile', edit_person_path(current_user.person)
+    %li=link_to 'Account', edit_user_path(current_user)
+    %li=link_to 'Services', services_path
+
+.span-19.prepend-5.last
+  %h2 Profile
+  = form_for @person do |person|
+    = person.error_messages
+
+    = person.fields_for :profile do |p|
+
+      %h3="#{t('.picture')}"
+      %div#image_picker
+        = p.hidden_field :image_url, :value => (@profile.image_url if @profile.image_url), :id => 'image_url_field'
+
+        - unless @photos.nil? || @photos.empty?
+          - for photo in @photos
+            - if @profile.image_url && @profile.image_url.include?(photo.url(:thumb_medium))
+              %div.small_photo{:id => photo.url(:thumb_medium), :class=>'selected'}
+                = check_box_tag 'checked_photo', true, true
+                = link_to image_tag(photo.url(:thumb_medium)), "#"
+            - else
+              %div.small_photo{:id => photo.url(:thumb_medium)}
+                = check_box_tag 'checked_photo'
+                = link_to image_tag(photo.url(:thumb_medium)), "#"
+
+        - else
+          =t('.you_dont_have_any_photos')
+          = link_to t('.albums'), albums_path(:aspect => 'all')
+          =t('.page_to_upload_some')
+
+      =will_paginate @photos
+
+      %br
+
+      %h3="#{t('.info')}"
+
+      %p
+        %b
+          ="#{t('.diaspora_username')}:"
+          = @person.diaspora_handle
+
+      %p
+        = p.label :first_name
+        = p.text_field :first_name, :value => @profile.first_name
+      %p
+        = p.label :last_name
+        = p.text_field :last_name, :value => @profile.last_name
+
+    .submit_block
+      = link_to t('.cancel'), edit_user_path(current_user)
+      = t('.or')
+      = person.submit t('.update_profile')
+
+  #content_bottom
+    .back
+      = link_to "⇧ #{t('.home')}", root_path
+

app/views/services/index.html.haml

@@ -6,8 +6,8 @@
   %h2
     Settings
   %ul#settings_nav
-    %li=link_to 'Profile', '#', :class => 'profile'
-    %li=link_to 'Account', '#', :class => 'account'
+    %li=link_to 'Profile', edit_person_path(current_user.person)
+    %li=link_to 'Account', edit_user_path(current_user)
     %li=link_to 'Services', services_path
 
 .span-19.prepend-5.last

app/views/users/_account.haml

@@ -1,41 +0,0 @@
--#   Copyright (c) 2010, Diaspora Inc.  This file is
--#   licensed under the Affero General Public License version 3 or later.  See
--#   the COPYRIGHT file.
-
-
-%h2 Account
-
-= link_to "invite friends", new_user_invitation_path(current_user)
-
-%br
-%br
-%br
-
-%h3 Change Password
-= form_for @user do |f|
-  = f.error_messages
-
-  %p
-    = f.label :password, "New Password"
-    = f.password_field :password
-  %p
-    = f.label :password_confirmation
-    = f.password_field :password_confirmation
-
-  .submit_block
-    = link_to "Cancel", edit_user_path(current_user)
-    or
-    = f.submit 'Change password'
-
-%h3 Export Data
-= link_to "download my xml", users_export_path, :class => "button"
-= link_to "download my photos", users_export_photos_path, :class => "button"
-
-%br
-%br
-%br
-
-%h3 Close Account
-= link_to "Close Account", current_user,
-            :confirm => "Are you sure?", :method => :delete,
-            :class => "button"

app/views/users/_profile.haml

@@ -1,58 +0,0 @@
--#   Copyright (c) 2010, Diaspora Inc.  This file is
--#   licensed under the Affero General Public License version 3 or later.  See
--#   the COPYRIGHT file.
-
-
-%h2 Profile
-= form_for @user do |f|
-  = f.error_messages
-
-  = f.fields_for :profile do |p|
-
-    %h3="#{t('.picture')}"
-    %div#image_picker
-      = p.hidden_field :image_url, :value => (@profile.image_url if @profile.image_url), :id => 'image_url_field'
-
-      - unless @photos.nil? || @photos.empty?
-        - for photo in @photos
-          - if @profile.image_url && @profile.image_url.include?(photo.url(:thumb_medium))
-            %div.small_photo{:id => photo.url(:thumb_medium), :class=>'selected'}
-              = check_box_tag 'checked_photo', true, true
-              = link_to image_tag(photo.url(:thumb_medium)), "#"
-          - else
-            %div.small_photo{:id => photo.url(:thumb_medium)}
-              = check_box_tag 'checked_photo'
-              = link_to image_tag(photo.url(:thumb_medium)), "#"
-
-      - else
-        =t('.you_dont_have_any_photos')
-        = link_to t('.albums'), albums_path(:aspect => 'all')
-        =t('.page_to_upload_some')
-
-    =will_paginate @photos
-
-    %br
-
-    %h3="#{t('.info')}"
-
-    %p
-      %b
-        ="#{t('.diaspora_username')}:"
-        = @user.diaspora_handle
-
-    %p
-      = p.label :first_name
-      = p.text_field :first_name, :value => @profile.first_name
-    %p
-      = p.label :last_name
-      = p.text_field :last_name, :value => @profile.last_name
-
-  .submit_block
-    = link_to t('.cancel'), edit_user_path(current_user)
-    = t('.or')
-    = f.submit t('.update_profile')
-
-#content_bottom
-  .back
-    = link_to "⇧ #{t('.home')}", root_path
-

app/views/users/edit.html.haml

@@ -16,14 +16,44 @@
   %h2
     Settings
   %ul#settings_nav
-    %li=link_to 'Profile', '#', :class => 'profile'
-    %li=link_to 'Account', '#', :class => 'account'
+    %li=link_to 'Profile', edit_person_path(current_user.person)
+    %li=link_to 'Account', edit_user_path(current_user)
     %li=link_to 'Services', services_path
 
 .span-19.prepend-5.last
-  #profile.settings_pane{:style=>"display:block;"}
-    = render 'users/profile'
+  %h2 Account
 
-  #account.settings_pane
-    = render 'users/account'
+  = link_to "invite friends", new_user_invitation_path(current_user)
 
+  %br
+  %br
+  %br
+
+  %h3 Change Password
+  = form_for @user do |f|
+    = f.error_messages
+
+    %p
+      = f.label :password, "New Password"
+      = f.password_field :password
+    %p
+      = f.label :password_confirmation
+      = f.password_field :password_confirmation
+
+    .submit_block
+      = link_to "Cancel", edit_user_path(current_user)
+      or
+      = f.submit 'Change password'
+
+  %h3 Export Data
+  = link_to "download my xml", users_export_path, :class => "button"
+  = link_to "download my photos", users_export_photos_path, :class => "button"
+
+  %br
+  %br
+  %br
+
+  %h3 Close Account
+  = link_to "Close Account", current_user,
+              :confirm => "Are you sure?", :method => :delete,
+              :class => "button"

config/locales/diaspora/en.yml

@@ -116,18 +116,6 @@ en:
   users:
       edit:
           editing_profile: "Editing profile"
-      profile:
-          cancel: "Cancel"
-          update_profile: "Update Profile"
-          home: "Home"
-          diaspora_username: "DIASPORA USERNAME"
-          info: "Info"
-          picture: "Picture"
-          editing_profile: "Editing profile"
-          albums: "Albums"
-          you_dont_have_any_photos: "You don't have any photos!  Go to the"
-          page_to_upload_some:  "page to upload some."
-          or: "or"
       destroy: "Account successfully closed."
   comments:
       comment:
@@ -218,6 +206,18 @@ en:
           are_you_sure: "Are you sure?"
           remove_friend: "remove friend"
           no_posts: "no posts to display!"
+      edit:
+          cancel: "Cancel"
+          update_profile: "Update Profile"
+          home: "Home"
+          diaspora_username: "DIASPORA USERNAME"
+          info: "Info"
+          picture: "Picture"
+          editing_profile: "Editing profile"
+          albums: "Albums"
+          you_dont_have_any_photos: "You don't have any photos!  Go to the"
+          page_to_upload_some:  "page to upload some."
+          or: "or"
   requests:
       new_request:
           add_a_new_friend_to: "Add a new friend to"

config/routes.rb

@@ -3,7 +3,7 @@
 #   the COPYRIGHT file.
 
 Diaspora::Application.routes.draw do
-  resources :people,          :only   => [:index, :show, :destroy]
+  resources :people
   resources :status_messages, :only   => [:create, :destroy, :show]
   resources :comments,        :except => [:index]
   resources :requests,        :except => [:edit, :update]

spec/controllers/people_controller_spec.rb

@@ -6,22 +6,22 @@ require 'spec_helper'
 
 describe PeopleController do
   render_views
-  before do
-    @user = Factory.create(:user)
 
-    sign_in :user, @user
-    @user.aspect(:name => "lame-os")
+  let(:user) { Factory(:user) }
+  let!(:aspect) { user.aspect(:name => "lame-os") }
+
+  before do
+    sign_in :user, user
   end
 
   it "index should yield search results for substring of person name" do
-    
     eugene = Factory.create(:person, :profile => {:first_name => "Eugene", :last_name => "w"})
     get :index, :q => "Eu"
     assigns[:people].should include eugene
   end
 
   it 'should go to the current_user show page' do
-    get :show, :id => @user.person.id
+    get :show, :id => user.person.id
   end
 
   it "doesn't error out on an invalid id" do
@@ -29,6 +29,25 @@ describe PeopleController do
   end
 
   it "doesn't error out on a nonexistent person" do
-    get :show, :id => @user.id
+    get :show, :id => user.id
+  end
+
+  describe '#update' do
+    context 'with a profile photo set' do
+      it "doesn't overwrite the profile photo when an empty string is passed in" do
+        user.person.profile.image_url = "http://tom.joindiaspora.com/images/user/tom.jpg"
+        user.person.profile.save
+        
+        params = {"profile"=> 
+                   {"image_url"  => "",
+                    "last_name"  => user.person.profile.last_name,
+                    "first_name" => user.person.profile.first_name}}
+
+        image_url = user.person.profile.image_url
+        put("update", :id => user.person.id, "person" => params)
+
+        user.person.profile.image_url.should == image_url
+      end
+    end
   end
 end

spec/controllers/users_controller_spec.rb

@@ -5,10 +5,14 @@
 require 'spec_helper'
 
 describe UsersController do
+
+  let(:user) { Factory(:user) }
+  let!(:aspect) { user.aspect(:name => "lame-os") }
+
+  let!(:old_password) { user.encrypted_password }
+    
   before do
-    @user = Factory.create(:user)
-    sign_in :user, @user
-    @user.aspect(:name => "lame-os")
+    sign_in :user, user
   end
 
   describe '#export' do
@@ -18,67 +22,29 @@ describe UsersController do
     end
   end
 
-
   describe '#update' do
-    context 'with a profile photo set' do
-      before do
-        @user.person.profile.image_url = "http://tom.joindiaspora.com/images/user/tom.jpg"
-        @user.person.profile.save
-        
-        @params = {"profile"=>
-          {"image_url"   => "",
-            "last_name"  => @user.person.profile.last_name,
-            "first_name" => @user.person.profile.first_name}}
-      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.id, "user" => @params)
-
-        @user.person.profile.image_url.should == image_url
-      end
     it "doesn't overwrite random attributes" do
-        new_user = Factory.create(:user)
-        @params[:owner_id] = new_user.id
-        person = @user.person
-        put('update', :id => @user.id, "user" => @params)
-        Person.find(person.id).owner_id.should == @user.id
-      end
+      params  = {:diaspora_handle => "notreal@stuff.com"}
+      proc{ put 'update', :id => user.id, "user" => params }.should_not change(user, :diaspora_handle)
     end
 
     context 'should allow the user to update their password' do
       it 'should change a users password ' do
-        old_password = @user.encrypted_password
-
-        put("update", :id => @user.id, "user"=> {"password" => "foobaz", 'password_confirmation' => "foobaz","profile"=>
-            {"image_url"   => "",
-            "last_name"  => @user.person.profile.last_name,
-            "first_name" => @user.person.profile.first_name}})
-
-        @user.reload
-        @user.encrypted_password.should_not == old_password
+        put("update", :id => user.id, "user"=> {"password" => "foobaz", 'password_confirmation' => "foobaz"})
+        user.reload
+        user.encrypted_password.should_not == old_password
       end
 
       it 'should not change a password if they do not match' do
-        old_password = @user.encrypted_password
-        put("update", :id => @user.id, "user"=> {"password" => "foobarz", 'password_confirmation' => "not_the_same","profile"=>
-            {"image_url"   => "",
-            "last_name"  => @user.person.profile.last_name,
-            "first_name" => @user.person.profile.first_name}})
-          @user.reload
-          @user.encrypted_password.should == old_password
+        put("update", :id => user.id, "user"=> {"password" => "foobarz", 'password_confirmation' => "not_the_same"})
+        user.reload
+        user.encrypted_password.should == old_password
       end
 
-
       it 'should not update if the password fields are left blank' do
-
-          old_password = @user.encrypted_password
-          put("update", :id => @user.id, "user"=> {"password" => "", 'password_confirmation' => "","profile"=>
-              {"image_url"   => "",
-              "last_name"  => @user.person.profile.last_name,
-              "first_name" => @user.person.profile.first_name}})
-            @user.reload
-            @user.encrypted_password.should == old_password
+        put("update", :id => user.id, "user"=> {"password" => "", 'password_confirmation' => ""})
+        user.reload
+        user.encrypted_password.should == old_password
       end
     end
   end