Merge branch 'mf-codequality-widget-backend-backstage' into 'master'

Move codeclimate functions out of ee directory

See merge request gitlab-org/gitlab!36114

app/models/ci/job_artifact.rb

@@ -245,6 +245,12 @@ module Ci
       self.update_column(:file_store, file.object_store)
     end
 
+    def self.associated_file_types_for(file_type)
+      return unless file_types.include?(file_type)
+
+      [file_type]
+    end
+
     def self.total_size
       self.sum(:size)
     end

app/models/ci/pipeline.rb

@@ -624,6 +624,38 @@ module Ci
       end
     end
 
+    def batch_lookup_report_artifact_for_file_type(file_type)
+      latest_report_artifacts
+        .values_at(*::Ci::JobArtifact.associated_file_types_for(file_type.to_s))
+        .flatten
+        .compact
+        .last
+    end
+
+    # This batch loads the latest reports for each CI job artifact
+    # type (e.g. sast, dast, etc.) in a single SQL query to eliminate
+    # the need to do N different `job_artifacts.where(file_type:
+    # X).last` calls.
+    #
+    # Return a hash of file type => array of 1 job artifact
+    def latest_report_artifacts
+      ::Gitlab::SafeRequestStore.fetch("pipeline:#{self.id}:latest_report_artifacts") do
+        # Note we use read_attribute(:project_id) to read the project
+        # ID instead of self.project_id. The latter appears to load
+        # the Project model. This extra filter doesn't appear to
+        # affect query plan but included to ensure we don't leak the
+        # wrong informaiton.
+        ::Ci::JobArtifact.where(
+          id: job_artifacts.with_reports
+            .select('max(ci_job_artifacts.id) as id')
+            .where(project_id: self.read_attribute(:project_id))
+            .group(:file_type)
+        )
+          .preload(:job)
+          .group_by(&:file_type)
+      end
+    end
+
     def has_kubernetes_active?
       project.deployment_platform&.active?
     end

app/presenters/ci/pipeline_presenter.rb

@@ -110,6 +110,17 @@ module Ci
       merge_request_presenter&.target_branch_link
     end
 
+    def downloadable_path_for_report_type(file_type)
+      if (job_artifact = batch_lookup_report_artifact_for_file_type(file_type)) &&
+          can?(current_user, :read_build, job_artifact.job)
+        download_project_job_artifacts_path(
+          job_artifact.project,
+          job_artifact.job,
+          file_type: file_type,
+          proxy: true)
+      end
+    end
+
     private
 
     def plain_ref_name

app/serializers/merge_request_widget_entity.rb

@@ -85,6 +85,26 @@ class MergeRequestWidgetEntity < Grape::Entity
     end
   end
 
+  expose :blob_path do
+    expose :head_path, if: -> (mr, _) { mr.source_branch_sha } do |merge_request|
+      project_blob_path(merge_request.project, merge_request.source_branch_sha)
+    end
+
+    expose :base_path, if: -> (mr, _) { mr.diff_base_sha } do |merge_request|
+      project_blob_path(merge_request.project, merge_request.diff_base_sha)
+    end
+  end
+
+  expose :codeclimate, if: -> (mr, _) { head_pipeline_downloadable_path_for_report_type(:codequality) } do
+    expose :head_path do |merge_request|
+      head_pipeline_downloadable_path_for_report_type(:codequality)
+    end
+
+    expose :base_path do |merge_request|
+      base_pipeline_downloadable_path_for_report_type(:codequality)
+    end
+  end
+
   private
 
   delegate :current_user, to: :request
@@ -103,6 +123,16 @@ class MergeRequestWidgetEntity < Grape::Entity
       can?(current_user, :read_build, merge_request.source_project) &&
       can?(current_user, :create_pipeline, merge_request.source_project)
   end
+
+  def head_pipeline_downloadable_path_for_report_type(file_type)
+    object.head_pipeline&.present(current_user: current_user)
+      &.downloadable_path_for_report_type(file_type)
+  end
+
+  def base_pipeline_downloadable_path_for_report_type(file_type)
+    object.base_pipeline&.present(current_user: current_user)
+      &.downloadable_path_for_report_type(file_type)
+  end
 end
 
 MergeRequestWidgetEntity.prepend_if_ee('EE::MergeRequestWidgetEntity')

ee/app/models/ee/ci/job_artifact.rb

@@ -61,13 +61,17 @@ module EE
       scope :coverage_fuzzing, -> do
         with_file_types(COVERAGE_FUZZING_REPORT_TYPES)
       end
+    end
+
+    class_methods do
+      extend ::Gitlab::Utils::Override
 
