Merge branch 'ajk-gql-dast-site' into 'master'

Improvements to dast site GQL resolvers

See merge request gitlab-org/gitlab!50426

app/graphql/mutations/concerns/mutations/authorizes_project.rb

-# frozen_string_literal: true
-
-module Mutations
-  module AuthorizesProject
-    include ResolvesProject
-
-    def authorized_find_project!(full_path:)
-      authorized_find!(full_path: full_path)
-    end
-
-    private
-
-    def find_object(full_path:)
-      resolve_project(full_path: full_path)
-    end
-  end
-end

app/graphql/mutations/concerns/mutations/finds_project.rb

+# frozen_string_literal: true
+
+module Mutations
+  module FindsProject
+    private
+
+    def find_object(full_path)
+      Project.find_by_full_path(full_path)
+    end
+  end
+end

ee/app/graphql/mutations/dast_on_demand_scans/create.rb

@@ -3,9 +3,9 @@
 module Mutations
   module DastOnDemandScans
     class Create < BaseMutation
-      InvalidGlobalID = Class.new(StandardError)
+      include FindsProject
 
-      include AuthorizesProject
+      InvalidGlobalID = Class.new(StandardError)
 
       graphql_name 'DastOnDemandScanCreate'
 
@@ -28,7 +28,7 @@ module Mutations
       authorize :create_on_demand_dast_scan
 
       def resolve(full_path:, dast_site_profile_id:, **args)
-        project = authorized_find_project!(full_path: full_path)
+        project = authorized_find!(full_path)
 
         dast_site_profile = find_dast_site_profile(project, dast_site_profile_id)
         dast_scanner_profile = find_dast_scanner_profile(project, args[:dast_scanner_profile_id])

ee/app/graphql/mutations/dast_scanner_profiles/create.rb

@@ -3,7 +3,7 @@
 module Mutations
   module DastScannerProfiles
     class Create < BaseMutation
-      include AuthorizesProject
+      include FindsProject
 
       graphql_name 'DastScannerProfileCreate'
 
@@ -53,7 +53,7 @@ module Mutations
       authorize :create_on_demand_dast_scan
 
       def resolve(full_path:, profile_name:, spider_timeout: nil, target_timeout: nil, scan_type:, use_ajax_spider:, show_debug_messages:)
