Merge branch 'mo-introduce-codequality-presenter' into 'master'

Persist code quality payload mr diff

See merge request gitlab-org/gitlab!52232

app/serializers/ci/codequality_mr_diff_entity.rb

+# frozen_string_literal: true
+
+module Ci
+  class CodequalityMrDiffEntity < Grape::Entity
+    expose :files
+  end
+end

app/serializers/ci/codequality_mr_diff_report_serializer.rb

+# frozen_string_literal: true
+
+module Ci
+  class CodequalityMrDiffReportSerializer < BaseSerializer
+    entity CodequalityMrDiffEntity
+  end
+end

app/services/ci/pipeline_artifacts/create_quality_report_service.rb

@@ -22,11 +22,17 @@ module Ci
 
       def build_carrierwave_file(pipeline)
         CarrierWaveStringFile.new_file(
-          file_content: pipeline.codequality_reports.to_json,
+          file_content: build_quality_mr_diff_report(pipeline),
           filename: Ci::PipelineArtifact::DEFAULT_FILE_NAMES.fetch(:code_quality),
           content_type: 'application/json'
         )
       end
+
+      def build_quality_mr_diff_report(pipeline)
+        mr_diff_report = Gitlab::Ci::Reports::CodequalityMrDiff.new(pipeline.codequality_reports)
+
+        Ci::CodequalityMrDiffReportSerializer.new.represent(mr_diff_report).to_json # rubocop: disable CodeReuse/Serializer
+      end
     end
   end
 end

lib/gitlab/ci/reports/codequality_mr_diff.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Reports
+      class CodequalityMrDiff
+        attr_reader :files
+
+        def initialize(raw_report)
+          @raw_report = raw_report
+          @files = {}
+          build_report!
+        end
+
+        private
+
+        def build_report!
+          codequality_files = @raw_report.all_degradations.each_with_object({}) do |degradation, codequality_files|
+            unless codequality_files[degradation.dig(:location, :path)].present?
+              codequality_files[degradation.dig(:location, :path)] = []
+            end
+
+            build_mr_diff_payload(codequality_files, degradation)
+          end
+
+          @files = codequality_files
+        end
+
+        def build_mr_diff_payload(codequality_files, degradation)
+          codequality_files[degradation.dig(:location, :path)] << {
+            line: degradation.dig(:location, :lines, :begin) || degradation.dig(:location, :positions, :begin, :line),
+            description: degradation[:description],
+            severity: degradation[:severity]
+          }
+        end
+      end
+    end
+  end
+end

spec/factories/ci/reports/codequality_degradations.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :codequality_degradation_1, class: Hash do
+    skip_create
+
+    initialize_with do
+      {
+        "categories": [
+          "Complexity"
+        ],
+        "check_name": "argument_count",
+        "content": {
+          "body": ""
+        },
+        "description": "Avoid parameter lists longer than 5 parameters. [12/5]",
+        "fingerprint": "15cdb5c53afd42bc22f8ca366a08d547",
+        "location": {
+          "path": "file_a.rb",
+          "lines": {
+            "begin": 10,
+            "end": 10
+          }
+        },
+        "other_locations": [],
+        "remediation_points": 900000,
+        "severity": "major",
+        "type": "issue",
+        "engine_name": "structure"
+      }.with_indifferent_access
+    end
+  end
+
+  factory :codequality_degradation_2, class: Hash do
+    skip_create
+
+    initialize_with do
+      {
+        "categories": [
+          "Complexity"
+        ],
+        "check_name": "argument_count",
+        "content": {
+          "body": ""
+        },
+        "description": "Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.",
+        "fingerprint": "f3bdc1e8c102ba5fbd9e7f6cda51c95e",
+        "location": {
+          "path": "file_a.rb",
+          "lines": {
+            "begin": 10,
+            "end": 10
+          }
+        },
+        "other_locations": [],
+        "remediation_points": 900000,
+        "severity": "major",
+        "type": "issue",
+        "engine_name": "structure"
+      }.with_indifferent_access
+    end
+  end
+
+  factory :codequality_degradation_3, class: Hash do
+    skip_create
+
+    initialize_with do
+      {
+        "type": "Issue",
+        "check_name": "Rubocop/Metrics/ParameterLists",
+        "description": "Avoid parameter lists longer than 5 parameters. [12/5]",
+        "categories": [
+          "Complexity"
+        ],
+        "remediation_points": 550000,
+        "location": {
+          "path": "file_b.rb",
+          "positions": {
+            "begin": {
+              "column": 14,
+              "line": 10
+            },
+            "end": {
+              "column": 39,
+              "line": 10
+            }
+          }
+        },
+        "content": {
+          "body": "This cop checks for methods with too many parameters.\nThe maximum number of parameters is configurable.\nKeyword arguments can optionally be excluded from the total count."
+        },
+        "engine_name": "rubocop",
+        "fingerprint": "ab5f8b935886b942d621399f5a2ca16e",
+        "severity": "minor"
+      }.with_indifferent_access
+    end
+  end
+end

spec/fixtures/api/schemas/entities/codequality_mr_diff_report.json

+{
+  "type": "object",
+  "description": "The schema used to display codequality report in mr diff",
+  "required": ["files"],
+  "properties": {
+    "patternProperties": {
+      ".*.": {
+        "type": "array",
+        "items": {
+          "required": ["line", "description", "severity"],
+          "properties": {
+            "line": { "type": "integer" },
+            "description": { "type": "string" },
+            "severity": { "type": "string" }
+          },
+          "additionalProperties": false
+        }
+      }
+    }
+  }
+}

spec/lib/gitlab/ci/reports/codequality_mr_diff_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Reports::CodequalityMrDiff do
+  let(:codequality_report) { Gitlab::Ci::Reports::CodequalityReports.new }
+  let(:degradation_1) { build(:codequality_degradation_1) }
+  let(:degradation_2) { build(:codequality_degradation_2) }
+  let(:degradation_3) { build(:codequality_degradation_3) }
+
+  describe '#initialize!' do
+    subject(:report) { described_class.new(codequality_report) }
+
+    context 'when quality has degradations' do
+      context 'with several degradations on the same line' do
+        before do
+          codequality_report.add_degradation(degradation_1)
+          codequality_report.add_degradation(degradation_2)
+        end
+
+        it 'generates quality report for mr diff' do
+          expect(report.files).to match(
+            "file_a.rb" => [
+              { line: 10, description: "Avoid parameter lists longer than 5 parameters. [12/5]", severity: "major" },
+              { line: 10, description: "Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.", severity: "major" }
+            ]
+          )
+        end
+      end
+
+      context 'with several degradations on several files' do
+        before do
+          codequality_report.add_degradation(degradation_1)
+          codequality_report.add_degradation(degradation_2)
+          codequality_report.add_degradation(degradation_3)
+        end
+
+        it 'returns quality report for mr diff' do
+          expect(report.files).to match(
+            "file_a.rb" => [
+              { line: 10, description: "Avoid parameter lists longer than 5 parameters. [12/5]", severity: "major" },
+              { line: 10, description: "Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.", severity: "major" }
+            ],
+            "file_b.rb" => [
+              { line: 10, description: "Avoid parameter lists longer than 5 parameters. [12/5]", severity: "minor" }
+            ]
+          )
+        end
+      end
+    end
+
+    context 'when quality has no degradation' do
+      it 'returns an empty hash' do
+        expect(report.files).to match({})
+      end
+    end
+  end
+end

spec/serializers/ci/codequality_mr_diff_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::CodequalityMrDiffEntity do
+  let(:entity) { described_class.new(mr_diff_report) }
+  let(:mr_diff_report) { Gitlab::Ci::Reports::CodequalityMrDiff.new(codequality_report) }
+  let(:codequality_report) { Gitlab::Ci::Reports::CodequalityReports.new }
+  let(:degradation_1) { build(:codequality_degradation_1) }
+  let(:degradation_2) { build(:codequality_degradation_2) }
+
+  describe '#as_json' do
+    subject(:report) { entity.as_json }
+
+    context 'when quality report has degradations' do
+      before do
+        codequality_report.add_degradation(degradation_1)
+        codequality_report.add_degradation(degradation_2)
+      end
+
+      it 'contains correct codequality mr diff report', :aggregate_failures do
+        expect(report[:files].keys).to eq(["file_a.rb"])
+        expect(report[:files]["file_a.rb"].first).to include(:line, :description, :severity)
+      end
+    end
+  end
+end

spec/serializers/ci/codequality_mr_diff_report_serializer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::CodequalityMrDiffReportSerializer do
+  let(:serializer) { described_class.new.represent(mr_diff_report) }
+  let(:mr_diff_report) { Gitlab::Ci::Reports::CodequalityMrDiff.new(codequality_report) }
+  let(:codequality_report) { Gitlab::Ci::Reports::CodequalityReports.new }
+  let(:degradation_1) { build(:codequality_degradation_1) }
+  let(:degradation_2) { build(:codequality_degradation_2) }
+
+  describe '#to_json' do
+    subject { serializer.as_json }
+
+    context 'when quality report has degradations' do
+      before do
+        codequality_report.add_degradation(degradation_1)
+        codequality_report.add_degradation(degradation_2)
+      end
+
+      it 'matches the schema' do
+        expect(subject).to match_schema('entities/codequality_mr_diff_report')
+      end
+    end
+
+    context 'when quality report has no degradations' do
+      it 'matches the schema' do
+        expect(subject).to match_schema('entities/codequality_mr_diff_report')
+      end
+    end
+  end
+end