Merge branch 'mo-add-group-coverage-data-finder-2' into 'master'

Add daily group coverage data new finder

See merge request gitlab-org/gitlab!54111

app/finders/ci/testing/daily_build_group_report_results_finder.rb

@@ -84,3 +84,5 @@ module Ci
     end
   end
 end
+
+Ci::Testing::DailyBuildGroupReportResultsFinder.prepend_if_ee('::EE::Ci::Testing::DailyBuildGroupReportResultsFinder')

app/models/ci/daily_build_group_report_result.rb

@@ -9,12 +9,14 @@ module Ci
 
     belongs_to :last_pipeline, class_name: 'Ci::Pipeline', foreign_key: :last_pipeline_id
     belongs_to :project
+    belongs_to :group
 
     validates :data, json_schema: { filename: "daily_build_group_report_result_data" }
 
     scope :with_included_projects, -> { includes(:project) }
     scope :by_ref_path, -> (ref_path) { where(ref_path: ref_path) }
     scope :by_projects, -> (ids) { where(project_id: ids) }
+    scope :by_group, -> (group_id) { where(group_id: group_id) }
     scope :with_coverage, -> { where("(data->'coverage') IS NOT NULL") }
     scope :with_default_branch, -> { where(default_branch: true) }
     scope :by_date, -> (start_date) { where(date: report_window(start_date)..Date.current) }

app/models/group.rb

@@ -48,6 +48,7 @@ class Group < Namespace
 
   has_many :labels, class_name: 'GroupLabel'
   has_many :variables, class_name: 'Ci::GroupVariable'
+  has_many :daily_build_group_report_results, class_name: 'Ci::DailyBuildGroupReportResult'
   has_many :custom_attributes, class_name: 'GroupCustomAttribute'
 
   has_many :boards

ee/app/controllers/groups/analytics/coverage_reports_controller.rb

@@ -32,7 +32,25 @@ class Groups::Analytics::CoverageReportsController < Groups::Analytics::Applicat
   end
 
   def report_results
-    Ci::DailyBuildGroupReportResultsByGroupFinder.new(**finder_params).execute
+    if ::Gitlab::Ci::Features.use_coverage_data_new_finder?(@group)
+      ::Ci::Testing::DailyBuildGroupReportResultsFinder.new(
+        params: new_finder_params,
+        current_user: current_user
+      ).execute
+    else
+      Ci::DailyBuildGroupReportResultsByGroupFinder.new(**finder_params).execute
+    end
+  end
+
+  def new_finder_params
+    {
+      group: @group,
+      coverage: true,
+      start_date: Date.parse(params.require(:start_date)),
+      end_date: Date.parse(params.require(:end_date)),
+      ref_path: params[:ref_path],
+      sort: true
+    }
   end
 
   def finder_params

ee/app/finders/ee/ci/testing/daily_build_group_report_results_finder.rb

+# frozen_string_literal: true
+
+module EE
+  module Ci
+    module Testing
+      # DailyBuildGroupReportResultsFinder
+      #
+      # Extends DailyBuildGroupReportResultsFinder
+      #
+      # Added arguments:
+      #   params:
+      #     group: integer
+      module DailyBuildGroupReportResultsFinder
+        extend ::Gitlab::Utils::Override
+
+        override :execute
+        def execute
+          return super unless params[:group]
+          return ::Ci::DailyBuildGroupReportResult.none unless query_allowed?
+
+          collection = ::Ci::DailyBuildGroupReportResult.by_group(params[:group])
+          collection = filter_report_results(collection)
+          collection
+        end
+
+        private
+
+        override :query_allowed?
+        def query_allowed?
+          return super unless params[:group]
+
+          can?(current_user, :read_group_build_report_results, params[:group])
+        end
+      end
+    end
+  end
+end

ee/spec/controllers/groups/analytics/coverage_reports_controller_spec.rb

@@ -8,8 +8,8 @@ RSpec.describe Groups::Analytics::CoverageReportsController do
   let_it_be(:project) { create(:project, namespace: group) }
   let_it_be(:ref_path) { 'refs/heads/master' }
 