-        project = authorized_find_project!(full_path: full_path)
+        project = authorized_find!(full_path)
 
         service = ::DastScannerProfiles::CreateService.new(project, current_user)
         result = service.execute(

ee/app/graphql/mutations/dast_scanner_profiles/delete.rb

@@ -3,7 +3,7 @@
 module Mutations
   module DastScannerProfiles
     class Delete < BaseMutation
-      include AuthorizesProject
+      include FindsProject
 
       graphql_name 'DastScannerProfileDelete'
 
@@ -24,7 +24,7 @@ module Mutations
         # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
         id = ScannerProfileID.coerce_isolated_input(id)
 
-        project = authorized_find_project!(full_path: full_path)
+        project = authorized_find!(full_path)
 
         service = ::DastScannerProfiles::DestroyService.new(project, current_user)
         result = service.execute(id: id.model_id)

ee/app/graphql/mutations/dast_site_profiles/create.rb

@@ -3,7 +3,7 @@
 module Mutations
   module DastSiteProfiles
     class Create < BaseMutation
-      include AuthorizesProject
+      include FindsProject
 
       graphql_name 'DastSiteProfileCreate'
 
@@ -26,7 +26,7 @@ module Mutations
       authorize :create_on_demand_dast_scan
 
       def resolve(full_path:, profile_name:, target_url: nil)
-        project = authorized_find_project!(full_path: full_path)
+        project = authorized_find!(full_path)
 
         service = ::DastSiteProfiles::CreateService.new(project, current_user)
         result = service.execute(name: profile_name, target_url: target_url)

ee/app/graphql/mutations/dast_site_profiles/delete.rb

@@ -3,8 +3,6 @@
 module Mutations
   module DastSiteProfiles
     class Delete < BaseMutation
-      include AuthorizesProject
-
       graphql_name 'DastSiteProfileDelete'
 
       argument :full_path, GraphQL::ID_TYPE,
@@ -18,7 +16,7 @@ module Mutations
       authorize :create_on_demand_dast_scan
 
       def resolve(full_path:, id:)
-        project = authorized_find_project!(full_path: full_path)
+        project = authorized_find!(full_path)
         # TODO: remove explicit coercion once compatibility layer is removed
         # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
         id = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(id)
@@ -28,10 +26,16 @@ module Mutations
         return { errors: dast_site_profile.errors.full_messages } unless dast_site_profile.destroy
 
         { errors: [] }
+      rescue ActiveRecord::RecordNotFound
+        raise_resource_not_available_error!
       end
 
       private
 
+      def find_object(full_path)
+        Project.find_by_full_path(full_path)
+      end
+
       def find_dast_site_profile(project:, global_id:)
         project.dast_site_profiles.find(global_id.model_id)
       rescue ActiveRecord::RecordNotFound

ee/app/graphql/mutations/dast_site_profiles/update.rb

@@ -3,7 +3,7 @@
 module Mutations
   module DastSiteProfiles
     class Update < BaseMutation
-      include AuthorizesProject
+      include FindsProject
 
       graphql_name 'DastSiteProfileUpdate'
 
@@ -33,7 +33,7 @@ module Mutations
         # TODO: remove explicit coercion once compatibility layer has been removed
         # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
         service_args[:id] = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(id).model_id
-        project = authorized_find_project!(full_path: full_path)
+        project = authorized_find!(full_path)
 
         service = ::DastSiteProfiles::UpdateService.new(project, current_user)
         result = service.execute(**service_args)

ee/app/graphql/mutations/dast_site_tokens/create.rb

@@ -3,7 +3,7 @@
 module Mutations
   module DastSiteTokens
     class Create < BaseMutation
-      include AuthorizesProject
+      include FindsProject
 
       graphql_name 'DastSiteTokenCreate'
 
@@ -30,7 +30,7 @@ module Mutations
       authorize :create_on_demand_dast_scan
 
       def resolve(full_path:, target_url:)
-        project = authorized_find_project!(full_path: full_path)
+        project = authorized_find!(full_path)
         raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless allowed?(project)
 
         response = ::DastSiteTokens::CreateService.new(

ee/app/graphql/mutations/dast_site_validations/create.rb

@@ -3,7 +3,7 @@
 module Mutations
   module DastSiteValidations
     class Create < BaseMutation
-      include AuthorizesProject
+      include FindsProject
 
       graphql_name 'DastSiteValidationCreate'
 
@@ -34,7 +34,7 @@ module Mutations
       authorize :create_on_demand_dast_scan
 
       def resolve(full_path:, dast_site_token_id:, validation_path:, strategy: :text_file)
-        project = authorized_find_project!(full_path: full_path)
+        project = authorized_find!(full_path)
         raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless allowed?(project)
 
         dast_site_token = dast_site_token_id.find

ee/spec/graphql/resolvers/dast_site_profile_resolver_spec.rb

@@ -5,14 +5,12 @@ require 'spec_helper'
 RSpec.describe Resolvers::DastSiteProfileResolver do
   include GraphqlHelpers
 
-  let_it_be(:current_user) { create(:user) }
   let_it_be(:project) { create(:project) }
+  let_it_be(:developer) { create(:user, developer_projects: [project] ) }
   let_it_be(:dast_site_profile1) { create(:dast_site_profile, project: project) }
   let_it_be(:dast_site_profile2) { create(:dast_site_profile, project: project) }
 
-  before do
-    project.add_maintainer(current_user)
-  end
+  let(:current_user) { developer }
 
   specify do
     expect(described_class).to have_nullable_graphql_type(Types::DastSiteProfileType.connection_type)
@@ -21,7 +19,7 @@ RSpec.describe Resolvers::DastSiteProfileResolver do
   context 'when resolving a single DAST site profile' do
     subject { sync(single_dast_site_profile(id: dast_site_profile1.to_global_id)) }
 
-    it { is_expected.to contain_exactly(dast_site_profile1) }
+    it { is_expected.to eq dast_site_profile1 }
   end
 
   context 'when resolving multiple DAST site profiles' do
@@ -32,11 +30,11 @@ RSpec.describe Resolvers::DastSiteProfileResolver do
 
   private
 
-  def dast_site_profiles(args = {}, context = { current_user: current_user })
-    resolve(described_class, obj: project, args: args, ctx: context)
+  def dast_site_profiles
+    resolve(described_class, obj: project, ctx: { current_user: current_user })
   end
 
-  def single_dast_site_profile(args = {}, context = { current_user: current_user })
-    resolve(described_class, obj: project, args: args, ctx: context)
+  def single_dast_site_profile(**args)
+    resolve(described_class.single, obj: project, args: args, ctx: { current_user: current_user })
   end
 end

ee/spec/graphql/resolvers/dast_site_validation_resolver_spec.rb

@@ -23,11 +23,10 @@ RSpec.describe Resolvers::DastSiteValidationResolver do
     expect(described_class).to have_nullable_graphql_type(Types::DastSiteValidationType.connection_type)
   end
 
-  subject { sync(resolver) }
-
   context 'when resolving multiple DAST site validations' do
+    subject { dast_site_validations(**args) }
+
     let(:args) { {} }
-    let(:resolver) { dast_site_validations(args) }
 
     it { is_expected.to contain_exactly(dast_site_validation3, dast_site_validation2, dast_site_validation1) }
 
@@ -52,7 +51,8 @@ RSpec.describe Resolvers::DastSiteValidationResolver do
 
   private
 
-  def dast_site_validations(args = {}, context = { current_user: current_user })
+  def dast_site_validations(**args)
+    context = { current_user: current_user }
     resolve(described_class, obj: project, args: args, ctx: context)
   end
 end