-      def self.associated_file_types_for(file_type)
-        return unless file_types.include?(file_type)
+      override :associated_file_types_for
+      def associated_file_types_for(file_type)
         return LICENSE_SCANNING_REPORT_FILE_TYPES if LICENSE_SCANNING_REPORT_FILE_TYPES.include?(file_type)
         return BROWSER_PERFORMANCE_REPORT_FILE_TYPES if BROWSER_PERFORMANCE_REPORT_FILE_TYPES.include?(file_type)
 
-        [file_type]
+        super
       end
     end
 

ee/app/models/ee/ci/pipeline.rb

@@ -94,11 +94,7 @@ module EE
       def batch_lookup_report_artifact_for_file_type(file_type)
         return unless available_licensed_report_type?(file_type)
 
-        latest_report_artifacts
-          .values_at(*::Ci::JobArtifact.associated_file_types_for(file_type.to_s))
-          .flatten
-          .compact
-          .last
+        super
       end
 
       def expose_license_scanning_data?
@@ -174,30 +170,6 @@ module EE
         ::MergeRequest.merge_train_ref?(ref)
       end
 
-      # This batch loads the latest reports for each CI job artifact
-      # type (e.g. sast, dast, etc.) in a single SQL query to eliminate
-      # the need to do N different `job_artifacts.where(file_type:
-      # X).last` calls.
-      #
-      # Return a hash of file type => array of 1 job artifact
-      def latest_report_artifacts
-        ::Gitlab::SafeRequestStore.fetch("pipeline:#{self.id}:latest_report_artifacts") do
-          # Note we use read_attribute(:project_id) to read the project
-          # ID instead of self.project_id. The latter appears to load
-          # the Project model. This extra filter doesn't appear to
-          # affect query plan but included to ensure we don't leak the
-          # wrong informaiton.
-          ::Ci::JobArtifact.where(
-            id: job_artifacts.with_reports
-              .select('max(ci_job_artifacts.id) as id')
-              .where(project_id: self.read_attribute(:project_id))
-              .group(:file_type)
-          )
-            .preload(:job)
-            .group_by(&:file_type)
-        end
-      end
-
       def available_licensed_report_type?(file_type)
         feature_names = REPORT_LICENSED_FEATURES.fetch(file_type)
         feature_names.nil? || feature_names.any? { |feature| project.feature_available?(feature) }

ee/app/presenters/ee/ci/pipeline_presenter.rb

@@ -28,17 +28,6 @@ module EE
         batch_lookup_report_artifact_for_file_type(:container_scanning)
       end
 
-      def downloadable_path_for_report_type(file_type)
-        if (job_artifact = batch_lookup_report_artifact_for_file_type(file_type)) &&
-            can?(current_user, :read_build, job_artifact.job)
-          download_project_job_artifacts_path(
-            job_artifact.project,
-            job_artifact.job,
-            file_type: job_artifact.file_type,
-            proxy: true)
-        end
-      end
-
       def degradation_threshold(file_type)
         if (job_artifact = batch_lookup_report_artifact_for_file_type(file_type)) &&
             can?(current_user, :read_build, job_artifact.job)

ee/app/serializers/ee/merge_request_widget_entity.rb

@@ -6,26 +6,6 @@ module EE
     extend ActiveSupport::Concern
 
     prepended do
-      expose :blob_path do
-        expose :head_path, if: -> (mr, _) { mr.head_pipeline_sha } do |merge_request|
-          project_blob_path(merge_request.project, merge_request.head_pipeline_sha)
-        end
-
-        expose :base_path, if: -> (mr, _) { mr.base_pipeline_sha } do |merge_request|
-          project_blob_path(merge_request.project, merge_request.base_pipeline_sha)
-        end
-      end
-
-      expose :codeclimate, if: -> (mr, _) { head_pipeline_downloadable_path_for_report_type(:codequality) } do
-        expose :head_path do |merge_request|
-          head_pipeline_downloadable_path_for_report_type(:codequality)
-        end
-
-        expose :base_path do |merge_request|
-          base_pipeline_downloadable_path_for_report_type(:codequality)
-        end
-      end
-
       expose :browser_performance, if: -> (mr, _) { head_pipeline_downloadable_path_for_report_type(:browser_performance) } do
         expose :degradation_threshold do |merge_request|
           merge_request.head_pipeline&.present(current_user: current_user)
@@ -145,15 +125,5 @@ module EE
 
       ::BlockingMergeRequestEntity.represent(blocking_mr, blocking_mr_options)
     end
-
-    def head_pipeline_downloadable_path_for_report_type(file_type)
-      object.head_pipeline&.present(current_user: current_user)
-        &.downloadable_path_for_report_type(file_type)
-    end
-
-    def base_pipeline_downloadable_path_for_report_type(file_type)
-      object.base_pipeline&.present(current_user: current_user)
-        &.downloadable_path_for_report_type(file_type)
-    end
   end
 end