-  let_it_be(:first_coverage) { create_daily_coverage('rspec', project, 79.0, '2020-03-09') }
-  let_it_be(:last_coverage) { create_daily_coverage('karma', project, 95.0, '2020-03-10') }
+  let_it_be(:first_coverage) { create_daily_coverage('rspec', project, 79.0, '2020-03-09', group) }
+  let_it_be(:last_coverage) { create_daily_coverage('karma', project, 95.0, '2020-03-10', group) }
 
   let_it_be(:valid_request_params) do
     {
@@ -59,71 +59,96 @@ RSpec.describe Groups::Analytics::CoverageReportsController do
         stub_licensed_features(group_coverage_reports: true)
       end
 
-      it 'responds 200 with CSV coverage data', :snowplow do
-        get :index, params: valid_request_params
-
-        expect_snowplow_event(
-          category: described_class.name,
-          action: 'download_code_coverage_csv',
-          label: 'group_id',
-          value: group.id
-        )
-
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(csv_response).to eq([
-          %w[date group_name project_name coverage],
-          [last_coverage.date.to_s, last_coverage.group_name, project.name, last_coverage.data['coverage'].to_s],
-          [first_coverage.date.to_s, first_coverage.group_name, project.name, first_coverage.data['coverage'].to_s]
-        ])
-      end
-
-      context 'with a project_id filter' do
-        let(:params) { valid_request_params.merge(project_ids: [project.id]) }
+      context 'when feature coverage_data_new_finder is enabled' do
+        before do
+          stub_feature_flags(coverage_data_new_finder: true)
+        end
 
         it 'responds 200 with CSV coverage data' do
-          expect(Ci::DailyBuildGroupReportResultsByGroupFinder).to receive(:new).with({
-            group: group,
-            current_user: user,
-            project_ids: [project.id.to_s],
-            start_date: Date.parse('2020-03-01'),
-            end_date: Date.parse('2020-03-31'),
-            ref_path: ref_path
-          }).and_call_original
-
-          get :index, params: params
+          get :index, params: valid_request_params
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(csv_response).to eq([
+            %w[date group_name project_name coverage],
+            [last_coverage.date.to_s, last_coverage.group_name, project.name, last_coverage.data['coverage'].to_s],
+            [first_coverage.date.to_s, first_coverage.group_name, project.name, first_coverage.data['coverage'].to_s]
+          ])
         end
       end
 
-      context 'when ref_path is nil' do
-        let(:ref_path) { nil }
+      context 'when feature coverage_data_new_finder is disabled' do
+        before do
+          stub_feature_flags(coverage_data_new_finder: false)
+        end
 
-        it 'responds HTTP 200' do
+        it 'responds 200 with CSV coverage data', :snowplow do
           get :index, params: valid_request_params
 
+          expect_snowplow_event(
+            category: described_class.name,
+            action: 'download_code_coverage_csv',
+            label: 'group_id',
+            value: group.id
+          )
+
           expect(response).to have_gitlab_http_status(:ok)
-          expect(csv_response.size).to eq(3)
+          expect(csv_response).to eq([
+            %w[date group_name project_name coverage],
+            [last_coverage.date.to_s, last_coverage.group_name, project.name, last_coverage.data['coverage'].to_s],
+            [first_coverage.date.to_s, first_coverage.group_name, project.name, first_coverage.data['coverage'].to_s]
+          ])
         end
-      end
 
-      it 'executes the same number of queries regardless of the number of records returned' do
-        control = ActiveRecord::QueryRecorder.new do
-          get :index, params: valid_request_params
+        context 'with a project_id filter' do
+          let(:params) { valid_request_params.merge(project_ids: [project.id]) }
+
+          it 'responds 200 with CSV coverage data' do
+            expect(Ci::DailyBuildGroupReportResultsByGroupFinder).to receive(:new).with({
+              group: group,
+              current_user: user,
+              project_ids: [project.id.to_s],
+              start_date: Date.parse('2020-03-01'),
+              end_date: Date.parse('2020-03-31'),
+              ref_path: ref_path
+            }).and_call_original
+
+            get :index, params: params
+
+            expect(response).to have_gitlab_http_status(:ok)
+          end
         end
 
-        expect(CSV.parse(response.body).length).to eq(3)
+        context 'when ref_path is nil' do
+          let(:ref_path) { nil }
 
-        create_daily_coverage('rspec', project, 79.0, '2020-03-10')
+          it 'responds HTTP 200' do
+            get :index, params: valid_request_params
 
-        expect { get :index, params: valid_request_params }.not_to exceed_query_limit(control)
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(csv_response.size).to eq(3)
+          end
+        end
 
-        expect(csv_response.length).to eq(4)
-      end
+        it 'executes the same number of queries regardless of the number of records returned' do
+          control = ActiveRecord::QueryRecorder.new do
+            get :index, params: valid_request_params
+          end
+
+          expect(CSV.parse(response.body).length).to eq(3)
+
+          create_daily_coverage('rspec', project, 79.0, '2020-03-10')
+
+          expect { get :index, params: valid_request_params }.not_to exceed_query_limit(control)
+
+          expect(csv_response.length).to eq(4)
+        end
 
-      context 'with an invalid format' do
-        it 'responds 404' do
-          get :index, params: valid_request_params.merge(format: :json)
+        context 'with an invalid format' do
+          it 'responds 404' do
+            get :index, params: valid_request_params.merge(format: :json)
 
-          expect(response).to have_gitlab_http_status(:not_found)
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
         end
       end
     end
@@ -131,14 +156,15 @@ RSpec.describe Groups::Analytics::CoverageReportsController do
 
   private
 
-  def create_daily_coverage(group_name, project, coverage, date)
+  def create_daily_coverage(group_name, project, coverage, date, group = nil)
     create(
       :ci_daily_build_group_report_result,
       project: project,
       ref_path: ref_path,
       group_name: group_name,
       data: { 'coverage' => coverage },
-      date: date
+      date: date,
+      group: group
     )
   end
 end

ee/spec/finders/ee/ci/testing/daily_build_group_report_results_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::Testing::DailyBuildGroupReportResultsFinder do
+  describe '#execute' do
+    let_it_be(:current_user) { create(:user) }
+    let_it_be(:group) { create(:group, :private) }
+    let_it_be(:rspec_project) { create(:project, namespace: group) }
+    let_it_be(:rspec_coverage) { create_daily_coverage('rspec', 95.0, '2020-03-10', rspec_project, group) }
+    let_it_be(:karma_project) { create(:project, namespace: group) }
+    let_it_be(:karma_coverage) { create_daily_coverage('karma', 89.0, '2020-03-09', karma_project, group) }
+    let_it_be(:generic_project) { create(:project) }
+    let_it_be(:generic_coverage) { create_daily_coverage('unreported', 95.0, '2020-03-10', generic_project) }
+
+    let(:ref_path) { 'refs/heads/master' }
+    let(:start_date) { '2020-03-09' }
+    let(:end_date) { '2020-03-10' }
+    let(:limit) { nil }
+    let(:sort) { true }
+
+    let(:params) do
+      {
+        group: group,
+        coverage: true,
+        ref_path: ref_path,
+        start_date: start_date,
+        end_date: end_date,
+        sort: sort,
+        limit: limit
+      }
+    end
+
+    let(:finder) { described_class.new(params: params, current_user: current_user) }
+
+    subject(:coverages) { finder.execute }
+
+    context 'with permissions' do
+      before do
+        group.add_reporter(current_user)
+      end
+
+      it 'returns coverages belonging to the group' do
+        expect(coverages).to contain_exactly(rspec_coverage, karma_coverage)
+      end
+
+      context 'with a limit below 1000' do
+        let(:limit) { 5 }
+
+        it 'uses the provided limit' do
+          expect(coverages.limit_value).to eq(5)
+        end
+      end
+
+      context 'with a limit above 1000' do
+        let(:limit) { 1001 }
+
+        it 'returns MAX_ITEMS as a limit' do
+          expect(coverages.limit_value).to eq(Ci::Testing::DailyBuildGroupReportResultsFinder::MAX_ITEMS)
+        end
+      end
+
+      context 'without a limit' do
+        it 'returns MAX_ITEMS as a limit' do
+          expect(coverages.limit_value).to eq(Ci::Testing::DailyBuildGroupReportResultsFinder::MAX_ITEMS)
+        end
+      end
+    end
+
+    context 'without permmissions' do
+      it 'returns an empty result' do
+        expect(coverages).to be_empty
+      end
+    end
+  end
+
+  private
+
+  def create_daily_coverage(group_name, coverage, date, project, group = nil)
+    create(
+      :ci_daily_build_group_report_result,
+      project: project,
+      ref_path: 'refs/heads/master',
+      group_name: group_name,
+      data: { 'coverage' => coverage },
+      date: date,
+      group: group
+    )
+  end
+end

spec/factories/ci/daily_build_group_report_results.rb

@@ -7,6 +7,7 @@ FactoryBot.define do
     project
     last_pipeline factory: :ci_pipeline
     group_name { 'rspec' }
+    group
     data do
       { 'coverage' => 77.0 }
     end

spec/models/ci/daily_build_group_report_result_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe Ci::DailyBuildGroupReportResult do
   describe 'associations' do
     it { is_expected.to belong_to(:last_pipeline) }
     it { is_expected.to belong_to(:project) }
+    it { is_expected.to belong_to(:group) }
   end
 
   describe 'validations' do
@@ -83,8 +84,9 @@ RSpec.describe Ci::DailyBuildGroupReportResult do
   end
 
   describe 'scopes' do
-    let_it_be(:project) { create(:project) }
-    let(:recent_build_group_report_result) { create(:ci_daily_build_group_report_result, project: project) }
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, group: group) }
+    let(:recent_build_group_report_result) { create(:ci_daily_build_group_report_result, project: project, group: group) }
     let(:old_build_group_report_result) do
       create(:ci_daily_build_group_report_result, date: 1.week.ago, project: project)
     end
@@ -97,6 +99,14 @@ RSpec.describe Ci::DailyBuildGroupReportResult do
       end
     end
 
+    describe '.by_group' do
+      subject { described_class.by_group(group) }
+
+      it 'returns records by group' do
+        expect(subject).to contain_exactly(recent_build_group_report_result)
+      end
+    end
+
     describe '.by_ref_path' do
       subject(:coverages) { described_class.by_ref_path(recent_build_group_report_result.ref_path) }
 

spec/models/group_spec.rb

@@ -32,6 +32,7 @@ RSpec.describe Group do
     it { is_expected.to have_many(:dependency_proxy_blobs) }
     it { is_expected.to have_many(:dependency_proxy_manifests) }
     it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::GroupDistribution').dependent(:destroy) }
+    it { is_expected.to have_many(:daily_build_group_report_results).class_name('Ci::DailyBuildGroupReportResult') }
 
     describe '#members & #requesters' do
       let(:requester) { create(:user) }