Merge remote branch 'upstream/master'

app/controllers/users_controller.rb

@@ -36,10 +36,8 @@ class UsersController < ApplicationController
         flash[:error] = "Password Change Failed"
       end
     else
-      data = clean_hash params[:user]
+      prep_image_url(params[:user])
-      prep_image_url(data)
+      if @user.update_profile params[:user][:profile]
-
-      if @user.update_profile data
         flash[:notice] = "Profile updated"
       else
         flash[:error] = "Failed to update profile"
@@ -120,15 +118,4 @@ class UsersController < ApplicationController
     end
   end
 
-  def clean_hash(params)
-    return {
-      :profile =>
-        {
-        :first_name => params[:profile][:first_name],
-        :last_name => params[:profile][:last_name],
-        :image_url => params[:profile][:image_url]
-        }
-    }
-  end
-
 end

app/helpers/application_helper.rb

@@ -60,7 +60,7 @@ module ApplicationHelper
     image_location = person.profile.image_url
     image_location ||= "/images/user/default.png"
 
-    image_tag image_location, :class => "avatar", :alt => person.real_name, :title => person.real_name
+    image_tag image_location, :class => "avatar", :alt => person.real_name, :title => person.real_name, "data-person_id" => person.id
   end
 
   def person_image_link(person)

app/models/user.rb

@@ -106,24 +106,36 @@ class User
 
   def move_friend(opts = {})
     return true if opts[:to] == opts[:from]
-    friend = Person.first(:_id => opts[:friend_id])
+    if opts[:friend_id] && opts[:to] && opts[:from] 
-    if self.friend_ids.include?(friend.id)
+      from_aspect = self.aspects.first(:_id => opts[:from])
-      from_aspect = self.aspect_by_id(opts[:from])
+      posts_to_move = from_aspect.posts.find_all_by_person_id(opts[:friend_id])
-      to_aspect = self.aspect_by_id(opts[:to])
+      if add_person_to_aspect(opts[:friend_id], opts[:to], :posts => posts_to_move)
-      if from_aspect && to_aspect
+        delete_person_from_aspect(opts[:friend_id], opts[:from], :posts => posts_to_move) 
-        posts_to_move = from_aspect.posts.find_all_by_person_id(friend.id)
-        to_aspect.people << friend
-        to_aspect.posts << posts_to_move
-        from_aspect.person_ids.delete(friend.id.to_id)
-        posts_to_move.each { |x| from_aspect.post_ids.delete(x.id) }
-        from_aspect.save
-        to_aspect.save
         return true
       end
     end
     false
   end
 
+  def add_person_to_aspect(person_id, aspect_id, opts = {})
+    raise "Can not add person to an aspect you do not own" unless aspect = self.aspects.find_by_id(aspect_id) 
+    raise "Can not add person you are not friends with" unless person = self.find_friend_by_id(person_id)
+    raise 'Can not add person who is already in the aspect' if aspect.person_ids.include?(person_id)
+    aspect.people << person 
+    opts[:posts] ||= self.raw_visible_posts.all(:person_id => person_id)
+    
+    aspect.posts += opts[:posts]
+    aspect.save
+  end
+
+  def delete_person_from_aspect(person_id, aspect_id, opts = {})
+    raise "Can not delete a person from an aspect you do not own" unless aspect = self.aspects.find_by_id(aspect_id)
+    aspect.person_ids.delete(person_id)
+    opts[:posts] ||= aspect.posts.all(:person_id => person_id)
+    aspect.posts -= opts[:posts]
+    aspect.save
+  end
+
   ######## Posting ########
   def post(class_name, options = {})
     if class_name == :photo
@@ -281,7 +293,7 @@ class User
 
   ########### Profile ######################
   def update_profile(params)
-    if self.person.update_attributes(params)
+    if self.person.profile.update_attributes(params)
       push_to_aspects profile, :all
       true
     else

app/views/photos/show.html.haml

@@ -76,6 +76,9 @@
 
   %div{:id => @photo.id}
     #show_photo
+      .edit_pane
+        .controls{:data=>{:actor=>"#{@photo.person.owner.id}",:actor_person=>"#{@photo.person.id}",:image_url=>"#{@photo.url(:thumb_medium)}"}}
+          = link_to 'make profile photo', '#', :class => "make_profile_photo"
         = linked_scaled_photo @photo, @album
       .caption
         -if current_user.owns? @photo

lib/diaspora/user/querying.rb