ee/spec/serializers/merge_request_widget_entity_spec.rb

@@ -20,17 +20,6 @@ RSpec.describe MergeRequestWidgetEntity do
     described_class.new(merge_request, current_user: user, request: request)
   end
 
-  it 'has blob path data' do
-    allow(merge_request).to receive_messages(
-      base_pipeline: pipeline,
-      head_pipeline: pipeline
-    )
-
-    expect(subject.as_json).to include(:blob_path)
-    expect(subject.as_json[:blob_path]).to include(:base_path)
-    expect(subject.as_json[:blob_path]).to include(:head_path)
-  end
-
   def create_all_artifacts
     artifacts = %i(codequality performance browser_performance load_performance)
 

spec/factories/ci/builds.rb

@@ -308,6 +308,12 @@ FactoryBot.define do
       end
     end
 
+    trait :codequality_report do
+      after(:build) do |build|
+        build.job_artifacts << create(:ci_job_artifact, :codequality, job: build)
+      end
+    end
+
     trait :test_reports do
       after(:build) do |build|
         build.job_artifacts << create(:ci_job_artifact, :junit, job: build)

spec/factories/ci/pipelines.rb

@@ -73,6 +73,14 @@ FactoryBot.define do
         end
       end
 
+      trait :with_codequality_report do
+        status { :success }
+
+        after(:build) do |pipeline, evaluator|
+          pipeline.builds << build(:ci_build, :codequality_report, pipeline: pipeline, project: pipeline.project)
+        end
+      end
+
       trait :with_test_reports do
         status { :success }
 

spec/models/ci/job_artifact_spec.rb

@@ -110,6 +110,21 @@ RSpec.describe Ci::JobArtifact do
     end
   end
 
+  describe '.associated_file_types_for' do
+    using RSpec::Parameterized::TableSyntax
+
+    subject { Ci::JobArtifact.associated_file_types_for(file_type) }
+
+    where(:file_type, :result) do
+      'codequality'         | %w(codequality)
+      'quality'             | nil
+    end
+
+    with_them do
+      it { is_expected.to eq result }
+    end
+  end
+
   describe '.erasable' do
     subject { described_class.erasable }
 

spec/models/ci/pipeline_spec.rb

@@ -2995,6 +2995,16 @@ RSpec.describe Ci::Pipeline, :mailer do
     end
   end
 
+  describe '#batch_lookup_report_artifact_for_file_type' do
+    context 'with code quality report artifact' do
+      let(:pipeline) { create(:ci_pipeline, :with_codequality_report, project: project) }
+
+      it "returns the code quality artifact" do
+        expect(pipeline.batch_lookup_report_artifact_for_file_type(:codequality)).to eq(pipeline.job_artifacts.sample)
+      end
+    end
+  end
+
   describe '#latest_report_builds' do
     it 'returns build with test artifacts' do
       test_build = create(:ci_build, :test_reports, pipeline: pipeline, project: project)

spec/serializers/merge_request_widget_entity_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe MergeRequestWidgetEntity do
 
   let(:project) { create :project, :repository }
   let(:resource) { create(:merge_request, source_project: project, target_project: project) }
+  let(:pipeline) { create(:ci_empty_pipeline, project: project) }
   let(:user) { create(:user) }
 
   let(:request) { double('request', current_user: user, project: project) }
@@ -53,6 +54,42 @@ RSpec.describe MergeRequestWidgetEntity do
       .to eq("/#{resource.project.full_path}/-/merge_requests/#{resource.iid}.diff")
   end
 
+  it 'has blob path data' do
+    allow(resource).to receive_messages(
+      base_pipeline: pipeline,
+      head_pipeline: pipeline
+    )
+
+    expect(subject).to include(:blob_path)
+    expect(subject[:blob_path]).to include(:base_path)
+    expect(subject[:blob_path]).to include(:head_path)
+  end
+
+  describe 'codequality report artifacts', :request_store do
+    before do
+      project.add_developer(user)
+
+      allow(resource).to receive_messages(
+        base_pipeline: pipeline,
+        head_pipeline: pipeline
+      )
+    end
+
+    context "with report artifacts" do
+      let(:pipeline) { create(:ci_pipeline, :with_codequality_report, project: project) }
+
+      it "has data entry" do
+        expect(subject).to include(:codeclimate)
+      end
+    end
+
+    context "without artifacts" do
+      it "does not have data entry" do
+        expect(subject).not_to include(:codeclimate)
+      end
+    end
+  end
+
   describe 'merge_request_add_ci_config_path' do
     let!(:project_auto_devops) { create(:project_auto_devops, :disabled, project: project) }