Add API route to (un)block a user

app/controllers/api/v1/users_controller.rb

@@ -17,6 +17,10 @@ module Api
         require_access_token %w[contacts:read]
       end
 
+      before_action only: %i[block] do
+        require_access_token %w[contacts:modify]
+      end
+
       before_action only: %i[show] do
         require_access_token %w[profile]
       end
@@ -81,6 +85,28 @@ module Api
         render_paged_api_response posts_page
       end
 
+      def block
+        person = Person.find_by!(guid: params[:user_id])
+        service = BlockService.new(current_user)
+        if request.request_method_symbol == :post
+          begin
+            service.block(person)
+            head :created
+          rescue ActiveRecord::RecordNotUnique
+            render_error 409, "User is already blocked"
+          end
+        elsif request.request_method_symbol == :delete
+          begin
+            service.unblock(person)
+            head :no_content
+          rescue ActiveRecord::RecordNotFound
+            render_error 410, "User is not blocked"
+          end
+        else
+          raise AbstractController::ActionNotFound
+        end
+      end
+
       private
 
       def aspects_service

app/controllers/blocks_controller.rb

@@ -4,9 +4,10 @@ class BlocksController < ApplicationController
   before_action :authenticate_user!
 
   def create
-    block = current_user.blocks.new(block_params)
-
-    send_message(block) if block.save
+    begin
+      block_service.block(Person.find_by!(id: block_params[:person_id]))
+    rescue ActiveRecord::RecordNotUnique
+    end
 
     respond_to do |format|
       format.json { head :no_content }
@@ -15,13 +16,13 @@ class BlocksController < ApplicationController
   end
 
   def destroy
-    block = current_user.blocks.find_by(id: params[:id])
-    notice = if block&.delete
-               ContactRetraction.for(block).defer_dispatch(current_user)
-               {notice: t("blocks.destroy.success")}
-             else
-               {error: t("blocks.destroy.failure")}
-             end
+    notice = nil
+    begin
+      block_service.remove_block(current_user.blocks.find_by!(id: params[:id]))
+      notice = {notice: t("blocks.destroy.success")}
+    rescue ActiveRecord::RecordNotFound
+      notice = {error: t("blocks.destroy.failure")}
+    end
 
     respond_to do |format|
       format.json { head :no_content }
@@ -31,17 +32,11 @@ class BlocksController < ApplicationController
 
   private
 
-  def send_message(block)
-    contact = current_user.contact_for(block.person)
-
-    if contact
-      current_user.disconnect(contact)
-    elsif block.person.remote?
-      Diaspora::Federation::Dispatcher.defer_dispatch(current_user, block)
-    end
-  end
-
   def block_params
     params.require(:block).permit(:person_id)
   end
+
+  def block_service
+    BlockService.new(current_user)
+  end
 end

app/services/block_service.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+class BlockService
+  def initialize(user)
+    @user = user
+  end
+
+  def block(person)
+    raise ActiveRecord::RecordNotUnique if @user.blocks.exists?(person: person)
+
+    block = @user.blocks.create!(person: person)
+    contact = @user.contact_for(person)
+
+    if contact
+      @user.disconnect(contact)
+    elsif block.person.remote?
+      Diaspora::Federation::Dispatcher.defer_dispatch(@user, block)
+    end
+  end
+
+  def unblock(person)
+    remove_block(@user.blocks.find_by!(person: person))
+  end
+
+  def remove_block(block)
+    block.destroy
+    ContactRetraction.for(block).defer_dispatch(@user)
+  end
+end

config/routes.rb

@@ -255,6 +255,8 @@ Rails.application.routes.draw do
       get :contacts
       get :photos
       get :posts
+      post :block
+      delete :block
     end
     resources :tag_followings, only: %i[index create destroy]
     get "search/users" => "search#user_index", :as => "user_index"

spec/controllers/blocks_controller_spec.rb

@@ -16,11 +16,6 @@ describe BlocksController, :type => :controller do
       post :create, params: {block: {person_id: eve.person.id}}, format: :json
       expect(response.status).to eq(204)
     end
-
-    it "calls #send_message" do
-      expect(@controller).to receive(:send_message).with(an_instance_of(Block))
-      post :create, params: {block: {person_id: bob.person.id}}, format: :json
-    end
   end
 
   describe "#destroy" do
@@ -66,33 +61,4 @@ describe BlocksController, :type => :controller do
       expect(flash[:error]).to eq(I18n.t("blocks.destroy.failure"))
     end
   end
-
-  describe "#send_message" do
-    before do
-      allow(@controller).to receive(:current_user).and_return(alice)
-    end
-
-    it "calls disconnect if there is a contact for a given user" do
-      block = alice.blocks.create(person: bob.person)
-      contact = alice.contact_for(bob.person)
-      expect(alice).to receive(:contact_for).and_return(contact)
-      expect(alice).to receive(:disconnect).with(contact)
-      expect(Diaspora::Federation::Dispatcher).not_to receive(:defer_dispatch)
-      @controller.send(:send_message, block)
-    end
-
-    it "queues a message with the block if the person is remote and there is no contact for a given user" do
-      block = alice.blocks.create(person: remote_raphael)
-      expect(alice).not_to receive(:disconnect)
-      expect(Diaspora::Federation::Dispatcher).to receive(:defer_dispatch).with(alice, block)
-      @controller.send(:send_message, block)
-    end
-
-    it "does nothing if the person is local and there is no contact for a given user" do
-      block = alice.blocks.create(person: eve.person)
-      expect(alice).not_to receive(:disconnect)
-      expect(Diaspora::Federation::Dispatcher).not_to receive(:defer_dispatch)
-      @controller.send(:send_message, block)
-    end
-  end
 end

