From 2c5916ab647c8ec99a46efbe0c47de5ee3f08be2 Mon Sep 17 00:00:00 2001 From: Panos Kanellidis <pkanellidis@gitlab.com> Date: Mon, 30 Oct 2023 05:56:01 +0000 Subject: [PATCH] Delete project subscription mutation --- doc/api/graphql/reference/index.md | 19 ++++ ee/app/graphql/ee/types/mutation_type.rb | 1 + .../ci/project_subscriptions/delete.rb | 46 +++++++++ .../ci/subscriptions/project_policy.rb | 6 ++ .../ci/delete_project_subscription_service.rb | 46 +++++++++ .../ci/subscriptions/project_policy_spec.rb | 6 ++ .../ci/project_subscriptions/delete_spec.rb | 96 +++++++++++++++++++ ...elete_project_subscription_service_spec.rb | 92 ++++++++++++++++++ 8 files changed, 312 insertions(+) create mode 100644 ee/app/graphql/mutations/ci/project_subscriptions/delete.rb create mode 100644 ee/app/services/ci/delete_project_subscription_service.rb create mode 100644 ee/spec/requests/api/graphql/mutations/ci/project_subscriptions/delete_spec.rb create mode 100644 ee/spec/services/ci/delete_project_subscription_service_spec.rb diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 5bc9e7e4b172..434c3fe70f14 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -5961,6 +5961,25 @@ Input type: `ProjectSubscriptionCreateInput` | <a id="mutationprojectsubscriptioncreateerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | <a id="mutationprojectsubscriptioncreatesubscription"></a>`subscription` | [`CiSubscriptionsProject`](#cisubscriptionsproject) | Project Subscription created by the mutation. | +### `Mutation.projectSubscriptionDelete` + +Input type: `ProjectSubscriptionDeleteInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="mutationprojectsubscriptiondeleteclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| <a id="mutationprojectsubscriptiondeletesubscriptionid"></a>`subscriptionId` | [`CiSubscriptionsProjectID!`](#cisubscriptionsprojectid) | ID of the subscription to delete. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="mutationprojectsubscriptiondeleteclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| <a id="mutationprojectsubscriptiondeleteerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +| <a id="mutationprojectsubscriptiondeleteproject"></a>`project` | [`Project`](#project) | Project after mutation. | + ### `Mutation.projectSyncFork` WARNING: diff --git a/ee/app/graphql/ee/types/mutation_type.rb b/ee/app/graphql/ee/types/mutation_type.rb index 2af1e3577465..17a737cc6385 100644 --- a/ee/app/graphql/ee/types/mutation_type.rb +++ b/ee/app/graphql/ee/types/mutation_type.rb @@ -8,6 +8,7 @@ module MutationType prepended do mount_mutation ::Mutations::Ci::Ai::GenerateConfig, alpha: { milestone: '16.0' } mount_mutation ::Mutations::Ci::ProjectSubscriptions::Create + mount_mutation ::Mutations::Ci::ProjectSubscriptions::Delete mount_mutation ::Mutations::ComplianceManagement::Frameworks::Destroy mount_mutation ::Mutations::ComplianceManagement::Frameworks::Update mount_mutation ::Mutations::ComplianceManagement::Frameworks::Create diff --git a/ee/app/graphql/mutations/ci/project_subscriptions/delete.rb b/ee/app/graphql/mutations/ci/project_subscriptions/delete.rb new file mode 100644 index 000000000000..d0ce3517c443 --- /dev/null +++ b/ee/app/graphql/mutations/ci/project_subscriptions/delete.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module Mutations + module Ci + module ProjectSubscriptions + class Delete < BaseMutation + graphql_name 'ProjectSubscriptionDelete' + + authorize :delete_project_subscription + + argument :subscription_id, Types::GlobalIDType[::Ci::Subscriptions::Project], + required: true, + description: 'ID of the subscription to delete.' + + field :project, + Types::ProjectType, + null: true, + description: "Project after mutation." + + attr_reader :subscription + + def resolve(subscription_id:) + @subscription = authorized_find!(id: subscription_id) + + response = ::Ci::DeleteProjectSubscriptionService + .new(subscription: subscription, user: current_user).execute + + if response.error? + result(errors: response.errors) + else + result(project: response.payload) + end + end + + private + + def result(project: nil, errors: []) + { + project: project, + errors: errors + } + end + end + end + end +end diff --git a/ee/app/policies/ci/subscriptions/project_policy.rb b/ee/app/policies/ci/subscriptions/project_policy.rb index 1ef623c2a63d..8ebfc7cf4164 100644 --- a/ee/app/policies/ci/subscriptions/project_policy.rb +++ b/ee/app/policies/ci/subscriptions/project_policy.rb @@ -3,6 +3,10 @@ module Ci module Subscriptions class ProjectPolicy < BasePolicy + condition(:admin_downstream_project) do + can?(:admin_project, @subject.downstream_project) + end + condition(:admin_access_to_both_projects) do can?(:admin_project, @subject.downstream_project) end @@ -14,6 +18,8 @@ class ProjectPolicy < BasePolicy rule { admin_access_to_both_projects & developer_access_to_downstream_project }.policy do enable :read_project_subscription end + + rule { admin_downstream_project }.policy { enable :delete_project_subscription } end end end diff --git a/ee/app/services/ci/delete_project_subscription_service.rb b/ee/app/services/ci/delete_project_subscription_service.rb new file mode 100644 index 000000000000..eda2980ae69d --- /dev/null +++ b/ee/app/services/ci/delete_project_subscription_service.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module Ci + class DeleteProjectSubscriptionService < BaseService + attr_accessor :subscription + + def initialize(subscription:, user:) + @subscription = subscription + @current_user = user + end + + def execute + validation_error = validate + return validation_error if validation_error + + result = subscription.destroy + + if result.errors.present? + return ServiceResponse.error( + message: result.errors.full_messages + ) + end + + ServiceResponse.success(payload: result.downstream_project) + end + + private + + def validate + unless subscription.present? + return ServiceResponse.error(message: "Failed to delete subscription.", + reason: "Subscription does not exist.") + end + + return if allowed? + + ServiceResponse.error(message: "Failed to delete subscription.", + reason: "Feature unavailable for this project.") + end + + def allowed? + subscription.downstream_project.licensed_feature_available?(:ci_project_subscriptions) && + can?(current_user, :delete_project_subscription, subscription) + end + end +end diff --git a/ee/spec/policies/ci/subscriptions/project_policy_spec.rb b/ee/spec/policies/ci/subscriptions/project_policy_spec.rb index a81c89fe48d3..a3115e505cfc 100644 --- a/ee/spec/policies/ci/subscriptions/project_policy_spec.rb +++ b/ee/spec/policies/ci/subscriptions/project_policy_spec.rb @@ -12,6 +12,10 @@ subject(:policy) { described_class.new(user, subscription) } + context 'when user does not have maintainer access to project' do + it { is_expected.to be_disallowed(:delete_project_subscription) } + end + context 'when user has no permissions' do it { is_expected.to be_disallowed(:read_project_subscription) } end @@ -22,6 +26,8 @@ end it { is_expected.to be_disallowed(:read_project_subscription) } + + it { is_expected.to be_allowed(:delete_project_subscription) } end context 'when user is a developer for the upstream project' do diff --git a/ee/spec/requests/api/graphql/mutations/ci/project_subscriptions/delete_spec.rb b/ee/spec/requests/api/graphql/mutations/ci/project_subscriptions/delete_spec.rb new file mode 100644 index 000000000000..3b3a5b39b03c --- /dev/null +++ b/ee/spec/requests/api/graphql/mutations/ci/project_subscriptions/delete_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Delete action for project subscriptions', feature_category: :continuous_integration do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:upstream_project) { create(:project, :repository, :public) } + let_it_be(:current_user) { create(:user) } + let_it_be(:subscription) do + create(:ci_subscriptions_project, downstream_project: project, upstream_project: upstream_project) + end + + let(:subscription_global_id) { subscription.to_global_id } + let(:mutation) do + graphql_mutation(:project_subscription_delete, subscription_id: subscription_global_id) do + <<~QL + project { + name + } + errors + QL + end + end + + let(:mutation_response) { graphql_mutation_response(:project_subscription_delete) } + + subject { post_graphql_mutation(mutation, current_user: current_user) } + + before do + stub_licensed_features(ci_project_subscriptions: true) + end + + context 'when the user is authorized' do + before_all do + project.add_maintainer(current_user) + end + + context 'when a successful result is yielded' do + it 'returns the project' do + subject + expect(mutation_response['project']['name']).to eq(project.name) + end + + it 'deletes the subscription' do + expect { subject }.to change { ::Ci::Subscriptions::Project.count }.by(-1) + end + end + + context 'when the subscription_id is invalid' do + let(:subscription_global_id) { 'gid://gitlab/Ci::Subscriptions::Project/-12' } + + it 'does not reduce the subscriptions' do + expect { subject }.to change { ::Ci::Subscriptions::Project.count }.by(0) + end + + it 'raises an exception' do + subject + expect(graphql_errors) + .to include(a_hash_including('message' => "The resource that you are attempting to access does " \ + "not exist or you don't have permission to perform this action")) + end + end + + context 'when the service returns an error' do + let(:service) { instance_double(::Ci::DeleteProjectSubscriptionService) } + let(:service_response) { ServiceResponse.error(message: 'An error message.') } + + before do + allow(::Ci::DeleteProjectSubscriptionService).to receive(:new) { service } + allow(service).to receive(:execute) { service_response } + end + + it_behaves_like 'a mutation that returns errors in the response', + errors: ['An error message.'] + + it 'does not create a new record' do + expect { subject }.not_to change { ::Ci::Subscriptions::Project.count } + end + end + end + + context 'when the user is not authorized' do + it 'does not reduce the subscriptions' do + expect { subject }.to change { ::Ci::Subscriptions::Project.count }.by(0) + end + + it 'returns an error' do + subject + expect(graphql_errors) + .to include(a_hash_including('message' => "The resource that you are attempting to access does " \ + "not exist or you don't have permission to perform this action")) + end + end +end diff --git a/ee/spec/services/ci/delete_project_subscription_service_spec.rb b/ee/spec/services/ci/delete_project_subscription_service_spec.rb new file mode 100644 index 000000000000..f6825e7929a0 --- /dev/null +++ b/ee/spec/services/ci/delete_project_subscription_service_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ci::DeleteProjectSubscriptionService, feature_category: :continuous_integration do + describe '#execute' do + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:upstream_project) { create(:project, :repository, :public) } + let_it_be(:current_user) { create(:user) } + let!(:subscription) do + create(:ci_subscriptions_project, downstream_project: project, upstream_project: upstream_project) + end + + subject(:result) do + described_class.new(subscription: subscription, user: current_user).execute + end + + before do + stub_licensed_features(ci_project_subscriptions: true) + end + + context 'when the user has permissions' do + before_all do + project.add_maintainer(current_user) + end + + it 'returns a success response with the payload' do + project = subject.payload + expect(project).to eq(subscription.downstream_project) + end + + it 'decreases the DB record by 1' do + expect { subject }.to change { ::Ci::Subscriptions::Project.count }.by(-1) + end + + context 'when the feature is locked' do + before do + stub_licensed_features(ci_project_subscriptions: false) + end + + it 'returns a service error with the relevant message' do + expect(result.payload).to eq({}) + expect(result.errors.first).to eq('Failed to delete subscription.') + expect(result.reason).to eq('Feature unavailable for this project.') + end + + it 'does not delete the record' do + expect { subject }.to change { ::Ci::Subscriptions::Project.count }.by(0) + end + end + + context 'when the subscription is null' do + let(:subscription) { nil } + + it 'returns a service error with the relevant message' do + result = subject + expect(result.payload).to eq({}) + expect(result.errors.first).to eq('Failed to delete subscription.') + expect(result.reason).to eq('Subscription does not exist.') + end + + it 'does not delete the record' do + expect { subject }.to change { ::Ci::Subscriptions::Project.count }.by(0) + end + end + + context 'when an error occurs while persisting' do + before do + subscription.errors.add(:downstream_project, :some_error, message: 'An error occurred.') + allow(subscription).to receive(:destroy) { subscription } + end + + it 'returns the error message' do + result = subject + expect(result.message).to eq(['Downstream project An error occurred.']) + end + end + end + + context 'when the user does not have permissions' do + it 'returns a service error with the relevant message' do + expect(result.payload).to eq({}) + expect(result.errors.first).to eq('Failed to delete subscription.') + expect(result.reason).to eq('Feature unavailable for this project.') + end + + it 'does not delete the record' do + expect { subject }.to change { ::Ci::Subscriptions::Project.count }.by(0) + end + end + end +end -- GitLab