From d151493a6b120b34d80844bbe6555fa85b8537f0 Mon Sep 17 00:00:00 2001 From: Vincent Pochet Date: Fri, 4 Oct 2024 16:03:56 +0200 Subject: [PATCH] misc: Remove the need for BaseService user argument --- .../mutations/payment_providers/destroy.rb | 8 ++++--- app/services/base_service.rb | 3 +-- .../payment_providers/destroy_service.rb | 16 +++++++++----- .../payment_providers/destroy_spec.rb | 21 +++++++++++++++++++ spec/services/base_service_spec.rb | 5 ++--- .../payment_providers/destroy_service_spec.rb | 17 ++++----------- 6 files changed, 44 insertions(+), 26 deletions(-) diff --git a/app/graphql/mutations/payment_providers/destroy.rb b/app/graphql/mutations/payment_providers/destroy.rb index 9dbdc11664e..559803b3c8f 100644 --- a/app/graphql/mutations/payment_providers/destroy.rb +++ b/app/graphql/mutations/payment_providers/destroy.rb @@ -15,9 +15,11 @@ class Destroy < BaseMutation field :id, ID, null: true def resolve(id:) - result = ::PaymentProviders::DestroyService - .new(context[:current_user]) - .destroy(id:) + payment_provider = ::PaymentProviders::BaseProvider.find_by( + id:, + organization_id: context[:current_user].organization_ids + ) + result = ::PaymentProviders::DestroyService.call(payment_provider) result.success? ? result.payment_provider : result_error(result) end diff --git a/app/services/base_service.rb b/app/services/base_service.rb index bf5eab19173..7c5fbeb3c19 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -156,10 +156,9 @@ def self.call_async(*, **, &) end end - def initialize(current_user = nil) + def initialize(args = nil) @result = Result.new @source = CurrentContext&.source - result.user = current_user.is_a?(User) ? current_user : nil end def call(**args, &block) diff --git a/app/services/payment_providers/destroy_service.rb b/app/services/payment_providers/destroy_service.rb index 95fb9ff1ab4..164faff9788 100644 --- a/app/services/payment_providers/destroy_service.rb +++ b/app/services/payment_providers/destroy_service.rb @@ -2,11 +2,13 @@ module PaymentProviders class DestroyService < BaseService - def destroy(id:) - payment_provider = PaymentProviders::BaseProvider.find_by( - id:, - organization_id: result.user.organization_ids - ) + def initialize(payment_provider) + @payment_provider = payment_provider + + super + end + + def call return result.not_found_failure!(resource: 'payment_provider') unless payment_provider customer_ids = payment_provider.customer_ids @@ -21,5 +23,9 @@ def destroy(id:) result.payment_provider = payment_provider result end + + private + + attr_reader :payment_provider end end diff --git a/spec/graphql/mutations/payment_providers/destroy_spec.rb b/spec/graphql/mutations/payment_providers/destroy_spec.rb index 76bfe489f62..9731190c316 100644 --- a/spec/graphql/mutations/payment_providers/destroy_spec.rb +++ b/spec/graphql/mutations/payment_providers/destroy_spec.rb @@ -32,4 +32,25 @@ data = result['data']['destroyPaymentProvider'] expect(data['id']).to eq(payment_provider.id) end + + context 'when payment provider is not attached to the organization' do + let(:payment_provider) { create(:stripe_provider) } + + it 'returns an error' do + result = execute_graphql( + current_user: membership.user, + permissions: required_permission, + query: mutation, + variables: { + input: {id: payment_provider.id} + } + ) + + aggregate_failures do + expect(result['errors'].first['message']).to eq('Resource not found') + expect(result['errors'].first['extensions']['code']).to eq('not_found') + expect(result['errors'].first['extensions']['status']).to eq(404) + end + end + end end diff --git a/spec/services/base_service_spec.rb b/spec/services/base_service_spec.rb index fcc83ec1a88..f1f05d1bbf3 100644 --- a/spec/services/base_service_spec.rb +++ b/spec/services/base_service_spec.rb @@ -11,10 +11,9 @@ context 'with current_user' do it 'assigns the current_user to the result' do - user = create(:user) - result = described_class.new(user).send :result + result = described_class.new.send :result - expect(result.user).to eq(user) + expect(result).to be_a(::BaseService::Result) end it 'does not assign the current_user to the result if it isn\'t a User' do diff --git a/spec/services/payment_providers/destroy_service_spec.rb b/spec/services/payment_providers/destroy_service_spec.rb index a13352bd447..254ac9d0207 100644 --- a/spec/services/payment_providers/destroy_service_spec.rb +++ b/spec/services/payment_providers/destroy_service_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' RSpec.describe PaymentProviders::DestroyService, type: :service do - subject(:destroy_service) { described_class.new(membership.user) } + subject(:destroy_service) { described_class.new(payment_provider) } let(:membership) { create(:membership) } let(:organization) { membership.organization } @@ -14,24 +14,15 @@ before { payment_provider } it 'destroys the payment_provider' do - expect { destroy_service.destroy(id: payment_provider.id) } + expect { destroy_service.call } .to change(PaymentProviders::BaseProvider, :count).by(-1) end context 'when payment provider is not found' do - it 'returns an error' do - result = destroy_service.destroy(id: nil) - - expect(result).not_to be_success - expect(result.error.error_code).to eq('payment_provider_not_found') - end - end - - context 'when payment provider is not attached to the organization' do - let(:payment_provider) { create(:stripe_provider) } + let(:payment_provider) { nil } it 'returns an error' do - result = destroy_service.destroy(id: payment_provider.id) + result = destroy_service.call expect(result).not_to be_success expect(result.error.error_code).to eq('payment_provider_not_found')