don't force remove contact on block

this creates inconsistent states, if you remove the block in the future

app/controllers/blocks_controller.rb

@@ -35,9 +35,7 @@ class BlocksController < ApplicationController
   private
 
   def disconnect_if_contact(person)
-    if contact = current_user.contact_for(person)
-      current_user.disconnect(contact, :force => true)
-    end
+    current_user.contact_for(person).try {|contact| current_user.disconnect(contact) }
   end
 
   def block_params

app/models/user/connecting.rb

@@ -27,29 +27,28 @@ class User
       contact
     end
 
-    def disconnect(contact, opts={force: false})
+    def disconnect(contact)
       logger.info "event=disconnect user=#{diaspora_handle} target=#{contact.person.diaspora_handle}"
 
       # TODO: send retraction
 
       contact.aspect_memberships.delete_all
 
-      if !contact.sharing || opts[:force]
-        contact.destroy
-      else
-        contact.update_attributes(receiving: false)
-      end
+      disconnect_contact(contact, direction: :receiving, destroy: !contact.sharing)
     end
 
     def disconnected_by(person)
       logger.info "event=disconnected_by user=#{diaspora_handle} target=#{person.diaspora_handle}"
-      contact = contact_for(person)
-      return unless contact
+      contact_for(person).try {|contact| disconnect_contact(contact, direction: :sharing, destroy: !contact.receiving) }
+    end
 
-      if contact.receiving
-        contact.update_attributes(sharing: false)
-      else
+    private
+
+    def disconnect_contact(contact, direction:, destroy:)
+      if destroy
         contact.destroy
+      else
+        contact.update_attributes(direction => false)
       end
     end
   end

spec/controllers/blocks_controller_spec.rb

@@ -55,7 +55,7 @@ describe BlocksController, :type => :controller do
     it "calls disconnect with the force option if there is a contact for a given user" do
       contact = alice.contact_for(bob.person)
       allow(alice).to receive(:contact_for).and_return(contact)
-      expect(alice).to receive(:disconnect).with(contact, hash_including(:force => true))
+      expect(alice).to receive(:disconnect).with(contact)
       @controller.send(:disconnect_if_contact, bob.person)
     end
 

spec/integration/receiving_spec.rb

@@ -45,9 +45,11 @@ describe 'a user receives a post', :type => :request do
     end
 
     it "does not remove visibility on disconnect" do
-      alice.disconnect(alice.contact_for(bob.person), force: true)
-      alice.reload
-      expect(ShareVisibility.find_by(user_id: alice.id, shareable_id: @status_message.id)).not_to be_nil
+      contact = alice.contact_for(bob.person)
+      alice.disconnect(contact)
+      contact.destroy
+
+      expect(ShareVisibility.exists?(user_id: alice.id, shareable_id: @status_message.id)).to be_truthy
     end
   end
 end