spec/integration/api/notifications_controller_spec.rb

@@ -199,15 +199,15 @@ describe Api::V1::NotificationsController do
     end
 
     context "fails" do
-      it "with proper invalid GUID" do
+      it "with invalid GUID" do
         patch(
           api_v1_notification_path("999_999_999"),
-          params: {access_token: access_token}
+          params: {read: true, access_token: access_token}
         )
-        confirm_api_error(response, 422, "Could not process the notifications request")
+        confirm_api_error(response, 404, "Notification with provided guid could not be found")
       end
 
-      it "with proper missing read field" do
+      it "with missing read field" do
         patch(
           api_v1_notification_path(@notification.guid),
           params: {access_token: access_token}

spec/integration/api/users_controller_spec.rb

@@ -6,7 +6,8 @@ describe Api::V1::UsersController do
   include PeopleHelper
 
   let(:full_scopes) {
-    %w[openid public:read public:modify private:read private:modify contacts:read profile profile:modify]
+    %w[openid public:read public:modify private:read private:modify
+       contacts:read contacts:modify profile profile:modify]
   }
 
   let(:auth) {
@@ -509,6 +510,99 @@ describe Api::V1::UsersController do
     end
   end
 
+  describe "#block" do
+    let(:person) { FactoryGirl.create(:user).person }
+
+    context "success" do
+      it "with proper credentials and flags" do
+        auth.user.share_with(person, auth.user.aspects.create(name: "Test"))
+        person.owner.share_with(auth.user.person, person.owner.aspects.create(name: "Victims"))
+
+        post(
+          api_v1_user_block_path(person.guid),
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(201)
+        expect(auth.user.blocks.exists?(person_id: person.id)).to be(true)
+        contact = auth.user.contact_for(person)
+        expect(contact.receiving).to be(false)
+
+        delete(
+          api_v1_user_block_path(person.guid),
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(204)
+        expect(auth.user.blocks.exists?(person: person)).to be(false)
+      end
+    end
+
+    context "fails" do
+      it "with invalid GUID" do
+        post(
+          api_v1_user_block_path("999_999_999"),
+          params: {access_token: access_token}
+        )
+        confirm_api_error(response, 404, "User not found")
+
+        delete(
+          api_v1_user_block_path("999_999_999"),
+          params: {access_token: access_token}
+        )
+        confirm_api_error(response, 404, "User not found")
+      end
+
+      it "to block already blocked user" do
+        post(
+          api_v1_user_block_path(person.guid),
+          params: {access_token: access_token}
+        )
+        expect(response.status).to eq(201)
+
+        post(
+          api_v1_user_block_path(person.guid),
+          params: {access_token: access_token}
+        )
+        confirm_api_error(response, 409, "User is already blocked")
+      end
+
+      it "to unblock not blocked user" do
+        delete(
+          api_v1_user_block_path(person.guid),
+          params: {access_token: access_token}
+        )
+        confirm_api_error(response, 410, "User is not blocked")
+      end
+
+      it "with insufficient credentials" do
+        post(
+          api_v1_user_block_path(person.guid),
+          params: {access_token: access_token_minimum_scopes}
+        )
+        expect(response.status).to eq(403)
+
+        delete(
+          api_v1_user_block_path(person.guid),
+          params: {access_token: access_token_minimum_scopes}
+        )
+        expect(response.status).to eq(403)
+      end
+
+      it "with improper credentials" do
+        post(
+          api_v1_user_block_path(person.guid),
+          params: {access_token: "999_999_999"}
+        )
+        expect(response.status).to eq(401)
+
+        delete(
+          api_v1_user_block_path(person.guid),
+          params: {access_token: "999_999_999"}
+        )
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
   def confirm_self_data_format(json)
     confirm_common_profile_elements(json)
     confirm_profile_details(json)

spec/services/block_serivce_spec.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+describe BlockService do
+  describe "#block" do
+    let(:service) { BlockService.new(alice) }
+
+    it "calls disconnect if there is a contact for a given user" do
+      contact = alice.contact_for(bob.person)
+      expect(alice).to receive(:contact_for).and_return(contact)
+      expect(alice).to receive(:disconnect).with(contact)
+      expect(Diaspora::Federation::Dispatcher).not_to receive(:defer_dispatch)
+      service.block(bob.person)
+    end
+
+    it "queues a message with the block if the person is remote and there is no contact for a given user" do
+      expect(alice).not_to receive(:disconnect)
+      expect(Diaspora::Federation::Dispatcher).to receive(:defer_dispatch)
+      service.block(remote_raphael)
+    end
+
+    it "does nothing if the person is local and there is no contact for a given user" do
+      expect(alice).not_to receive(:disconnect)
+      expect(Diaspora::Federation::Dispatcher).not_to receive(:defer_dispatch)
+      service.block(eve.person)
+    end
+  end
+end