From de1e0eade13ed77b6d003e91437cfa5d46abf8b7 Mon Sep 17 00:00:00 2001
From: Jerry Seto <jseto@gitlab.com>
Date: Fri, 10 May 2024 20:49:40 +0000
Subject: [PATCH] Fix error on deleting exclusions and limit creating
exclusions
---
.../integrations/exclusions/create.rb | 9 +++++--
.../integrations/exclusions/delete.rb | 2 +-
.../exclusions/destroy_service.rb | 5 +++-
doc/api/graphql/reference/index.md | 4 ++--
.../integrations/exclusions/create_spec.rb | 16 +++++++++++++
.../integrations/exclusions/delete_spec.rb | 24 ++++++++++++-------
6 files changed, 46 insertions(+), 14 deletions(-)
diff --git a/app/graphql/mutations/integrations/exclusions/create.rb b/app/graphql/mutations/integrations/exclusions/create.rb
index 7949b2a267bc..483e3b0fba8d 100644
--- a/app/graphql/mutations/integrations/exclusions/create.rb
+++ b/app/graphql/mutations/integrations/exclusions/create.rb
@@ -6,6 +6,7 @@ module Exclusions
class Create < BaseMutation
graphql_name 'IntegrationExclusionCreate'
include ResolvesIds
+ MAX_PROJECT_IDS = 100
field :exclusions, [::Types::Integrations::ExclusionType],
null: true,
@@ -19,14 +20,18 @@ class Create < BaseMutation
argument :project_ids,
[::Types::GlobalIDType[::Project]],
required: true,
- description: 'Ids of projects to exclude.'
+ description: "IDs of projects to exclude up to a maximum of #{MAX_PROJECT_IDS}."
authorize :admin_all_resources
def resolve(integration_name:, project_ids:)
authorize!(:global)
- projects = Project.id_in(resolve_ids(project_ids))
+ if project_ids.length > MAX_PROJECT_IDS
+ raise Gitlab::Graphql::Errors::ArgumentError, "Count of projectIds should be less than #{MAX_PROJECT_IDS}"
+ end
+
+ projects = Project.id_in(resolve_ids(project_ids)).limit(MAX_PROJECT_IDS)
result = ::Integrations::Exclusions::CreateService.new(
current_user: current_user,
diff --git a/app/graphql/mutations/integrations/exclusions/delete.rb b/app/graphql/mutations/integrations/exclusions/delete.rb
index d01d8c6776ed..bf76c07a1508 100644
--- a/app/graphql/mutations/integrations/exclusions/delete.rb
+++ b/app/graphql/mutations/integrations/exclusions/delete.rb
@@ -19,7 +19,7 @@ class Delete < BaseMutation
argument :project_ids,
[::Types::GlobalIDType[::Project]],
required: true,
- description: 'Id of excluded project.'
+ description: 'IDs of excluded projects.'
authorize :admin_all_resources
diff --git a/app/services/integrations/exclusions/destroy_service.rb b/app/services/integrations/exclusions/destroy_service.rb
index cee0af8f9c88..6fd1fd201617 100644
--- a/app/services/integrations/exclusions/destroy_service.rb
+++ b/app/services/integrations/exclusions/destroy_service.rb
@@ -19,7 +19,10 @@ def destroy_exclusions
instance_integration = integration_class.for_instance.first
- return ServiceResponse.success(payload: exclusions.destroy_all) unless instance_integration # rubocop: disable Cop/DestroyAll -- We load exclusions so we can have the deleted exclusions in the response
+ unless instance_integration
+ integration_class.id_in(exclusions.map(&:id)).delete_all
+ return ServiceResponse.success(payload: exclusions)
+ end
::Integrations::Propagation::BulkUpdateService.new(instance_integration, exclusions).execute
ServiceResponse.success(payload: exclusions)
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 561f34b80461..61bb881aeb4c 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -5460,7 +5460,7 @@ Input type: `IntegrationExclusionCreateInput`
| ---- | ---- | ----------- |
| <a id="mutationintegrationexclusioncreateclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationintegrationexclusioncreateintegrationname"></a>`integrationName` | [`IntegrationType!`](#integrationtype) | Type of integration to exclude. |
-| <a id="mutationintegrationexclusioncreateprojectids"></a>`projectIds` | [`[ProjectID!]!`](#projectid) | Ids of projects to exclude. |
+| <a id="mutationintegrationexclusioncreateprojectids"></a>`projectIds` | [`[ProjectID!]!`](#projectid) | IDs of projects to exclude up to a maximum of 100. |
#### Fields
@@ -5484,7 +5484,7 @@ Input type: `IntegrationExclusionDeleteInput`
| ---- | ---- | ----------- |
| <a id="mutationintegrationexclusiondeleteclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationintegrationexclusiondeleteintegrationname"></a>`integrationName` | [`IntegrationType!`](#integrationtype) | Type of integration. |
-| <a id="mutationintegrationexclusiondeleteprojectids"></a>`projectIds` | [`[ProjectID!]!`](#projectid) | Id of excluded project. |
+| <a id="mutationintegrationexclusiondeleteprojectids"></a>`projectIds` | [`[ProjectID!]!`](#projectid) | IDs of excluded projects. |
#### Fields
diff --git a/spec/requests/api/graphql/mutations/integrations/exclusions/create_spec.rb b/spec/requests/api/graphql/mutations/integrations/exclusions/create_spec.rb
index 5d3e19c100df..153a5ef51e5d 100644
--- a/spec/requests/api/graphql/mutations/integrations/exclusions/create_spec.rb
+++ b/spec/requests/api/graphql/mutations/integrations/exclusions/create_spec.rb
@@ -56,5 +56,21 @@
expect(existing_exclusion.inherit_from_id).to be_nil
end
end
+
+ context 'and the number of project ids exceeds the maximum' do
+ let(:stubbed_limit) { 1 }
+ let(:project_ids) { [project, project2].map { |p| p.to_global_id.to_s } }
+
+ before do
+ stub_const("#{described_class.name}::MAX_PROJECT_IDS", stubbed_limit)
+ end
+
+ it 'responds with an error' do
+ resolve_mutation
+ expect(graphql_errors.first['message']).to eq(
+ "Count of projectIds should be less than #{stubbed_limit}"
+ )
+ end
+ end
end
end
diff --git a/spec/requests/api/graphql/mutations/integrations/exclusions/delete_spec.rb b/spec/requests/api/graphql/mutations/integrations/exclusions/delete_spec.rb
index 2e4dce2e4d08..2b1e41ff56b4 100644
--- a/spec/requests/api/graphql/mutations/integrations/exclusions/delete_spec.rb
+++ b/spec/requests/api/graphql/mutations/integrations/exclusions/delete_spec.rb
@@ -41,20 +41,28 @@
end
context 'and there are integrations' do
- let!(:instance_integration) { create(:beyond_identity_integration) }
let!(:existing_exclusion) do
create(:beyond_identity_integration, project: project, active: false, inherit_from_id: nil,
instance: false)
end
- it 'enables the integration for the specified project' do
- resolve_mutation
+ context 'and the integration is active for the instance' do
+ let!(:instance_integration) { create(:beyond_identity_integration) }
- existing_exclusion.reload
- expect(existing_exclusion).to be_activated
- expect(existing_exclusion.inherit_from_id).to eq(instance_integration.id)
- exclusion_response = graphql_data['integrationExclusionDelete']['exclusions'][0]
- expect(exclusion_response['project']['id']).to eq(project.to_global_id.to_s)
+ it 'enables the integration for the specified project' do
+ resolve_mutation
+
+ existing_exclusion.reload
+ expect(existing_exclusion).to be_activated
+ expect(existing_exclusion.inherit_from_id).to eq(instance_integration.id)
+ exclusion_response = graphql_data['integrationExclusionDelete']['exclusions'][0]
+ expect(exclusion_response['project']['id']).to eq(project.to_global_id.to_s)
+ end
+ end
+
+ it 'deletes the integration' do
+ expect { resolve_mutation }.to change { Integration.count }.from(1).to(0)
+ expect(graphql_errors).to be_nil
end
end
end
--
GitLab