@@ -34,6 +34,11 @@ module Diaspora
         aspects.detect{|x| x.id == id }
       end
 
+      def find_friend_by_id(id)
+        id = id.to_id
+        friends.detect{|x| x.id == id }
+      end
+
       def aspects_with_post( id )
         self.aspects.find_all_by_post_ids( id.to_id )
       end

public/javascripts/view.js

@@ -67,7 +67,6 @@ $(document).ready(function(){
     };
   });
 
-
 });//end document ready
 
 
@@ -105,3 +104,20 @@ function openVideo(type, videoid, link) {
   $(container).slideDown('fast', function() { });
   link.onclick = function() { $(container).slideToggle('fast', function() { } ); }
 }
+
+$(".make_profile_photo").live("click", function(){
+  var user_id   = $(this).closest(".controls").attr('data-actor');
+      person_id = $(this).closest(".controls").attr('data-actor_person');
+      photo_url = $(this).closest(".controls").attr('data-image_url');
+
+  $.ajax({
+    type: "PUT",
+    url: '/users/'+user_id,
+    data: {"user":{"profile":{ "image_url": photo_url }}},
+    success: function(){
+      $("img[data-person_id='"+ person_id +"']").each( function() {
+        $(this).attr('src', photo_url);
+      });
+    }
+  });
+});

public/stylesheets/sass/application.sass

@@ -733,6 +733,11 @@ h1.big_text
   :display inline
   :float right
 
+  form
+    :margin
+      :right 0
+      :top 0
+
 .back
   :font
     :size 12px
@@ -936,6 +941,7 @@ h1.big_text
       :color #333
 
     :border-radius 5px
+    :color #ccc
   
     a
       :color #ccc
@@ -1083,3 +1089,29 @@ header
   img
     :height 27px
     :width 27px
+
+
+.edit_pane
+  :display inline
+  :position relative
+
+  .controls
+    :display none
+    :background
+      :color rgba(51,51,51,0.9)
+    :padding 10px
+    :position absolute
+    :right 0
+
+    a
+      :font
+        :weight bold
+      :color #eee
+      :text-shadow 0 1px #000
+
+      &:hover
+        :color #fff
+    
+  &:hover
+    .controls
+      :display inline

spec/controllers/aspects_controller_spec.rb

@@ -46,4 +46,17 @@ describe AspectsController do
       end
     end
   end
+
+  describe "#update" do
+    before do
+      @aspect = @user.aspect(:name => "Bruisers")
+    end
+    it "doesn't overwrite random attributes" do
+      new_user = Factory.create :user
+      params = {"name" => "Bruisers"}
+      params[:user_id] = new_user.id
+      put('update', :id => @aspect.id, "aspect" => params)
+      Aspect.find(@aspect.id).user_id.should == @user.id
+    end
+  end
 end

spec/controllers/users_controller_spec.rb

@@ -24,17 +24,26 @@ describe UsersController 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"=> {"profile"=>
+        put("update", :id => @user.id, "user" => @params)
-          {"image_url"   => "",
-            "last_name"  => @user.person.profile.last_name,
-            "first_name" => @user.person.profile.first_name}})
 
         @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
     end
 
     context 'should allow the user to update their password' do

spec/models/aspect_spec.rb

@@ -5,123 +5,129 @@
 require 'spec_helper'
 
 describe Aspect do
-  before do
+  let(:user ) { Factory.create(:user) }
-    @user = Factory.create(:user)
+  let(:friend) { Factory.create(:person) }
-    @friend = Factory.create(:person)
+  let(:user2) { Factory.create(:user) }
-    @user2 = Factory.create(:user)
+  let(:friend_2) { Factory.create(:person) }
-    @friend_2 = Factory.create(:person)
+
-  end
+  let(:aspect) {user.aspect(:name => 'losers')}
+  let(:aspect2) {user2.aspect(:name => 'failures')}
+  let(:aspect1) {user.aspect(:name => 'cats')}
+  let(:not_friend) { Factory(:person, :diaspora_handle => "not@person.com")}
+  let(:user3) {Factory(:user)}
+  let(:aspect3) {user3.aspect(:name => "lala")}
 
   describe 'creation' do
     it 'should have a name' do
-      aspect = @user.aspect(:name => 'losers')
+      aspect = user.aspect(:name => 'losers')
       aspect.name.should == "losers"
     end
 
     it 'should be creatable with people' do
