Merge branch '321674-allow-to-filter-alert-management-integrations-by-id-take-2' into 'master'

Allow filtering alertManagementIntegrations GraphQL by ID [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!57590

app/graphql/gitlab_schema.rb

@@ -111,6 +111,7 @@ class GitlabSchema < GraphQL::Schema
     #
     # Options:
     #  * :expected_type [Class] - the type of object this GlobalID should refer to.
+    #  * :expected_type [[Class]] - array of the types of object this GlobalID should refer to.
     #
     # e.g.
     #
@@ -120,14 +121,14 @@ class GitlabSchema < GraphQL::Schema
     #   gid.model_class == ::Project
     # ```
     def parse_gid(global_id, ctx = {})
-      expected_type = ctx[:expected_type]
+      expected_types = Array(ctx[:expected_type])
       gid = GlobalID.parse(global_id)
 
       raise Gitlab::Graphql::Errors::ArgumentError, "#{global_id} is not a valid GitLab ID." unless gid
 
-      if expected_type && gid.model_class.ancestors.exclude?(expected_type)
-        vars = { global_id: global_id, expected_type: expected_type }
-        msg = _('%{global_id} is not a valid ID for %{expected_type}.') % vars
+      if expected_types.any? && expected_types.none? { |type| gid.model_class.ancestors.include?(type) }
+        vars = { global_id: global_id, expected_types: expected_types.join(', ') }
+        msg = _('%{global_id} is not a valid ID for %{expected_types}.') % vars
         raise Gitlab::Graphql::Errors::ArgumentError, msg
       end
 

app/graphql/resolvers/alert_management/http_integrations_resolver.rb

@@ -3,19 +3,39 @@
 module Resolvers
   module AlertManagement
     class HttpIntegrationsResolver < BaseResolver
+      include ::Gitlab::Graphql::Laziness
+
       alias_method :project, :object
 
+      argument :id, Types::GlobalIDType[::AlertManagement::HttpIntegration],
+               required: false,
+               description: 'ID of the integration.'
+
       type Types::AlertManagement::HttpIntegrationType.connection_type, null: true
 
-      def resolve(**args)
-        http_integrations
+      def resolve(id: nil)
+        return [] unless Ability.allowed?(current_user, :admin_operations, project)
+
+        if id
+          integrations_by(gid: id)
+        else
+          http_integrations
+        end
       end
 
       private
 
-      def http_integrations
-        return [] unless Ability.allowed?(current_user, :admin_operations, project)
+      def integrations_by(gid:)
+        id = Types::GlobalIDType[::AlertManagement::HttpIntegration].coerce_isolated_input(gid)
+        object = GitlabSchema.find_by_gid(id)
+
+        defer { object }.then do |integration|
+          ret = integration if project == integration&.project
+          Array.wrap(ret)
+        end
+      end
 
+      def http_integrations
         ::AlertManagement::HttpIntegrationsFinder.new(project, {}).execute
       end
     end

app/graphql/resolvers/alert_management/integrations_resolver.rb

@@ -3,27 +3,60 @@
 module Resolvers
   module AlertManagement
     class IntegrationsResolver < BaseResolver
+      include ::Gitlab::Graphql::Laziness
+
       alias_method :project, :object
 
+      argument :id, ::Types::GlobalIDType,
+               required: false,
+               description: 'ID of the integration.'
+
       type Types::AlertManagement::IntegrationType.connection_type, null: true
 
-      def resolve(**args)
-        http_integrations + prometheus_integrations
+      def resolve(id: nil)
+        if id
+          integrations_by(gid: id)
+        else
+          http_integrations + prometheus_integrations
+        end
       end
 
       private
 
+      def integrations_by(gid:)
+        object = GitlabSchema.object_from_id(gid, expected_type: expected_integration_types)
+        defer { object }.then do |integration|
+          ret = integration if project == integration&.project
+          Array.wrap(ret)
+        end
+      end
+
       def prometheus_integrations
-        return [] unless Ability.allowed?(current_user, :admin_project, project)
+        return [] unless prometheus_integrations_allowed?
 
         Array(project.prometheus_service)
       end
 
       def http_integrations
-        return [] unless Ability.allowed?(current_user, :admin_operations, project)
+        return [] unless http_integrations_allowed?
 
         ::AlertManagement::HttpIntegrationsFinder.new(project, {}).execute
       end
+
+      def prometheus_integrations_allowed?
+        Ability.allowed?(current_user, :admin_project, project)
+      end
+
+      def http_integrations_allowed?
+        Ability.allowed?(current_user, :admin_operations, project)
+      end
+
+      def expected_integration_types
+        [].tap do |types|
+          types << ::AlertManagement::HttpIntegration if http_integrations_allowed?
+          types << ::PrometheusService if prometheus_integrations_allowed?
+        end
+      end
     end
   end
 end

changelogs/unreleased/321674-allow-to-filter-alert-management-integrations-by-id-take-2.yml

+---
+title: Allow filtering GraphQL alertManagementIntegrations and alertManagementHttpIntegrations
+  by ID
+merge_request: 57590
+author:
+type: added

ee/spec/requests/api/graphql/project/alert_management/http_integrations_spec.rb

@@ -6,6 +6,8 @@ RSpec.describe 'getting Alert Management HTTP Integrations' do
   include ::Gitlab::Routing.url_helpers
   include GraphqlHelpers
 
+  let(:params) { {} }
+
   let_it_be(:payload_example) do
     {
       alert: {
@@ -51,7 +53,7 @@ RSpec.describe 'getting Alert Management HTTP Integrations' do
     graphql_query_for(
       'project',
       { 'fullPath' => project.full_path },
-      query_graphql_field('alertManagementHttpIntegrations', {}, fields)
+      query_graphql_field('alertManagementHttpIntegrations', params, fields)
     )
   end
 
@@ -98,62 +100,91 @@ RSpec.describe 'getting Alert Management HTTP Integrations' do
         post_graphql(query, current_user: current_user)
       end
 
-      it_behaves_like 'a working graphql query'
+      context 'when no extra params given' do
+        it_behaves_like 'a working graphql query'
+
+        specify { expect(integrations.size).to eq(2) }
+
+        it 'returns the correct properties of the integrations' do
+          expect(integrations).to include(
+            {
+              'id' => global_id_of(active_http_integration),
+              'type' => 'HTTP',
+              'name' => active_http_integration.name,
+              'active' => active_http_integration.active,
+              'token' => active_http_integration.token,
+              'url' => active_http_integration.url,
+              'apiUrl' => nil,
+              'payloadExample' => payload_example.to_json,
+              'payloadAttributeMappings' => [
+                {
+                  'fieldName' => 'TITLE',
+                  'label' => nil,
+                  'path' => %w(alert name),
+                  'type' => 'STRING'
+                },
+                {
+                  'fieldName' => 'DESCRIPTION',
+                  'label' => 'Description',
+                  'path' => %w(alert desc),
+                  'type' => 'STRING'
+                }
+              ],
+              'payloadAlertFields' => [
+                {
+                  'label' => 'alert/name',
+                  'path' => %w(alert name),
+                  'type' => 'STRING'
+                },
+                {
+                  'label' => 'alert/desc',
+                  'path' => %w(alert desc),
+                  'type' => 'STRING'
+                }
+              ]
+            },
+            {
+              'id' => global_id_of(inactive_http_integration),
+              'type' => 'HTTP',
+              'name' => inactive_http_integration.name,
+              'active' => inactive_http_integration.active,
+              'token' => inactive_http_integration.token,
+              'url' => inactive_http_integration.url,
+              'apiUrl' => nil,
+              'payloadExample' => "{}",
+              'payloadAttributeMappings' => [],
+              'payloadAlertFields' => []
+            }
+          )
+        end
+      end
 
-      specify { expect(integrations.size).to eq(2) }
-
-      it 'returns the correct properties of the integrations' do
-        expect(integrations).to include(
-          {
-            'id' => GitlabSchema.id_from_object(active_http_integration).to_s,
-            'type' => 'HTTP',
-            'name' => active_http_integration.name,
-            'active' => active_http_integration.active,
-            'token' => active_http_integration.token,
-            'url' => active_http_integration.url,
-            'apiUrl' => nil,
-            'payloadExample' => payload_example.to_json,
-            'payloadAttributeMappings' => [
-              {
-                'fieldName' => 'TITLE',
-                'label' => nil,
-                'path' => %w(alert name),
-                'type' => 'STRING'
-              },
-              {
-                'fieldName' => 'DESCRIPTION',
-                'label' => 'Description',
-                'path' => %w(alert desc),
-                'type' => 'STRING'
-              }
-            ],
-            'payloadAlertFields' => [
-              {
-                'label' => 'alert/name',
-                'path' => %w(alert name),
-                'type' => 'STRING'
-              },
-              {
-                'label' => 'alert/desc',
-                'path' => %w(alert desc),
-                'type' => 'STRING'
-              }
-            ]
-          },
-          {
-            'id' => GitlabSchema.id_from_object(inactive_http_integration).to_s,
-            'type' => 'HTTP',
-            'name' => inactive_http_integration.name,
-            'active' => inactive_http_integration.active,
-            'token' => inactive_http_integration.token,
-            'url' => inactive_http_integration.url,
-            'apiUrl' => nil,
-            'payloadExample' => "{}",
-            'payloadAttributeMappings' => [],
-            'payloadAlertFields' => []
-          }
-        )
+      context 'when HTTP Integration ID is given' do
+        let(:params) { { id: global_id_of(inactive_http_integration) } }
+
+        it_behaves_like 'a working graphql query'
+
+        specify { expect(integrations).to be_one }
+
+        it 'returns the correct properties of the integration' do
+          expect(integrations).to include(
+            {
+              'id' => global_id_of(inactive_http_integration),
+              'type' => 'HTTP',
+              'name' => inactive_http_integration.name,
+              'active' => inactive_http_integration.active,
+              'token' => inactive_http_integration.token,
+              'url' => inactive_http_integration.url,
+              'apiUrl' => nil,
+              'payloadExample' => "{}",
+              'payloadAttributeMappings' => [],
+              'payloadAlertFields' => []
+            }
+          )
+        end
       end
+
+      it_behaves_like 'GraphQL query with several integrations requested', graphql_query_name: 'alertManagementHttpIntegrations'
     end
   end
 end

locale/gitlab.pot

@@ -553,7 +553,7 @@ msgstr ""
 msgid "%{gitlab_experience_text}. We won't share this information with anyone."
 msgstr ""
 
-msgid "%{global_id} is not a valid ID for %{expected_type}."
+msgid "%{global_id} is not a valid ID for %{expected_types}."
 msgstr ""
 
 msgid "%{group_docs_link_start}Groups%{group_docs_link_end} allow you to manage and collaborate across multiple projects. Members of a group have access to all of its projects."

spec/graphql/gitlab_schema_spec.rb

@@ -206,18 +206,22 @@ RSpec.describe GitlabSchema do
   describe '.parse_gid' do
     let_it_be(:global_id) { 'gid://gitlab/TestOne/2147483647' }
 
+    subject(:parse_gid) { described_class.parse_gid(global_id) }
+
     before do
       test_base = Class.new
       test_one = Class.new(test_base)
       test_two = Class.new(test_base)
+      test_three = Class.new(test_base)
 
       stub_const('TestBase', test_base)
       stub_const('TestOne', test_one)
       stub_const('TestTwo', test_two)
+      stub_const('TestThree', test_three)
     end
 
     it 'parses the gid' do
-      gid = described_class.parse_gid(global_id)
+      gid = parse_gid
 
       expect(gid.model_id).to eq '2147483647'
       expect(gid.model_class).to eq TestOne
@@ -227,7 +231,7 @@ RSpec.describe GitlabSchema do
       let_it_be(:global_id) { 'malformed://gitlab/TestOne/2147483647' }
 
       it 'raises an error' do
-        expect { described_class.parse_gid(global_id) }
+        expect { parse_gid }
           .to raise_error(Gitlab::Graphql::Errors::ArgumentError, "#{global_id} is not a valid GitLab ID.")
       end
     end
@@ -249,6 +253,33 @@ RSpec.describe GitlabSchema do
         expect { described_class.parse_gid(global_id, expected_type: TestTwo) }
           .to raise_error(Gitlab::Graphql::Errors::ArgumentError, "#{global_id} is not a valid ID for TestTwo.")
       end
+
+      context 'when expected_type is an array' do
+        subject(:parse_gid) { described_class.parse_gid(global_id, expected_type: [TestOne, TestTwo]) }
+
+        context 'when global_id is of type TestOne' do
+          it 'returns an object of an expected type' do
+            expect(parse_gid.model_class).to eq TestOne
+          end
+        end
+
+        context 'when global_id is of type TestTwo' do
+          let_it_be(:global_id) { 'gid://gitlab/TestTwo/2147483647' }
+
+          it 'returns an object of an expected type' do
+            expect(parse_gid.model_class).to eq TestTwo
+          end
+        end
+
+        context 'when global_id is of type TestThree' do
+          let_it_be(:global_id) { 'gid://gitlab/TestThree/2147483647' }
+
+          it 'rejects an unknown type' do
+            expect { parse_gid }
+              .to raise_error(Gitlab::Graphql::Errors::ArgumentError, "#{global_id} is not a valid ID for TestOne, TestTwo.")
+          end
+        end
+      end
     end
   end
 

spec/graphql/resolvers/alert_management/http_integrations_resolver_spec.rb

@@ -14,7 +14,9 @@ RSpec.describe Resolvers::AlertManagement::HttpIntegrationsResolver do
   let_it_be(:inactive_http_integration) { create(:alert_management_http_integration, :inactive, project: project) }
   let_it_be(:other_proj_integration) { create(:alert_management_http_integration) }
 
-  subject { sync(resolve_http_integrations) }
+  let(:params) { {} }
+
+  subject { sync(resolve_http_integrations(params)) }
 
   before do
     project.add_developer(developer)
@@ -41,11 +43,25 @@ RSpec.describe Resolvers::AlertManagement::HttpIntegrationsResolver do
     let(:current_user) { maintainer }
 
     it { is_expected.to contain_exactly(active_http_integration) }
+
+    context 'when HTTP Integration ID is given' do
+      context 'when integration is from the current project' do
+        let(:params) { { id: global_id_of(inactive_http_integration) } }
+
+        it { is_expected.to contain_exactly(inactive_http_integration) }
+      end
+
+      context 'when integration is from other project' do
+        let(:params) { { id: global_id_of(other_proj_integration) } }
+
+        it { is_expected.to be_empty }
+      end
+    end
   end
 
   private
 
   def resolve_http_integrations(args = {}, context = { current_user: current_user })
-    resolve(described_class, obj: project, ctx: context)
+    resolve(described_class, obj: project, args: args, ctx: context)
   end
 end

spec/graphql/resolvers/alert_management/integrations_resolver_spec.rb

@@ -7,12 +7,16 @@ RSpec.describe Resolvers::AlertManagement::IntegrationsResolver do
 
   let_it_be(:current_user) { create(:user) }
   let_it_be(:project) { create(:project) }
+  let_it_be(:project2) { create(:project) }
   let_it_be(:prometheus_integration) { create(:prometheus_service, project: project) }
   let_it_be(:active_http_integration) { create(:alert_management_http_integration, project: project) }
   let_it_be(:inactive_http_integration) { create(:alert_management_http_integration, :inactive, project: project) }
-  let_it_be(:other_proj_integration) { create(:alert_management_http_integration) }
+  let_it_be(:other_proj_integration) { create(:alert_management_http_integration, project: project2) }
+  let_it_be(:other_proj_prometheus_integration) { create(:prometheus_service, project: project2) }
 
-  subject { sync(resolve_http_integrations) }
+  let(:params) { {} }
+
+  subject { sync(resolve_http_integrations(params)) }
 
   specify do
     expect(described_class).to have_nullable_graphql_type(Types::AlertManagement::IntegrationType.connection_type)
@@ -25,14 +29,43 @@ RSpec.describe Resolvers::AlertManagement::IntegrationsResolver do
   context 'user has permission' do
     before do
       project.add_maintainer(current_user)
+      project2.add_maintainer(current_user)
     end
 
     it { is_expected.to contain_exactly(active_http_integration, prometheus_integration) }
+
+    context 'when HTTP Integration ID is given' do
+      context 'when integration is from the current project' do
+        let(:params) { { id: global_id_of(inactive_http_integration) } }
+
+        it { is_expected.to contain_exactly(inactive_http_integration) }
+      end
+
+      context 'when integration is from other project' do
+        let(:params) { { id: global_id_of(other_proj_integration) } }
+
+        it { is_expected.to be_empty }
+      end
+    end
+
+    context 'when Prometheus Integration ID is given' do
+      context 'when integration is from the current project' do
+        let(:params) { { id: global_id_of(prometheus_integration) } }
+
+        it { is_expected.to contain_exactly(prometheus_integration) }
+      end
+
+      context 'when integration is from other project' do
+        let(:params) { { id: global_id_of(other_proj_prometheus_integration) } }
+
+        it { is_expected.to be_empty }
+      end
+    end
   end
 
   private
 
   def resolve_http_integrations(args = {}, context = { current_user: current_user })
-    resolve(described_class, obj: project, ctx: context)
+    resolve(described_class, obj: project, args: args, ctx: context)
   end
 end

spec/requests/api/graphql/project/alert_management/integrations_spec.rb

@@ -13,6 +13,8 @@ RSpec.describe 'getting Alert Management Integrations' do
   let_it_be(:inactive_http_integration) { create(:alert_management_http_integration, :inactive, project: project) }
   let_it_be(:other_project_http_integration) { create(:alert_management_http_integration) }
 
+  let(:params) { {} }
+
   let(:fields) do
     <<~QUERY
       nodes {
@@ -25,7 +27,7 @@ RSpec.describe 'getting Alert Management Integrations' do
     graphql_query_for(
       'project',
       { 'fullPath' => project.full_path },
-      query_graphql_field('alertManagementIntegrations', {}, fields)
+      query_graphql_field('alertManagementIntegrations', params, fields)
     )
   end
 
@@ -50,34 +52,78 @@ RSpec.describe 'getting Alert Management Integrations' do
         post_graphql(query, current_user: current_user)
       end
 
-      let(:http_integration) { integrations.first }
-      let(:prometheus_integration) { integrations.second }
+      context 'when no extra params given' do
+        let(:http_integration) { integrations.first }
+        let(:prometheus_integration) { integrations.second }
 
-      it_behaves_like 'a working graphql query'
+        it_behaves_like 'a working graphql query'
+
+        it { expect(integrations.size).to eq(2) }
+
+        it 'returns the correct properties of the integrations' do
+          expect(http_integration).to include(
+            'id' => global_id_of(active_http_integration),
+            'type' => 'HTTP',
+            'name' => active_http_integration.name,
+            'active' => active_http_integration.active,
+            'token' => active_http_integration.token,
+            'url' => active_http_integration.url,
+            'apiUrl' => nil
+          )
 
-      it { expect(integrations.size).to eq(2) }
-
-      it 'returns the correct properties of the integrations' do
-        expect(http_integration).to include(
-          'id' => GitlabSchema.id_from_object(active_http_integration).to_s,
-          'type' => 'HTTP',
-          'name' => active_http_integration.name,
-          'active' => active_http_integration.active,
-          'token' => active_http_integration.token,
-          'url' => active_http_integration.url,
-          'apiUrl' => nil
-        )
-
-        expect(prometheus_integration).to include(
-          'id' => GitlabSchema.id_from_object(prometheus_service).to_s,
-          'type' => 'PROMETHEUS',
-          'name' => 'Prometheus',
-          'active' => prometheus_service.manual_configuration?,
-          'token' => project_alerting_setting.token,
-          'url' => "http://localhost/#{project.full_path}/prometheus/alerts/notify.json",
-          'apiUrl' => prometheus_service.api_url
-        )
+          expect(prometheus_integration).to include(
+            'id' => global_id_of(prometheus_service),
+            'type' => 'PROMETHEUS',
+            'name' => 'Prometheus',
+            'active' => prometheus_service.manual_configuration?,
+            'token' => project_alerting_setting.token,
+            'url' => "http://localhost/#{project.full_path}/prometheus/alerts/notify.json",
+            'apiUrl' => prometheus_service.api_url
+          )
+        end
       end
+
+      context 'when HTTP Integration ID is given' do
+        let(:params) { { id: global_id_of(active_http_integration) } }
+
+        it_behaves_like 'a working graphql query'
+
+        it { expect(integrations).to be_one }
+
+        it 'returns the correct properties of the HTTP integration' do
+          expect(integrations.first).to include(
+            'id' => global_id_of(active_http_integration),
+            'type' => 'HTTP',
+            'name' => active_http_integration.name,
+            'active' => active_http_integration.active,
+            'token' => active_http_integration.token,
+            'url' => active_http_integration.url,
+            'apiUrl' => nil
+          )
+        end
+      end
+
+      context 'when Prometheus Integration ID is given' do
+        let(:params) { { id: global_id_of(prometheus_service) } }
+
+        it_behaves_like 'a working graphql query'
+
+        it { expect(integrations).to be_one }
+
+        it 'returns the correct properties of the Prometheus Integration' do
+          expect(integrations.first).to include(
+            'id' => global_id_of(prometheus_service),
+            'type' => 'PROMETHEUS',
+            'name' => 'Prometheus',
+            'active' => prometheus_service.manual_configuration?,
+            'token' => project_alerting_setting.token,
+            'url' => "http://localhost/#{project.full_path}/prometheus/alerts/notify.json",
+            'apiUrl' => prometheus_service.api_url
+          )
+        end
+      end
+
+      it_behaves_like 'GraphQL query with several integrations requested', graphql_query_name: 'alertManagementIntegrations'
     end
   end
 end

spec/support/shared_examples/requests/api/graphql/projects/alert_management/integrations_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'GraphQL query with several integrations requested' do |graphql_query_name:|
+  context 'when several HTTP integrations requested' do
+    let(:params_ai) { { id: global_id_of(active_http_integration) } }
+    let(:params_ii) { { id: global_id_of(inactive_http_integration) } }
+    let(:fields) { "nodes { id name }" }
+
+    let(:single_selection_query) do
+      graphql_query_for(
+        'project',
+        { 'fullPath' => project.full_path },
+        <<~QUERY
+        ai: #{query_graphql_field(graphql_query_name, params_ai, fields)}
+        QUERY
+      )
+    end
+
+    let(:multi_selection_query) do
+      graphql_query_for(
+        'project',
+        { 'fullPath' => project.full_path },
+        <<~QUERY
+        ai: #{query_graphql_field(graphql_query_name, params_ai, fields)}
+        ii: #{query_graphql_field(graphql_query_name, params_ii, fields)}
+        QUERY
+      )
+    end
+
+    it 'returns the correct properties of the integrations', :aggregate_failures do
+      post_graphql(multi_selection_query, current_user: current_user)
+
+      expect(graphql_data.dig('project', 'ai', 'nodes')).to include(
+        'id' => global_id_of(active_http_integration),
+        'name' => active_http_integration.name
+      )
+
+      expect(graphql_data.dig('project', 'ii', 'nodes')).to include(
+        'id' => global_id_of(inactive_http_integration),
+        'name' => inactive_http_integration.name
+      )
+    end
+
+    it 'batches queries' do
+      expect { post_graphql(multi_selection_query, current_user: current_user) }
+        .to issue_same_number_of_queries_as { post_graphql(single_selection_query, current_user: current_user) }.ignoring_cached_queries
+    end
+  end
+end