diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index a7cf3effb..065a4057b 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/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 diff --git a/app/controllers/blocks_controller.rb b/app/controllers/blocks_controller.rb index 39d8cb91c..480bd7538 100644 --- a/app/controllers/blocks_controller.rb +++ b/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 diff --git a/app/services/block_service.rb b/app/services/block_service.rb new file mode 100644 index 000000000..bc768287e --- /dev/null +++ b/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 diff --git a/config/routes.rb b/config/routes.rb index 4651d6db6..9e6cad3f1 100644 --- a/config/routes.rb +++ b/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" diff --git a/spec/controllers/blocks_controller_spec.rb b/spec/controllers/blocks_controller_spec.rb index 11acfdccb..29470e583 100644 --- a/spec/controllers/blocks_controller_spec.rb +++ b/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 diff --git a/spec/integration/api/notifications_controller_spec.rb b/spec/integration/api/notifications_controller_spec.rb index d61eba2b0..f889367b7 100644 --- a/spec/integration/api/notifications_controller_spec.rb +++ b/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} diff --git a/spec/integration/api/users_controller_spec.rb b/spec/integration/api/users_controller_spec.rb index c67a6b3cb..c68ded1f3 100644 --- a/spec/integration/api/users_controller_spec.rb +++ b/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) diff --git a/spec/services/block_serivce_spec.rb b/spec/services/block_serivce_spec.rb new file mode 100644 index 000000000..46e62e24c --- /dev/null +++ b/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