-      aspect = @user.aspect(:name => 'losers', :people => [@friend, @friend_2])
+      aspect = user.aspect(:name => 'losers', :people => [friend, friend_2])
       aspect.people.size.should == 2
     end
 
     it 'should be able to have other users' do
-      aspect = @user.aspect(:name => 'losers', :people => [@user2.person])
+      aspect = user.aspect(:name => 'losers', :people => [user2.person])
-      aspect.people.include?(@user.person).should be false
+      aspect.people.include?(user.person).should be false
-      aspect.people.include?(@user2.person).should be true
+      aspect.people.include?(user2.person).should be true
       aspect.people.size.should == 1
     end
 
     it 'should be able to have users and people' do
-      aspect = @user.aspect(:name => 'losers', :people => [@user2.person, @friend_2])
+      aspect = user.aspect(:name => 'losers', :people => [user2.person, friend_2])
-      aspect.people.include?(@user.person).should be false
+      aspect.people.include?(user.person).should be false
-      aspect.people.include?(@user2.person).should be true
+      aspect.people.include?(user2.person).should be true
-      aspect.people.include?(@friend_2).should be true
+      aspect.people.include?(friend_2).should be true
       aspect.people.size.should == 2
     end
   end
 
   describe 'validation' do
     before do
-      @aspect = @user.aspect(:name => 'losers')
+      aspect
     end
     it 'has a unique name for one user' do
-      aspect2 = @user.aspect(:name => @aspect.name)
+      aspect2 = user.aspect(:name => aspect.name)
       aspect2.valid?.should be_false
     end
 
     it 'has no uniqueness between users' do
-      aspect2 = @user2.aspect(:name => @aspect.name)
+      aspect2 = user2.aspect(:name => aspect.name)
       aspect2.valid?.should be_true
     end
   end
 
   describe 'querying' do
     before do
-      @aspect = @user.aspect(:name => 'losers')
+      aspect
-      @user.activate_friend(@friend, @aspect)
+      user.activate_friend(friend, aspect)
-      @aspect2 = @user2.aspect(:name => 'failures')
+      aspect2
-      friend_users(@user, @aspect, @user2, @aspect2)
+      friend_users(user, aspect, user2, aspect2)
-      @aspect.reload
+      aspect.reload
+      user.reload
     end
 
     it 'belong to a user' do
-      @aspect.user.id.should == @user.id
+      aspect.user.id.should == user.id
-      @user.aspects.size.should == 1 
+      user.aspects.should == [aspect]
     end
 
     it 'should have people' do
-      @aspect.people.all.include?(@friend).should be true
+      aspect.people.all.include?(friend).should be true
-      @aspect.people.size.should == 2
+      aspect.people.size.should == 2
     end
 
     it 'should be accessible through the user' do
-      aspects = @user.aspects_with_person(@friend)
+      aspects = user.aspects_with_person(friend)
       aspects.size.should == 1
-      aspects.first.id.should == @aspect.id
+      aspects.first.id.should == aspect.id
       aspects.first.people.size.should == 2
-      aspects.first.people.include?(@friend).should be true
+      aspects.first.people.include?(friend).should be true
-      aspects.first.people.include?(@user2.person).should be true
+      aspects.first.people.include?(user2.person).should be true
     end
   end
 
   describe 'posting' do
 
     it 'should add post to aspect via post method' do
-      aspect = @user.aspect(:name => 'losers', :people => [@friend])
+      aspect = user.aspect(:name => 'losers', :people => [friend])
 
-      status_message = @user.post( :status_message, :message => "hey", :to => aspect.id )
+      status_message = user.post( :status_message, :message => "hey", :to => aspect.id )
 
       aspect.reload
       aspect.posts.include?(status_message).should be true
     end
 
     it 'should add post to aspect via receive method' do
-      aspect  = @user.aspect(:name => 'losers')
+      aspect  = user.aspect(:name => 'losers')
-      aspect2 = @user2.aspect(:name => 'winners')
+      aspect2 = user2.aspect(:name => 'winners')
-      friend_users(@user, aspect, @user2, aspect2)
+      friend_users(user, aspect, user2, aspect2)
 
-      message = @user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
+      message = user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
 
-      @user.receive message.to_diaspora_xml, @user2.person
+      user.receive message.to_diaspora_xml, user2.person
 
       aspect.reload
       aspect.posts.include?(message).should be true
-      @user.visible_posts(:by_members_of => aspect).include?(message).should be true
+      user.visible_posts(:by_members_of => aspect).include?(message).should be true
     end
 
     it 'should retract the post from the aspects as well' do
-      aspect  = @user.aspect(:name => 'losers')
+      aspect  = user.aspect(:name => 'losers')
-      aspect2 = @user2.aspect(:name => 'winners')
+      aspect2 = user2.aspect(:name => 'winners')
-      friend_users(@user, aspect, @user2, aspect2)
+      friend_users(user, aspect, user2, aspect2)
 
-      message = @user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
+      message = user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
 
-      @user.receive message.to_diaspora_xml, @user2.person
+      user.receive message.to_diaspora_xml, user2.person
       aspect.reload
 
       aspect.post_ids.include?(message.id).should be true
 
-      retraction = @user2.retract(message)
+      retraction = user2.retract(message)
-      @user.receive retraction.to_diaspora_xml, @user2.person
+      user.receive retraction.to_diaspora_xml, user2.person
 
 
       aspect.reload
@@ -129,59 +135,128 @@ describe Aspect do
     end
   end
 
-  describe "aspect editing" do
+  context "aspect management" do
+
+
     before do
-      @aspect = @user.aspect(:name => 'losers')
+      friend_users(user, aspect, user2, aspect2)
-      @aspect2 = @user2.aspect(:name => 'failures')
+      aspect.reload
-      friend_users(@user, @aspect, @user2, @aspect2)
+      user.reload
-      @aspect.reload
-      @aspect3 = @user.aspect(:name => 'cats')
-      @user.reload
     end
     
-    it 'should be able to move a friend from one of users existing aspects to another' do
-      @user.move_friend(:friend_id => @user2.person.id, :from => @aspect.id, :to => @aspect3.id)
-      @aspect.reload
-      @aspect3.reload
 
-      @aspect.person_ids.include?(@user2.person.id).should be false
+    describe "#add_person_to_aspect" do
-      @aspect3.people.include?(@user2.person).should be true
+      it 'adds the user to the aspect' do
+        aspect1.people.should_not include user2.person
+        user.add_person_to_aspect(user2.person.id, aspect1.id)
+        aspect1.reload
+        aspect1.people.should include user2.person
+      end
+
+      it 'raises if its an aspect that the user does not own'do
+        proc{user.add_person_to_aspect(user2.person.id, aspect2.id) }.should raise_error /Can not add person to an aspect you do not own/
+      end
+
+      it 'does not allow to have duplicate people in an aspect' do
+        proc{user.add_person_to_aspect(not_friend.id, aspect1.id) }.should raise_error /Can not add person you are not friends with/
+      end
+
+      it 'does not allow you to add a person if they are already in the aspect' do
+        proc{user.add_person_to_aspect(user2.person.id, aspect.id) }.should raise_error /Can not add person who is already in the aspect/
+      end
+    end
+
+    describe '#delete_person_from_aspect' do
+      it 'deletes a user from the aspect' do
+         user.add_person_to_aspect(user2.person.id, aspect1.id)
+         user.reload
+         user.aspects.find_by_id(aspect1.id).people.include?(user2.person).should be true
+         user.delete_person_from_aspect(user2.person.id, aspect1.id)
+         user.reload
+         user.aspects.find_by_id(aspect1.id).people.include?(user2.person).should be false
+      end
+
+      it 'should check to make sure you have the aspect ' do
+        proc{user.delete_person_from_aspect(user2.person.id, aspect2.id) }.should raise_error /Can not delete a person from an aspect you do not own/
+      end
+    end
+
+    context 'moving and removing posts' do
+
+      let(:message) { user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)}
+      let(:message2){user3.post(:status_message, :message => "other post", :to => aspect3.id)}
+
+      before do
+        friend_users(user, aspect, user3, aspect3)
+        user.receive message.to_diaspora_xml, user2.person
+        user.receive message2.to_diaspora_xml, user3.person
+        aspect.reload
+        @post_count  = aspect.posts.count
+        @post_count1 = aspect1.posts.count
+
+        user.reload
+      end
+      
+      it 'moves the persons posts into the new aspect' do
+        user.add_person_to_aspect(user2.person.id, aspect1.id, :posts => [message] )
+        aspect1.reload
+        aspect1.posts.should == [message]
+      end
+
+      
+      it 'should remove the users posts from that aspect' do
+        user.delete_person_from_aspect(user2.person.id, aspect.id)
+        aspect.reload
+        aspect.posts.count.should == @post_count - 1
+      end
+
+      it 'should not delete other peoples posts' do
+        user.delete_person_from_aspect(user2.person.id, aspect.id)
+        aspect.reload
+        aspect.posts.should == [message2]
+      end
+
+      describe '#move_friend' do
+        it 'should be able to move a friend from one of users existing aspects to another' do
+          user.move_friend(:friend_id => user2.person.id, :from => aspect.id, :to => aspect1.id)
+          aspect.reload
+          aspect1.reload
+
+          aspect.person_ids.include?(user2.person.id).should be false
+          aspect1.people.include?(user2.person).should be true
         end
 
         it "should not move a person who is not a friend" do
