From a8655e2e8dce5a0433ddabf4c164316d1e70c821 Mon Sep 17 00:00:00 2001 From: Ruxton Date: Mon, 29 Apr 2013 21:18:24 +0800 Subject: [PATCH 1/4] Add tweet_id column to post --- db/migrate/20130429073928_add_tweet_id_to_post.rb | 6 ++++++ db/schema.rb | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20130429073928_add_tweet_id_to_post.rb diff --git a/db/migrate/20130429073928_add_tweet_id_to_post.rb b/db/migrate/20130429073928_add_tweet_id_to_post.rb new file mode 100644 index 000000000..3d42da98c --- /dev/null +++ b/db/migrate/20130429073928_add_tweet_id_to_post.rb @@ -0,0 +1,6 @@ +class AddTweetIdToPost < ActiveRecord::Migration + def change + add_column :posts, :tweet_id, :string + add_index :posts, :tweet_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 79e696a65..22f5dda6d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20130404211624) do +ActiveRecord::Schema.define(:version => 20130429073928) do create_table "account_deletions", :force => true do |t| t.string "diaspora_handle" @@ -307,6 +307,7 @@ ActiveRecord::Schema.define(:version => 20130404211624) do t.string "frame_name" t.boolean "favorite", :default => false t.string "facebook_id" + t.string "tweet_id" end add_index "posts", ["author_id", "root_guid"], :name => "index_posts_on_author_id_and_root_guid", :unique => true @@ -316,6 +317,7 @@ ActiveRecord::Schema.define(:version => 20130404211624) do add_index "posts", ["root_guid"], :name => "index_posts_on_root_guid" add_index "posts", ["status_message_guid", "pending"], :name => "index_posts_on_status_message_guid_and_pending" add_index "posts", ["status_message_guid"], :name => "index_posts_on_status_message_guid" + add_index "posts", ["tweet_id"], :name => "index_posts_on_tweet_id" add_index "posts", ["type", "pending", "id"], :name => "index_posts_on_type_and_pending_and_id" create_table "profiles", :force => true do |t| From ba0e2509c90215bd6096b0f0766e19095092a9a6 Mon Sep 17 00:00:00 2001 From: Ruxton Date: Mon, 29 Apr 2013 21:19:11 +0800 Subject: [PATCH 2/4] Twitter service removes status from twitter when post is revoked --- Changelog.md | 6 +++--- app/models/services/twitter.rb | 14 ++++++++++++-- lib/postzord/dispatcher.rb | 5 ++++- spec/models/services/twitter_spec.rb | 13 +++++++++++-- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/Changelog.md b/Changelog.md index cf62724cc..ce62a1def 100644 --- a/Changelog.md +++ b/Changelog.md @@ -10,6 +10,7 @@ ## Features +* Deleting a post that was shared to Twitter now deletes it from Twitter too [#4135](https://github.com/diaspora/diaspora/pull/4135) # 0.1.0.0 @@ -139,7 +140,7 @@ everything is set up. * Fix reshares in single post-view [#4056](https://github.com/diaspora/diaspora/issues/4056) * Fix mobile view of deleted reshares. [#4063](https://github.com/diaspora/diaspora/issues/4063) * Hide comment button in the mobile view when not signed in. [#4065](https://github.com/diaspora/diaspora/issues/4065) -* Send profile alongside notification [#3976] (https://github.com/diaspora/diaspora/issues/3976) +* Send profile alongside notification [#3976](https://github.com/diaspora/diaspora/issues/3976) * Fix off-center close button image on intro popovers [#3841](https://github.com/diaspora/diaspora/pull/3841) * Remove unnecessary dotted CSS borders. [#2940](https://github.com/diaspora/diaspora/issues/2940) * Fix default image url in profiles table. [#3795](https://github.com/diaspora/diaspora/issues/3795) @@ -159,7 +160,7 @@ everything is set up. ## Features -* Deleting a post that was shared to Facebook now deletes it from Facebook too [#3980]( https://github.com/diaspora/diaspora/pull/3980) +* Deleting a post that was shared to Facebook now deletes it from Facebook too [#3980](https://github.com/diaspora/diaspora/pull/3980) * Include reshares in a users public atom feed [#1781](https://github.com/diaspora/diaspora/issues/1781) * Add the ability to upload photos from the mobile site. [#4004](https://github.com/diaspora/diaspora/issues/4004) * Show timestamp when hovering on comment time-ago string. [#4042](https://github.com/diaspora/diaspora/issues/4042) @@ -212,7 +213,6 @@ everything is set up. * uglifier 1.3.0 -> 2.0.1 * unicorn 4.6.0 -> 4.6.2 - # 0.0.3.4 * Bump Rails to 3.2.13, fixes CVE-2013-1854, CVE-2013-1855, CVE-2013-1856 and CVE-2013-1857. [Read more](http://weblog.rubyonrails.org/2013/3/18/SEC-ANN-Rails-3-2-13-3-1-12-and-2-3-18-have-been-released/) diff --git a/app/models/services/twitter.rb b/app/models/services/twitter.rb index f874ecb48..377b11d0e 100644 --- a/app/models/services/twitter.rb +++ b/app/models/services/twitter.rb @@ -9,8 +9,9 @@ class Services::Twitter < Service def post(post, url='') Rails.logger.debug("event=post_to_service type=twitter sender_id=#{self.user_id}") message = public_message(post, url) - - client.update(message) + tweet = client.update(message) + post.tweet_id = tweet.id + post.save end @@ -28,6 +29,15 @@ class Services::Twitter < Service client.user(nickname).profile_image_url_https("original") end + def delete_post(service_post_id) + Rails.logger.debug("event=delete_from_service type=twitter sender_id=#{self.user_id}") + delete_from_twitter(service_post_id) + end + + def delete_from_twitter(service_post_id) + client.status_destroy(service_post_id) + end + private def client @client ||= Twitter::Client.new( diff --git a/lib/postzord/dispatcher.rb b/lib/postzord/dispatcher.rb index a3b986b84..997322cef 100644 --- a/lib/postzord/dispatcher.rb +++ b/lib/postzord/dispatcher.rb @@ -151,9 +151,12 @@ class Postzord::Dispatcher end end if @object.instance_of?(SignedRetraction) - services.select { |service| service.respond_to? :delete_post }.each do |service| + services.select { |service| service.provider == "facebook" }.each do |service| Workers::DeletePostFromService.perform_async(service.id, @object.target.facebook_id) end + services.select { |service| service.provider == "twitter" }.each do |service| + Workers::DeletePostFromService.perform_async(service.id, @object.target.tweet_id) + end end end diff --git a/spec/models/services/twitter_spec.rb b/spec/models/services/twitter_spec.rb index 8d888fe6c..2fe9adbea 100644 --- a/spec/models/services/twitter_spec.rb +++ b/spec/models/services/twitter_spec.rb @@ -10,19 +10,28 @@ describe Services::Twitter do end describe '#post' do + + before do + Twitter::Client.any_instance.stub(:update) { Twitter::Tweet.new(id: "1234") } + end + it 'posts a status message to twitter' do Twitter::Client.any_instance.should_receive(:update).with(instance_of(String)) @service.post(@post) end - it 'swallows exception raised by twitter always being down' do + it 'sets the tweet_id on the post' do + @service.post(@post) + @post.tweet_id.should match "1234" + end + + it 'swallows exception raised by twitter always being down' do pending Twitter::Client.any_instance.should_receive(:update).and_raise(StandardError) @service.post(@post) end it 'should call public message' do - Twitter::Client.any_instance.stub(:update) url = "foo" @service.should_receive(:public_message).with(@post, url) @service.post(@post, url) From 20c38a0489c03ff9c0e451845754b05931192ebf Mon Sep 17 00:00:00 2001 From: Ruxton Date: Mon, 29 Apr 2013 22:30:40 +0800 Subject: [PATCH 3/4] Fix dispatcher_spec to depend on Tumblr which isn't retracting --- spec/lib/postzord/dispatcher_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/postzord/dispatcher_spec.rb b/spec/lib/postzord/dispatcher_spec.rb index 23af27bf0..c5b7c80e4 100644 --- a/spec/lib/postzord/dispatcher_spec.rb +++ b/spec/lib/postzord/dispatcher_spec.rb @@ -321,7 +321,7 @@ describe Postzord::Dispatcher do it "doesn't queue a job if we can't delete the post from the service" do retraction = SignedRetraction.build(alice, FactoryGirl.create(:status_message)) - service = Services::Twitter.new(access_token: "nope") + service = Services::Tumblr.new(access_token: "nope") mailman = Postzord::Dispatcher.build(alice, retraction, :url => "http://joindiaspora.com/p/123", :services => [service]) Workers::DeletePostFromService.should_not_receive(:perform_async).with(anything, anything) From 48b48470bcae0fb8c6c69009b9cfafdbca1fc79e Mon Sep 17 00:00:00 2001 From: Ruxton Date: Mon, 20 May 2013 11:44:59 +0800 Subject: [PATCH 4/4] Service.delete_post now more generic, for future implementations Service.delete_post now accepts post to be more generic and supports deleting post from any service that overrides delete_post(post) --- Changelog.md | 2 +- app/models/service.rb | 4 ++++ app/models/services/facebook.rb | 8 +++++--- app/models/services/twitter.rb | 8 +++++--- app/workers/delete_post_from_service.rb | 5 +++-- lib/postzord/dispatcher.rb | 13 ++++--------- spec/lib/postzord/dispatcher_spec.rb | 10 +--------- 7 files changed, 23 insertions(+), 27 deletions(-) diff --git a/Changelog.md b/Changelog.md index ce62a1def..f9bbbed58 100644 --- a/Changelog.md +++ b/Changelog.md @@ -10,7 +10,7 @@ ## Features -* Deleting a post that was shared to Twitter now deletes it from Twitter too [#4135](https://github.com/diaspora/diaspora/pull/4135) +* Deleting a post that was shared to Twitter now deletes it from Twitter too [#4156](https://github.com/diaspora/diaspora/pull/4156) # 0.1.0.0 diff --git a/app/models/service.rb b/app/models/service.rb index beb41fa96..070a81732 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -37,6 +37,10 @@ class Service < ActiveRecord::Base nil end + def delete_post(post) + #don't do anything (should be overriden by service extensions) + end + end require 'services/facebook' require 'services/twitter' diff --git a/app/models/services/facebook.rb b/app/models/services/facebook.rb index 93ba07ef5..83d4da631 100644 --- a/app/models/services/facebook.rb +++ b/app/models/services/facebook.rb @@ -37,9 +37,11 @@ class Services::Facebook < Service "https://graph.facebook.com/#{self.uid}/picture?type=large&access_token=#{URI.escape(self.access_token)}" end - def delete_post(service_post_id) - Rails.logger.debug("event=delete_from_service type=facebook sender_id=#{self.user_id}") - delete_from_facebook("https://graph.facebook.com/#{service_post_id}/", {:access_token => self.access_token}) + def delete_post(post) + if post.present? && post.facebbook_id.present? + Rails.logger.debug("event=delete_from_service type=facebook sender_id=#{self.user_id}") + delete_from_facebook("https://graph.facebook.com/#{post.facebook_id}/", {:access_token => self.access_token}) + end end def delete_from_facebook(url, body) diff --git a/app/models/services/twitter.rb b/app/models/services/twitter.rb index 377b11d0e..a640f6f5c 100644 --- a/app/models/services/twitter.rb +++ b/app/models/services/twitter.rb @@ -29,9 +29,11 @@ class Services::Twitter < Service client.user(nickname).profile_image_url_https("original") end - def delete_post(service_post_id) - Rails.logger.debug("event=delete_from_service type=twitter sender_id=#{self.user_id}") - delete_from_twitter(service_post_id) + def delete_post(post) + if post.present? && post.tweet_id.present? + Rails.logger.debug("event=delete_from_service type=twitter sender_id=#{self.user_id}") + delete_from_twitter(post.tweet_id) + end end def delete_from_twitter(service_post_id) diff --git a/app/workers/delete_post_from_service.rb b/app/workers/delete_post_from_service.rb index b790f8d88..9efd3bb18 100644 --- a/app/workers/delete_post_from_service.rb +++ b/app/workers/delete_post_from_service.rb @@ -6,9 +6,10 @@ module Workers class DeletePostFromService < Base sidekiq_options queue: :http_service - def perform(service_id, service_post_id) + def perform(service_id, post_id) service = Service.find_by_id(service_id) - service.delete_post(service_post_id) + post = Post.find_by_id(post_id) + service.delete_post(post) end end end diff --git a/lib/postzord/dispatcher.rb b/lib/postzord/dispatcher.rb index 997322cef..01ead3da0 100644 --- a/lib/postzord/dispatcher.rb +++ b/lib/postzord/dispatcher.rb @@ -145,17 +145,12 @@ class Postzord::Dispatcher if @object.respond_to?(:public) && @object.public deliver_to_hub end - if @object.instance_of?(StatusMessage) - services.each do |service| + services.each do |service| + if @object.instance_of?(StatusMessage) Workers::PostToService.perform_async(service.id, @object.id, url) end - end - if @object.instance_of?(SignedRetraction) - services.select { |service| service.provider == "facebook" }.each do |service| - Workers::DeletePostFromService.perform_async(service.id, @object.target.facebook_id) - end - services.select { |service| service.provider == "twitter" }.each do |service| - Workers::DeletePostFromService.perform_async(service.id, @object.target.tweet_id) + if @object.instance_of?(SignedRetraction) + Workers::DeletePostFromService.perform_async(service.id, @object.target.id) end end end diff --git a/spec/lib/postzord/dispatcher_spec.rb b/spec/lib/postzord/dispatcher_spec.rb index c5b7c80e4..d5fc319e8 100644 --- a/spec/lib/postzord/dispatcher_spec.rb +++ b/spec/lib/postzord/dispatcher_spec.rb @@ -318,15 +318,7 @@ describe Postzord::Dispatcher do Workers::DeletePostFromService.should_receive(:perform_async).with(anything, anything) mailman.post end - - it "doesn't queue a job if we can't delete the post from the service" do - retraction = SignedRetraction.build(alice, FactoryGirl.create(:status_message)) - service = Services::Tumblr.new(access_token: "nope") - mailman = Postzord::Dispatcher.build(alice, retraction, :url => "http://joindiaspora.com/p/123", :services => [service]) - - Workers::DeletePostFromService.should_not_receive(:perform_async).with(anything, anything) - mailman.post - end + end describe '#and_notify_local_users' do