From 80a9b97f8df264dd63e1cacd07528eef2e6448ff Mon Sep 17 00:00:00 2001 From: Raphael Date: Wed, 15 Dec 2010 12:33:02 -0800 Subject: [PATCH] add_person_to_aspect is now add_contact_to_aspect, some methods now take models rather than ids --- app/controllers/aspects_controller.rb | 42 ++++++------ app/controllers/people_controller.rb | 1 + app/controllers/photos_controller.rb | 6 +- app/helpers/aspects_helper.rb | 8 +-- app/models/user.rb | 21 ++---- app/views/people/_aspect_list.haml | 8 +-- app/views/shared/_contact_list.html.haml | 2 +- public/stylesheets/sass/application.sass | 71 ++++++++++----------- spec/controllers/aspects_controller_spec.rb | 18 ++++-- spec/lib/diaspora/exporter_spec.rb | 2 +- spec/models/aspect_spec.rb | 66 ++++++++----------- spec/models/user/connecting_spec.rb | 4 +- spec/models/user/posting_spec.rb | 7 +- spec/models/user/receive_spec.rb | 3 +- 14 files changed, 124 insertions(+), 135 deletions(-) diff --git a/app/controllers/aspects_controller.rb b/app/controllers/aspects_controller.rb index 16690dfa9..5d1f66058 100644 --- a/app/controllers/aspects_controller.rb +++ b/app/controllers/aspects_controller.rb @@ -91,7 +91,11 @@ class AspectsController < ApplicationController end def move_contact - unless current_user.move_contact( :person_id => params[:person_id], :from => params[:from], :to => params[:to][:to]) + @person = Person.find(params[:person_id]) + @from_aspect = current_user.aspects.find(params[:from]) + @to_aspect = current_user.aspects.find(params[:to][:to]) + + unless current_user.move_contact( @person, @from_aspect, @to_aspect) flash[:error] = I18n.t 'aspects.move_contact.error',:inspect => params.inspect end if aspect = current_user.aspect_by_id(params[:to][:to]) @@ -104,30 +108,22 @@ class AspectsController < ApplicationController end def add_to_aspect - begin current_user.add_person_to_aspect( params[:person_id], params[:aspect_id]) + @person = Person.find(params[:person_id]) + @aspect = current_user.aspects.find(params[:aspect_id]) + @contact = current_user.contact_for(@person) - @aspect = current_user.aspects.find(params[:aspect_id]) - @aspect_id = @aspect.id - @person_id = params[:person_id] + current_user.add_contact_to_aspect(@contact, @aspect) + flash.now[:notice] = I18n.t 'aspects.add_to_aspect.success' - flash.now[:notice] = I18n.t 'aspects.add_to_aspect.success' - - respond_to do |format| - format.js { render :json => { - :button_html => render_to_string(:partial => 'aspects/add_to_aspect', - :locals => {:aspect_id => @aspect_id, - :person_id => @person_id}), - :badge_html => render_to_string(:partial => 'aspects/aspect_badge', - :locals => {:aspect => @aspect}) - }} - format.html{ redirect_to aspect_path(@aspect_id)} - end - rescue Exception => e - flash.now[:error] = I18n.t 'aspects.add_to_aspect.failure' - respond_to do |format| - format.js { render :text => e, :status => 403 } - format.html{ redirect_to aspect_path(@aspect_id)} - end + respond_to do |format| + format.js { render :json => { + :button_html => render_to_string(:partial => 'aspects/add_to_aspect', + :locals => {:aspect_id => @aspect.id, + :person_id => @person.id}), + :badge_html => render_to_string(:partial => 'aspects/aspect_badge', + :locals => {:aspect => @aspect}) + }} + format.html{ redirect_to aspect_path(@aspect.id)} end end diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 478122e9c..5fae95ea4 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -51,6 +51,7 @@ class PeopleController < ApplicationController @profile = @person.profile @contact = current_user.contact_for(@person) + @aspects_with_person = [] if @contact @aspects_with_person = @contact.aspects diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb index 779e1c21c..5caa2b3ed 100644 --- a/app/controllers/photos_controller.rb +++ b/app/controllers/photos_controller.rb @@ -13,9 +13,13 @@ class PhotosController < ApplicationController @person = Person.find_by_id(params[:person_id]) if @person + @incoming_request = Request.to(current_user).from(@person).first + @outgoing_request = Request.from(current_user).to(@person).first + @profile = @person.profile @contact = current_user.contact_for(@person) @is_contact = @person != current_user.person && @contact + @aspects_with_person = [] if @contact @aspects_with_person = @contact.aspects @@ -125,7 +129,7 @@ class PhotosController < ApplicationController def show @photo = current_user.find_visible_post_by_id params[:id] - if @photo + if @photo @parent = @photo.status_message #if photo is not an attachment, fetch comments for self diff --git a/app/helpers/aspects_helper.rb b/app/helpers/aspects_helper.rb index ad3b9ab48..2536132da 100644 --- a/app/helpers/aspects_helper.rb +++ b/app/helpers/aspects_helper.rb @@ -23,11 +23,11 @@ module AspectsHelper link_to image_tag('icons/monotone_check_yes.png'), {:controller => "aspects", :action => 'remove_from_aspect', :aspect_id => aspect_id, :person_id => person_id}, :remote => true, :class => 'added button' end - def aspect_membership_button(aspect_id, contact) - unless contact.aspect_ids.include?(aspect_id) - add_to_aspect_button(aspect_id, contact.person.id) + def aspect_membership_button(aspect_id, contact, person) + if contact.nil? || !contact.aspect_ids.include?(aspect_id) + add_to_aspect_button(aspect_id, person.id) else - remove_from_aspect_button(aspect_id, contact.person.id) + remove_from_aspect_button(aspect_id, person.id) end end end diff --git a/app/models/user.rb b/app/models/user.rb index 017bf36f3..f3eb0367b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -96,26 +96,19 @@ class User end end - def move_contact(opts = {}) - if opts[:to] == opts[:from] + def move_contact(person, to_aspect, from_aspect) + contact = contact_for(person) + if to_aspect == from_aspect true - elsif opts[:person_id] && opts[:to] && opts[:from] - from_aspect = self.aspects.find_by_id(opts[:from]) - - if add_person_to_aspect(opts[:person_id], opts[:to]) - delete_person_from_aspect(opts[:person_id], opts[:from]) - end + elsif add_contact_to_aspect(contact, to_aspect) + delete_person_from_aspect(person.id, from_aspect.id) end end - def add_person_to_aspect(person_id, aspect_id) - contact = contact_for(Person.find(person_id)) - 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 connected to" unless contact - raise 'Can not add person who is already in the aspect' if aspect.contacts.include?(contact) + def add_contact_to_aspect(contact, aspect) + return true if contact.aspect_ids.include?(aspect.id) contact.aspects << aspect contact.save! - aspect.save! end def delete_person_from_aspect(person_id, aspect_id, opts = {}) diff --git a/app/views/people/_aspect_list.haml b/app/views/people/_aspect_list.haml index 1642060d4..47605869d 100644 --- a/app/views/people/_aspect_list.haml +++ b/app/views/people/_aspect_list.haml @@ -23,13 +23,13 @@ }); .aspects - .badges + .badges{:class => ("hidden" if !contact)} - for aspect in aspects_with_person = render :partial => 'aspects/aspect_badge', :locals => {:aspect => aspect} .right = link_to "edit aspect membership", "#", :id=> "edit_contact_aspects" - .edit + .edit{:class => ("hidden" if contact)} .contact_list %ul - for aspect in aspects_with_person @@ -37,14 +37,14 @@ %span.name = link_to aspect.name, aspect .right - = aspect_membership_button(aspect.id, contact) + = aspect_membership_button(aspect.id, contact, person) - for aspect in aspects_without_person %li %span.name = link_to aspect.name, aspect .right - = aspect_membership_button(aspect.id, contact) + = aspect_membership_button(aspect.id, contact, person) .right = link_to "done editing", "#", :id => "done_contact_aspects" diff --git a/app/views/shared/_contact_list.html.haml b/app/views/shared/_contact_list.html.haml index 250774105..ed98c7c11 100644 --- a/app/views/shared/_contact_list.html.haml +++ b/app/views/shared/_contact_list.html.haml @@ -11,5 +11,5 @@ %span.name = link_to hash[:person].name, hash[:person] .right - = aspect_membership_button(aspect_id, hash[:contact]) + = aspect_membership_button(aspect_id, hash[:contact], hash[:person]) diff --git a/public/stylesheets/sass/application.sass b/public/stylesheets/sass/application.sass index ea0ffb5e5..732b8ba3c 100644 --- a/public/stylesheets/sass/application.sass +++ b/public/stylesheets/sass/application.sass @@ -61,14 +61,14 @@ form :width 100% :padding 1em - + :-moz-box-shadow 0 1px 2px #333 :-webkit-box-shadow 0 1px 2px #333 :box-shadow 0 1px 2px #333 :font :weight bold - + #flash_notice :background :color rgb(126,240,77) @@ -160,7 +160,7 @@ header :background :color #000 - + :-webkit-border-radius 5px :-moz-border-radius 5px :border-radius 5px @@ -170,7 +170,7 @@ header :top 5px :right 0 - + a :padding :right 15px @@ -213,7 +213,7 @@ header :height 20px :width 20px :position absolute - :left 2px + :left 2px :top 2px :display block @@ -339,11 +339,11 @@ li.message .stream_photo :float left - :margin + :margin :top 6px .photo_description - :margin + :margin :top 6px :padding :left 220px @@ -370,7 +370,7 @@ li.message :weight bold :margin :right 5px - + &:hover div.info, .time a @@ -595,7 +595,7 @@ ul.comments > li :margin :bottom 2em - + .stream, #profile, .comments @@ -661,7 +661,7 @@ li.message:hover :display inline-block :background :color #fff - + :padding 10px :bottom 30px @@ -878,7 +878,7 @@ label :-webkit-border-radius 5px :-moz-border-radius 5px - :border-radius 5px + :border-radius 5px :background -webkit-gradient( linear, left bottom, left top, color-stop(0, rgb(0,123,194)), color-stop(1, rgb(65,182,250))) :background -moz-linear-gradient( center bottom, rgb(0,123,194) 0%, rgb(65,182,250) 100%) @@ -930,9 +930,9 @@ label :margin 0 :padding 4px 0 a - :-webkit-border-radius 3px 3px 0 0 - :-moz-border-radius 3px 3px 0 0 - :border-radius 3px 3px 0 0 + :-webkit-border-radius 3px 3px 0 0 + :-moz-border-radius 3px 3px 0 0 + :border-radius 3px 3px 0 0 :text-shadow 0 1px 0 #444 @@ -976,7 +976,7 @@ label input :display inline :background-color #888 - :border + :border :top 1px solid #111 :font @@ -1061,8 +1061,8 @@ label .draggable_info :position absolute :display none - :right 15px - :bottom 10px + :right 15px + :bottom 10px :font :style italic :size 14px @@ -1263,7 +1263,7 @@ ul#settings_nav :padding 6px :border :bottom 1px solid #ddd - + .floating.empty_message :margin @@ -1289,7 +1289,7 @@ h1,h2,h3,h4 :color #ccc :margin :top 0.5em - + h2,h3,h4 .description :font @@ -1323,7 +1323,7 @@ input[type="search"] h2 :display inline - + .right :margin :top 10px @@ -1364,7 +1364,7 @@ input[type="search"] .photo_options :display inline :position absolute - :bottom 5px + :bottom 5px :right 13px :color #999 @@ -1399,7 +1399,7 @@ ul.aspects :-moz-border-radius 10px :border-radius 10px - :line-height 16px + :line-height 16px :text-indent 6px :font-size 16px @@ -1450,7 +1450,7 @@ ul.aspects :color #fff :padding 12px :bottom 10px - + a :color #fafafa :background @@ -1474,7 +1474,7 @@ ul.aspects :display inline :font :weight normal - + &:after :content ", " @@ -1536,7 +1536,7 @@ h3 span.current_gs_step :position fixed :bottom 21px :right 12px - + a :background :color rgb(30,30,30) @@ -1553,7 +1553,7 @@ h3 span.current_gs_step :min-width 200px :padding 12px :color #fff - + #profile ul#aspects_for_person > li @@ -1627,11 +1627,11 @@ ul#request_result :-webkit-border-radius 5px :-moz-border-radius 5px :border-radius 5px - + :background :color rgb(252,252,252) - :margin + :margin :bottom 1.5em :-webkit-box-shadow 0 1px #fff @@ -1711,21 +1711,21 @@ footer &:hover :color #ccc - + ul#landing_nav :margin 0 :padding 0 :font :size 14px - + > li :display inline :margin :right 0.5em a :color #107FC9 - + &.login :padding 5px 8px @@ -1771,7 +1771,7 @@ ul#landing_nav :display block :color #888 :text-shadow 0 1px 0 #fff - + #mce-error-response :color red @@ -1779,7 +1779,7 @@ ul#landing_nav :color green input[type='text'] - :top -1px + :top -1px :margin 0 :right -3px :width 300px @@ -1860,7 +1860,7 @@ ul#landing_nav :top 14px :left 540px :color #888 - + ul#press_logos :margin 0 @@ -2015,9 +2015,6 @@ h3,h4 :height auto :max-height auto :width 298px - - .edit - :display none .aspects .aspect_badge :font diff --git a/spec/controllers/aspects_controller_spec.rb b/spec/controllers/aspects_controller_spec.rb index 53c6a0e01..2f73c17ce 100644 --- a/spec/controllers/aspects_controller_spec.rb +++ b/spec/controllers/aspects_controller_spec.rb @@ -17,7 +17,7 @@ describe AspectsController do connect_users(@user, @aspect, @user2, @aspect2) - @contact = @user.contact_for(@user2.person) + @contact = @user.contact_for(@user2.person) @user.getting_started = false @user.save sign_in :user, @user @@ -146,7 +146,12 @@ describe AspectsController do end describe "#move_contact" do - let(:opts) { {:person_id => "person_id", :from => "from_aspect_id", :to => {:to => "to_aspect_id"}} } + let(:opts) { { + :person_id => "person_id", + :from => "from_aspect_id", + :to => + {:to => "to_aspect_id"} + } } it 'calls the move_contact_method' do pending "need to figure out what is the deal with remote requests" @controller.stub!(:current_user).and_return(@user) @@ -222,8 +227,11 @@ describe AspectsController do describe "#add_to_aspect" do context 'with a non-contact' do - it 'creates a pending contact' do - pending + before do + @person = Factory(:person) + end + it 'calls send_contact_request_to' do + end end it 'adds the users to the aspect' do @@ -238,7 +246,7 @@ describe AspectsController do describe "#remove_from_aspect" do it 'removes contacts from an aspect' do - @user.add_person_to_aspect( @user2.person.id, @aspect1.id) + @user.add_contact_to_aspect(@contact, @aspect1) @aspect.reload @aspect.contacts.include?(@contact).should be true post 'remove_from_aspect', :format => 'js', :person_id => @user2.person.id, :aspect_id => @aspect.id diff --git a/spec/lib/diaspora/exporter_spec.rb b/spec/lib/diaspora/exporter_spec.rb index 00791fd3b..e69704f70 100644 --- a/spec/lib/diaspora/exporter_spec.rb +++ b/spec/lib/diaspora/exporter_spec.rb @@ -51,7 +51,7 @@ describe Diaspora::Exporter do before do connect_users(@user1, @aspect1, @user3, @aspect3) - @user1.add_person_to_aspect(@user3.person.id, @aspect.id) + @user1.add_contact_to_aspect(@user1.contact_for(@user3.person), @aspect) @user1.reload end diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb index e9736a95b..b693eeaa0 100644 --- a/spec/models/aspect_spec.rb +++ b/spec/models/aspect_spec.rb @@ -13,7 +13,6 @@ describe Aspect do let(:aspect) {user.aspects.create(:name => 'losers')} let(:aspect2) {user2.aspects.create(:name => 'failures')} let(:aspect1) {user.aspects.create(:name => 'cats')} - let(:not_contact) { Factory(:person, :diaspora_handle => "not@person.com")} let(:user3) {make_user} let(:aspect3) {user3.aspects.create(:name => "lala")} @@ -96,10 +95,10 @@ describe Aspect do it 'returns multiple aspects if the person is there' do user.reload - user.add_person_to_aspect(connected_person.id, aspect1.id) + contact = user.contact_for(connected_person) + user.add_contact_to_aspect(contact, aspect1) aspects = user.aspects_with_person(connected_person) aspects.count.should == 2 - contact = user.contact_for(connected_person) aspects.each{ |asp| asp.contacts.include?(contact).should be_true } aspects.include?(aspect_without_contact).should be_false end @@ -147,42 +146,34 @@ describe Aspect do end context "aspect management" do - let(:contact){user.contact_for(user2.person)} before do connect_users(user, aspect, user2, aspect2) aspect.reload user.reload + @contact = user.contact_for(user2.person) end - - describe "#add_person_to_aspect" do - it 'adds the user to the aspect' do - aspect1.contacts.include?(contact).should be_false - user.add_person_to_aspect(user2.person.id, aspect1.id) + + describe "#add_contact_to_aspect" do + it 'adds the contact to the aspect' do + aspect1.contacts.include?(@contact).should be_false + user.add_contact_to_aspect(@contact, aspect1) aspect1.reload - aspect1.contacts.include?(contact).should be_true + aspect1.contacts.include?(@contact).should be_true 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 contacts in an aspect' do - proc{user.add_person_to_aspect(not_contact.id, aspect1.id) }.should raise_error /Can not add person you are not connected to/ - 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/ + it 'returns true if they are already in the aspect' do + user.add_contact_to_aspect(@contact, aspect).should == true 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.add_contact_to_aspect(@contact, aspect1) user.reload user.delete_person_from_aspect(user2.person.id, aspect1.id) user.reload - aspect1.reload.contacts.include?(contact).should be false + aspect1.reload.contacts.include?(@contact).should be false end it 'should check to make sure you have the aspect ' do @@ -190,7 +181,7 @@ describe Aspect do end it 'deletes no posts' do - user.add_person_to_aspect(user2.person.id, aspect1.id) + user.add_contact_to_aspect(@contact, aspect1) user.reload user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id) lambda{ @@ -203,9 +194,9 @@ describe Aspect do end it 'should allow a force removal of a contact from an aspect' do - contact.aspect_ids.should_receive(:count).exactly(0).times + @contact.aspect_ids.should_receive(:count).exactly(0).times - user.add_person_to_aspect(user2.person.id, aspect1.id) + user.add_contact_to_aspect(@contact, aspect1) user.delete_person_from_aspect(user2.person.id, aspect.id, :force => true) end @@ -217,7 +208,7 @@ describe Aspect do aspect.reload user.reload end - + it 'should keep the contact\'s posts in previous aspect' do aspect.post_ids.count.should == 1 user.delete_person_from_aspect(user2.person.id, aspect.id, :force => true) @@ -235,34 +226,29 @@ describe Aspect do describe '#move_contact' do it 'should be able to move a contact from one of users existing aspects to another' do - user.move_contact(:person_id => user2.person.id, :from => aspect.id, :to => aspect1.id) + user.move_contact(user2.person, aspect1, aspect) aspect.reload aspect1.reload - aspect.contacts.include?(contact).should be_false - aspect1.contacts.include?(contact).should be_true + aspect.contacts.include?(@contact).should be_false + aspect1.contacts.include?(@contact).should be_true end it "should not move a person who is not a contact" do - proc{ user.move_contact(:person_id => connected_person.id, :from => aspect.id, :to => aspect1.id) }.should raise_error /Can not add person you are not connected to/ + proc{ + user.move_contact(connected_person, aspect1, aspect) + }.should raise_error + aspect.reload aspect1.reload aspect.contacts.first(:person_id => connected_person.id).should be_nil aspect1.contacts.first(:person_id => connected_person.id).should be_nil end - it "should not move a person to a aspect that's not his" do - proc {user.move_contact(:person_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 - aspect2.reload - aspect.contacts.include?(contact).should be true - aspect2.contacts.include?(contact).should be false - 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_receive(:add_contact_to_aspect).and_return(false) user.should_not_receive(:delete_person_from_aspect) - user.move_contact(:person_id => user2.person.id, :from => aspect.id, :to => aspect1.id) + user.move_contact(user2.person, aspect1, aspect) end end end diff --git a/spec/models/user/connecting_spec.rb b/spec/models/user/connecting_spec.rb index 2d63fc408..f229bc48c 100644 --- a/spec/models/user/connecting_spec.rb +++ b/spec/models/user/connecting_spec.rb @@ -242,7 +242,9 @@ describe Diaspora::UserModules::Connecting do end it 'should remove the contact from all aspects they are in' do - user.add_person_to_aspect(user2.person.id, aspect1.id) + user.add_contact_to_aspect( + user.contact_for(user2.person), + aspect1) aspect.reload.contacts.count.should == 1 aspect1.reload.contacts.count.should == 1 lambda { user.disconnected_by user2.person }.should change { diff --git a/spec/models/user/posting_spec.rb b/spec/models/user/posting_spec.rb index c19ecef1a..15ec8c601 100644 --- a/spec/models/user/posting_spec.rb +++ b/spec/models/user/posting_spec.rb @@ -37,7 +37,7 @@ describe User do aspect1.reload.post_ids.should include @post.id end - it 'sockets the post to the poster' do + it 'sockets the post to the poster' do @post.should_receive(:socket_to_uid).with(user.id, anything) user.add_to_streams(@post, @aspect_ids) end @@ -110,7 +110,7 @@ describe User do user.dispatch_post(status, :to => "all") end end - + describe '#update_post' do it 'should update fields' do photo = user.post(:photo, :user_file => uploaded_photo, :caption => "Old caption", :to => aspect.id) @@ -134,7 +134,8 @@ describe User do connect_users(user, aspect, user2, aspect2) connect_users(user, aspect, user3, aspect3) connect_users(user, aspect1, user4, aspect4) - user.add_person_to_aspect(user2.person.id, aspect1.id) + contact = user.contact_for(user2.person) + user.add_contact_to_aspect(contact, aspect1) user.reload end diff --git a/spec/models/user/receive_spec.rb b/spec/models/user/receive_spec.rb index 86ac26b38..11841fc47 100644 --- a/spec/models/user/receive_spec.rb +++ b/spec/models/user/receive_spec.rb @@ -20,7 +20,8 @@ describe User do end it 'should stream only one message to the everyone aspect when a multi-aspected contacts posts' do - user.add_person_to_aspect(user2.person.id, user.aspects.create(:name => "villains").id) + contact = user.contact_for(user2.person) + user.add_contact_to_aspect(contact, user.aspects.create(:name => "villains")) status = user2.post(:status_message, :message => "Users do things", :to => aspect2.id) xml = status.to_diaspora_xml Diaspora::WebSocket.should_receive(:queue_to_user).exactly(:once)