-      @user.move_friend(:friend_id => @friend.id, :from => @aspect.id, :to => @aspect3.id)
+          proc{ user.move_friend(:friend_id => friend.id, :from => aspect.id, :to => aspect1.id) }.should raise_error /Can not add person you are not friends with/
-      @aspect.reload
+          aspect.reload
-      @aspect3.reload
+          aspect1.reload
-      @aspect.people.include?(@friend).should be false
+          aspect.people.include?(friend).should be false
-      @aspect3.people.include?(@friend).should be false
+          aspect1.people.include?(friend).should be false
         end
 
         it "should not move a person to a aspect that's not his" do
-      @user.move_friend(:friend_id => @user2.person.id, :from => @aspect.id, :to => @aspect2.id)
+          proc {user.move_friend(:friend_id => user2.person.id, :from => aspect.id, :to => aspect2.id )}.should raise_error /Can not add person to an aspect you do not own/
-      @aspect.reload
+          aspect.reload
-      @aspect2.reload
+          aspect2.reload
-      @aspect.people.include?(@user2.person).should be true
+          aspect.people.include?(user2.person).should be true
-      @aspect2.people.include?(@user2.person).should be false
+          aspect2.people.include?(user2.person).should be false
         end
 
-    it 'should move all the by that user to the new aspect' do
+        it 'should move all posts by that user to the new aspect' do
-      message = @user2.post(:status_message, :message => "Hey Dude", :to => @aspect2.id)
+          user.move_friend(:friend_id => user2.person.id, :from => aspect.id, :to => aspect1.id)
-
+          aspect.reload
-      @user.receive message.to_diaspora_xml, @user2.person
+          aspect1.reload
-      @aspect.reload
-
-      @aspect.posts.count.should == 1
-      @aspect3.posts.count.should == 0
-
-      @user.reload
-      @user.move_friend(:friend_id => @user2.person.id, :from => @aspect.id, :to => @aspect3.id)
-      @aspect.reload
-      @aspect3.reload
-
-      @aspect3.posts.count.should == 1
-      @aspect.posts.count.should == 0
 
+          aspect1.posts.count.should == @post_count1 + 1
+          aspect.posts.count.should == @post_count - 1
         end
 
+        it 'does not try to delete if add person did not go through' do
+          user.should_receive(:add_person_to_aspect).and_return(false)
+          user.should_not_receive(:delete_person_from_aspect)
+          user.move_friend(:friend_id => user2.person.id, :from => aspect.id, :to => aspect1.id)
+        end
+      end
+    end
   end
 end

spec/models/user/visible_posts_spec.rb

@@ -23,6 +23,7 @@ describe User do
 
   before do
     friend_users(user, first_aspect, user2, user2.aspects.first)
+    friend_users(user, second_aspect, user3, user3.aspects.first)
   end
 
   describe "#visible_posts" do
@@ -48,7 +49,6 @@ describe User do
     end
 
     it "queries by aspect" do
-      friend_users(user, second_aspect, user3, user3.aspects.first)
       friend_users(user, second_aspect, user4, user4.aspects.first)
 
       user.receive status_message4.to_diaspora_xml, user2.person
@@ -67,6 +67,19 @@ describe User do
         user.find_visible_post_by_id(status_message1.id).should == nil
       end
     end
+
+    describe '#find_friend_by_id' do
+      it 'should find both friends' do
+        user.reload
+        user.find_friend_by_id(user2.person.id).should == user2.person
+        user.find_friend_by_id(user3.person.id).should == user3.person
+      end
+
+      it 'should not find a non-friend' do
+        user3.find_friend_by_id(user4.person.id).should be nil
+      end
+
+    end
   end
 
   context 'albums' do