From 78bced56bb4874796b29b85b617a4b20f7ea9368 Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 22 Jul 2011 16:00:19 -0700 Subject: [PATCH] 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