Merge branch 'mo-add-coquality-merge-request' into 'master'

Add coquality comparison for merge request

See merge request gitlab-org/gitlab!48830

app/controllers/projects/merge_requests_controller.rb

@@ -21,7 +21,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     :exposed_artifacts,
     :coverage_reports,
     :terraform_reports,
-    :accessibility_reports
+    :accessibility_reports,
+    :codequality_reports
   ]
   before_action :set_issuables_index, only: [:index]
   before_action :authenticate_user!, only: [:assign_related_issues]
@@ -194,6 +195,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     end
   end
 
+  def codequality_reports
+    reports_response(@merge_request.compare_codequality_reports)
+  end
+
   def terraform_reports
     reports_response(@merge_request.find_terraform_reports)
   end

app/models/merge_request.rb

@@ -1461,6 +1461,20 @@ class MergeRequest < ApplicationRecord
     compare_reports(Ci::GenerateCoverageReportsService)
   end
 
+  def has_codequality_reports?
+    return false unless Feature.enabled?(:codequality_mr_diff, project)
+
+    actual_head_pipeline&.has_reports?(Ci::JobArtifact.codequality_reports)
+  end
+
+  def compare_codequality_reports
+    unless has_codequality_reports?
+      return { status: :error, status_reason: _('This merge request does not have codequality reports') }
+    end
+
+    compare_reports(Ci::CompareCodequalityReportsService)
+  end
+
   def find_terraform_reports
     unless has_terraform_reports?
       return { status: :error, status_reason: 'This merge request does not have terraform reports' }

app/services/ci/compare_codequality_reports_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class CompareCodequalityReportsService < CompareReportsBaseService
+    def comparer_class
+      Gitlab::Ci::Reports::CodequalityReportsComparer
+    end
+
+    def serializer_class
+      CodequalityReportsComparerSerializer
+    end
+
+    def get_report(pipeline)
+      pipeline&.codequality_reports
+    end
+  end
+end

config/feature_flags/development/codequality_mr_diff.yml

+---
+name: codequality_mr_diff
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47938
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/284140
+milestone: '13.7'
+type: development
+group: group::testing
+default_enabled: false

config/routes/merge_requests.rb

@@ -17,6 +17,7 @@ resources :merge_requests, concerns: :awardable, except: [:new, :create, :show],
     get :accessibility_reports
     get :coverage_reports
     get :terraform_reports
+    get :codequality_reports
 
     scope constraints: ->(req) { req.format == :json }, as: :json do
       get :commits

locale/gitlab.pot

@@ -28185,6 +28185,9 @@ msgstr ""
 msgid "This merge request does not have accessibility reports"
 msgstr ""
 
+msgid "This merge request does not have codequality reports"
+msgstr ""
+
 msgid "This merge request is closed. To apply this suggestion, edit this file directly."
 msgstr ""
 

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -1498,6 +1498,121 @@ RSpec.describe Projects::MergeRequestsController do
     end
   end
 
+  describe 'GET codequality_reports' do
+    let_it_be(:merge_request) do
+      create(:merge_request,
+        :with_diffs,
+        :with_merge_request_pipeline,
+        target_project: project,
+        source_project: project
+      )
+    end
+
+    let(:pipeline) do
+      create(:ci_pipeline,
+        :success,
+        project: merge_request.source_project,
+        ref: merge_request.source_branch,
+        sha: merge_request.diff_head_sha)
+    end
+
+    before do
+      allow_any_instance_of(MergeRequest)
+        .to receive(:compare_codequality_reports)
+        .and_return(codequality_comparison)
+
+      allow_any_instance_of(MergeRequest)
+        .to receive(:actual_head_pipeline)
+        .and_return(pipeline)
+    end
+
+    subject do
+      get :codequality_reports, params: {
+        namespace_id: project.namespace.to_param,
+        project_id: project,
+        id: merge_request.iid
+      },
+      format: :json
+    end
+
+    context 'permissions on a public project with private CI/CD' do
+      let(:project) { project_public_with_private_builds }
+      let(:codequality_comparison) { { status: :parsed, data: { summary: 1 } } }
+
+      context 'while signed out' do
+        before do
+          sign_out(user)
+        end
+
+        it 'responds with a 404' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:not_found)
+          expect(response.body).to be_blank
+        end
+      end
+
+      context 'while signed in as an unrelated user' do
+        before do
+          sign_in(create(:user))
+        end
+
+        it 'responds with a 404' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:not_found)
+          expect(response.body).to be_blank
+        end
+      end
+    end
+
+    context 'when pipeline has jobs with codequality reports' do
+      before do
+        allow_any_instance_of(MergeRequest)
+          .to receive(:has_codequality_reports?)
+          .and_return(true)
+      end
+
+      context 'when processing codequality reports is in progress' do
+        let(:codequality_comparison) { { status: :parsing } }
+
+        it 'sends polling interval' do
+          expect(Gitlab::PollingInterval).to receive(:set_header)
+
+          subject
+        end
+
+        it 'returns 204 HTTP status' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:no_content)
+        end
+      end
+
+      context 'when processing codequality reports is completed' do
+        let(:codequality_comparison) { { status: :parsed, data: { summary: 1 } } }
+
+        it 'returns codequality reports' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to eq({ 'summary' => 1 })
+        end
+      end
+    end
+
+    context 'when pipeline has job without a codequality report' do
+      let(:codequality_comparison) { { status: :error, status_reason: 'no codequality report' } }
+
+      it 'returns a 400' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response).to eq({ 'status_reason' => 'no codequality report' })
+      end
+    end
+  end
+
   describe 'POST remove_wip' do
     before do
       merge_request.title = merge_request.wip_title

