From 06f886ad770d95b7396a068ad15c153c624632d9 Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Wed, 25 May 2011 17:22:36 -0700 Subject: [PATCH 01/28] WIP reshare --- app/controllers/people_controller.rb | 5 ++--- app/views/shared/_stream_element.html.haml | 3 +++ config/routes.rb | 2 ++ features/disconnects_users.feature | 2 +- features/step_definitions/posts_steps.rb | 5 +++++ spec/controllers/people_controller_spec.rb | 19 +++++++++++++++++++ spec/factories.rb | 5 +++++ 7 files changed, 37 insertions(+), 4 deletions(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 2f2eeb3ff..6f5c627b7 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -96,12 +96,11 @@ class PeopleController < ApplicationController else @commenting_disabled = false end - @posts = current_user.posts_from(@person).where(:type => ["StatusMessage", "ActivityStreams::Photo"]).includes(:comments).limit(15).where(StatusMessage.arel_table[:created_at].lt(max_time)) + @posts = current_user.posts_from(@person).where(:type => ["StatusMessage", "Reshare", "ActivityStreams::Photo"]).includes(:comments).limit(15).where(StatusMessage.arel_table[:created_at].lt(max_time)) else @commenting_disabled = true - @posts = @person.posts.where(:type => ["StatusMessage", "ActivityStreams::Photo"], :public => true).includes(:comments).limit(15).where(StatusMessage.arel_table[:created_at].lt(max_time)).order('posts.created_at DESC') + @posts = @person.posts.where(:type => ["StatusMessage", "Reshare", "ActivityStreams::Photo"], :public => true).includes(:comments).limit(15).where(StatusMessage.arel_table[:created_at].lt(max_time)).order('posts.created_at DESC') end - @posts = PostsFake.new(@posts) end diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml index 0c1d4769a..55f27f1ef 100644 --- a/app/views/shared/_stream_element.html.haml +++ b/app/views/shared/_stream_element.html.haml @@ -53,6 +53,9 @@ %span.like_action = like_action(post, current_user) · + %span.reshare_action + = link_to "Reshare", reshares_path(:root_id => post.id), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" + · = link_to t('comments.new_comment.comment'), '#', :class => 'focus_comment_textarea' .likes.on_post diff --git a/config/routes.rb b/config/routes.rb index 9b792f21e..dca33bae5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -6,6 +6,8 @@ Diaspora::Application.routes.draw do # Posting and Reading + resources :reshares + resources :aspects do put :toggle_contact_visibility end diff --git a/features/disconnects_users.feature b/features/disconnects_users.feature index fdede04d0..ebffd1407 100644 --- a/features/disconnects_users.feature +++ b/features/disconnects_users.feature @@ -1,6 +1,6 @@ @javascript Feature: disconnecting users - In order to deal with life + In order to help users feel secure on Diaspora As a User I want to be able to disconnect from others diff --git a/features/step_definitions/posts_steps.rb b/features/step_definitions/posts_steps.rb index 2ab36b55a..6e3661333 100644 --- a/features/step_definitions/posts_steps.rb +++ b/features/step_definitions/posts_steps.rb @@ -8,3 +8,8 @@ end Then /^I should see an uploaded image within the photo drop zone$/ do find("#photodropzone img")["src"].should include("uploads/images") end + +Given /^"([^"]*)" has a public post with text "([^"]*)"$/ do |email, text| + user = User.find_by_email(email) + user.post(:status_message, :text => text, :public => true, :to => user.aspects) +end diff --git a/spec/controllers/people_controller_spec.rb b/spec/controllers/people_controller_spec.rb index 7bb2b4d95..f6aea6246 100644 --- a/spec/controllers/people_controller_spec.rb +++ b/spec/controllers/people_controller_spec.rb @@ -137,6 +137,7 @@ describe PeopleController do response.body.match(profile.first_name).should be_false end + context "when the person is the current user" do it "succeeds" do get :show, :id => @user.person.to_param @@ -204,6 +205,12 @@ describe PeopleController do @public_posts.first.save end + it "posts include reshares" do + reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id) + get :show, :id => @user.person.id + assigns[:posts].post_fakes.map{|x| x.id}.should include(reshare.id) + end + it "assigns only public posts" do get :show, :id => @person.id assigns[:posts].models.should =~ @public_posts @@ -251,6 +258,12 @@ describe PeopleController do assigns(:posts).models.should =~ posts_user_can_see end + it "posts include reshares" do + reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id) + get :show, :id => @user.person.id + assigns[:posts].post_fakes.map{|x| x.id}.should include(reshare.id) + end + it 'sets @commenting_disabled to true' do get :show, :id => @person.id assigns(:commenting_disabled).should == false @@ -283,6 +296,12 @@ describe PeopleController do assigns[:posts].models.should =~ [public_post] end + it "posts include reshares" do + reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id) + get :show, :id => @user.person.id + assigns[:posts].post_fakes.map{|x| x.id}.should include(reshare.id) + end + it 'sets @commenting_disabled to true' do get :show, :id => @person.id assigns(:commenting_disabled).should == true diff --git a/spec/factories.rb b/spec/factories.rb index 7d9ebe9ca..dbd6da864 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -91,6 +91,11 @@ Factory.define(:photo) do |p| end end +Factory.define :reshare do |r| + r.association(:root, :public => true, :factory => :status_message) + r.association(:author, :factory => :person) +end + Factory.define :service do |service| service.nickname "sirrobertking" service.type "Services::Twitter" From fa9269541fa89b4ed4061cd19a99b4bc89b0a184 Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Wed, 25 May 2011 19:01:17 -0700 Subject: [PATCH 02/28] wip removed some generated specs --- app/controllers/aspects_controller.rb | 2 +- app/controllers/reshares_controller.rb | 13 +++++++ app/helpers/reshares_helper.rb | 2 + app/models/reshare.rb | 19 ++++++++++ app/views/reshares/create.js.erb | 1 + app/views/shared/_stream.haml | 2 +- app/views/shared/_stream_element.html.haml | 11 +++++- app/views/shared/_stream_element_shim.haml | 5 +++ .../20110525213325_add_root_id_to_posts.rb | 9 +++++ features/repost.feature | 29 ++++++++++++++ public/stylesheets/sass/application.sass | 4 ++ spec/controllers/aspects_controller_spec.rb | 6 +++ spec/controllers/people_controller_spec.rb | 6 +-- spec/controllers/reshares_controller_spec.rb | 38 +++++++++++++++++++ spec/helpers/reshares_helper_spec.rb | 15 ++++++++ spec/models/reshare_spec.rb | 20 ++++++++++ 16 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 app/controllers/reshares_controller.rb create mode 100644 app/helpers/reshares_helper.rb create mode 100644 app/models/reshare.rb create mode 100644 app/views/reshares/create.js.erb create mode 100644 app/views/shared/_stream_element_shim.haml create mode 100644 db/migrate/20110525213325_add_root_id_to_posts.rb create mode 100644 features/repost.feature create mode 100644 spec/controllers/reshares_controller_spec.rb create mode 100644 spec/helpers/reshares_helper_spec.rb create mode 100644 spec/models/reshare_spec.rb diff --git a/app/controllers/aspects_controller.rb b/app/controllers/aspects_controller.rb index ad4958ae3..c2051bef8 100644 --- a/app/controllers/aspects_controller.rb +++ b/app/controllers/aspects_controller.rb @@ -44,7 +44,7 @@ class AspectsController < ApplicationController @aspect_ids = @aspects.map { |a| a.id } posts = current_user.visible_posts(:by_members_of => @aspect_ids, - :type => ['StatusMessage','ActivityStreams::Photo'], + :type => ['StatusMessage','Reshare', 'ActivityStreams::Photo'], :order => session[:sort_order] + ' DESC', :max_time => params[:max_time].to_i ).includes(:mentions => {:person => :profile}) diff --git a/app/controllers/reshares_controller.rb b/app/controllers/reshares_controller.rb new file mode 100644 index 000000000..d28a0c0a5 --- /dev/null +++ b/app/controllers/reshares_controller.rb @@ -0,0 +1,13 @@ +class ResharesController < ApplicationController + before_filter :authenticate_user! + respond_to :js + + def create + @reshare = current_user.build_post(:reshare, :root_id => params[:root_id]) + if @reshare.save! + current_user.add_to_streams(@reshare, current_user.aspects) + end + + respond_with @reshare + end +end diff --git a/app/helpers/reshares_helper.rb b/app/helpers/reshares_helper.rb new file mode 100644 index 000000000..f1b23f522 --- /dev/null +++ b/app/helpers/reshares_helper.rb @@ -0,0 +1,2 @@ +module ResharesHelper +end diff --git a/app/models/reshare.rb b/app/models/reshare.rb new file mode 100644 index 000000000..2b0366518 --- /dev/null +++ b/app/models/reshare.rb @@ -0,0 +1,19 @@ +class Reshare < Post + belongs_to :root, :class_name => 'Post' + validate :root_must_be_public + attr_accessible :root_id, + + before_validation do + self.public = true + end + + delegate :photos, :text, :comments, :to => :root + private + + def root_must_be_public + if self.root.nil? || !self.root.public + errors[:base] << "you must reshare public posts" + return false + end + end +end diff --git a/app/views/reshares/create.js.erb b/app/views/reshares/create.js.erb new file mode 100644 index 000000000..f25d9629f --- /dev/null +++ b/app/views/reshares/create.js.erb @@ -0,0 +1 @@ +$('.stream_element[data-guid=<%=params[:root_id]%>]').addClass('reshared'); diff --git a/app/views/shared/_stream.haml b/app/views/shared/_stream.haml index 95ee72a0f..1d79fb636 100644 --- a/app/views/shared/_stream.haml +++ b/app/views/shared/_stream.haml @@ -3,7 +3,7 @@ -# the COPYRIGHT file. -= render :partial => 'shared/stream_element', += render :partial => 'shared/stream_element_shim', :collection => posts, :as => :post, :locals => { :commenting_disabled => defined?(@commenting_disabled)} diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml index 55f27f1ef..0a2049f31 100644 --- a/app/views/shared/_stream_element.html.haml +++ b/app/views/shared/_stream_element.html.haml @@ -4,11 +4,18 @@ .stream_element{:id => post.guid} - if current_user && post.author.owner_id == current_user.id + - if reshare + .reshare_attribution + = "reshared by #{reshare.author.name}" .right.controls = link_to image_tag('deletelabel.png'), post_path(post), :confirm => t('are_you_sure'), :method => :delete, :remote => true, :class => "delete stream_element_delete", :title => t('delete') + - else - .right.controls - = link_to image_tag('deletelabel.png'), post_visibility_path(:id => "42", :post_id => post.id), :method => :put, :remote => true, :class => "delete stream_element_delete", :title => t('hide') + .right + - if reshare + .reshare_attribution= "reshared by #{reshare.author.name}" + .controls + = link_to image_tag('deletelabel.png'), post_visibility_path(:id => "42", :post_id => post.id), :method => :put, :remote => true, :class => "delete stream_element_delete", :title => t('hide') .undo_text.hidden = t('post_visibilites.update.post_hidden', :name => post.author.name) = link_to t('undo'), post_visibility_path(:id => "42", :post_id => post.id), :method => :put, :remote => true, :class => "delete stream_element_delete" diff --git a/app/views/shared/_stream_element_shim.haml b/app/views/shared/_stream_element_shim.haml new file mode 100644 index 000000000..a5dd321c4 --- /dev/null +++ b/app/views/shared/_stream_element_shim.haml @@ -0,0 +1,5 @@ + +-if post.model.is_a?(Reshare) + = render 'shared/stream_element', :post => post.root, :all_aspects => all_aspects, :commenting_disabled => commenting_disabled, :reshare => post +- else + = render 'shared/stream_element', :post => post, :all_aspects => all_aspects, :commenting_disabled => commenting_disabled, :reshare => nil diff --git a/db/migrate/20110525213325_add_root_id_to_posts.rb b/db/migrate/20110525213325_add_root_id_to_posts.rb new file mode 100644 index 000000000..7f6969b54 --- /dev/null +++ b/db/migrate/20110525213325_add_root_id_to_posts.rb @@ -0,0 +1,9 @@ +class AddRootIdToPosts < ActiveRecord::Migration + def self.up + add_column :posts, :root_id, :integer + end + + def self.down + remove_column :posts, :root_id + end +end diff --git a/features/repost.feature b/features/repost.feature new file mode 100644 index 000000000..1be45daaf --- /dev/null +++ b/features/repost.feature @@ -0,0 +1,29 @@ +@javascript +Feature: public repost + In order to make Diaspora more viral + As a User + I want to reshare my friends post + + Background: + Given a user named "Bob Jones" with email "bob@bob.bob" + And a user named "Alice Smith" with email "alice@alice.alice" + And a user with email "bob@bob.bob" is connected with "alice@alice.alice" + And "bob@bob.bob" has a public post with text "reshare this!" + And I sign in as "alice@alice.alice" + And I preemptively confirm the alert + And I follow "Reshare" + And I wait for the ajax to finish + + Scenario: shows up on the profile page + Then I should see a ".reshared" + And I am on "alice@alice.alice"'s page + Then I should see "reshare this!" + Then I should see a ".reshared" + And I should see "Bob" + + Scenario: shows up on the aspects page + Then I should see a ".reshared" + And I follow "All Aspects" + Then I should see "reshare this!" + Then I should see a ".reshared" + And I should see "Bob" diff --git a/public/stylesheets/sass/application.sass b/public/stylesheets/sass/application.sass index b02d1881e..d3d462a10 100644 --- a/public/stylesheets/sass/application.sass +++ b/public/stylesheets/sass/application.sass @@ -376,6 +376,10 @@ ul.as-selections &:hover :text :decoration none +.reshare_attribution + :font-size smaller +.reshared + :background-color #eee .stream_element :position relative diff --git a/spec/controllers/aspects_controller_spec.rb b/spec/controllers/aspects_controller_spec.rb index 792ac7a9e..f0a2794a6 100644 --- a/spec/controllers/aspects_controller_spec.rb +++ b/spec/controllers/aspects_controller_spec.rb @@ -179,6 +179,12 @@ describe AspectsController do assigns(:posts).models.length.should == 2 end + it "posts include reshares" do + reshare = alice.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id, :to => alice.aspects) + get :index + assigns[:posts].post_fakes.map{|x| x.id}.should include(reshare.id) + end + it "can filter to a single aspect" do get :index, :a_ids => [@alices_aspect_2.id.to_s] assigns(:posts).models.length.should == 1 diff --git a/spec/controllers/people_controller_spec.rb b/spec/controllers/people_controller_spec.rb index f6aea6246..7feb2b478 100644 --- a/spec/controllers/people_controller_spec.rb +++ b/spec/controllers/people_controller_spec.rb @@ -206,7 +206,7 @@ describe PeopleController do end it "posts include reshares" do - reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id) + reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id, :to => alice.aspects) get :show, :id => @user.person.id assigns[:posts].post_fakes.map{|x| x.id}.should include(reshare.id) end @@ -259,7 +259,7 @@ describe PeopleController do end it "posts include reshares" do - reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id) + reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id, :to => alice.aspects) get :show, :id => @user.person.id assigns[:posts].post_fakes.map{|x| x.id}.should include(reshare.id) end @@ -297,7 +297,7 @@ describe PeopleController do end it "posts include reshares" do - reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id) + reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id, :to => alice.aspects) get :show, :id => @user.person.id assigns[:posts].post_fakes.map{|x| x.id}.should include(reshare.id) end diff --git a/spec/controllers/reshares_controller_spec.rb b/spec/controllers/reshares_controller_spec.rb new file mode 100644 index 000000000..9a780a905 --- /dev/null +++ b/spec/controllers/reshares_controller_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' +describe ResharesController do + + describe '#create' do + + it 'requires authentication' do + post :create, :format => :js + response.should_not be_success + end + + context 'with an authenticated user' do + + before do + + sign_in :user, bob + @post_id = Factory(:status_message, :public => true).id + @controller.stub(:current_user).and_return(bob) + end + + it 'succeeds' do + post :create, :format => :js, :root_id => @post_id + puts response.code + response.should be_success + end + + it 'creates a reshare' do + expect{ + post :create, :format => :js, :root_id => @post_id + }.should change(Reshare, :count).by(1) + end + + it 'after save, calls add to streams' do + bob.should_receive(:add_to_streams) + post :create, :format => :js, :root_id => @post_id + end + end + end +end diff --git a/spec/helpers/reshares_helper_spec.rb b/spec/helpers/reshares_helper_spec.rb new file mode 100644 index 000000000..524a2bd0c --- /dev/null +++ b/spec/helpers/reshares_helper_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +# Specs in this file have access to a helper object that includes +# the ResharesHelper. For example: +# +# describe ResharesHelper do +# describe "string concat" do +# it "concats two strings with spaces" do +# helper.concat_strings("this","that").should == "this that" +# end +# end +# end +describe ResharesHelper do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb new file mode 100644 index 000000000..7fcd620ae --- /dev/null +++ b/spec/models/reshare_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Reshare do + it 'has a valid Factory' do + Factory(:reshare).should be_valid + end + + it 'requires root' do + reshare = Factory.build(:reshare, :root => nil) + reshare.should_not be_valid + end + + it 'require public root' do + Factory.build(:reshare, :root => Factory.build(:status_message, :public => false)).should_not be_valid + end + + it 'forces public' do + Factory(:reshare, :public => false).public.should be_true + end +end From ce9477976603cdb000d6e7095b13dc548235f99c Mon Sep 17 00:00:00 2001 From: Ilya Zhitomirskiy Date: Thu, 26 May 2011 14:41:38 -0700 Subject: [PATCH 03/28] not showing the reshare button to the users on things they can't reshare --- app/helpers/sockets_helper.rb | 1 + app/models/reshare.rb | 3 ++- app/views/shared/_stream_element.html.haml | 9 +++++--- app/views/status_messages/create.js.erb | 3 ++- features/repost.feature | 26 +++++++++++++++++++++- features/step_definitions/posts_steps.rb | 6 +++++ 6 files changed, 42 insertions(+), 6 deletions(-) diff --git a/app/helpers/sockets_helper.rb b/app/helpers/sockets_helper.rb index eaa246585..799d7f36d 100644 --- a/app/helpers/sockets_helper.rb +++ b/app/helpers/sockets_helper.rb @@ -28,6 +28,7 @@ module SocketsHelper post_hash = {:post => object, :author => object.author, :photos => object.photos, + :reshare => nil, :comments => object.comments.map{|c| {:comment => c, :author => c.author diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 2b0366518..05f29367f 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -1,12 +1,13 @@ class Reshare < Post belongs_to :root, :class_name => 'Post' validate :root_must_be_public - attr_accessible :root_id, + attr_accessible :root_id, :public before_validation do self.public = true end + delegate :photos, :text, :comments, :to => :root private diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml index 0a2049f31..6f83771c8 100644 --- a/app/views/shared/_stream_element.html.haml +++ b/app/views/shared/_stream_element.html.haml @@ -59,10 +59,13 @@ - unless (defined?(@commenting_disabled) && @commenting_disabled) %span.like_action = like_action(post, current_user) + + - unless(post.author_id == current_user.person.id) || (!post.public?) + · + %span.reshare_action + = link_to "Reshare", reshares_path(:root_id => post.id), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" · - %span.reshare_action - = link_to "Reshare", reshares_path(:root_id => post.id), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" - · + = link_to t('comments.new_comment.comment'), '#', :class => 'focus_comment_textarea' .likes.on_post diff --git a/app/views/status_messages/create.js.erb b/app/views/status_messages/create.js.erb index 02965ff00..7d2d2a0eb 100644 --- a/app/views/status_messages/create.js.erb +++ b/app/views/status_messages/create.js.erb @@ -5,7 +5,8 @@ :author => @status_message.author, :photos => @status_message.photos, :comments => [], - :all_aspects => current_user.aspects + :all_aspects => current_user.aspects, + :reshare => nil } ), :post_id => @status_message.guid}.to_json.html_safe%> diff --git a/features/repost.feature b/features/repost.feature index 1be45daaf..a54e1e798 100644 --- a/features/repost.feature +++ b/features/repost.feature @@ -8,13 +8,30 @@ Feature: public repost Given a user named "Bob Jones" with email "bob@bob.bob" And a user named "Alice Smith" with email "alice@alice.alice" And a user with email "bob@bob.bob" is connected with "alice@alice.alice" + + + Scenario: does not show the reshare button on my own posts + And "bob@bob.bob" has a non public post with text "reshare this!" + And I sign in as "bob@bob.bob" + Then I should not see "Reshare" + + Scenario: does not show a reshare button on other private pots + And "bob@bob.bob" has a non public post with text "reshare this!" + And I sign in as "alice@alice.alice" + Then I should not see "Reshare" + + Scenario: does shows the reshare button on my own posts + And "bob@bob.bob" has a public post with text "reshare this!" + And I sign in as "alice@alice.alice" + Then I should see "Reshare" + + Scenario: shows up on the profile page And "bob@bob.bob" has a public post with text "reshare this!" And I sign in as "alice@alice.alice" And I preemptively confirm the alert And I follow "Reshare" And I wait for the ajax to finish - Scenario: shows up on the profile page Then I should see a ".reshared" And I am on "alice@alice.alice"'s page Then I should see "reshare this!" @@ -22,8 +39,15 @@ Feature: public repost And I should see "Bob" Scenario: shows up on the aspects page + And "bob@bob.bob" has a public post with text "reshare this!" + And I sign in as "alice@alice.alice" + And I preemptively confirm the alert + And I follow "Reshare" + And I wait for the ajax to finish + Then I should see a ".reshared" And I follow "All Aspects" Then I should see "reshare this!" Then I should see a ".reshared" And I should see "Bob" + diff --git a/features/step_definitions/posts_steps.rb b/features/step_definitions/posts_steps.rb index 6e3661333..d9879e72a 100644 --- a/features/step_definitions/posts_steps.rb +++ b/features/step_definitions/posts_steps.rb @@ -13,3 +13,9 @@ Given /^"([^"]*)" has a public post with text "([^"]*)"$/ do |email, text| user = User.find_by_email(email) user.post(:status_message, :text => text, :public => true, :to => user.aspects) end + +Given /^"([^"]*)" has a non public post with text "([^"]*)"$/ do |email, text| + user = User.find_by_email(email) + user.post(:status_message, :text => text, :public => false, :to => user.aspects) +end + From d1bbbdbc4c2c91af69fa841572d9116d93f1ff97 Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Tue, 12 Jul 2011 15:04:15 -0700 Subject: [PATCH 04/28] fixed all specs; rebased. --- app/views/posts/show.html.haml | 8 ++++---- app/views/shared/_stream_element.html.haml | 11 ++++++----- app/views/shared/_stream_element_shim.haml | 7 +++---- db/schema.rb | 1 + features/repost.feature | 8 ++++++-- public/stylesheets/sass/application.sass | 4 ---- spec/controllers/reshares_controller_spec.rb | 4 ---- 7 files changed, 20 insertions(+), 23 deletions(-) diff --git a/app/views/posts/show.html.haml b/app/views/posts/show.html.haml index 52fab9ea3..d06204d3a 100644 --- a/app/views/posts/show.html.haml +++ b/app/views/posts/show.html.haml @@ -4,7 +4,7 @@ .span-20.append-2.prepend-2.last #main_stream.stream.status_message_show - = render 'shared/stream_element', :post => @post, :all_aspects => @post.aspects -%br -%br -%br + = render 'shared/stream_element_shim', :post => @post, :commenting_disabled => defined?(@commenting_disabled) + %br + %br + %br diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml index 6f83771c8..2e619e288 100644 --- a/app/views/shared/_stream_element.html.haml +++ b/app/views/shared/_stream_element.html.haml @@ -11,11 +11,8 @@ = link_to image_tag('deletelabel.png'), post_path(post), :confirm => t('are_you_sure'), :method => :delete, :remote => true, :class => "delete stream_element_delete", :title => t('delete') - else - .right - - if reshare - .reshare_attribution= "reshared by #{reshare.author.name}" - .controls - = link_to image_tag('deletelabel.png'), post_visibility_path(:id => "42", :post_id => post.id), :method => :put, :remote => true, :class => "delete stream_element_delete", :title => t('hide') + .right.controls + = link_to image_tag('deletelabel.png'), post_visibility_path(:id => "42", :post_id => post.id), :method => :put, :remote => true, :class => "delete stream_element_delete", :title => t('hide') .undo_text.hidden = t('post_visibilites.update.post_hidden', :name => post.author.name) = link_to t('undo'), post_visibility_path(:id => "42", :post_id => post.id), :method => :put, :remote => true, :class => "delete stream_element_delete" @@ -27,6 +24,10 @@ %span.from = person_link(post.author, :class => 'hovercardable') %time.time.timeago{:datetime => post.created_at, :integer => time_for_sort(post).to_i} + - if reshare + %span.reshared + = "reshared by" + = person_link(reshare.author, :class => "hovercardable") %span.details – %span.timeago diff --git a/app/views/shared/_stream_element_shim.haml b/app/views/shared/_stream_element_shim.haml index a5dd321c4..7a2512880 100644 --- a/app/views/shared/_stream_element_shim.haml +++ b/app/views/shared/_stream_element_shim.haml @@ -1,5 +1,4 @@ - --if post.model.is_a?(Reshare) - = render 'shared/stream_element', :post => post.root, :all_aspects => all_aspects, :commenting_disabled => commenting_disabled, :reshare => post +-if (defined?(post.model) && post.model.is_a?(Reshare)) || post.instance_of?(Reshare) + = render 'shared/stream_element', :post => post.root, :commenting_disabled => commenting_disabled, :reshare => post - else - = render 'shared/stream_element', :post => post, :all_aspects => all_aspects, :commenting_disabled => commenting_disabled, :reshare => nil + = render 'shared/stream_element', :post => post, :commenting_disabled => commenting_disabled, :reshare => nil diff --git a/db/schema.rb b/db/schema.rb index a8ddf606d..da4789c6a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -268,6 +268,7 @@ ActiveRecord::Schema.define(:version => 20110707234802) do t.string "provider_display_name" t.string "actor_url" t.integer "objectId" + t.integer "root_id" t.string "status_message_guid" t.integer "likes_count", :default => 0 end diff --git a/features/repost.feature b/features/repost.feature index a54e1e798..775557730 100644 --- a/features/repost.feature +++ b/features/repost.feature @@ -28,11 +28,14 @@ Feature: public repost Scenario: shows up on the profile page And "bob@bob.bob" has a public post with text "reshare this!" And I sign in as "alice@alice.alice" + And I preemptively confirm the alert And I follow "Reshare" And I wait for the ajax to finish + And I wait for 2 seconds + + - Then I should see a ".reshared" And I am on "alice@alice.alice"'s page Then I should see "reshare this!" Then I should see a ".reshared" @@ -45,8 +48,9 @@ Feature: public repost And I follow "Reshare" And I wait for the ajax to finish + And I go to the home page Then I should see a ".reshared" - And I follow "All Aspects" + And I follow "Your Aspects" Then I should see "reshare this!" Then I should see a ".reshared" And I should see "Bob" diff --git a/public/stylesheets/sass/application.sass b/public/stylesheets/sass/application.sass index d3d462a10..b02d1881e 100644 --- a/public/stylesheets/sass/application.sass +++ b/public/stylesheets/sass/application.sass @@ -376,10 +376,6 @@ ul.as-selections &:hover :text :decoration none -.reshare_attribution - :font-size smaller -.reshared - :background-color #eee .stream_element :position relative diff --git a/spec/controllers/reshares_controller_spec.rb b/spec/controllers/reshares_controller_spec.rb index 9a780a905..23508cf86 100644 --- a/spec/controllers/reshares_controller_spec.rb +++ b/spec/controllers/reshares_controller_spec.rb @@ -2,16 +2,13 @@ require 'spec_helper' describe ResharesController do describe '#create' do - it 'requires authentication' do post :create, :format => :js response.should_not be_success end context 'with an authenticated user' do - before do - sign_in :user, bob @post_id = Factory(:status_message, :public => true).id @controller.stub(:current_user).and_return(bob) @@ -19,7 +16,6 @@ describe ResharesController do it 'succeeds' do post :create, :format => :js, :root_id => @post_id - puts response.code response.should be_success end From f3a515eef165095b79de85418e33eb968e65e59a Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Tue, 12 Jul 2011 19:03:35 -0700 Subject: [PATCH 05/28] DG IZ reshare retractions is WIP --- app/controllers/reshares_controller.rb | 1 + app/helpers/stream_helper.rb | 4 ++ app/models/post.rb | 3 ++ app/models/reshare.rb | 2 - app/models/user.rb | 3 +- app/views/reshares/_reshare.haml | 25 +++++++++ app/views/shared/_stream.haml | 2 +- app/views/shared/_stream_element.html.haml | 15 +++--- app/views/shared/_stream_element_shim.haml | 4 -- features/repost.feature | 52 +++++++++++++++++-- features/step_definitions/custom_web_steps.rb | 4 ++ features/step_definitions/posts_steps.rb | 3 ++ lib/postzord/dispatch.rb | 5 +- public/stylesheets/sass/application.sass | 30 ++++++----- spec/controllers/reshares_controller_spec.rb | 5 ++ spec/lib/postzord/dispatch_spec.rb | 18 +++++-- spec/models/reshare_spec.rb | 16 ++++++ 17 files changed, 153 insertions(+), 39 deletions(-) create mode 100644 app/views/reshares/_reshare.haml delete mode 100644 app/views/shared/_stream_element_shim.haml diff --git a/app/controllers/reshares_controller.rb b/app/controllers/reshares_controller.rb index d28a0c0a5..9520715c7 100644 --- a/app/controllers/reshares_controller.rb +++ b/app/controllers/reshares_controller.rb @@ -6,6 +6,7 @@ class ResharesController < ApplicationController @reshare = current_user.build_post(:reshare, :root_id => params[:root_id]) if @reshare.save! current_user.add_to_streams(@reshare, current_user.aspects) + current_user.dispatch_post(@reshare, :url => post_url(@reshare), :additional_subscribers => @reshare.root.author) end respond_with @reshare diff --git a/app/helpers/stream_helper.rb b/app/helpers/stream_helper.rb index 566a92d54..55d006cd6 100644 --- a/app/helpers/stream_helper.rb +++ b/app/helpers/stream_helper.rb @@ -28,4 +28,8 @@ module StreamHelper def comments_expanded false end + + def reshare?(post) + (defined?(post.model) && post.model.is_a?(Reshare)) || post.instance_of?(Reshare) + end end diff --git a/app/models/post.rb b/app/models/post.rb index 45dec7028..0b43778ce 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -24,6 +24,9 @@ class Post < ActiveRecord::Base has_many :contacts, :through => :post_visibilities has_many :mentions, :dependent => :destroy + has_many :reshares, :class_name => "Reshare" + has_many :resharers, :through => :reshares, :foreign_key => :root_id, :source => :author + belongs_to :author, :class_name => 'Person' def diaspora_handle diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 05f29367f..9fffce176 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -7,8 +7,6 @@ class Reshare < Post self.public = true end - - delegate :photos, :text, :comments, :to => :root private def root_must_be_public diff --git a/app/models/user.rb b/app/models/user.rb index e6df4fe04..23621fed4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -136,7 +136,8 @@ class User < ActiveRecord::Base end def dispatch_post(post, opts = {}) - mailman = Postzord::Dispatch.new(self, post) + additional_people = opts.delete(:additional_subscribers) + mailman = Postzord::Dispatch.new(self, post, :additional_subscribers => additional_people) mailman.post(opts) end diff --git a/app/views/reshares/_reshare.haml b/app/views/reshares/_reshare.haml new file mode 100644 index 000000000..456b3555c --- /dev/null +++ b/app/views/reshares/_reshare.haml @@ -0,0 +1,25 @@ +-# Copyright (c) 2010, Diaspora Inc. This file is +-# licensed under the Affero General Public License version 3 or later. See +-# the COPYRIGHT file. + + +.reshare + - if post + = person_image_link(post.author, :size => :thumb_small) + + .content + .right + = link_to "Show Original", post_path(post) + %span.from + = person_link(post.author, :class => "hovercardable") + + - if post.activity_streams? + = link_to image_tag(post.image_url, 'data-small-photo' => post.image_url, 'data-full-photo' => post.image_url, :class => 'stream-photo'), post.object_url, :class => "stream-photo-link" + - else + = render 'status_messages/status_message', :post => post, :photos => post.photos + - if (post.author_id != current_user.person.id) && (post.public?) && !reshare?(post) + %span.reshare_action + = link_to "Reshare", reshares_path(:root_id => post.id), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" + + - else + Original post deleted by author diff --git a/app/views/shared/_stream.haml b/app/views/shared/_stream.haml index 1d79fb636..95ee72a0f 100644 --- a/app/views/shared/_stream.haml +++ b/app/views/shared/_stream.haml @@ -3,7 +3,7 @@ -# the COPYRIGHT file. -= render :partial => 'shared/stream_element_shim', += render :partial => 'shared/stream_element', :collection => posts, :as => :post, :locals => { :commenting_disabled => defined?(@commenting_disabled)} diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml index 2e619e288..a4a1f735f 100644 --- a/app/views/shared/_stream_element.html.haml +++ b/app/views/shared/_stream_element.html.haml @@ -4,15 +4,13 @@ .stream_element{:id => post.guid} - if current_user && post.author.owner_id == current_user.id - - if reshare - .reshare_attribution - = "reshared by #{reshare.author.name}" .right.controls = link_to image_tag('deletelabel.png'), post_path(post), :confirm => t('are_you_sure'), :method => :delete, :remote => true, :class => "delete stream_element_delete", :title => t('delete') - else .right.controls - = link_to image_tag('deletelabel.png'), post_visibility_path(:id => "42", :post_id => post.id), :method => :put, :remote => true, :class => "delete stream_element_delete", :title => t('hide') + = link_to image_tag('deletelabel.png'), post_path(post), :confirm => t('are_you_sure'), :method => :delete, :remote => true, :class => "delete stream_element_delete", :title => t('delete') + .undo_text.hidden = t('post_visibilites.update.post_hidden', :name => post.author.name) = link_to t('undo'), post_visibility_path(:id => "42", :post_id => post.id), :method => :put, :remote => true, :class => "delete stream_element_delete" @@ -24,10 +22,7 @@ %span.from = person_link(post.author, :class => 'hovercardable') %time.time.timeago{:datetime => post.created_at, :integer => time_for_sort(post).to_i} - - if reshare - %span.reshared - = "reshared by" - = person_link(reshare.author, :class => "hovercardable") + %span.details – %span.timeago @@ -35,6 +30,8 @@ - if post.activity_streams? = link_to image_tag(post.image_url, 'data-small-photo' => post.image_url, 'data-full-photo' => post.image_url, :class => 'stream-photo'), post.object_url, :class => "stream-photo-link" + - elsif reshare?(post) + = render 'reshares/reshare', :reshare => post, :post => post.root - else = render 'status_messages/status_message', :post => post, :photos => post.photos @@ -61,7 +58,7 @@ %span.like_action = like_action(post, current_user) - - unless(post.author_id == current_user.person.id) || (!post.public?) + - if (post.author_id != current_user.person.id) && (post.public?) && !reshare?(post) · %span.reshare_action = link_to "Reshare", reshares_path(:root_id => post.id), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" diff --git a/app/views/shared/_stream_element_shim.haml b/app/views/shared/_stream_element_shim.haml deleted file mode 100644 index 7a2512880..000000000 --- a/app/views/shared/_stream_element_shim.haml +++ /dev/null @@ -1,4 +0,0 @@ --if (defined?(post.model) && post.model.is_a?(Reshare)) || post.instance_of?(Reshare) - = render 'shared/stream_element', :post => post.root, :commenting_disabled => commenting_disabled, :reshare => post -- else - = render 'shared/stream_element', :post => post, :commenting_disabled => commenting_disabled, :reshare => nil diff --git a/features/repost.feature b/features/repost.feature index 775557730..4dd29df78 100644 --- a/features/repost.feature +++ b/features/repost.feature @@ -34,11 +34,9 @@ Feature: public repost And I wait for the ajax to finish And I wait for 2 seconds - - And I am on "alice@alice.alice"'s page Then I should see "reshare this!" - Then I should see a ".reshared" + Then I should see a ".reshare" And I should see "Bob" Scenario: shows up on the aspects page @@ -49,9 +47,53 @@ Feature: public repost And I wait for the ajax to finish And I go to the home page - Then I should see a ".reshared" + Then I should see a ".reshare" And I follow "Your Aspects" Then I should see "reshare this!" - Then I should see a ".reshared" + Then I should see a ".reshare" And I should see "Bob" + Scenario: can be retracted + And "bob@bob.bob" has a public post with text "reshare this!" + And I sign in as "alice@alice.alice" + And I preemptively confirm the alert + And I follow "Reshare" + And I wait for the ajax to finish + + And I go to the home page + Then I should see a ".reshare" + And I follow "Your Aspects" + Then I should see "reshare this!" + Then I should see a ".reshare" + And I should see "Bob" + + And I go to the destroy user session page + And I sign in as "bob@bob.bob" + + And The user deletes their first post + + And I go to the destroy user session page + And I sign in as "alice@alice.alice" + + And I go to the home page + Then I should see "Original post deleted by author" + + Scenario: Keeps track of the number of reshares + And "bob@bob.bob" has a public post with text "reshare this!" + And I sign in as "alice@alice.alice" + And I preemptively confirm the alert + And I follow "Reshare" + And I wait for the ajax to finish + + And I go to the home page + Then I should see a ".reshare" + And I follow "Your Aspects" + Then I should see "reshare this!" + Then I should see a ".reshare" + And I should see "Bob" + + And I go to the destroy user session page + And I sign in as "bob@bob.bob" + And I should see "1 Reshare" + + Scenario: Can have text diff --git a/features/step_definitions/custom_web_steps.rb b/features/step_definitions/custom_web_steps.rb index 63c0e9b34..8233da35e 100644 --- a/features/step_definitions/custom_web_steps.rb +++ b/features/step_definitions/custom_web_steps.rb @@ -44,6 +44,10 @@ When /^I click to delete the first post$/ do page.execute_script('$(".stream_element").first().find(".stream_element_delete").first().click()') end +When /^I click to delete the ([\d])(nd|rd|st|th) post$/ do |number, stuff| + page.execute_script('$(".stream_element:nth-child('+ number +'").first().find(".stream_element_delete").first().click()') +end + When /^I click to delete the first comment$/ do page.execute_script('$(".comment.posted").first().find(".comment_delete").click()') end diff --git a/features/step_definitions/posts_steps.rb b/features/step_definitions/posts_steps.rb index d9879e72a..94bf863df 100644 --- a/features/step_definitions/posts_steps.rb +++ b/features/step_definitions/posts_steps.rb @@ -19,3 +19,6 @@ Given /^"([^"]*)" has a non public post with text "([^"]*)"$/ do |email, text| user.post(:status_message, :text => text, :public => false, :to => user.aspects) end +When /^The user deletes their first post$/ do + @me.posts.first.destroy +end diff --git a/lib/postzord/dispatch.rb b/lib/postzord/dispatch.rb index 75ba6db1f..cb99b3635 100644 --- a/lib/postzord/dispatch.rb +++ b/lib/postzord/dispatch.rb @@ -3,7 +3,9 @@ # the COPYRIGHT file. class Postzord::Dispatch - def initialize(user, object) + + # @note Takes :additional_subscribers param to add to subscribers to dispatch to + def initialize(user, object, opts={}) unless object.respond_to? :to_diaspora_xml raise 'this object does not respond_to? to_diaspora xml. try including Diaspora::Webhooks into your object' end @@ -12,6 +14,7 @@ class Postzord::Dispatch @object = object @xml = @object.to_diaspora_xml @subscribers = @object.subscribers(@sender) + @subscribers = @subscribers | [*opts[:additional_subscribers]] if opts[:additional_subscribers] end def salmon diff --git a/public/stylesheets/sass/application.sass b/public/stylesheets/sass/application.sass index b02d1881e..5a4b4ee1e 100644 --- a/public/stylesheets/sass/application.sass +++ b/public/stylesheets/sass/application.sass @@ -475,6 +475,7 @@ ul.as-selections .from a :color $blue + .status_message_show .stream_element .content @@ -554,7 +555,8 @@ ul.as-selections ul.comments, ul.show_comments, -.likes_container +.likes_container, +.stream_element .reshare .avatar :width 30px @@ -589,18 +591,18 @@ ul.show_comments, :height 250px :width 400px - .content - :margin - :top 0px - :bottom -2px - :padding - :left 36px - :right 10px + .content + :margin + :top 0px + :bottom -2px + :padding + :left 36px + :right 10px - p - :margin - :bottom 0 - :top 0 + p + :margin + :bottom 0 + :top 0 .right :right 4px @@ -609,6 +611,10 @@ ul.show_comments, :position absolute :display inline +.stream_element .reshare + :padding 10px + :border 1px solid #eee + ul.show_comments :padding :bottom 6px diff --git a/spec/controllers/reshares_controller_spec.rb b/spec/controllers/reshares_controller_spec.rb index 23508cf86..5dbaaab7a 100644 --- a/spec/controllers/reshares_controller_spec.rb +++ b/spec/controllers/reshares_controller_spec.rb @@ -29,6 +29,11 @@ describe ResharesController do bob.should_receive(:add_to_streams) post :create, :format => :js, :root_id => @post_id end + + it 'calls dispatch' do + bob.should_receive(:dispatch_post).with(anything, hash_including(:additional_subscribers)) + post :create, :format => :js, :root_id => @post_id + end end end end diff --git a/spec/lib/postzord/dispatch_spec.rb b/spec/lib/postzord/dispatch_spec.rb index 6991e8b93..da8181c54 100644 --- a/spec/lib/postzord/dispatch_spec.rb +++ b/spec/lib/postzord/dispatch_spec.rb @@ -23,10 +23,20 @@ describe Postzord::Dispatch do zord.instance_variable_get(:@object).should == @sm end - it 'sets @subscribers from object' do - @sm.should_receive(:subscribers).and_return(@subscribers) - zord = Postzord::Dispatch.new(alice, @sm) - zord.instance_variable_get(:@subscribers).should == @subscribers + context 'setting @subscribers' do + it 'sets @subscribers from object' do + @sm.should_receive(:subscribers).and_return(@subscribers) + zord = Postzord::Dispatch.new(alice, @sm) + zord.instance_variable_get(:@subscribers).should == @subscribers + end + + it 'accepts additional subscribers from opts' do + new_person = Factory(:person) + + @sm.should_receive(:subscribers).and_return(@subscribers) + zord = Postzord::Dispatch.new(alice, @sm, :additional_subscribers => new_person) + zord.instance_variable_get(:@subscribers).should == @subscribers | [new_person] + end end it 'sets the @sender_person object' do diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb index 7fcd620ae..cd4a87bc5 100644 --- a/spec/models/reshare_spec.rb +++ b/spec/models/reshare_spec.rb @@ -17,4 +17,20 @@ describe Reshare do it 'forces public' do Factory(:reshare, :public => false).public.should be_true end + + describe "#receive" do + before do + @reshare = Factory.build(:reshare, :root => Factory.build(:status_message, :public => false)) + @root = @reshare.root + @reshare.receive(@root.author.owner, @reshare.author) + end + + it 'increments the reshare count' do + @root.resharers.count.should == 1 + end + + it 'adds the resharer to the re-sharers of the post' do + @root.resharers.should include(@reshare.author) + end + end end From 7b3180e5da4a6dab67e543f237b3a9b6de1bc72f Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Wed, 13 Jul 2011 13:37:36 -0700 Subject: [PATCH 06/28] user can retract a reshared post --- app/models/post.rb | 4 +-- app/models/reshare.rb | 14 ++++++++++ app/models/user.rb | 7 ++++- app/views/posts/show.html.haml | 2 +- app/views/reshares/_reshare.haml | 2 +- spec/models/reshare_spec.rb | 2 +- spec/models/user_spec.rb | 45 ++++++++++++++++++++++++++++++++ 7 files changed, 70 insertions(+), 6 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 0b43778ce..01da0e94b 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -24,8 +24,8 @@ class Post < ActiveRecord::Base has_many :contacts, :through => :post_visibilities has_many :mentions, :dependent => :destroy - has_many :reshares, :class_name => "Reshare" - has_many :resharers, :through => :reshares, :foreign_key => :root_id, :source => :author + has_many :reshares, :class_name => "Reshare", :foreign_key => :root_id + has_many :resharers, :class_name => 'Person', :through => :reshares, :source => :author belongs_to :author, :class_name => 'Person' diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 9fffce176..08853dee9 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -7,6 +7,20 @@ class Reshare < Post self.public = true end + def receive(user, person) + local_reshare = Reshare.where(:guid => self.guid).first + if local_reshare.root.author_id == user.person.id + local_reshare.root.reshares << local_reshare + + if user.contact_for(person) + local_reshare.receive(user, person) + end + + else + super(user, person) + end + end + private def root_must_be_public diff --git a/app/models/user.rb b/app/models/user.rb index 23621fed4..a1566f4d3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -237,7 +237,12 @@ class User < ActiveRecord::Base retraction = Retraction.for(target) end - mailman = Postzord::Dispatch.new(self, retraction) + opts = {} + if target.is_a?(Post) + opts[:additional_subscribers] = target.resharers + end + + mailman = Postzord::Dispatch.new(self, retraction, opts) mailman.post retraction.perform(self) diff --git a/app/views/posts/show.html.haml b/app/views/posts/show.html.haml index d06204d3a..716a1fd64 100644 --- a/app/views/posts/show.html.haml +++ b/app/views/posts/show.html.haml @@ -4,7 +4,7 @@ .span-20.append-2.prepend-2.last #main_stream.stream.status_message_show - = render 'shared/stream_element_shim', :post => @post, :commenting_disabled => defined?(@commenting_disabled) + = render 'shared/stream_element', :post => @post, :commenting_disabled => defined?(@commenting_disabled) %br %br %br diff --git a/app/views/reshares/_reshare.haml b/app/views/reshares/_reshare.haml index 456b3555c..2a71f683a 100644 --- a/app/views/reshares/_reshare.haml +++ b/app/views/reshares/_reshare.haml @@ -17,7 +17,7 @@ = link_to image_tag(post.image_url, 'data-small-photo' => post.image_url, 'data-full-photo' => post.image_url, :class => 'stream-photo'), post.object_url, :class => "stream-photo-link" - else = render 'status_messages/status_message', :post => post, :photos => post.photos - - if (post.author_id != current_user.person.id) && (post.public?) && !reshare?(post) + - if defined?(current_user) && current_user && (post.author_id != current_user.person.id) && (post.public?) && !reshare?(post) %span.reshare_action = link_to "Reshare", reshares_path(:root_id => post.id), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb index cd4a87bc5..523f6cc78 100644 --- a/spec/models/reshare_spec.rb +++ b/spec/models/reshare_spec.rb @@ -20,7 +20,7 @@ describe Reshare do describe "#receive" do before do - @reshare = Factory.build(:reshare, :root => Factory.build(:status_message, :public => false)) + @reshare = Factory.create(:reshare, :root => Factory(:status_message, :author => bob.person, :public => true)) @root = @reshare.root @reshare.receive(@root.author.owner, @reshare.author) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b34d92e3d..7cc3944c4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -752,4 +752,49 @@ describe User do end end end + + describe '#retract' do + before do + @retraction = mock + end + + context "regular retractions" do + before do + Retraction.stub(:for).and_return(@retraction) + @retraction.stub(:perform) + + @post = Factory(:status_message, :author => bob.person, :public => true) + end + + it 'sends a retraction' do + dispatcher = mock + Postzord::Dispatch.should_receive(:new).with(bob, @retraction, anything()).and_return(dispatcher) + dispatcher.should_receive(:post) + + bob.retract(@post) + end + + it 'adds resharers of target post as additional subsctibers' do + person = Factory(:person) + reshare = Factory(:reshare, :root => @post, :author => person) + @post.reshares << reshare + + dispatcher = mock + Postzord::Dispatch.should_receive(:new).with(bob, @retraction, {:additional_subscribers => [person]}).and_return(dispatcher) + dispatcher.should_receive(:post) + + bob.retract(@post) + end + + it 'performs the retraction' do + + end + end + + context "relayable retractions" do + it 'sends a relayable retraction if the object is relayable' do + + end + end + end end From 509a435cc9c6b608923b3a03347584bda76be5e7 Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Wed, 13 Jul 2011 19:40:08 -0700 Subject: [PATCH 07/28] moving to not including the post in the reshare xml --- app/controllers/publics_controller.rb | 2 +- app/models/reshare.rb | 22 +++++++++ config/routes.rb | 2 +- spec/models/reshare_spec.rb | 67 +++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 2 deletions(-) diff --git a/app/controllers/publics_controller.rb b/app/controllers/publics_controller.rb index dc1a338fb..1b59bd1ef 100644 --- a/app/controllers/publics_controller.rb +++ b/app/controllers/publics_controller.rb @@ -66,7 +66,7 @@ class PublicsController < ApplicationController end def post - @post = Post.where(:id => params[:id], :public => true).includes(:author, :comments => :author).first + @post = Post.where(:guid => params[:guid], :public => true).includes(:author, :comments => :author).first #hax to upgrade logged in users who can comment if @post diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 08853dee9..fb1c59542 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -3,10 +3,32 @@ class Reshare < Post validate :root_must_be_public attr_accessible :root_id, :public + xml_attr :root_diaspora_id + xml_attr :root_guid + + attr_accessible :root_diaspora_id, :root_guid + before_validation do self.public = true end + def root_guid + self.root.guid + end + def root_guid= rg + #self.root = Post.where(:guid => rg).first + debugger + person = Person.where(:diaspora_handle => self[:root_diaspora_id]).first + Faraday.get(person.url + public_post_path(:guid => rg)) + end + + def root_diaspora_id + self.root.author.diaspora_handle + end + def root_diaspora_id= id + Webfinger.new(id).fetch + end + def receive(user, person) local_reshare = Reshare.where(:guid => self.guid).first if local_reshare.root.author_id == user.person.id diff --git a/config/routes.rb b/config/routes.rb index dca33bae5..42ff03d3f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,7 +18,7 @@ Diaspora::Application.routes.draw do resources :likes, :only => [:create, :destroy, :index] resources :comments, :only => [:create, :destroy, :index] end - get 'p/:id' => 'publics#post', :as => 'public_post' + get 'p/:guid' => 'publics#post', :as => 'public_post' # roll up likes into a nested resource above resources :comments, :only => [:create, :destroy] do diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb index 523f6cc78..10b93d261 100644 --- a/spec/models/reshare_spec.rb +++ b/spec/models/reshare_spec.rb @@ -1,6 +1,13 @@ require 'spec_helper' describe Reshare do + include ActionView::Helpers::UrlHelper + include Rails.application.routes.url_helpers + def controller + mock() + end + + it 'has a valid Factory' do Factory(:reshare).should be_valid end @@ -33,4 +40,64 @@ describe Reshare do @root.resharers.should include(@reshare.author) end end + + describe "XML" do + before do + @reshare = Factory(:reshare) + @xml = @reshare.to_xml.to_s + pp @xml + end + + context 'serialization' do + it 'serializes root_diaspora_id' do + @xml.should include("root_diaspora_id") + end + + it 'serializes root_guid' do + @xml.should include("root_guid") + end + end + + context 'marshalling' do + context 'local' do + before do + @original_author = @reshare.root.author.dup + @root_object = @reshare.root.dup + end + + it 'fetches the root post from root_guid' do + Reshare.from_xml(@xml).root.should == @root_object + end + + it 'fetches the root author from root_diaspora_id' do + Reshare.from_xml(@xml).root.author.should == @original_author + end + end + + context 'remote' do + before do + @original_profile = @reshare.root.author.profile + @original_author = @reshare.root.author.delete + @root_object = @reshare.root.delete + end + + it 'fetches the root post from root_guid' do + @original_profile.save! + pp @original_profile + @original_author.save! + pp @original_author + Faraday.should_receive(:get).with(@original_author.url + public_post_path(:guid => @reshare.guid)).and_return(@root_object.to_diaspora_xml) + Reshare.from_xml(@xml).root.should == @root_object + end + + it 'fetches the root author from root_diaspora_id' do + person = Factory.build(:person) + wf_prof_mock = mock + wf_prof_mock.should_receive(:fetch).and_return(person) + Webfinger.should_receive(:new).and_return(wf_prof_mock) + Reshare.from_xml(@xml) + end + end + end + end end From bdc4b9f746eca12b50b5d37fd329eeb0bfa75cc5 Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Thu, 14 Jul 2011 15:32:04 -0700 Subject: [PATCH 08/28] marsharing reshares now grabs the post from the original poster, also fetcheer that person if they don't exist locally still need to ping for xml and give the right xml back --- app/controllers/publics_controller.rb | 7 ++++- app/models/reshare.rb | 23 ++++++++-------- app/views/shared/_stream_element.html.haml | 2 +- features/posts.feature | 1 + spec/controllers/publics_controller_spec.rb | 27 ++++++++++++++++--- spec/models/reshare_spec.rb | 30 +++++++++++++-------- 6 files changed, 63 insertions(+), 27 deletions(-) diff --git a/app/controllers/publics_controller.rb b/app/controllers/publics_controller.rb index 1b59bd1ef..c58b7f917 100644 --- a/app/controllers/publics_controller.rb +++ b/app/controllers/publics_controller.rb @@ -66,7 +66,12 @@ class PublicsController < ApplicationController end def post - @post = Post.where(:guid => params[:guid], :public => true).includes(:author, :comments => :author).first + + if params[:guid].to_s.length <= 8 + @post = Post.where(:id => params[:guid], :public => true).includes(:author, :comments => :author).first + else + @post = Post.where(:guid => params[:guid], :public => true).includes(:author, :comments => :author).first + end #hax to upgrade logged in users who can comment if @post diff --git a/app/models/reshare.rb b/app/models/reshare.rb index fb1c59542..8eb0303d7 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -1,4 +1,5 @@ class Reshare < Post + belongs_to :root, :class_name => 'Post' validate :root_must_be_public attr_accessible :root_id, :public @@ -6,8 +7,6 @@ class Reshare < Post xml_attr :root_diaspora_id xml_attr :root_guid - attr_accessible :root_diaspora_id, :root_guid - before_validation do self.public = true end @@ -15,19 +14,10 @@ class Reshare < Post def root_guid self.root.guid end - def root_guid= rg - #self.root = Post.where(:guid => rg).first - debugger - person = Person.where(:diaspora_handle => self[:root_diaspora_id]).first - Faraday.get(person.url + public_post_path(:guid => rg)) - end def root_diaspora_id self.root.author.diaspora_handle end - def root_diaspora_id= id - Webfinger.new(id).fetch - end def receive(user, person) local_reshare = Reshare.where(:guid => self.guid).first @@ -45,6 +35,17 @@ class Reshare < Post private + def after_parse + root_author = Webfinger.new(@root_diaspora_id).fetch + root_author.save! + + unless self.root = Post.where(:guid => @root_guid).first + self.root = Diaspora::Parser.from_xml(Faraday.get(root_author.url + "/p/#{@root_guid}").body) + self.root.save! + end + + end + def root_must_be_public if self.root.nil? || !self.root.public errors[:base] << "you must reshare public posts" diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml index a4a1f735f..0a4a3ef13 100644 --- a/app/views/shared/_stream_element.html.haml +++ b/app/views/shared/_stream_element.html.haml @@ -9,7 +9,7 @@ - else .right.controls - = link_to image_tag('deletelabel.png'), post_path(post), :confirm => t('are_you_sure'), :method => :delete, :remote => true, :class => "delete stream_element_delete", :title => t('delete') + = link_to image_tag('deletelabel.png'), post_visibility_path(:id => "42", :post_id => post.id), :method => :put, :remote => true, :class => "delete stream_element_delete", :title => t('hide') .undo_text.hidden = t('post_visibilites.update.post_hidden', :name => post.author.name) diff --git a/features/posts.feature b/features/posts.feature index 74b9f024e..ed6b226c7 100644 --- a/features/posts.feature +++ b/features/posts.feature @@ -64,6 +64,7 @@ Feature: posting And I am on "bob@bob.bob"'s page And I hover over the ".stream_element" + And I preemptively confirm the alert And I click to delete the first post And I wait for the ajax to finish And I go to "bob@bob.bob"'s page diff --git a/spec/controllers/publics_controller_spec.rb b/spec/controllers/publics_controller_spec.rb index 0e4b6d465..8f55400fc 100644 --- a/spec/controllers/publics_controller_spec.rb +++ b/spec/controllers/publics_controller_spec.rb @@ -65,22 +65,43 @@ describe PublicsController do it 'shows a public post' do status = alice.post(:status_message, :text => "hello", :public => true, :to => 'all') - get :post, :id => status.id + get :post, :guid => status.id response.status= 200 end it 'does not show a private post' do status = alice.post(:status_message, :text => "hello", :public => false, :to => 'all') - get :post, :id => status.id + get :post, :guid => status.id response.status = 302 end it 'redirects to the proper show page if the user has visibility of the post' do status = alice.post(:status_message, :text => "hello", :public => true, :to => 'all') sign_in bob - get :post, :id => status.id + get :post, :guid => status.id response.should be_redirect end + + # We want to be using guids from now on for this post route, but do not want to break + # preexisiting permalinks. We can assume a guid is 8 characters long as we have + # guids set to hex(8) since we started using them. + context 'id/guid switch' do + before do + @status = alice.post(:status_message, :text => "hello", :public => true, :to => 'all') + end + + it 'assumes guids less than 8 chars are ids and not guids' do + Post.should_receive(:where).with(hash_including(:id => @status.id)).and_return(Post) + get :post, :guid => @status.id + response.status= 200 + end + + it 'assumes guids more than (or equal to) 8 chars are actually guids' do + Post.should_receive(:where).with(hash_including(:guid => @status.guid)).and_return(Post) + get :post, :guid => @status.guid + response.status= 200 + end + end end describe '#hcard' do diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb index 10b93d261..c84890052 100644 --- a/spec/models/reshare_spec.rb +++ b/spec/models/reshare_spec.rb @@ -45,7 +45,6 @@ describe Reshare do before do @reshare = Factory(:reshare) @xml = @reshare.to_xml.to_s - pp @xml end context 'serialization' do @@ -76,25 +75,34 @@ describe Reshare do context 'remote' do before do - @original_profile = @reshare.root.author.profile - @original_author = @reshare.root.author.delete @root_object = @reshare.root.delete end it 'fetches the root post from root_guid' do - @original_profile.save! - pp @original_profile - @original_author.save! - pp @original_author - Faraday.should_receive(:get).with(@original_author.url + public_post_path(:guid => @reshare.guid)).and_return(@root_object.to_diaspora_xml) - Reshare.from_xml(@xml).root.should == @root_object + response = mock + response.stub(:body).and_return(@root_object.to_diaspora_xml) + Faraday.default_connection.should_receive(:get).with(@reshare.root.author.url + public_post_path(:guid => @root_object.guid)).and_return(response) + + root = Reshare.from_xml(@xml).root + + [:text, :guid, :diaspora_handle, :type].each do |attr| + root.send(attr).should == @reshare.root.send(attr) + end end it 'fetches the root author from root_diaspora_id' do - person = Factory.build(:person) + @original_profile = @reshare.root.author.profile + @original_author = @reshare.root.author.delete + wf_prof_mock = mock - wf_prof_mock.should_receive(:fetch).and_return(person) + wf_prof_mock.should_receive(:fetch).and_return(@original_author) Webfinger.should_receive(:new).and_return(wf_prof_mock) + + response = mock + response.stub(:body).and_return(@root_object.to_diaspora_xml) + + Faraday.default_connection.should_receive(:get).with(@original_author.url + public_post_path(:guid => @root_object.guid)).and_return(response) + Reshare.from_xml(@xml) end end From a32ef3c1d803f09b82f91a46bc9ca69091df718b Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Thu, 14 Jul 2011 15:56:55 -0700 Subject: [PATCH 09/28] added xml response to the post --- app/controllers/publics_controller.rb | 26 +++++++++++++-------- app/models/reshare.rb | 2 +- spec/controllers/publics_controller_spec.rb | 6 +++++ spec/models/reshare_spec.rb | 4 ++-- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/app/controllers/publics_controller.rb b/app/controllers/publics_controller.rb index c58b7f917..e0e3941e2 100644 --- a/app/controllers/publics_controller.rb +++ b/app/controllers/publics_controller.rb @@ -12,6 +12,9 @@ class PublicsController < ApplicationController skip_before_filter :set_grammatical_gender before_filter :allow_cross_origin, :only => [:hcard, :host_meta, :webfinger] + respond_to :html + respond_to :xml, :only => :post + def allow_cross_origin headers["Access-Control-Allow-Origin"] = "*" end @@ -77,17 +80,20 @@ class PublicsController < ApplicationController if @post if user_signed_in? && current_user.find_visible_post_by_id(@post.id) redirect_to post_path(@post) - return - end - - @landing_page = true - @person = @post.author - if @person.owner_id - I18n.locale = @person.owner.language - render "#{@post.class.to_s.underscore}", :layout => 'application' else - flash[:error] = I18n.t('posts.show.not_found') - redirect_to root_url + @landing_page = true + @person = @post.author + if @person.owner_id + I18n.locale = @person.owner.language + + respond_to do |format| + format.all{ render "#{@post.class.to_s.underscore}", :layout => 'application'} + format.xml{ render :xml => @post.to_diaspora_xml } + end + else + flash[:error] = I18n.t('posts.show.not_found') + redirect_to root_url + end end else flash[:error] = I18n.t('posts.show.not_found') diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 8eb0303d7..9b80ba533 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -40,7 +40,7 @@ class Reshare < Post root_author.save! unless self.root = Post.where(:guid => @root_guid).first - self.root = Diaspora::Parser.from_xml(Faraday.get(root_author.url + "/p/#{@root_guid}").body) + self.root = Diaspora::Parser.from_xml(Faraday.get(root_author.url + "/p/#{@root_guid}.xml").body) self.root.save! end diff --git a/spec/controllers/publics_controller_spec.rb b/spec/controllers/publics_controller_spec.rb index 8f55400fc..b99c0be69 100644 --- a/spec/controllers/publics_controller_spec.rb +++ b/spec/controllers/publics_controller_spec.rb @@ -82,6 +82,12 @@ describe PublicsController do response.should be_redirect end + it 'responds with diaspora xml if format is xml' do + status = alice.post(:status_message, :text => "hello", :public => true, :to => 'all') + get :post, :guid => status.guid, :format => :xml + response.body.should == status.to_diaspora_xml + end + # We want to be using guids from now on for this post route, but do not want to break # preexisiting permalinks. We can assume a guid is 8 characters long as we have # guids set to hex(8) since we started using them. diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb index c84890052..f029d5b2d 100644 --- a/spec/models/reshare_spec.rb +++ b/spec/models/reshare_spec.rb @@ -81,7 +81,7 @@ describe Reshare do it 'fetches the root post from root_guid' do response = mock response.stub(:body).and_return(@root_object.to_diaspora_xml) - Faraday.default_connection.should_receive(:get).with(@reshare.root.author.url + public_post_path(:guid => @root_object.guid)).and_return(response) + Faraday.default_connection.should_receive(:get).with(@reshare.root.author.url + public_post_path(:guid => @root_object.guid, :format => "xml")).and_return(response) root = Reshare.from_xml(@xml).root @@ -101,7 +101,7 @@ describe Reshare do response = mock response.stub(:body).and_return(@root_object.to_diaspora_xml) - Faraday.default_connection.should_receive(:get).with(@original_author.url + public_post_path(:guid => @root_object.guid)).and_return(response) + Faraday.default_connection.should_receive(:get).with(@original_author.url + public_post_path(:guid => @root_object.guid, :format => "xml")).and_return(response) Reshare.from_xml(@xml) end From a8400ad4607e518ecf1b9a1d416956f2bd455317 Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Thu, 14 Jul 2011 19:08:03 -0700 Subject: [PATCH 10/28] 2 more specs failing. wip --- app/models/reshare.rb | 14 ++++++++---- spec/models/reshare_spec.rb | 43 ++++++++++++++++++++++++------------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 9b80ba533..716780a7d 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -39,11 +39,17 @@ class Reshare < Post root_author = Webfinger.new(@root_diaspora_id).fetch root_author.save! - unless self.root = Post.where(:guid => @root_guid).first - self.root = Diaspora::Parser.from_xml(Faraday.get(root_author.url + "/p/#{@root_guid}.xml").body) - self.root.save! + local_post = Post.where(:guid => @root_guid).select('id').first + + unless local_post && self.root_id = local_post.id + received_post = Diaspora::Parser.from_xml(Faraday.get(root_author.url + "/p/#{@root_guid}.xml").body) + unless post = received_post.class.where(:guid => received_post.guid).first + post = received_post + post.save + end + + self.root_id = post.id end - end def root_must_be_public diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb index f029d5b2d..c3b02d9ed 100644 --- a/spec/models/reshare_spec.rb +++ b/spec/models/reshare_spec.rb @@ -78,21 +78,11 @@ describe Reshare do @root_object = @reshare.root.delete end - it 'fetches the root post from root_guid' do - response = mock - response.stub(:body).and_return(@root_object.to_diaspora_xml) - Faraday.default_connection.should_receive(:get).with(@reshare.root.author.url + public_post_path(:guid => @root_object.guid, :format => "xml")).and_return(response) - - root = Reshare.from_xml(@xml).root - - [:text, :guid, :diaspora_handle, :type].each do |attr| - root.send(attr).should == @reshare.root.send(attr) - end - end - it 'fetches the root author from root_diaspora_id' do - @original_profile = @reshare.root.author.profile - @original_author = @reshare.root.author.delete + @original_profile = @reshare.root.author.profile.dup + @reshare.root.author.profile.delete + @original_author = @reshare.root.author.dup + @reshare.root.author.delete wf_prof_mock = mock wf_prof_mock.should_receive(:fetch).and_return(@original_author) @@ -102,9 +92,32 @@ describe Reshare do response.stub(:body).and_return(@root_object.to_diaspora_xml) Faraday.default_connection.should_receive(:get).with(@original_author.url + public_post_path(:guid => @root_object.guid, :format => "xml")).and_return(response) - Reshare.from_xml(@xml) end + + context 'saving the post' do + before do + response = mock + response.stub(:body).and_return(@root_object.to_diaspora_xml) + Faraday.default_connection.should_receive(:get).with(@reshare.root.author.url + public_post_path(:guid => @root_object.guid, :format => "xml")).and_return(response) + end + + it 'fetches the root post from root_guid' do + root = Reshare.from_xml(@xml).root + + [:text, :guid, :diaspora_handle, :type, :public].each do |attr| + root.send(attr).should == @reshare.root.send(attr) + end + end + + it 'correctly saves the type' do + Reshare.from_xml(@xml).root.reload.type.should == "StatusMessage" + end + + it 'correctly sets the author' do + Reshare.from_xml(@xml).root.reload.author.reload.should == @original_author + end + end end end end From 987d44c41c0851e64fbabdc7cb04828738810520 Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Mon, 18 Jul 2011 15:43:36 -0700 Subject: [PATCH 11/28] fixed reshare specs; all specs green --- app/models/reshare.rb | 8 +++++++- spec/models/reshare_spec.rb | 25 +++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 716780a7d..3ea3f0439 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -45,7 +45,13 @@ class Reshare < Post received_post = Diaspora::Parser.from_xml(Faraday.get(root_author.url + "/p/#{@root_guid}.xml").body) unless post = received_post.class.where(:guid => received_post.guid).first post = received_post - post.save + + if root_author.diaspora_handle != post.diaspora_handle + raise "Diaspora ID (#{post.diaspora_handle}) in the root does not match the Diaspora ID (#{root_author.diaspora_handle}) specified in the reshare!" + end + + post.author_id = root_author.id + post.save! end self.root_id = post.id diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb index c3b02d9ed..02a74db0a 100644 --- a/spec/models/reshare_spec.rb +++ b/spec/models/reshare_spec.rb @@ -75,7 +75,8 @@ describe Reshare do context 'remote' do before do - @root_object = @reshare.root.delete + @root_object = @reshare.root + @root_object.delete end it 'fetches the root author from root_diaspora_id' do @@ -84,6 +85,8 @@ describe Reshare do @original_author = @reshare.root.author.dup @reshare.root.author.delete + @original_author.profile = @original_profile + wf_prof_mock = mock wf_prof_mock.should_receive(:fetch).and_return(@original_author) Webfinger.should_receive(:new).and_return(wf_prof_mock) @@ -99,7 +102,7 @@ describe Reshare do before do response = mock response.stub(:body).and_return(@root_object.to_diaspora_xml) - Faraday.default_connection.should_receive(:get).with(@reshare.root.author.url + public_post_path(:guid => @root_object.guid, :format => "xml")).and_return(response) + Faraday.default_connection.stub(:get).with(@reshare.root.author.url + public_post_path(:guid => @root_object.guid, :format => "xml")).and_return(response) end it 'fetches the root post from root_guid' do @@ -115,8 +118,26 @@ describe Reshare do end it 'correctly sets the author' do + @original_author = @reshare.root.author Reshare.from_xml(@xml).root.reload.author.reload.should == @original_author end + + it 'verifies that the author of the post received is the same as the author in the reshare xml' do + @original_author = @reshare.root.author.dup + @xml = @reshare.to_xml.to_s + + different_person = Factory.create(:person) + + wf_prof_mock = mock + wf_prof_mock.should_receive(:fetch).and_return(different_person) + Webfinger.should_receive(:new).and_return(wf_prof_mock) + + different_person.stub(:url).and_return(@original_author.url) + + lambda{ + Reshare.from_xml(@xml) + }.should raise_error /^Diaspora ID \(.+\) in the root does not match the Diaspora ID \(.+\) specified in the reshare!$/ + end end end end From 2d6f51f68c43653bb761933241f16acebc8f4956 Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Tue, 19 Jul 2011 11:17:46 -0700 Subject: [PATCH 12/28] repost wip --- app/models/relayable_retraction.rb | 2 +- app/models/user.rb | 16 +++++++++---- app/views/shared/_stream_element.html.haml | 2 +- features/repost.feature | 3 +-- spec/models/retraction_spec.rb | 26 ++++++++++++++++------ spec/models/user_spec.rb | 20 +++++++++++++---- 6 files changed, 50 insertions(+), 19 deletions(-) diff --git a/app/models/relayable_retraction.rb b/app/models/relayable_retraction.rb index 1f593f5cc..554f678ad 100644 --- a/app/models/relayable_retraction.rb +++ b/app/models/relayable_retraction.rb @@ -51,7 +51,7 @@ class RelayableRetraction retraction.sender = sender retraction.target = target retraction.target_author_signature = retraction.sign_with_key(sender.encryption_key) if sender.person == target.author - retraction.parent_author_signature = retraction.sign_with_key(sender.encryption_key) if sender.person == target.parent.author + retraction.parent_author_signature = retraction.sign_with_key(sender.encryption_key) if defined?(target.parent) && sender.person == target.parent.author retraction end diff --git a/app/models/user.rb b/app/models/user.rb index a1566f4d3..b93c093ac 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -230,21 +230,29 @@ class User < ActiveRecord::Base end ######### Posts and Such ############### - def retract(target) + def retract(target, opts={}) if target.respond_to?(:relayable?) && target.relayable? retraction = RelayableRetraction.build(self, target) else retraction = Retraction.for(target) end - opts = {} - if target.is_a?(Post) - opts[:additional_subscribers] = target.resharers + if target.is_a?(Post) && target.reshares.size != 0 + reshare_retraction = RelayableRetraction.build(self, target) end +# if target.is_a?(Post) +# opts[:additional_subscribers] = target.resharers +# end + mailman = Postzord::Dispatch.new(self, retraction, opts) mailman.post + if reshare_retraction + mailman = Postzord::Dispatch.new(self, reshare_retraction) + mailman.post + end + retraction.perform(self) retraction diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml index 0a4a3ef13..8c63d93b2 100644 --- a/app/views/shared/_stream_element.html.haml +++ b/app/views/shared/_stream_element.html.haml @@ -61,7 +61,7 @@ - if (post.author_id != current_user.person.id) && (post.public?) && !reshare?(post) · %span.reshare_action - = link_to "Reshare", reshares_path(:root_id => post.id), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" + = link_to "#{(post.reshares.size unless post.reshares.size == 0)} Reshare", reshares_path(:root_id => post.id), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" · = link_to t('comments.new_comment.comment'), '#', :class => 'focus_comment_textarea' diff --git a/features/repost.feature b/features/repost.feature index 4dd29df78..39567dc9f 100644 --- a/features/repost.feature +++ b/features/repost.feature @@ -91,9 +91,8 @@ Feature: public repost Then I should see "reshare this!" Then I should see a ".reshare" And I should see "Bob" + And I go to the home page - And I go to the destroy user session page - And I sign in as "bob@bob.bob" And I should see "1 Reshare" Scenario: Can have text diff --git a/spec/models/retraction_spec.rb b/spec/models/retraction_spec.rb index 6aee4a074..0d96356c5 100644 --- a/spec/models/retraction_spec.rb +++ b/spec/models/retraction_spec.rb @@ -8,7 +8,7 @@ describe Retraction do before do @aspect = alice.aspects.first alice.contacts.create(:person => eve.person, :aspects => [@aspect]) - @post = alice.post :status_message, :text => "Destroy!", :to => @aspect.id + @post = alice.post(:status_message, :public => true, :text => "Destroy!", :to => @aspect.id) end describe 'serialization' do @@ -20,12 +20,24 @@ describe Retraction do end describe '#subscribers' do - it 'returns the subscribers to the post for all objects other than person' do - retraction = Retraction.for(@post) - obj = retraction.instance_variable_get(:@object) - wanted_subscribers = obj.subscribers(alice) - obj.should_receive(:subscribers).with(alice).and_return(wanted_subscribers) - retraction.subscribers(alice).map{|s| s.id}.should =~ wanted_subscribers.map{|s| s.id} + context 'posts' do + before do + @retraction = Retraction.for(@post) + @obj = @retraction.instance_variable_get(:@object) + @wanted_subscribers = @obj.subscribers(alice) + end + + it 'returns the subscribers to the post for all objects other than person' do + @retraction.subscribers(alice).map(&:id).should =~ @wanted_subscribers.map(&:id) + end + + it 'does not return the authors of reshares' do + @post.reshares << Factory.create(:reshare, :root => @post, :author => bob.person) + @post.save! + + @wanted_subscribers -= [bob.person] + @retraction.subscribers(alice).map(&:id).should =~ @wanted_subscribers.map(&:id) + end end context 'setting subscribers' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7cc3944c4..23a25a49b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -756,14 +756,14 @@ describe User do describe '#retract' do before do @retraction = mock + + @post = Factory(:status_message, :author => bob.person, :public => true) end context "regular retractions" do before do Retraction.stub(:for).and_return(@retraction) @retraction.stub(:perform) - - @post = Factory(:status_message, :author => bob.person, :public => true) end it 'sends a retraction' do @@ -787,13 +787,25 @@ describe User do end it 'performs the retraction' do - + pending end end context "relayable retractions" do - it 'sends a relayable retraction if the object is relayable' do + before do + @post.reshares << Factory.create(:reshare, :author => remote_raphael) + @post.save! + end + it 'sends a relayable retraction if the object is relayable' do + r_ret = RelayableRetraction.build(bob, @post) + RelayableRetraction.should_receive(:build).and_return(r_ret) + + dis = mock + dis.should_receive(:post) + Postzord::Dispatch.should_receive(:new).with(bob, r_ret).and_return(dis) + + bob.retract(@post) end end end From bd1c8efe54f44083cbe81a04633621225ad4d43b Mon Sep 17 00:00:00 2001 From: Ilyaaaaaaaaaaaaa Zhitomirskiy Date: Tue, 19 Jul 2011 15:04:34 -0700 Subject: [PATCH 13/28] wip integration servers --- app/models/app_config.rb | 2 +- app/models/retraction.rb | 2 ++ config/application.yml.example | 7 ++++ features/step_definitions/oauth_steps.rb | 8 +++-- spec/multi_server/server_spec.rb | 11 ++++-- spec/support/server.rb | 44 ++++++++++++++++++++++-- 6 files changed, 65 insertions(+), 9 deletions(-) diff --git a/app/models/app_config.rb b/app/models/app_config.rb index 573b63567..e3aea84cb 100644 --- a/app/models/app_config.rb +++ b/app/models/app_config.rb @@ -6,7 +6,7 @@ require 'uri' class AppConfig < Settingslogic def self.source_file_name - if Rails.env == 'test' || ENV["CI"] + if Rails.env == 'test' || ENV["CI"] || Rails.env.include?("integration") File.join(Rails.root, "config", "application.yml.example") else File.join(Rails.root, "config", "application.yml") diff --git a/app/models/retraction.rb b/app/models/retraction.rb index e0f3ce63d..8b95aae7d 100644 --- a/app/models/retraction.rb +++ b/app/models/retraction.rb @@ -15,6 +15,8 @@ class Retraction def subscribers(user) unless self.type == 'Person' @subscribers ||= self.object.subscribers(user) + @subscribers -= self.object.resharers + @subscribers else raise 'HAX: you must set the subscribers manaully before unfriending' if @subscribers.nil? @subscribers diff --git a/config/application.yml.example b/config/application.yml.example index 0be3aa125..ea01833be 100644 --- a/config/application.yml.example +++ b/config/application.yml.example @@ -154,3 +154,10 @@ test: open_invitations: false +integration_1: + <<: *defaults + pod_url: "http://localhost:45789" + +integration_2: + <<: *defaults + pod_url: "http://localhost:34658" diff --git a/features/step_definitions/oauth_steps.rb b/features/step_definitions/oauth_steps.rb index c8df986db..a14d83db7 100644 --- a/features/step_definitions/oauth_steps.rb +++ b/features/step_definitions/oauth_steps.rb @@ -59,7 +59,7 @@ class Chubbies def self.run @pid = fork do - Process.exec "cd #{Rails.root}/spec/chubbies/ && bundle exec rackup -p #{PORT} 2> /dev/null 1> /dev/null" + Process.exec "cd #{Rails.root}/spec/chubbies/ && bundle exec #{run_command} #{nullify}" end at_exit do @@ -94,9 +94,13 @@ class Chubbies end end + def self.run_command + "rackup -p #{PORT}" + end + def self.get_pid @pid ||= lambda { - processes = `ps ax -o pid,command | grep "rackup -p #{PORT}"`.split("\n") + processes = `ps ax -o pid,command | grep "#{run_command}"`.split("\n") processes = processes.select{|p| !p.include?("grep") } processes.first.split(" ").first }.call diff --git a/spec/multi_server/server_spec.rb b/spec/multi_server/server_spec.rb index a5b5e364a..a600bce2e 100644 --- a/spec/multi_server/server_spec.rb +++ b/spec/multi_server/server_spec.rb @@ -2,13 +2,18 @@ require 'spec_helper' WebMock::Config.instance.allow_localhost = true -if Server.all && Server.all.first && Server.all.first.running? +unless Server.all.empty? describe Server do before(:all) do WebMock::Config.instance.allow_localhost = true + Server.all.each{|s| s.kill if s.running?} + Server.all.each{|s| s.run} end after(:all) do + Server.all.each{|s| s.kill if s.running?} + sleep(1) + Server.all.each{|s| puts "Server at port #{s.port} still running." if s.running?} WebMock::Config.instance.allow_localhost = false end @@ -25,12 +30,12 @@ if Server.all && Server.all.first && Server.all.first.running? it 'takes an environment' do server = Server.new("integration_1") server.env.should == "integration_1" - server.port.should == ActiveRecord::Base.configurations["integration_1"]["app_server_port"] end end describe "#running?" do it "verifies that the server is running" do - Server.new("integration_1").running?.should be_true + server = Server.new("integration_1") + server.running?.should be_true end end describe '#db' do diff --git a/spec/support/server.rb b/spec/support/server.rb index 0b6d4cd78..0a848955a 100644 --- a/spec/support/server.rb +++ b/spec/support/server.rb @@ -8,11 +8,48 @@ class Server attr_reader :port, :env def initialize(env) - @config = ActiveRecord::Base.configurations[env] - @port = @config["app_server_port"] + @db_config = ActiveRecord::Base.configurations[env] + @app_config = (YAML.load_file AppConfig.source)[env] + @port = URI::parse(@app_config["pod_url"]).port @env = env + ensure_database_setup end + def ensure_database_setup + `cd #{Rails.root} && RAILS_ENV=#{@env} bundle exec rake db:create` + tables_exist = self.db do + ActiveRecord::Base.connection.tables.include?("users") + end + if tables_exist + truncate_database + else + `cd #{Rails.root} && RAILS_ENV=#{@env} bundle exec rake db:schema:load` + end + end + + def run + @pid = fork do + Process.exec "cd #{Rails.root} && RAILS_ENV=#{@env} bundle exec #{run_command}" + end + end + + def kill + puts "I am trying to kill the server" + `kill -9 #{get_pid}` + end + + def run_command + "rails s thin -p #{@port}" + end + + def get_pid + @pid = lambda { + processes = `ps ax -o pid,command | grep "#{run_command}"`.split("\n") + processes = processes.select{|p| !p.include?("grep") } + processes.first.split(" ").first + }.call + end + def running? begin RestClient.get("localhost:#{@port}/users/sign_in") @@ -25,8 +62,9 @@ class Server def db former_env = Rails.env ActiveRecord::Base.establish_connection(env) - yield + result = yield ActiveRecord::Base.establish_connection(former_env) + result end def truncate_database From 70be713b158b181b548ce1b35abca65cad7d4f2a Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Tue, 19 Jul 2011 16:10:42 -0700 Subject: [PATCH 14/28] Drop the integration databases in rebuild --- lib/tasks/db.rake | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index ceae7f271..77b35bc08 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -4,7 +4,7 @@ namespace :db do desc "rebuild and prepare test db" - task :rebuild => [:drop, :create, :migrate, :seed,'db:test:prepare'] + task :rebuild => [:drop, :drop_integration, :create, :migrate, :seed, 'db:test:prepare'] namespace :integration do # desc 'Check for pending migrations and load the integration schema' @@ -81,6 +81,14 @@ namespace :db do end task :add_user => :environment + task :drop_integration do + ActiveRecord::Base.configurations.keys.select{ |k| + k.include?("integration") + }.each{ |k| + drop_database ActiveRecord::Base.configurations[k] + } + end + task :fix_diaspora_handle do puts "fixing the people in this seed" require File.join(File.dirname(__FILE__), '..', '..', 'config', 'environment') From 537766d0d70ee26b5fce3a770062f15f66e0f790 Mon Sep 17 00:00:00 2001 From: Ilyaaaaaaaaaaaaa Zhitomirskiy Date: Tue, 19 Jul 2011 18:17:08 -0700 Subject: [PATCH 15/28] Started working on repost integration test --- app/models/reshare.rb | 2 +- spec/multi_server/repost_spec.rb | 52 ++++++++++++++++++++++++++++++++ spec/multi_server/server_spec.rb | 2 -- spec/support/server.rb | 22 ++++++++++++-- 4 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 spec/multi_server/repost_spec.rb diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 3ea3f0439..8a9bed4a3 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -59,7 +59,7 @@ class Reshare < Post end def root_must_be_public - if self.root.nil? || !self.root.public + if !self.root.public errors[:base] << "you must reshare public posts" return false end diff --git a/spec/multi_server/repost_spec.rb b/spec/multi_server/repost_spec.rb new file mode 100644 index 000000000..4e4bde01d --- /dev/null +++ b/spec/multi_server/repost_spec.rb @@ -0,0 +1,52 @@ + +require 'spec_helper' + + +unless Server.all.empty? + describe "reposting" do + before(:all) do + WebMock::Config.instance.allow_localhost = true + #Server.all.each{|s| s.kill if s.running?} + #Server.all.each{|s| s.run} + end + + after(:all) do + #Server.all.each{|s| s.kill if s.running?} + #sleep(1) + #Server.all.each{|s| puts "Server at port #{s.port} still running." if s.running?} + WebMock::Config.instance.allow_localhost = false + end + + it 'fetches the original post from the root server' do + Server.all[0].in_scope do + original_poster = Factory.create(:user_with_aspect, :username => "original_poster") + resharer = Factory.create(:user_with_aspect, :username => "resharer") + + connect_users_with_aspects(original_poster, resharer) + + original_post = original_poster.post(:status_message, + :public => true, + :text => "Awesome Sauce!", + :to => 'all') + resharer.post(:reshare, :root_id => original_post.id, :to => 'all') + end + + Server.all[1].in_scope do + recipient = Factory.create(:user_with_aspect, :username => "resharer") + end + + Server.all[0].in_scope do + r = User.find_by_username("resharer") + person = Webfinger.new("recipient@localhost:#{Server.all[1].port}").fetch + r.share_with(person, r.aspects.first) + end + Server.all[1].in_scope do + r = User.find_by_username("recipient") + person = Webfinger.new("resharer@localhost:#{Server.all[0].port}").fetch + r.share_with(person, r.aspects.first) + end + + + end + end +end diff --git a/spec/multi_server/server_spec.rb b/spec/multi_server/server_spec.rb index a600bce2e..e8e01fea3 100644 --- a/spec/multi_server/server_spec.rb +++ b/spec/multi_server/server_spec.rb @@ -1,7 +1,6 @@ #This is a spec for the class that runs the servers used in the other multi-server specs require 'spec_helper' -WebMock::Config.instance.allow_localhost = true unless Server.all.empty? describe Server do before(:all) do @@ -57,4 +56,3 @@ unless Server.all.empty? end end end -WebMock::Config.instance.allow_localhost = false diff --git a/spec/support/server.rb b/spec/support/server.rb index 0a848955a..eb31db5e6 100644 --- a/spec/support/server.rb +++ b/spec/support/server.rb @@ -62,8 +62,11 @@ class Server def db former_env = Rails.env ActiveRecord::Base.establish_connection(env) - result = yield - ActiveRecord::Base.establish_connection(former_env) + begin + result = yield + ensure + ActiveRecord::Base.establish_connection(former_env) + end result end @@ -72,4 +75,19 @@ class Server DatabaseCleaner.clean_with(:truncation) end end + + def in_scope + pod_url = "http://localhost:#{@port}/" + AppConfig.stub!(:pod_url).and_return(pod_url) + AppConfig.stub!(:pod_uri).and_return(URI.parse(pod_url)) + begin + result = db do + yield + end + ensure + AppConfig.unstub!(:pod_url) + AppConfig.unstub!(:pod_uri) + end + result + end end From e19fb6d0f885afaaaa48708da2f29f60ecfc2bed Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Wed, 20 Jul 2011 02:20:23 -0700 Subject: [PATCH 16/28] Use authority instead of host in order to get the port as well. --- app/models/app_config.rb | 2 +- app/models/user.rb | 2 +- app/views/publics/host_meta.erb | 2 +- app/views/publics/host_meta.html.erb | 2 +- config/initializers/mailer_config.rb | 2 +- spec/support/server.rb | 8 ++++---- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/models/app_config.rb b/app/models/app_config.rb index e3aea84cb..338036002 100644 --- a/app/models/app_config.rb +++ b/app/models/app_config.rb @@ -125,7 +125,7 @@ HELP def self.pod_uri if @@pod_uri.nil? begin - @@pod_uri = URI.parse(self.pod_url) + @@pod_uri = Addressable::URI.parse(self.pod_url) rescue puts "WARNING: pod url " + self.pod_url + " is not a legal URI" end diff --git a/app/models/user.rb b/app/models/user.rb index b93c093ac..a37445555 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -340,7 +340,7 @@ class User < ActiveRecord::Base end self.person = Person.new(opts[:person]) - self.person.diaspora_handle = "#{opts[:username]}@#{AppConfig[:pod_uri].host}" + self.person.diaspora_handle = "#{opts[:username]}@#{AppConfig[:pod_uri].authority}" self.person.url = AppConfig[:pod_url] diff --git a/app/views/publics/host_meta.erb b/app/views/publics/host_meta.erb index faaa0e0a2..5bb6786c5 100644 --- a/app/views/publics/host_meta.erb +++ b/app/views/publics/host_meta.erb @@ -1,7 +1,7 @@ - <%= AppConfig[:pod_uri].host %> + <%= AppConfig[:pod_uri].authority %> Resource Descriptor diff --git a/app/views/publics/host_meta.html.erb b/app/views/publics/host_meta.html.erb index 8b0ad3aa8..fa8cd9879 100644 --- a/app/views/publics/host_meta.html.erb +++ b/app/views/publics/host_meta.html.erb @@ -1,7 +1,7 @@ - <%= AppConfig[:pod_uri].host %> + <%= AppConfig[:pod_uri].authority %> Resource Descriptor diff --git a/config/initializers/mailer_config.rb b/config/initializers/mailer_config.rb index 0f154d3e9..a42f539c3 100644 --- a/config/initializers/mailer_config.rb +++ b/config/initializers/mailer_config.rb @@ -3,7 +3,7 @@ # the COPYRIGHT file. Diaspora::Application.configure do - config.action_mailer.default_url_options = {:host => AppConfig[:pod_uri].host} + config.action_mailer.default_url_options = {:host => AppConfig[:pod_uri].authority } unless Rails.env == 'test' || AppConfig[:mailer_on] != true if AppConfig[:mailer_method] == "sendmail" config.action_mailer.delivery_method = :sendmail diff --git a/spec/support/server.rb b/spec/support/server.rb index eb31db5e6..27df791f7 100644 --- a/spec/support/server.rb +++ b/spec/support/server.rb @@ -9,7 +9,7 @@ class Server attr_reader :port, :env def initialize(env) @db_config = ActiveRecord::Base.configurations[env] - @app_config = (YAML.load_file AppConfig.source)[env] + @app_config = (YAML.load_file AppConfig.source)[env] @port = URI::parse(@app_config["pod_url"]).port @env = env ensure_database_setup @@ -18,7 +18,7 @@ class Server def ensure_database_setup `cd #{Rails.root} && RAILS_ENV=#{@env} bundle exec rake db:create` tables_exist = self.db do - ActiveRecord::Base.connection.tables.include?("users") + ActiveRecord::Base.connection.tables.include?("users") end if tables_exist truncate_database @@ -49,7 +49,7 @@ class Server processes.first.split(" ").first }.call end - + def running? begin RestClient.get("localhost:#{@port}/users/sign_in") @@ -79,7 +79,7 @@ class Server def in_scope pod_url = "http://localhost:#{@port}/" AppConfig.stub!(:pod_url).and_return(pod_url) - AppConfig.stub!(:pod_uri).and_return(URI.parse(pod_url)) + AppConfig.stub!(:pod_uri).and_return(Addressable::URI.parse(pod_url)) begin result = db do yield From 7382dfb7be498d3c9d2b37311fd88ac98158074d Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Wed, 20 Jul 2011 13:10:29 -0700 Subject: [PATCH 17/28] Multi-server spec now successfully passes along a reshare --- app/models/person.rb | 1 - app/models/reshare.rb | 14 +++++++------ config/application.yml.example | 2 ++ config/environments/integration_1.rb | 3 +-- spec/factories.rb | 2 +- spec/multi_server/repost_spec.rb | 31 +++++++++++++++++++--------- spec/support/server.rb | 16 ++++++++------ spec/support/user_methods.rb | 8 ++++++- 8 files changed, 50 insertions(+), 27 deletions(-) diff --git a/app/models/person.rb b/app/models/person.rb index 68d18a011..756a00ef7 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -235,7 +235,6 @@ class Person < ActiveRecord::Base protected def clean_url - self.url ||= "http://localhost:3000/" if self.class == User if self.url self.url = 'http://' + self.url unless self.url.match(/https?:\/\//) self.url = self.url + '/' if self.url[-1, 1] != '/' diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 8a9bed4a3..80bbeb47f 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -1,4 +1,4 @@ -class Reshare < Post +class Reshare < Post belongs_to :root, :class_name => 'Post' validate :root_must_be_public @@ -7,12 +7,12 @@ class Reshare < Post xml_attr :root_diaspora_id xml_attr :root_guid - before_validation do + before_validation do self.public = true end def root_guid - self.root.guid + self.root.guid end def root_diaspora_id @@ -21,9 +21,9 @@ class Reshare < Post def receive(user, person) local_reshare = Reshare.where(:guid => self.guid).first - if local_reshare.root.author_id == user.person.id + if local_reshare && local_reshare.root.author_id == user.person.id local_reshare.root.reshares << local_reshare - + if user.contact_for(person) local_reshare.receive(user, person) end @@ -42,7 +42,9 @@ class Reshare < Post local_post = Post.where(:guid => @root_guid).select('id').first unless local_post && self.root_id = local_post.id - received_post = Diaspora::Parser.from_xml(Faraday.get(root_author.url + "/p/#{@root_guid}.xml").body) + response = Faraday.get(root_author.url + "/p/#{@root_guid}.xml") + received_post = Diaspora::Parser.from_xml(response.body) + unless post = received_post.class.where(:guid => received_post.guid).first post = received_post diff --git a/config/application.yml.example b/config/application.yml.example index ea01833be..6577ffd9d 100644 --- a/config/application.yml.example +++ b/config/application.yml.example @@ -157,7 +157,9 @@ test: integration_1: <<: *defaults pod_url: "http://localhost:45789" + enable_splunk_logging: false integration_2: <<: *defaults pod_url: "http://localhost:34658" + enable_splunk_logging: false diff --git a/config/environments/integration_1.rb b/config/environments/integration_1.rb index 1279e645a..86f1c3043 100644 --- a/config/environments/integration_1.rb +++ b/config/environments/integration_1.rb @@ -15,13 +15,12 @@ Diaspora::Application.configure do # Show full error reports and disable caching config.consider_all_requests_local = true - config.action_view.debug_rjs = true config.action_controller.perform_caching = false # Don't care if the mailer can't send config.action_mailer.raise_delivery_errors = false config.active_support.deprecation = :log - #config.threadsafe! + config.threadsafe! # Monkeypatch around the nasty "2.5MB exception page" issue, caused by very large environment vars # This snippet via: http://stackoverflow.com/questions/3114993/exception-pages-in-development-mode-take-upwards-of-15-30-seconds-to-render-why diff --git a/spec/factories.rb b/spec/factories.rb index dbd6da864..b637cc97d 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -19,7 +19,7 @@ end Factory.define :person do |p| p.sequence(:diaspora_handle) { |n| "bob-person-#{n}#{r_str}@aol.com" } - p.sequence(:url) { |n| "http://google-#{n}#{r_str}.com/" } + p.sequence(:url) { |n| AppConfig[:pod_url] } p.serialized_public_key OpenSSL::PKey::RSA.generate(1024).public_key.export p.after_build do |person| person.profile ||= Factory.build(:profile, :person => person) diff --git a/spec/multi_server/repost_spec.rb b/spec/multi_server/repost_spec.rb index 4e4bde01d..48577eb4c 100644 --- a/spec/multi_server/repost_spec.rb +++ b/spec/multi_server/repost_spec.rb @@ -6,11 +6,13 @@ unless Server.all.empty? describe "reposting" do before(:all) do WebMock::Config.instance.allow_localhost = true + enable_typhoeus #Server.all.each{|s| s.kill if s.running?} #Server.all.each{|s| s.run} end after(:all) do + disable_typhoeus #Server.all.each{|s| s.kill if s.running?} #sleep(1) #Server.all.each{|s| puts "Server at port #{s.port} still running." if s.running?} @@ -18,34 +20,43 @@ unless Server.all.empty? end it 'fetches the original post from the root server' do - Server.all[0].in_scope do + original_post = nil + Server[0].in_scope do original_poster = Factory.create(:user_with_aspect, :username => "original_poster") resharer = Factory.create(:user_with_aspect, :username => "resharer") connect_users_with_aspects(original_poster, resharer) - original_post = original_poster.post(:status_message, + original_post = original_poster.post(:status_message, :public => true, :text => "Awesome Sauce!", :to => 'all') - resharer.post(:reshare, :root_id => original_post.id, :to => 'all') end - Server.all[1].in_scope do - recipient = Factory.create(:user_with_aspect, :username => "resharer") + Server[1].in_scope do + recipient = Factory.create(:user_with_aspect, :username => "recipient") end - Server.all[0].in_scope do + Server[0].in_scope do r = User.find_by_username("resharer") - person = Webfinger.new("recipient@localhost:#{Server.all[1].port}").fetch + person = Webfinger.new("recipient@localhost:#{Server[1].port}").fetch r.share_with(person, r.aspects.first) end - Server.all[1].in_scope do + Server[1].in_scope do r = User.find_by_username("recipient") - person = Webfinger.new("resharer@localhost:#{Server.all[0].port}").fetch + person = Webfinger.new("resharer@localhost:#{Server[0].port}").fetch r.share_with(person, r.aspects.first) end - + + Server[0].in_scope do + r = User.find_by_username("resharer") + r.post(:reshare, :root_id => original_post.id, :to => 'all') + debugger + end + + Server[1].in_scope do + Post.exists?(:guid => original_post.guid).should be_true + end end end diff --git a/spec/support/server.rb b/spec/support/server.rb index 27df791f7..898a7614a 100644 --- a/spec/support/server.rb +++ b/spec/support/server.rb @@ -1,5 +1,10 @@ #This class is for running servers in the background during integration testing. This will not run on Windows. class Server + + def self.[] index + self.all[index] + end + def self.all @servers ||= ActiveRecord::Base.configurations.keys.select{ |k| k.include?("integration") @@ -29,7 +34,7 @@ class Server def run @pid = fork do - Process.exec "cd #{Rails.root} && RAILS_ENV=#{@env} bundle exec #{run_command}" + Process.exec "cd #{Rails.root} && RAILS_ENV=#{@env} bundle exec #{run_command} 2> /dev/null" end end @@ -39,7 +44,7 @@ class Server end def run_command - "rails s thin -p #{@port}" + "rails s -p #{@port}" end def get_pid @@ -78,15 +83,14 @@ class Server def in_scope pod_url = "http://localhost:#{@port}/" - AppConfig.stub!(:pod_url).and_return(pod_url) - AppConfig.stub!(:pod_uri).and_return(Addressable::URI.parse(pod_url)) + old_pod_url = AppConfig[:pod_url] + AppConfig[:pod_url] = pod_url begin result = db do yield end ensure - AppConfig.unstub!(:pod_url) - AppConfig.unstub!(:pod_uri) + AppConfig[:pod_url] = old_pod_url end result end diff --git a/spec/support/user_methods.rb b/spec/support/user_methods.rb index ff9925b41..63063be5f 100644 --- a/spec/support/user_methods.rb +++ b/spec/support/user_methods.rb @@ -1,4 +1,8 @@ class User + include Rails.application.routes.url_helpers + def default_url_options + {:host => AppConfig[:pod_url]} + end alias_method :share_with_original, :share_with @@ -16,7 +20,9 @@ class User aspects = self.aspects_from_ids(opts[:to]) add_to_streams(p, aspects) - dispatch_post(p, :to => opts[:to]) + dispatch_opts = {:url => post_url(p), :to => opts[:to]} + dispatch_opts.merge!(:additional_subscribers => p.root.author) if class_name == :reshare + dispatch_post(p, dispatch_opts) end unless opts[:created_at] p.created_at = Time.now - 1 From a1bf22fe7e4cbcb535ebfd6d108cf09d37856ebe Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Wed, 20 Jul 2011 19:00:28 -0700 Subject: [PATCH 18/28] More work on retracting posts with reshares, one failing spec on User#retract --- app/models/relayable_retraction.rb | 69 +++--------------------------- app/models/reshare.rb | 3 +- app/models/user.rb | 17 +++----- spec/models/person_spec.rb | 2 +- spec/models/user_spec.rb | 28 ++++++------ spec/multi_server/repost_spec.rb | 30 +++++++++---- 6 files changed, 52 insertions(+), 97 deletions(-) diff --git a/app/models/relayable_retraction.rb b/app/models/relayable_retraction.rb index 554f678ad..d2218af68 100644 --- a/app/models/relayable_retraction.rb +++ b/app/models/relayable_retraction.rb @@ -2,81 +2,28 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -class RelayableRetraction - include ROXML - include Diaspora::Webhooks - include Diaspora::Encryptable - +class RelayableRetraction < SignedRetraction xml_name :relayable_retraction - xml_attr :target_guid - xml_attr :target_type - xml_attr :sender_handle xml_attr :parent_author_signature - xml_attr :target_author_signature - attr_accessor :target_guid, - :target_type, - :parent_author_signature, - :target_author_signature, - :sender + attr_accessor :parent_author_signature def signable_accessors - accessors = self.class.roxml_attrs.collect do |definition| - definition.accessor - end - ['target_author_signature', 'parent_author_signature'].each do |acc| - accessors.delete acc - end - accessors - end - - def sender_handle= new_sender_handle - @sender = Person.where(:diaspora_handle => new_sender_handle).first - end - - def sender_handle - @sender.diaspora_handle - end - - def diaspora_handle - self.sender_handle - end - - def subscribers(user) - self.target.subscribers(user) + super - ['parent_author_signature'] end def self.build(sender, target) - retraction = self.new - retraction.sender = sender - retraction.target = target - retraction.target_author_signature = retraction.sign_with_key(sender.encryption_key) if sender.person == target.author + retraction = super retraction.parent_author_signature = retraction.sign_with_key(sender.encryption_key) if defined?(target.parent) && sender.person == target.parent.author retraction end - def target - @target ||= self.target_type.constantize.where(:guid => target_guid).first - end - - def guid - target_guid - end - def target= new_target - @target = new_target - @target_type = new_target.class.to_s - @target_guid = new_target.guid - end - def parent self.target.parent end - def perform receiving_user - Rails.logger.debug "Performing retraction for #{target_guid}" - self.target.unsocket_from_user receiving_user if target.respond_to? :unsocket_from_user - self.target.destroy - Rails.logger.info(:event => :retraction, :status => :complete, :target_type => self.target_type, :guid => self.target_guid) + def diaspora_handle + self.sender_handle end def receive(recipient, sender) @@ -101,8 +48,4 @@ class RelayableRetraction def parent_author_signature_valid? verify_signature(self.parent_author_signature, self.parent.author) end - - def target_author_signature_valid? - verify_signature(self.target_author_signature, self.target.author) - end end diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 80bbeb47f..72e3ccbca 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -3,6 +3,7 @@ class Reshare < Post belongs_to :root, :class_name => 'Post' validate :root_must_be_public attr_accessible :root_id, :public + validates_presence_of :root, :on => :create xml_attr :root_diaspora_id xml_attr :root_guid @@ -61,7 +62,7 @@ class Reshare < Post end def root_must_be_public - if !self.root.public + if self.root && !self.root.public errors[:base] << "you must reshare public posts" return false end diff --git a/app/models/user.rb b/app/models/user.rb index a37445555..fdd391b7f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -233,26 +233,19 @@ class User < ActiveRecord::Base def retract(target, opts={}) if target.respond_to?(:relayable?) && target.relayable? retraction = RelayableRetraction.build(self, target) + elsif target.is_a? Post + retraction = SignedRetraction.build(self, target) else retraction = Retraction.for(target) end - if target.is_a?(Post) && target.reshares.size != 0 - reshare_retraction = RelayableRetraction.build(self, target) - end - -# if target.is_a?(Post) -# opts[:additional_subscribers] = target.resharers -# end + if target.is_a?(Post) + opts[:additional_subscribers] = target.resharers + end mailman = Postzord::Dispatch.new(self, retraction, opts) mailman.post - if reshare_retraction - mailman = Postzord::Dispatch.new(self, reshare_retraction) - mailman.post - end - retraction.perform(self) retraction diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index b58bc862b..b1def8023 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -41,7 +41,7 @@ describe Person do context 'local people' do it 'uses the pod config url to set the diaspora_handle' do new_person = User.build(:username => "foo123", :email => "foo123@example.com", :password => "password", :password_confirmation => "password").person - new_person.diaspora_handle.should == "foo123@#{AppConfig[:pod_uri].host}" + new_person.diaspora_handle.should == "foo123@#{AppConfig[:pod_uri].authority}" end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 23a25a49b..f8c7fdaf7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -760,9 +760,9 @@ describe User do @post = Factory(:status_message, :author => bob.person, :public => true) end - context "regular retractions" do + context "posts" do before do - Retraction.stub(:for).and_return(@retraction) + SignedRetraction.stub(:build).and_return(@retraction) @retraction.stub(:perform) end @@ -791,21 +791,25 @@ describe User do end end - context "relayable retractions" do + context "signed retractions" do before do - @post.reshares << Factory.create(:reshare, :author => remote_raphael) + @post.reshares << Factory.create(:reshare,:root_id => @post.id, :author => Factory(:user).person) @post.save! end - it 'sends a relayable retraction if the object is relayable' do - r_ret = RelayableRetraction.build(bob, @post) - RelayableRetraction.should_receive(:build).and_return(r_ret) - - dis = mock - dis.should_receive(:post) - Postzord::Dispatch.should_receive(:new).with(bob, r_ret).and_return(dis) + it 'relays the signed retraction onwards to recipients of reshares' do + r_ret = SignedRetraction.build(bob, @post) + SignedRetraction.should_receive(:build).and_return(r_ret) - bob.retract(@post) + dis = mock + dis_2 = mock + dis.should_receive(:post) + dis_2.should_receive(:post) + Postzord::Dispatch.should_receive(:new).with(bob, r_ret, anything()).and_return(dis, dis_2) + + fantasy_resque do + bob.retract(@post) + end end end end diff --git a/spec/multi_server/repost_spec.rb b/spec/multi_server/repost_spec.rb index 48577eb4c..c8ea9f535 100644 --- a/spec/multi_server/repost_spec.rb +++ b/spec/multi_server/repost_spec.rb @@ -18,16 +18,16 @@ unless Server.all.empty? #Server.all.each{|s| puts "Server at port #{s.port} still running." if s.running?} WebMock::Config.instance.allow_localhost = false end - - it 'fetches the original post from the root server' do - original_post = nil + before do + Server.all.each{|s| s.truncate_database; puts "Truncating database for server #{s}" } + @original_post = nil Server[0].in_scope do original_poster = Factory.create(:user_with_aspect, :username => "original_poster") resharer = Factory.create(:user_with_aspect, :username => "resharer") connect_users_with_aspects(original_poster, resharer) - original_post = original_poster.post(:status_message, + @original_post = original_poster.post(:status_message, :public => true, :text => "Awesome Sauce!", :to => 'all') @@ -50,14 +50,28 @@ unless Server.all.empty? Server[0].in_scope do r = User.find_by_username("resharer") - r.post(:reshare, :root_id => original_post.id, :to => 'all') - debugger + r.post(:reshare, :root_id => @original_post.id, :to => 'all') + end + end + + it 'fetches the original post from the root server' do + Server[1].in_scope do + Post.exists?(:guid => @original_post.guid).should be_true + end + end + + it 'relays the retraction for the root post to recipients of the reshare' do + Server[0].in_scope do + poster = User.find_by_username 'original_poster' + fantasy_resque do + poster.retract @original_post + end end Server[1].in_scope do - Post.exists?(:guid => original_post.guid).should be_true + Post.exists?(:guid => @original_post.guid).should be_false end - end + end end From 17774d8b3b94ab0f3c6917c65d30552959fa387e Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Thu, 21 Jul 2011 19:15:47 -0700 Subject: [PATCH 19/28] Getting repost back to almost green. --- app/models/signed_retraction.rb | 94 ++++++++++++++++++++++++ features/step_definitions/oauth_steps.rb | 4 + lib/tasks/db.rake | 2 +- spec/models/reshare_spec.rb | 8 +- spec/models/user_spec.rb | 16 ++-- spec/multi_server/repost_spec.rb | 2 +- spec/support/server.rb | 2 +- 7 files changed, 113 insertions(+), 15 deletions(-) create mode 100644 app/models/signed_retraction.rb diff --git a/app/models/signed_retraction.rb b/app/models/signed_retraction.rb new file mode 100644 index 000000000..75a4e5477 --- /dev/null +++ b/app/models/signed_retraction.rb @@ -0,0 +1,94 @@ +# Copyright (c) 2010, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +class SignedRetraction + include ROXML + include Diaspora::Webhooks + include Diaspora::Encryptable + + xml_name :signed_retraction + xml_attr :target_guid + xml_attr :target_type + xml_attr :sender_handle + xml_attr :target_author_signature + + attr_accessor :target_guid, + :target_type, + :target_author_signature, + :sender + + def signable_accessors + accessors = self.class.roxml_attrs.collect do |definition| + definition.accessor + end + accessors - ['target_author_signature'] + end + + def sender_handle= new_sender_handle + @sender = Person.where(:diaspora_handle => new_sender_handle).first + end + + def sender_handle + @sender.diaspora_handle + end + + def diaspora_handle + self.target.diaspora_handle + end + + def subscribers(user) + self.target.subscribers(user) + end + + def self.build(sender, target) + retraction = self.new + retraction.sender = sender + retraction.target = target + retraction.target_author_signature = retraction.sign_with_key(sender.encryption_key) if sender.person == target.author + retraction + end + + def target + @target ||= self.target_type.constantize.where(:guid => target_guid).first + end + + def guid + target_guid + end + def target= new_target + @target = new_target + @target_type = new_target.class.to_s + @target_guid = new_target.guid + end + + def perform receiving_user + Rails.logger.debug "Performing retraction for #{target_guid}" + puts "Performing retraction for #{target_guid}" + if reshare = Reshare.where(:author_id => receiving_user.person.id, :root_id => target.id).first + Postzord::Dispatch.new(receiving_user, self).post + end + self.target.unsocket_from_user receiving_user if target.respond_to? :unsocket_from_user + self.target.destroy + Rails.logger.info(:event => :retraction, :status => :complete, :target_type => self.target_type, :guid => self.target_guid) + end + + def receive(recipient, sender) + if self.target.nil? + Rails.logger.info("event=retraction status=abort reason='no post found' sender=#{sender.diaspora_handle} target_guid=#{target_guid}") + return + elsif self.target_author_signature_valid? + #this is a retraction from the upstream owner + self.perform(recipient) + else + Rails.logger.info("event=receive status=abort reason='object signature not valid' recipient=#{recipient.diaspora_handle} sender=#{self.parent.author.diaspora_handle} payload_type=#{self.class} parent_id=#{self.parent.id}") + return + end + self + end + + def target_author_signature_valid? + verify_signature(self.target_author_signature, self.target.author) + end +end + diff --git a/features/step_definitions/oauth_steps.rb b/features/step_definitions/oauth_steps.rb index a14d83db7..444648fc3 100644 --- a/features/step_definitions/oauth_steps.rb +++ b/features/step_definitions/oauth_steps.rb @@ -71,6 +71,10 @@ class Chubbies end end + def self.nullify + "2> /dev/null > /dev/null" + end + def self.kill `kill -9 #{get_pid}` end diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index 77b35bc08..705d12fed 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -85,7 +85,7 @@ namespace :db do ActiveRecord::Base.configurations.keys.select{ |k| k.include?("integration") }.each{ |k| - drop_database ActiveRecord::Base.configurations[k] + drop_database ActiveRecord::Base.configurations[k] rescue Mysql2::Error } end diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb index 02a74db0a..cc24ef8e9 100644 --- a/spec/models/reshare_spec.rb +++ b/spec/models/reshare_spec.rb @@ -49,19 +49,19 @@ describe Reshare do context 'serialization' do it 'serializes root_diaspora_id' do - @xml.should include("root_diaspora_id") + @xml.should include("root_diaspora_id") end it 'serializes root_guid' do - @xml.should include("root_guid") + @xml.should include("root_guid") end end context 'marshalling' do context 'local' do before do - @original_author = @reshare.root.author.dup - @root_object = @reshare.root.dup + @original_author = @reshare.root.author + @root_object = @reshare.root end it 'fetches the root post from root_guid' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f8c7fdaf7..b71da5ee6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -114,13 +114,13 @@ describe User do alice.email = eve.email alice.should_not be_valid end - + it "requires a vaild email address" do alice.email = "somebody@anywhere" alice.should_not be_valid end end - + describe "of unconfirmed_email" do it "unconfirmed_email address can be nil/blank" do alice.unconfirmed_email = nil @@ -134,7 +134,7 @@ describe User do alice.unconfirmed_email = "new@email.com" alice.should be_valid end - + it "requires a vaild unconfirmed_email address" do alice.unconfirmed_email = "somebody@anywhere" alice.should_not be_valid @@ -679,7 +679,7 @@ describe User do user.confirm_email_token.size.should eql(30) end end - + describe '#mail_confirm_email' do it 'enqueues a mail job on user with unconfirmed email' do user.update_attribute(:unconfirmed_email, "alice@newmail.com") @@ -712,14 +712,14 @@ describe User do user.unconfirmed_email.should_not eql(nil) user.confirm_email_token.should_not eql(nil) end - + it 'returns false and does not change anything on blank token' do user.confirm_email("").should eql(false) user.email.should_not eql("alice@newmail.com") user.unconfirmed_email.should_not eql(nil) user.confirm_email_token.should_not eql(nil) end - + it 'returns false and does not change anything on blank token' do user.confirm_email(nil).should eql(false) user.email.should_not eql("alice@newmail.com") @@ -803,8 +803,8 @@ describe User do dis = mock dis_2 = mock - dis.should_receive(:post) - dis_2.should_receive(:post) + dis.should_receive(:post).and_return{ r_ret.perform(@post.reshares.first.author.owner)} + #dis_2.should_receive(:post) Postzord::Dispatch.should_receive(:new).with(bob, r_ret, anything()).and_return(dis, dis_2) fantasy_resque do diff --git a/spec/multi_server/repost_spec.rb b/spec/multi_server/repost_spec.rb index c8ea9f535..39bacb164 100644 --- a/spec/multi_server/repost_spec.rb +++ b/spec/multi_server/repost_spec.rb @@ -19,7 +19,7 @@ unless Server.all.empty? WebMock::Config.instance.allow_localhost = false end before do - Server.all.each{|s| s.truncate_database; puts "Truncating database for server #{s}" } + Server.all.each{|s| s.truncate_database; } @original_post = nil Server[0].in_scope do original_poster = Factory.create(:user_with_aspect, :username => "original_poster") diff --git a/spec/support/server.rb b/spec/support/server.rb index 898a7614a..043224752 100644 --- a/spec/support/server.rb +++ b/spec/support/server.rb @@ -34,7 +34,7 @@ class Server def run @pid = fork do - Process.exec "cd #{Rails.root} && RAILS_ENV=#{@env} bundle exec #{run_command} 2> /dev/null" + Process.exec "cd #{Rails.root} && RAILS_ENV=#{@env} bundle exec #{run_command}"# 2> /dev/null" end end From ad5dba052ca7c2d67e9e7c64ff8d46f84b2ab791 Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 22 Jul 2011 13:12:12 -0700 Subject: [PATCH 20/28] First reshare retraction spec is green --- app/models/signed_retraction.rb | 11 ++++++----- spec/models/signed_retraction_spec.rb | 23 +++++++++++++++++++++++ spec/models/user_spec.rb | 22 ---------------------- spec/multi_server/server_spec.rb | 10 +++++----- 4 files changed, 34 insertions(+), 32 deletions(-) create mode 100644 spec/models/signed_retraction_spec.rb diff --git a/app/models/signed_retraction.rb b/app/models/signed_retraction.rb index 75a4e5477..a499c7d87 100644 --- a/app/models/signed_retraction.rb +++ b/app/models/signed_retraction.rb @@ -22,7 +22,7 @@ class SignedRetraction accessors = self.class.roxml_attrs.collect do |definition| definition.accessor end - accessors - ['target_author_signature'] + accessors - ['target_author_signature', 'sender_handle'] end def sender_handle= new_sender_handle @@ -34,7 +34,7 @@ class SignedRetraction end def diaspora_handle - self.target.diaspora_handle + self.sender_handle end def subscribers(user) @@ -64,9 +64,10 @@ class SignedRetraction def perform receiving_user Rails.logger.debug "Performing retraction for #{target_guid}" - puts "Performing retraction for #{target_guid}" if reshare = Reshare.where(:author_id => receiving_user.person.id, :root_id => target.id).first - Postzord::Dispatch.new(receiving_user, self).post + onward_retraction = self.dup + onward_retraction.sender = receiving_user.person + Postzord::Dispatch.new(receiving_user, onward_retraction).post end self.target.unsocket_from_user receiving_user if target.respond_to? :unsocket_from_user self.target.destroy @@ -81,7 +82,7 @@ class SignedRetraction #this is a retraction from the upstream owner self.perform(recipient) else - Rails.logger.info("event=receive status=abort reason='object signature not valid' recipient=#{recipient.diaspora_handle} sender=#{self.parent.author.diaspora_handle} payload_type=#{self.class} parent_id=#{self.parent.id}") + Rails.logger.info("event=receive status=abort reason='object signature not valid' recipient=#{recipient.diaspora_handle} sender=#{self.sender_handle} payload_type=#{self.class}") return end self diff --git a/spec/models/signed_retraction_spec.rb b/spec/models/signed_retraction_spec.rb new file mode 100644 index 000000000..de5b38da3 --- /dev/null +++ b/spec/models/signed_retraction_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe SignedRetraction do + before do + @post = Factory(:status_message, :author => bob.person, :public => true) + @resharer = Factory(:user) + @post.reshares << Factory.create(:reshare,:root_id => @post.id, :author => @resharer.person) + @post.save! + end + describe '#perform' do + it "dispatches the retraction onward to recipients of the recipient's reshare" do + retraction = SignedRetraction.build(bob, @post) + onward_retraction = retraction.dup + retraction.should_receive(:dup).and_return(onward_retraction) + + dis = mock + Postzord::Dispatch.should_receive(:new).with(@resharer, onward_retraction).and_return(dis) + dis.should_receive(:post) + + retraction.perform(@resharer) + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b71da5ee6..a3abdba86 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -790,27 +790,5 @@ describe User do pending end end - - context "signed retractions" do - before do - @post.reshares << Factory.create(:reshare,:root_id => @post.id, :author => Factory(:user).person) - @post.save! - end - - it 'relays the signed retraction onwards to recipients of reshares' do - r_ret = SignedRetraction.build(bob, @post) - SignedRetraction.should_receive(:build).and_return(r_ret) - - dis = mock - dis_2 = mock - dis.should_receive(:post).and_return{ r_ret.perform(@post.reshares.first.author.owner)} - #dis_2.should_receive(:post) - Postzord::Dispatch.should_receive(:new).with(bob, r_ret, anything()).and_return(dis, dis_2) - - fantasy_resque do - bob.retract(@post) - end - end - end end end diff --git a/spec/multi_server/server_spec.rb b/spec/multi_server/server_spec.rb index e8e01fea3..35f046af3 100644 --- a/spec/multi_server/server_spec.rb +++ b/spec/multi_server/server_spec.rb @@ -5,14 +5,14 @@ unless Server.all.empty? describe Server do before(:all) do WebMock::Config.instance.allow_localhost = true - Server.all.each{|s| s.kill if s.running?} - Server.all.each{|s| s.run} + #Server.all.each{|s| s.kill if s.running?} + #Server.all.each{|s| s.run} end after(:all) do - Server.all.each{|s| s.kill if s.running?} - sleep(1) - Server.all.each{|s| puts "Server at port #{s.port} still running." if s.running?} + #Server.all.each{|s| s.kill if s.running?} + #sleep(1) + #Server.all.each{|s| puts "Server at port #{s.port} still running." if s.running?} WebMock::Config.instance.allow_localhost = false end From 4b1d5b772d4d812c231dbd713c5dcf9197b28002 Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 22 Jul 2011 14:19:58 -0700 Subject: [PATCH 21/28] Failing spec for the second retraction for a reshared post, reshares need to refer to guids --- app/models/reshare.rb | 12 ++++++++++-- spec/models/signed_retraction_spec.rb | 24 ++++++++++++++++++++++++ spec/multi_server/repost_spec.rb | 1 - 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 72e3ccbca..9ba63bcd9 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -43,8 +43,7 @@ class Reshare < Post local_post = Post.where(:guid => @root_guid).select('id').first unless local_post && self.root_id = local_post.id - response = Faraday.get(root_author.url + "/p/#{@root_guid}.xml") - received_post = Diaspora::Parser.from_xml(response.body) + received_post = self.class.fetch_post(root_author, @root_guid) unless post = received_post.class.where(:guid => received_post.guid).first post = received_post @@ -61,6 +60,15 @@ class Reshare < Post end end + # Fetch a remote public post, used for receiving reshares of unknown posts + # @param [Person] author the remote post's author + # @param [String] guid the remote post's guid + # @return [Post] an unsaved remote post + def self.fetch_post author, guid + response = Faraday.get(root_author.url + "/p/#{@root_guid}.xml") + Diaspora::Parser.from_xml(response.body) + end + def root_must_be_public if self.root && !self.root.public errors[:base] << "you must reshare public posts" diff --git a/spec/models/signed_retraction_spec.rb b/spec/models/signed_retraction_spec.rb index de5b38da3..7c86a2ac6 100644 --- a/spec/models/signed_retraction_spec.rb +++ b/spec/models/signed_retraction_spec.rb @@ -19,5 +19,29 @@ describe SignedRetraction do retraction.perform(@resharer) end + it 'relays the retraction onward even if the post does not exist' do + remote_post = Factory(:status_message, :public => true) + bob.post(:reshare, :root_id => remote_post.id) + alice.post(:reshare, :root_id => remote_post.id) + + remote_retraction = SignedRetraction.new.tap{|r| + r.target_type = remote_post.type + r.target_guid = remote_post.guid + r.sender = remote_post.author + r.stub!(:target_author_signature_valid?).and_return(true) + } + + remote_retraction.dup.perform(bob) + Post.exists?(:id => remote_post.id).should be_false + + dis = mock + Postzord::Dispatch.should_receive(:new){ |sender, retraction| + sender.should == alice + retraction.sender.should == alice.person + dis + } + dis.should_receive(:post) + remote_retraction.perform(alice) + end end end diff --git a/spec/multi_server/repost_spec.rb b/spec/multi_server/repost_spec.rb index 39bacb164..1f3e70921 100644 --- a/spec/multi_server/repost_spec.rb +++ b/spec/multi_server/repost_spec.rb @@ -72,6 +72,5 @@ unless Server.all.empty? Post.exists?(:guid => @original_post.guid).should be_false end end - end end From 5a12636967703521025625d27f4a1b1f2182bcf4 Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 22 Jul 2011 14:38:56 -0700 Subject: [PATCH 22/28] Clarify Reshare#after_parse --- app/models/reshare.rb | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 9ba63bcd9..cb1cbad64 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -40,23 +40,20 @@ class Reshare < Post root_author = Webfinger.new(@root_diaspora_id).fetch root_author.save! - local_post = Post.where(:guid => @root_guid).select('id').first + if local_post = Post.where(:guid => @root_guid).select('id').first + self.root_id = local_post.id + return + else + fetched_post = self.class.fetch_post(root_author, @root_guid) - unless local_post && self.root_id = local_post.id - received_post = self.class.fetch_post(root_author, @root_guid) - - unless post = received_post.class.where(:guid => received_post.guid).first - post = received_post - - if root_author.diaspora_handle != post.diaspora_handle - raise "Diaspora ID (#{post.diaspora_handle}) in the root does not match the Diaspora ID (#{root_author.diaspora_handle}) specified in the reshare!" - end - - post.author_id = root_author.id - post.save! + if root_author.diaspora_handle != fetched_post.diaspora_handle + raise "Diaspora ID (#{fetched_post.diaspora_handle}) in the root does not match the Diaspora ID (#{root_author.diaspora_handle}) specified in the reshare!" end - self.root_id = post.id + fetched_post.author_id = root_author.id + fetched_post.save! + + self.root_id = fetched_post.id end end @@ -65,7 +62,7 @@ class Reshare < Post # @param [String] guid the remote post's guid # @return [Post] an unsaved remote post def self.fetch_post author, guid - response = Faraday.get(root_author.url + "/p/#{@root_guid}.xml") + response = Faraday.get(author.url + "/p/#{guid}.xml") Diaspora::Parser.from_xml(response.body) end From 78bced56bb4874796b29b85b617a4b20f7ea9368 Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 22 Jul 2011 16:00:19 -0700 Subject: [PATCH 23/28] Reshares and reshare retractions are green. --- app/controllers/reshares_controller.rb | 4 +- app/models/post.rb | 2 +- app/models/reshare.rb | 40 ++++++++----------- app/models/signed_retraction.rb | 8 ++-- app/views/reshares/create.js.erb | 2 +- app/views/shared/_stream_element.html.haml | 2 +- .../20110525213325_add_root_id_to_posts.rb | 4 +- db/schema.rb | 2 +- spec/controllers/aspects_controller_spec.rb | 2 +- spec/controllers/people_controller_spec.rb | 6 +-- spec/controllers/reshares_controller_spec.rb | 12 +++--- spec/models/reshare_spec.rb | 6 +++ spec/models/retraction_spec.rb | 2 +- spec/models/signed_retraction_spec.rb | 6 +-- spec/multi_server/repost_spec.rb | 2 +- 15 files changed, 50 insertions(+), 50 deletions(-) diff --git a/app/controllers/reshares_controller.rb b/app/controllers/reshares_controller.rb index 9520715c7..d1511f634 100644 --- a/app/controllers/reshares_controller.rb +++ b/app/controllers/reshares_controller.rb @@ -3,9 +3,9 @@ class ResharesController < ApplicationController respond_to :js def create - @reshare = current_user.build_post(:reshare, :root_id => params[:root_id]) + @reshare = current_user.build_post(:reshare, :root_guid => params[:root_guid]) if @reshare.save! - current_user.add_to_streams(@reshare, current_user.aspects) + current_user.add_to_streams(@reshare, current_user.aspects) current_user.dispatch_post(@reshare, :url => post_url(@reshare), :additional_subscribers => @reshare.root.author) end diff --git a/app/models/post.rb b/app/models/post.rb index 01da0e94b..eb7dff48a 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -24,7 +24,7 @@ class Post < ActiveRecord::Base has_many :contacts, :through => :post_visibilities has_many :mentions, :dependent => :destroy - has_many :reshares, :class_name => "Reshare", :foreign_key => :root_id + has_many :reshares, :class_name => "Reshare", :foreign_key => :root_guid, :primary_key => :guid has_many :resharers, :class_name => 'Person', :through => :reshares, :source => :author belongs_to :author, :class_name => 'Person' diff --git a/app/models/reshare.rb b/app/models/reshare.rb index cb1cbad64..5040b85be 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -1,8 +1,8 @@ class Reshare < Post - belongs_to :root, :class_name => 'Post' + belongs_to :root, :class_name => 'Post', :foreign_key => :root_guid, :primary_key => :guid validate :root_must_be_public - attr_accessible :root_id, :public + attr_accessible :root_guid, :public validates_presence_of :root, :on => :create xml_attr :root_diaspora_id @@ -12,25 +12,21 @@ class Reshare < Post self.public = true end - def root_guid - self.root.guid - end - def root_diaspora_id self.root.author.diaspora_handle end - def receive(user, person) + def receive(recipient, sender) local_reshare = Reshare.where(:guid => self.guid).first - if local_reshare && local_reshare.root.author_id == user.person.id + if local_reshare && local_reshare.root.author_id == recipient.person.id local_reshare.root.reshares << local_reshare - if user.contact_for(person) - local_reshare.receive(user, person) + if recipient.contact_for(sender) + local_reshare.receive(recipient, sender) end else - super(user, person) + super(recipient, sender) end end @@ -38,23 +34,19 @@ class Reshare < Post def after_parse root_author = Webfinger.new(@root_diaspora_id).fetch - root_author.save! + root_author.save! unless root_author.persisted? - if local_post = Post.where(:guid => @root_guid).select('id').first - self.root_id = local_post.id - return - else - fetched_post = self.class.fetch_post(root_author, @root_guid) + return if Post.exists?(:guid => self.root_guid) - if root_author.diaspora_handle != fetched_post.diaspora_handle - raise "Diaspora ID (#{fetched_post.diaspora_handle}) in the root does not match the Diaspora ID (#{root_author.diaspora_handle}) specified in the reshare!" - end + fetched_post = self.class.fetch_post(root_author, self.root_guid) - fetched_post.author_id = root_author.id - fetched_post.save! - - self.root_id = fetched_post.id + if root_author.diaspora_handle != fetched_post.diaspora_handle + raise "Diaspora ID (#{fetched_post.diaspora_handle}) in the root does not match the Diaspora ID (#{root_author.diaspora_handle}) specified in the reshare!" end + + #Todo, this is a bug if it is necessary. The marshalling process should set the author. + fetched_post.author_id = root_author.id + fetched_post.save! end # Fetch a remote public post, used for receiving reshares of unknown posts diff --git a/app/models/signed_retraction.rb b/app/models/signed_retraction.rb index a499c7d87..ea152ab76 100644 --- a/app/models/signed_retraction.rb +++ b/app/models/signed_retraction.rb @@ -64,13 +64,15 @@ class SignedRetraction def perform receiving_user Rails.logger.debug "Performing retraction for #{target_guid}" - if reshare = Reshare.where(:author_id => receiving_user.person.id, :root_id => target.id).first + if reshare = Reshare.where(:author_id => receiving_user.person.id, :root_guid => target_guid).first onward_retraction = self.dup onward_retraction.sender = receiving_user.person Postzord::Dispatch.new(receiving_user, onward_retraction).post end - self.target.unsocket_from_user receiving_user if target.respond_to? :unsocket_from_user - self.target.destroy + if target + self.target.unsocket_from_user receiving_user if target.respond_to? :unsocket_from_user + self.target.destroy + end Rails.logger.info(:event => :retraction, :status => :complete, :target_type => self.target_type, :guid => self.target_guid) end diff --git a/app/views/reshares/create.js.erb b/app/views/reshares/create.js.erb index f25d9629f..3c179af45 100644 --- a/app/views/reshares/create.js.erb +++ b/app/views/reshares/create.js.erb @@ -1 +1 @@ -$('.stream_element[data-guid=<%=params[:root_id]%>]').addClass('reshared'); +$('.stream_element[data-guid=<%=params[:root_guid]%>]').addClass('reshared'); diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml index 8c63d93b2..bb5de8f9c 100644 --- a/app/views/shared/_stream_element.html.haml +++ b/app/views/shared/_stream_element.html.haml @@ -61,7 +61,7 @@ - if (post.author_id != current_user.person.id) && (post.public?) && !reshare?(post) · %span.reshare_action - = link_to "#{(post.reshares.size unless post.reshares.size == 0)} Reshare", reshares_path(:root_id => post.id), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" + = link_to "#{(post.reshares.size unless post.reshares.size == 0)} Reshare", reshares_path(:root_guid => post.guid), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" · = link_to t('comments.new_comment.comment'), '#', :class => 'focus_comment_textarea' diff --git a/db/migrate/20110525213325_add_root_id_to_posts.rb b/db/migrate/20110525213325_add_root_id_to_posts.rb index 7f6969b54..8deeed9d4 100644 --- a/db/migrate/20110525213325_add_root_id_to_posts.rb +++ b/db/migrate/20110525213325_add_root_id_to_posts.rb @@ -1,9 +1,9 @@ class AddRootIdToPosts < ActiveRecord::Migration def self.up - add_column :posts, :root_id, :integer + add_column :posts, :root_guid, :string, :limit => 30 end def self.down - remove_column :posts, :root_id + remove_column :posts, :root_guid end end diff --git a/db/schema.rb b/db/schema.rb index da4789c6a..4ce633fcb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -268,7 +268,7 @@ ActiveRecord::Schema.define(:version => 20110707234802) do t.string "provider_display_name" t.string "actor_url" t.integer "objectId" - t.integer "root_id" + t.string "root_guid", :limit => 30 t.string "status_message_guid" t.integer "likes_count", :default => 0 end diff --git a/spec/controllers/aspects_controller_spec.rb b/spec/controllers/aspects_controller_spec.rb index f0a2794a6..5c2185320 100644 --- a/spec/controllers/aspects_controller_spec.rb +++ b/spec/controllers/aspects_controller_spec.rb @@ -180,7 +180,7 @@ describe AspectsController do end it "posts include reshares" do - reshare = alice.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id, :to => alice.aspects) + reshare = alice.post(:reshare, :public => true, :root_guid => Factory(:status_message, :public => true).guid, :to => alice.aspects) get :index assigns[:posts].post_fakes.map{|x| x.id}.should include(reshare.id) end diff --git a/spec/controllers/people_controller_spec.rb b/spec/controllers/people_controller_spec.rb index 7feb2b478..6b7e73735 100644 --- a/spec/controllers/people_controller_spec.rb +++ b/spec/controllers/people_controller_spec.rb @@ -206,7 +206,7 @@ describe PeopleController do end it "posts include reshares" do - reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id, :to => alice.aspects) + reshare = @user.post(:reshare, :public => true, :root_guid => Factory(:status_message, :public => true).guid, :to => alice.aspects) get :show, :id => @user.person.id assigns[:posts].post_fakes.map{|x| x.id}.should include(reshare.id) end @@ -259,7 +259,7 @@ describe PeopleController do end it "posts include reshares" do - reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id, :to => alice.aspects) + reshare = @user.post(:reshare, :public => true, :root_guid => Factory(:status_message, :public => true).guid, :to => alice.aspects) get :show, :id => @user.person.id assigns[:posts].post_fakes.map{|x| x.id}.should include(reshare.id) end @@ -297,7 +297,7 @@ describe PeopleController do end it "posts include reshares" do - reshare = @user.post(:reshare, :public => true, :root_id => Factory(:status_message, :public => true).id, :to => alice.aspects) + reshare = @user.post(:reshare, :public => true, :root_guid => Factory(:status_message, :public => true).guid, :to => alice.aspects) get :show, :id => @user.person.id assigns[:posts].post_fakes.map{|x| x.id}.should include(reshare.id) end diff --git a/spec/controllers/reshares_controller_spec.rb b/spec/controllers/reshares_controller_spec.rb index 5dbaaab7a..daea3162b 100644 --- a/spec/controllers/reshares_controller_spec.rb +++ b/spec/controllers/reshares_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe ResharesController do - describe '#create' do + describe '#create' do it 'requires authentication' do post :create, :format => :js response.should_not be_success @@ -10,29 +10,29 @@ describe ResharesController do context 'with an authenticated user' do before do sign_in :user, bob - @post_id = Factory(:status_message, :public => true).id + @post_guid = Factory(:status_message, :public => true).guid @controller.stub(:current_user).and_return(bob) end it 'succeeds' do - post :create, :format => :js, :root_id => @post_id + post :create, :format => :js, :root_guid => @post_guid response.should be_success end it 'creates a reshare' do expect{ - post :create, :format => :js, :root_id => @post_id + post :create, :format => :js, :root_guid => @post_guid }.should change(Reshare, :count).by(1) end it 'after save, calls add to streams' do bob.should_receive(:add_to_streams) - post :create, :format => :js, :root_id => @post_id + post :create, :format => :js, :root_guid => @post_guid end it 'calls dispatch' do bob.should_receive(:dispatch_post).with(anything, hash_including(:additional_subscribers)) - post :create, :format => :js, :root_id => @post_id + post :create, :format => :js, :root_guid => @post_guid end end end diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb index cc24ef8e9..1839cc363 100644 --- a/spec/models/reshare_spec.rb +++ b/spec/models/reshare_spec.rb @@ -50,10 +50,12 @@ describe Reshare do context 'serialization' do it 'serializes root_diaspora_id' do @xml.should include("root_diaspora_id") + @xml.should include(@reshare.author.diaspora_handle) end it 'serializes root_guid' do @xml.should include("root_guid") + @xml.should include(@reshare.root.guid) end end @@ -64,6 +66,10 @@ describe Reshare do @root_object = @reshare.root end + it 'marshals the guid' do + Reshare.from_xml(@xml).root_guid.should == @root_object.guid + end + it 'fetches the root post from root_guid' do Reshare.from_xml(@xml).root.should == @root_object end diff --git a/spec/models/retraction_spec.rb b/spec/models/retraction_spec.rb index 0d96356c5..b5a0b1c84 100644 --- a/spec/models/retraction_spec.rb +++ b/spec/models/retraction_spec.rb @@ -33,7 +33,7 @@ describe Retraction do it 'does not return the authors of reshares' do @post.reshares << Factory.create(:reshare, :root => @post, :author => bob.person) - @post.save! + @post.save! @wanted_subscribers -= [bob.person] @retraction.subscribers(alice).map(&:id).should =~ @wanted_subscribers.map(&:id) diff --git a/spec/models/signed_retraction_spec.rb b/spec/models/signed_retraction_spec.rb index 7c86a2ac6..cc9e7ef61 100644 --- a/spec/models/signed_retraction_spec.rb +++ b/spec/models/signed_retraction_spec.rb @@ -4,7 +4,7 @@ describe SignedRetraction do before do @post = Factory(:status_message, :author => bob.person, :public => true) @resharer = Factory(:user) - @post.reshares << Factory.create(:reshare,:root_id => @post.id, :author => @resharer.person) + @post.reshares << Factory.create(:reshare, :root => @post, :author => @resharer.person) @post.save! end describe '#perform' do @@ -21,8 +21,8 @@ describe SignedRetraction do end it 'relays the retraction onward even if the post does not exist' do remote_post = Factory(:status_message, :public => true) - bob.post(:reshare, :root_id => remote_post.id) - alice.post(:reshare, :root_id => remote_post.id) + bob.post(:reshare, :root_guid => remote_post.guid) + alice.post(:reshare, :root_guid => remote_post.guid) remote_retraction = SignedRetraction.new.tap{|r| r.target_type = remote_post.type diff --git a/spec/multi_server/repost_spec.rb b/spec/multi_server/repost_spec.rb index 1f3e70921..a734ec2dd 100644 --- a/spec/multi_server/repost_spec.rb +++ b/spec/multi_server/repost_spec.rb @@ -50,7 +50,7 @@ unless Server.all.empty? Server[0].in_scope do r = User.find_by_username("resharer") - r.post(:reshare, :root_id => @original_post.id, :to => 'all') + r.post(:reshare, :root_guid => @original_post.guid, :to => 'all') end end From 0e2936f069ebdc9a5748d024063b4ff622415c73 Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 22 Jul 2011 16:03:54 -0700 Subject: [PATCH 24/28] Reshare spec, not repost spec --- spec/multi_server/{repost_spec.rb => reshare_spec.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/multi_server/{repost_spec.rb => reshare_spec.rb} (100%) diff --git a/spec/multi_server/repost_spec.rb b/spec/multi_server/reshare_spec.rb similarity index 100% rename from spec/multi_server/repost_spec.rb rename to spec/multi_server/reshare_spec.rb From f74bf047982e456564be0d284e82e9c0135790a1 Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 22 Jul 2011 16:44:24 -0700 Subject: [PATCH 25/28] Fix infieldLabels selector --- public/javascripts/view.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/public/javascripts/view.js b/public/javascripts/view.js index 57918d31d..41ed4820e 100644 --- a/public/javascripts/view.js +++ b/public/javascripts/view.js @@ -17,11 +17,11 @@ var View = { }); Diaspora.widgets.subscribe("stream/scrolled", function() { - $('#main_stream .comments label').inFieldLabels(); + $('#main_stream label').inFieldLabels(); }); Diaspora.widgets.subscribe("stream/reloaded", function() { - $('#main_stream .comments label').inFieldLabels(); + $('#main_stream label').inFieldLabels(); }); From 99aafb18cd041cf382fbd1fc795231e5d3ec62df Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 22 Jul 2011 17:13:31 -0700 Subject: [PATCH 26/28] Reshares show up on sharing, prevent multi-resharing, still need to make that ui nice --- app/models/reshare.rb | 4 ++-- app/views/reshares/_reshare.haml | 4 ++-- app/views/reshares/create.js.erb | 5 ++++- spec/factories.rb | 1 + 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 5040b85be..f23569791 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -4,6 +4,7 @@ class Reshare < Post validate :root_must_be_public attr_accessible :root_guid, :public validates_presence_of :root, :on => :create + validates_uniqueness_of :root_guid, :scope => :author_id xml_attr :root_diaspora_id xml_attr :root_guid @@ -40,12 +41,11 @@ class Reshare < Post fetched_post = self.class.fetch_post(root_author, self.root_guid) + #Why are we checking for this? if root_author.diaspora_handle != fetched_post.diaspora_handle raise "Diaspora ID (#{fetched_post.diaspora_handle}) in the root does not match the Diaspora ID (#{root_author.diaspora_handle}) specified in the reshare!" end - #Todo, this is a bug if it is necessary. The marshalling process should set the author. - fetched_post.author_id = root_author.id fetched_post.save! end diff --git a/app/views/reshares/_reshare.haml b/app/views/reshares/_reshare.haml index 2a71f683a..1c773cdca 100644 --- a/app/views/reshares/_reshare.haml +++ b/app/views/reshares/_reshare.haml @@ -8,7 +8,7 @@ = person_image_link(post.author, :size => :thumb_small) .content - .right + .right = link_to "Show Original", post_path(post) %span.from = person_link(post.author, :class => "hovercardable") @@ -19,7 +19,7 @@ = render 'status_messages/status_message', :post => post, :photos => post.photos - if defined?(current_user) && current_user && (post.author_id != current_user.person.id) && (post.public?) && !reshare?(post) %span.reshare_action - = link_to "Reshare", reshares_path(:root_id => post.id), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" + = link_to "Reshare", reshares_path(:root_guid => post.guid), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" - else Original post deleted by author diff --git a/app/views/reshares/create.js.erb b/app/views/reshares/create.js.erb index 3c179af45..ec6f7e515 100644 --- a/app/views/reshares/create.js.erb +++ b/app/views/reshares/create.js.erb @@ -1 +1,4 @@ -$('.stream_element[data-guid=<%=params[:root_guid]%>]').addClass('reshared'); +$('.stream_element#<%=params[:root_guid]%>').addClass('reshared'); +ContentUpdater.addPostToStream( + "<%= escape_javascript(render(:partial => 'shared/stream_element', :locals => {:post => @reshare }))=%>" +); diff --git a/spec/factories.rb b/spec/factories.rb index b637cc97d..ab39142e6 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -41,6 +41,7 @@ Factory.define :like do |x| end Factory.define :user do |u| + u.getting_started false u.sequence(:username) { |n| "bob#{n}#{r_str}" } u.sequence(:email) { |n| "bob#{n}#{r_str}@pivotallabs.com" } u.password "bluepin7" From 00885be738a2e737e7ed18d016eb7a537213d49e Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 22 Jul 2011 17:43:05 -0700 Subject: [PATCH 27/28] Provide feedback when a user tries to double-reshare --- app/controllers/reshares_controller.rb | 2 +- app/helpers/reshares_helper.rb | 7 +++++++ app/views/reshares/create.js.erb | 13 +++++++++---- config/locales/diaspora/en-US.yml | 1 + config/locales/diaspora/en.yml | 10 +++++++--- config/locales/javascript/javascript.en.yml | 2 ++ 6 files changed, 27 insertions(+), 8 deletions(-) diff --git a/app/controllers/reshares_controller.rb b/app/controllers/reshares_controller.rb index d1511f634..856426ef7 100644 --- a/app/controllers/reshares_controller.rb +++ b/app/controllers/reshares_controller.rb @@ -4,7 +4,7 @@ class ResharesController < ApplicationController def create @reshare = current_user.build_post(:reshare, :root_guid => params[:root_guid]) - if @reshare.save! + if @reshare.save current_user.add_to_streams(@reshare, current_user.aspects) current_user.dispatch_post(@reshare, :url => post_url(@reshare), :additional_subscribers => @reshare.root.author) end diff --git a/app/helpers/reshares_helper.rb b/app/helpers/reshares_helper.rb index f1b23f522..8c6cf0ea6 100644 --- a/app/helpers/reshares_helper.rb +++ b/app/helpers/reshares_helper.rb @@ -1,2 +1,9 @@ module ResharesHelper + def reshare_error_message(reshare) + if @reshare.errors[:root_guid].present? + escape_javascript(@reshare.errors[:root_guid].first) + else + escape_javascript(t('reshares.create.failure')) + end + end end diff --git a/app/views/reshares/create.js.erb b/app/views/reshares/create.js.erb index ec6f7e515..f56a8218c 100644 --- a/app/views/reshares/create.js.erb +++ b/app/views/reshares/create.js.erb @@ -1,4 +1,9 @@ -$('.stream_element#<%=params[:root_guid]%>').addClass('reshared'); -ContentUpdater.addPostToStream( - "<%= escape_javascript(render(:partial => 'shared/stream_element', :locals => {:post => @reshare }))=%>" -); +<% if @reshare.persisted? %> + $('.stream_element#<%=params[:root_guid]%>').addClass('reshared'); + ContentUpdater.addPostToStream( + "<%= escape_javascript(render(:partial => 'shared/stream_element', :locals => {:post => @reshare }))=%>" + ); + <% else %> + Diaspora.widgets.flashes.render({success:false, + notice: "<%= reshare_error_message(@reshare) %>"}); +<% end %> diff --git a/config/locales/diaspora/en-US.yml b/config/locales/diaspora/en-US.yml index 55523b72a..8cb2569f2 100644 --- a/config/locales/diaspora/en-US.yml +++ b/config/locales/diaspora/en-US.yml @@ -26,3 +26,4 @@ attributes: from_id: taken: "is a duplicate of a pre-existing request." + diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index b08d09126..3ca403ccf 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -65,8 +65,10 @@ en: attributes: from_id: taken: "is a duplicate of a pre-existing request." - - + reshare: + attributes: + root_guid: + taken: "You've already reshared that post!" error_messages: helper: invalid_fields: "Invalid Fields" @@ -579,7 +581,9 @@ en: few: "%{count} new requests!" many: "%{count} new requests!" other: "%{count} new requests!" - + reshares: + create: + failure: "There was an error resharing this post." services: index: logged_in_as: "logged in as" diff --git a/config/locales/javascript/javascript.en.yml b/config/locales/javascript/javascript.en.yml index 41e44f1db..bd393d873 100644 --- a/config/locales/javascript/javascript.en.yml +++ b/config/locales/javascript/javascript.en.yml @@ -47,3 +47,5 @@ en: comments: show: "show all comments" hide: "hide comments" + reshares: + duplicate:"You've already reshared that post!" From 241999f72a89f91e67223519991bd0c0cdf24a0b Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 22 Jul 2011 17:58:52 -0700 Subject: [PATCH 28/28] MS RS translated reshares --- app/helpers/reshares_helper.rb | 4 ++++ app/views/reshares/_reshare.haml | 6 +++--- app/views/shared/_stream_element.html.haml | 2 +- config/locales/diaspora/en.yml | 10 ++++++++++ 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/app/helpers/reshares_helper.rb b/app/helpers/reshares_helper.rb index 8c6cf0ea6..6ba1a0241 100644 --- a/app/helpers/reshares_helper.rb +++ b/app/helpers/reshares_helper.rb @@ -6,4 +6,8 @@ module ResharesHelper escape_javascript(t('reshares.create.failure')) end end + + def reshare_link post + link_to t("reshares.reshare.reshare", :count => post.reshares.size), reshares_path(:root_guid => post.guid), :method => :post, :remote => true, :confirm => t('reshares.reshare.reshare_confirmation', :author => post.author.name, :text => post.text) + end end diff --git a/app/views/reshares/_reshare.haml b/app/views/reshares/_reshare.haml index 1c773cdca..c504e292c 100644 --- a/app/views/reshares/_reshare.haml +++ b/app/views/reshares/_reshare.haml @@ -9,7 +9,7 @@ .content .right - = link_to "Show Original", post_path(post) + = link_to t("show_original"), post_path(post) %span.from = person_link(post.author, :class => "hovercardable") @@ -19,7 +19,7 @@ = render 'status_messages/status_message', :post => post, :photos => post.photos - if defined?(current_user) && current_user && (post.author_id != current_user.person.id) && (post.public?) && !reshare?(post) %span.reshare_action - = link_to "Reshare", reshares_path(:root_guid => post.guid), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" + = reshare_link(post) - else - Original post deleted by author + = t('.deleted') diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml index bb5de8f9c..5de61e812 100644 --- a/app/views/shared/_stream_element.html.haml +++ b/app/views/shared/_stream_element.html.haml @@ -61,7 +61,7 @@ - if (post.author_id != current_user.person.id) && (post.public?) && !reshare?(post) · %span.reshare_action - = link_to "#{(post.reshares.size unless post.reshares.size == 0)} Reshare", reshares_path(:root_guid => post.guid), :method => :post, :remote => true, :confirm => "Reshare: #{post.author.name} - #{post.text}?" + = reshare_link(post) · = link_to t('comments.new_comment.comment'), '#', :class => 'focus_comment_textarea' diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 3ca403ccf..24a291caf 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -582,6 +582,16 @@ en: many: "%{count} new requests!" other: "%{count} new requests!" reshares: + reshare: + reshare: + zero: "Reshare" + one: "1 Reshare" + few: "%{count} Reshares" + many: "%{count} Reshares" + other: "%{count} Reshares" + show_original: "Show Original" + reshare_confirmation: "Reshare %{author} - %{text}?" + deleted: "Original post deleted by author." create: failure: "There was an error resharing this post." services: