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

@@ -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
 

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'

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)
+  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/#{service_post_id}/", {:access_token => self.access_token})
+      delete_from_facebook("https://graph.facebook.com/#{post.facebook_id}/", {:access_token => self.access_token})
+    end
   end
 
   def delete_from_facebook(url, body)

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)
+  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(service_post_id)
+      delete_from_twitter(post.tweet_id)
+    end
   end
 
   def delete_from_twitter(service_post_id)

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

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|
+      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.id)
-        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

spec/lib/postzord/dispatcher_spec.rb

@@ -319,14 +319,6 @@ describe Postzord::Dispatcher do
         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