spec/factories/merge_requests.rb

@@ -159,6 +159,18 @@ FactoryBot.define do
       end
     end
 
+    trait :with_codequality_reports do
+      after(:build) do |merge_request|
+        merge_request.head_pipeline = build(
+          :ci_pipeline,
+          :success,
+          :with_codequality_reports,
+          project: merge_request.source_project,
+          ref: merge_request.source_branch,
+          sha: merge_request.diff_head_sha)
+      end
+    end
+
     trait :unique_branches do
       source_branch { generate(:branch) }
       target_branch { generate(:branch) }

spec/models/merge_request_spec.rb

@@ -1929,6 +1929,32 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
   end
 
+  describe '#has_codequality_reports?' do
+    subject { merge_request.has_codequality_reports? }
+
+    let(:project) { create(:project, :repository) }
+
+    context 'when head pipeline has a codequality report' do
+      let(:merge_request) { create(:merge_request, :with_codequality_reports, source_project: project) }
+
+      it { is_expected.to be_truthy }
+
+      context 'when feature flag is disabled' do
+        before do
+          stub_feature_flags(codequality_mr_diff: false)
+        end
+
+        it { is_expected.to be_falsey }
+      end
+    end
+
+    context 'when head pipeline does not have a codequality report' do
+      let(:merge_request) { create(:merge_request, source_project: project) }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe '#has_terraform_reports?' do
     context 'when head pipeline has terraform reports' do
       it 'returns true' do
@@ -2205,6 +2231,62 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
   end
 
+  describe '#compare_codequality_reports' do
+    let_it_be(:project) { create(:project, :repository) }
+    let_it_be(:merge_request, reload: true) { create(:merge_request, :with_codequality_reports, source_project: project) }
+    let_it_be(:pipeline) { merge_request.head_pipeline }
+
+    subject { merge_request.compare_codequality_reports }
+
+    context 'when head pipeline has codequality report' do
+      let(:job) do
+        create(:ci_build, options: { artifacts: { reports: { codeclimate: ['codequality.json'] } } }, pipeline: pipeline)
+      end
+
+      let(:artifacts_metadata) { create(:ci_job_artifact, :metadata, job: job) }
+
+      context 'when reactive cache worker is parsing results asynchronously' do
+        it 'returns parsing status' do
+          expect(subject[:status]).to eq(:parsing)
+        end
+      end
+
+      context 'when reactive cache worker is inline' do
+        before do
+          synchronous_reactive_cache(merge_request)
+        end
+
+        it 'returns parsed status' do
+          expect(subject[:status]).to eq(:parsed)
+          expect(subject[:data]).to be_present
+        end
+
+        context 'when an error occurrs' do
+          before do
+            merge_request.update!(head_pipeline: nil)
+          end
+
+          it 'returns an error status' do
+            expect(subject[:status]).to eq(:error)
+            expect(subject[:status_reason]).to eq("This merge request does not have codequality reports")
+          end
+        end
+
+        context 'when cached result is not latest' do
+          before do
+            allow_next_instance_of(Ci::CompareCodequalityReportsService) do |service|
+              allow(service).to receive(:latest?).and_return(false)
+            end
+          end
+
+          it 'raises an InvalidateReactiveCache error' do
+            expect { subject }.to raise_error(ReactiveCaching::InvalidateReactiveCache)
+          end
+        end
+      end
+    end
+  end
+
   describe '#all_commit_shas' do
     context 'when merge request is persisted' do
       let(:all_commit_shas) do

spec/services/ci/compare_codequality_reports_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::CompareCodequalityReportsService do
+  let(:service) { described_class.new(project) }
+  let(:project) { create(:project, :repository) }
+
+  describe '#execute' do
+    subject { service.execute(base_pipeline, head_pipeline) }
+
+    context 'when head pipeline has a codequality report' do
+      let(:base_pipeline) { nil }
+      let(:head_pipeline) { create(:ci_pipeline, :with_codequality_reports, project: project) }
+
+      it 'returns status and data' do
+        expect(subject[:status]).to eq(:parsed)
+        expect(subject[:data]).to match_schema('entities/codequality_reports_comparer')
+      end
+    end
+
+    context 'when base and head pipelines have codequality reports' do
+      let(:base_pipeline) { create(:ci_pipeline, :with_codequality_reports, project: project) }
+      let(:head_pipeline) { create(:ci_pipeline, :with_codequality_reports, project: project) }
+
+      it 'returns status and data' do
+        expect(subject[:status]).to eq(:parsed)
+        expect(subject[:data]).to match_schema('entities/codequality_reports_comparer')
+      end
+    end
+  end
+
+  describe '#latest?' do
+    subject { service.latest?(base_pipeline, head_pipeline, data) }
+
+    let!(:base_pipeline) { nil }
+    let!(:head_pipeline) { create(:ci_pipeline, :with_codequality_reports, project: project) }
+    let!(:key) { service.send(:key, base_pipeline, head_pipeline) }
+
+    context 'when cache key is latest' do
+      let(:data) { { key: key } }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when cache key is outdated' do
+      before do
+        head_pipeline.update_column(:updated_at, 10.minutes.ago)
+      end
+
+      let(:data) { { key: key } }
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when cache key is empty' do
+      let(:data) { { key: nil } }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+end