Merge branch '355088-add-training-url' into 'master'

Add security training urls to `ProjectType`

See merge request gitlab-org/gitlab!82555

doc/api/graphql/reference/index.md

@@ -15148,6 +15148,18 @@ Returns [`[ProjectSecurityTraining!]`](#projectsecuritytraining).
 | ---- | ---- | ----------- |
 | <a id="projectsecuritytrainingprovidersonlyenabled"></a>`onlyEnabled` | [`Boolean`](#boolean) | Filter the list by only enabled security trainings. |
 
+##### `Project.securityTrainingUrls`
+
+Security training URLs for the enabled training providers of the project.
+
+Returns [`[SecurityTrainingUrl!]`](#securitytrainingurl).
+
+###### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="projectsecuritytrainingurlsidentifierexternalids"></a>`identifierExternalIds` | [`[String!]!`](#string) | List of external IDs of vulnerability identifiers. |
+
 ##### `Project.sentryDetailedError`
 
 Detailed version of a Sentry error on the project.
@@ -16961,7 +16973,6 @@ Represents a vulnerability.
 | <a id="vulnerabilityresolvedby"></a>`resolvedBy` | [`UserCore`](#usercore) | User that resolved the vulnerability. |
 | <a id="vulnerabilityresolvedondefaultbranch"></a>`resolvedOnDefaultBranch` | [`Boolean!`](#boolean) | Indicates whether the vulnerability is fixed on the default branch or not. |
 | <a id="vulnerabilityscanner"></a>`scanner` | [`VulnerabilityScanner`](#vulnerabilityscanner) | Scanner metadata for the vulnerability. |
-| <a id="vulnerabilitysecuritytrainingurls"></a>`securityTrainingUrls` | [`[SecurityTrainingUrl!]`](#securitytrainingurl) | Security training URLs for the vulnerability. |
 | <a id="vulnerabilityseverity"></a>`severity` | [`VulnerabilitySeverity`](#vulnerabilityseverity) | Severity of the vulnerability (INFO, UNKNOWN, LOW, MEDIUM, HIGH, CRITICAL). |
 | <a id="vulnerabilitystate"></a>`state` | [`VulnerabilityState`](#vulnerabilitystate) | State of the vulnerability (DETECTED, CONFIRMED, RESOLVED, DISMISSED). |
 | <a id="vulnerabilitytitle"></a>`title` | [`String`](#string) | Title of the vulnerability. |

ee/app/finders/security/training_providers/base_url_finder.rb

@@ -12,9 +12,10 @@ module Security
       self.reactive_cache_key = ->(finder) { finder.full_url }
       self.reactive_cache_worker_finder = ->(id, *args) { from_cache(id) }
 
-      def initialize(provider, identifier)
+      def initialize(project, provider, identifier_external_id)
+        @project = project
         @provider = provider
-        @identifier = identifier
+        @identifier_external_id = identifier_external_id
       end
 
       def execute
@@ -26,18 +27,17 @@ module Security
       end
 
       def self.from_cache(id)
-        project_id, provider_id, identifier_id = id.split('-')
+        project_id, provider_id, identifier_external_id = id.split('-')
 
         project = Project.find(project_id)
         provider = ::Security::TrainingProvider.find(provider_id)
-        identifier = project.vulnerability_identifiers.find(identifier_id)
 
-        new(provider, identifier)
+        new(project, provider, identifier_external_id)
       end
 
       private
 
-      attr_reader :provider, :identifier
+      attr_reader :project, :provider, :identifier_external_id
 
       def response_url
         strong_memoize(:response_url) do
@@ -52,7 +52,7 @@ module Security
       # Required for ReactiveCaching; Usage overridden by
       # self.reactive_cache_worker_finder
       def id
-        "#{identifier.project.id}-#{provider.id}-#{identifier.id}"
+        "#{project.id}-#{provider.id}-#{identifier_external_id}"
       end
     end
   end

ee/app/finders/security/training_providers/kontra_url_finder.rb

@@ -15,7 +15,7 @@ module Security
       end
 
       def full_url
-        Gitlab::Utils.append_path(provider.url, "?#{identifier.external_type}=#{identifier.external_id}")
+        Gitlab::Utils.append_path(provider.url, "?cwe=#{identifier_external_id}")
       end
     end
   end

ee/app/finders/security/training_providers/secure_code_warrior_url_finder.rb

@@ -9,7 +9,7 @@ module Security
       end
 
       def full_url
-        Gitlab::Utils.append_path(provider.url, "?Id=gitlab&MappingList=#{identifier.external_type}&MappingKey=#{identifier.external_id}")
+        Gitlab::Utils.append_path(provider.url, "?Id=gitlab&MappingList=cwe&MappingKey=#{identifier_external_id}")
       end
     end
   end

ee/app/finders/security/training_urls_finder.rb

@@ -2,25 +2,27 @@
 
 module Security
   class TrainingUrlsFinder
-    def initialize(vulnerability)
-      @vulnerability = vulnerability
+    def initialize(project, identifier_external_ids)
+      @project = project
+      @identifier_external_ids = identifier_external_ids
     end
 
     def execute
-      cwe_identifiers = @vulnerability.identifiers&.with_external_type('cwe')
-      return [] if cwe_identifiers.blank?
+      return [] if identifier_external_ids.blank?
 
-      security_training_urls(cwe_identifiers)
+      security_training_urls(identifier_external_ids)
     end
 
     private
 
-    def security_training_urls(cwe_identifiers)
+    attr_reader :project, :identifier_external_ids
+
+    def security_training_urls(identifier_external_ids)
       [].tap do |content_urls|
         training_providers.each do |provider|
-          cwe_identifiers.each do |identifier|
+          identifier_external_ids.each do |identifier_external_id|
             class_name = "::Security::TrainingProviders::#{provider.name.delete(' ')}UrlFinder".safe_constantize
-            content_url = class_name.new(provider, identifier).execute if class_name
+            content_url = class_name.new(project, provider, identifier_external_id).execute if class_name
             content_urls << content_url if content_url
           end
         end
@@ -28,7 +30,7 @@ module Security
     end
 
     def training_providers
-      ::Security::TrainingProvider.for_project(@vulnerability.project, only_enabled: true).ordered_by_is_primary_desc
+      ::Security::TrainingProvider.for_project(project, only_enabled: true).ordered_by_is_primary_desc
     end
   end
 end

ee/app/graphql/ee/types/project_type.rb

@@ -207,6 +207,12 @@ module EE
               null: true,
               method: :itself,
               description: "Project's DORA metrics."
+
+        field :security_training_urls,
+              [::Types::Security::TrainingUrlType],
+              null: true,
+              description: 'Security training URLs for the enabled training providers of the project.',
+              resolver: ::Resolvers::SecurityTrainingUrlsResolver
       end
 
       def api_fuzzing_ci_configuration

ee/app/graphql/resolvers/security_training_urls_resolver.rb

@@ -4,8 +4,15 @@ module Resolvers
   class SecurityTrainingUrlsResolver < BaseResolver
     type [::Types::Security::TrainingUrlType], null: true
 
-    def resolve
-      ::Security::TrainingUrlsFinder.new(object).execute
+    argument :identifier_external_ids,
+         [GraphQL::Types::String],
+         required: true,
+         description: 'List of external IDs of vulnerability identifiers.'
+
+    alias_method :project, :object
+
+    def resolve(**args)
+      ::Security::TrainingUrlsFinder.new(project, args[:identifier_external_ids]).execute
     end
   end
 end

ee/app/graphql/types/vulnerability_type.rb

@@ -106,10 +106,6 @@ module Types
           description: 'Indicates whether the vulnerability is a false positive.',
           resolver_method: :false_positive?
 
-    field :security_training_urls, [::Types::Security::TrainingUrlType], null: true,
-          description: 'Security training URLs for the vulnerability.',
-          resolver: ::Resolvers::SecurityTrainingUrlsResolver
-
     def confirmed_by
       ::Gitlab::Graphql::Loaders::BatchModelLoader.new(::User, object.confirmed_by_id).find
     end

ee/spec/finders/security/training_providers/base_url_finder_spec.rb

@@ -10,14 +10,14 @@ RSpec.describe Security::TrainingProviders::BaseUrlFinder do
 
   describe '#execute' do
     it 'raises an error if full_url is not implemented' do
-      expect { described_class.new(nil, nil).execute }.to raise_error(
+      expect { described_class.new(nil, nil, nil).execute }.to raise_error(
         NotImplementedError,
         'full_url must be overwritten to return training url'
       )
     end
 
     context 'when response_url is nil' do
-      let_it_be(:finder) { described_class.new(provider, identifier) }
+      let_it_be(:finder) { described_class.new(identifier.project, provider, identifier.external_id) }
 
       before do
         allow_next_instance_of(described_class) do |instance|
@@ -26,12 +26,12 @@ RSpec.describe Security::TrainingProviders::BaseUrlFinder do
       end
 
       it 'returns a nil url with status pending' do
-        expect(described_class.new(provider, identifier).execute).to eq({ name: provider.name, url: nil, status: 'pending' })
+        expect(described_class.new(identifier.project, provider, identifier.external_id).execute).to eq({ name: provider.name, url: nil, status: 'pending' })
       end
     end
 
     context 'when response_url is not nil' do
-      let_it_be(:finder) { described_class.new(provider, identifier) }
+      let_it_be(:finder) { described_class.new(identifier.project, provider, identifier.external_id) }
 
       before do
         allow_next_instance_of(described_class) do |instance|
@@ -40,12 +40,12 @@ RSpec.describe Security::TrainingProviders::BaseUrlFinder do
       end
 
       it 'returns a url with status completed' do
-        expect(described_class.new(provider, identifier).execute).to eq({ name: provider.name, url: dummy_url, status: 'completed' })
+        expect(described_class.new(identifier.project, provider, identifier.external_id).execute).to eq({ name: provider.name, url: dummy_url, status: 'completed' })
       end
     end
 
     context 'when response_url is not nil, but the url is' do
-      let_it_be(:finder) { described_class.new(provider, identifier) }
+      let_it_be(:finder) { described_class.new(identifier.project, provider, identifier.external_id) }
 
       before do
         allow_next_instance_of(described_class) do |instance|
@@ -54,14 +54,14 @@ RSpec.describe Security::TrainingProviders::BaseUrlFinder do
       end
 
       it 'returns nil' do
-        expect(described_class.new(provider, identifier).execute).to be_nil
+        expect(described_class.new(identifier.project, provider, identifier.external_id).execute).to be_nil
       end
     end
   end
 
   describe '.from_cache' do
     it 'returns instance of finder object' do
-      expect(described_class.from_cache("#{identifier.project.id}-#{provider.id}-#{identifier.id}")).to be_an_instance_of(described_class)
+      expect(described_class.from_cache("#{identifier.project.id}-#{provider.id}-#{identifier.external_id}")).to be_an_instance_of(described_class)
     end
   end
 end

ee/spec/finders/security/training_providers/kontra_url_finder_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe Security::TrainingProviders::KontraUrlFinder do
 
   describe '#calculate_reactive_cache' do
     context 'when response is nil' do
-      let_it_be(:finder) { described_class.new(provider, identifier) }
+      let_it_be(:finder) {described_class.new(identifier.project, provider, identifier.external_id) }
 
       before do
         synchronous_reactive_cache(finder)
@@ -25,7 +25,7 @@ RSpec.describe Security::TrainingProviders::KontraUrlFinder do
     end
 
     context 'when response is not nil' do
-      let_it_be(:finder) { described_class.new(provider, identifier) }
+      let_it_be(:finder) { described_class.new(identifier.project, provider, identifier.external_id) }
       let_it_be(:response) { { 'link' => dummy_url } }
 
       before do
@@ -41,7 +41,7 @@ RSpec.describe Security::TrainingProviders::KontraUrlFinder do
 
   describe '#full_url' do
     it 'returns full url path' do
-      expect(described_class.new(provider, identifier).full_url).to eq('example.com/?cwe=2')
+      expect(described_class.new(identifier.project, provider, identifier.external_id).full_url).to eq('example.com/?cwe=2')
     end
   end
 end

ee/spec/finders/security/training_providers/secure_code_warrior_url_finder_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe Security::TrainingProviders::SecureCodeWarriorUrlFinder do
 
   describe '#execute' do
     context 'when response is nil' do
-      let_it_be(:finder) { described_class.new(provider, identifier) }
+      let_it_be(:finder) { described_class.new(identifier.project, provider, identifier.external_id) }
 
       before do
         synchronous_reactive_cache(finder)
@@ -25,7 +25,7 @@ RSpec.describe Security::TrainingProviders::SecureCodeWarriorUrlFinder do
     end
 
     context 'when response is not nil' do
-      let_it_be(:finder) { described_class.new(provider, identifier) }
+      let_it_be(:finder) { described_class.new(identifier.project, provider, identifier.external_id) }
       let_it_be(:response) { { 'url' => dummy_url } }
 
       before do
@@ -41,7 +41,7 @@ RSpec.describe Security::TrainingProviders::SecureCodeWarriorUrlFinder do
 
   describe '#full_url' do
     it 'returns full url path' do
-      expect(described_class.new(provider, identifier).full_url).to eq('example.com/?Id=gitlab&MappingList=cwe&MappingKey=2')
+      expect(described_class.new(identifier.project, provider, identifier.external_id).full_url).to eq('example.com/?Id=gitlab&MappingList=cwe&MappingKey=2')
     end
   end
 end

ee/spec/finders/security/training_urls_finder_spec.rb

@@ -5,21 +5,20 @@ require 'spec_helper'
 RSpec.describe Security::TrainingUrlsFinder do
   let_it_be(:project) { create(:project) }
   let_it_be(:vulnerability) { create(:vulnerability, :with_findings, project: project) }
+  let_it_be(:identifier) { create(:vulnerabilities_identifier, project: project, external_type: 'cwe', external_id: 2) }
 
-  subject { described_class.new(vulnerability).execute }
+  subject { described_class.new(project, identifier_external_ids).execute }
 
   context 'no identifier with cwe external type' do
+    let(:identifier_external_ids) { [] }
+
     it 'returns empty list' do
       is_expected.to be_empty
     end
   end
 
   context 'identifiers with cwe external type' do
-    let_it_be(:identifier) { create(:vulnerabilities_identifier, external_type: "cwe") }
-
-    before do
-      vulnerability.identifiers << identifier
-    end
+    let(:identifier_external_ids) { [identifier.external_id] }
 
     context 'when there is no training provider enabled for project' do
       it 'returns empty list' do

ee/spec/graphql/resolvers/security_training_urls_resolver_spec.rb

@@ -6,9 +6,9 @@ RSpec.describe Resolvers::SecurityTrainingUrlsResolver do
   include GraphqlHelpers
 
   describe '#resolve' do
-    subject { resolve(described_class, obj: vulnerability) }
+    subject { resolve(described_class, obj: project) }
 
-    let_it_be(:vulnerability) { create(:vulnerability, :with_findings) }
+    let_it_be(:project) { create(:project) }
 
     it 'calls TrainingUrlsFinder#execute' do
       expect_next_instance_of(::Security::TrainingUrlsFinder) do |finder|

ee/spec/graphql/types/project_type_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe GitlabSchema.types['Project'] do
       security_dashboard_path iterations iteration_cadences repository_size_excess actual_repository_size_limit
       code_coverage_summary api_fuzzing_ci_configuration corpuses path_locks incident_management_escalation_policies
       incident_management_escalation_policy scan_execution_policies network_policies incident_management_timeline_events
-      incident_management_timeline_event
+      incident_management_timeline_event security_training_urls
     ]
 
     expect(described_class).to include_graphql_fields(*expected_fields)

ee/spec/graphql/types/vulnerability_type_spec.rb

@@ -39,8 +39,7 @@ RSpec.describe GitlabSchema.types['Vulnerability'] do
        confirmed_by
        resolved_by
        dismissed_by
-       details
-       security_training_urls]
+       details]
   end
 
   before do