Merge branch 'mo-coverage-finder-graphql-cleanup' into 'master'

Refactor code coverage summary graphql resolver

See merge request gitlab-org/gitlab!55658

app/models/ci/daily_build_group_report_result.rb

@@ -4,7 +4,6 @@ module Ci
   class DailyBuildGroupReportResult < ApplicationRecord
     extend Gitlab::Ci::Model
 
-    REPORT_WINDOW = 90.days
     PARAM_TYPES = %w[coverage].freeze
 
     belongs_to :last_pipeline, class_name: 'Ci::Pipeline', foreign_key: :last_pipeline_id
@@ -13,13 +12,11 @@ module Ci
 
     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) }
     scope :by_dates, -> (start_date, end_date) { where(date: start_date..end_date) }
     scope :ordered_by_date_and_group_name, -> { order(date: :desc, group_name: :asc) }
 
@@ -29,13 +26,6 @@ module Ci
       def upsert_reports(data)
         upsert_all(data, unique_by: :index_daily_build_group_report_results_unique_columns) if data.any?
       end
-
-      def report_window(start_date)
-        default_date = REPORT_WINDOW.ago.to_date
-        date = Date.parse(start_date) rescue default_date
-
-        [date, default_date].max
-      end
     end
   end
 end

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

@@ -9,6 +9,8 @@ module EE
     # Added arguments:
     #   params:
     #     group: integer
+    #     group_activity: boolean
+    #
     module DailyBuildGroupReportResultsFinder
       extend ::Gitlab::Utils::Override
 
@@ -24,6 +26,17 @@ module EE
 
       private
 
+      override :filter_report_results
+      def filter_report_results(collection)
+        collection = super(collection)
+        collection = by_activity_per_group(collection)
+        collection
+      end
+
+      def by_activity_per_group(items)
+        params[:group_activity].present? ? items.activity_per_group : items
+      end
+
       override :query_allowed?
       def query_allowed?
         return super unless params[:group]

ee/app/graphql/resolvers/ci/code_coverage_activities_resolver.rb

@@ -12,16 +12,24 @@ module Resolvers
       alias_method :group, :object
 
       def resolve(**args)
-        project_ids = group.projects.select(:id)
-        start_date = args[:start_date].to_s
+        code_coverage_params = params(args)
 
-        ::Ci::DailyBuildGroupReportResult
-          .with_included_projects
-          .by_projects(project_ids)
-          .with_coverage
-          .with_default_branch
-          .by_date(start_date)
-          .activity_per_group
+        ::Ci::DailyBuildGroupReportResultsFinder.new(
+          params: code_coverage_params,
+          current_user: current_user
+        ).execute
+      end
+
+      private
+
+      def params(args)
+        {
+          group: group,
+          coverage: true,
+          start_date: args.dig(:start_date).to_s,
+          end_date: Date.current.to_s,
+          group_activity: true
+        }
       end
     end
   end

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

@@ -17,7 +17,8 @@ RSpec.describe Ci::DailyBuildGroupReportResultsFinder do
     let(:start_date) { '2020-03-09' }
     let(:end_date) { '2020-03-10' }
     let(:limit) { nil }
-    let(:sort) { true }
+    let(:sort) { false }
+    let(:group_activity) { nil }
 
     let(:params) do
       {
@@ -27,7 +28,8 @@ RSpec.describe Ci::DailyBuildGroupReportResultsFinder do
         start_date: start_date,
         end_date: end_date,
         sort: sort,
-        limit: limit
+        limit: limit,
+        group_activity: group_activity
       }
     end
 
@@ -40,8 +42,12 @@ RSpec.describe Ci::DailyBuildGroupReportResultsFinder do
         group.add_reporter(current_user)
       end
 
-      it 'returns coverages belonging to the group' do
-        expect(coverages).to contain_exactly(rspec_coverage, karma_coverage)
+      context 'when sort is true' do
+        let(:sort) { true }
+
+        it 'returns coverages belonging to the group' do
+          expect(coverages).to contain_exactly(rspec_coverage, karma_coverage)
+        end
       end
 
       context 'with a limit below 1000' do
@@ -65,6 +71,31 @@ RSpec.describe Ci::DailyBuildGroupReportResultsFinder do
           expect(coverages.limit_value).to eq(Ci::DailyBuildGroupReportResultsFinder::MAX_ITEMS)
         end
       end
+
+      context 'with group_activity' do
+        let(:group_activity) { true }
+
+        let_it_be(:karma_coverage_2) { create_daily_coverage('karma', 89.0, '2020-03-10', karma_project, group) }
+
+        it 'returns the group activity aggregated data' do
+          expected_results = [
+            {
+              average_coverage: 89.0,
+              coverage_count: 1,
+              project_count: 1,
+              date: '2020-03-09'.to_date
+            },
+            {
+              average_coverage: 92.0,
+              coverage_count: 2,
+              project_count: 2,
+              date: '2020-03-10'.to_date
+            }
+          ]
+
+          expect(coverages).to eq(expected_results)
+        end
+      end
     end
 
     context 'without permmissions' do

ee/spec/graphql/resolvers/ci/code_coverage_activities_resolver_spec.rb

@@ -16,8 +16,8 @@ RSpec.describe Resolvers::Ci::CodeCoverageActivitiesResolver do
     let(:start_date) { 1.day.ago.to_date.to_s }
 
     context 'when group has projects with coverage' do
-      let!(:coverage_1) { create(:ci_daily_build_group_report_result, project: project_1) }
-      let!(:coverage_2) { create(:ci_daily_build_group_report_result, project: project_2) }
+      let!(:coverage_1) { create(:ci_daily_build_group_report_result, project: project_1, group: group) }
+      let!(:coverage_2) { create(:ci_daily_build_group_report_result, project: project_2, group: group) }
 
       it 'returns coverage activity for the group' do
         expected_results = expected_activities(
@@ -42,8 +42,8 @@ RSpec.describe Resolvers::Ci::CodeCoverageActivitiesResolver do
     end
 
     context 'when coverage is included within start date' do
-      let!(:coverage_1) { create(:ci_daily_build_group_report_result, project: project_1, date: 1.week.ago) }
-      let!(:coverage_2) { create(:ci_daily_build_group_report_result, project: project_1, date: 1.week.ago, group_name: 'karma') }
+      let!(:coverage_1) { create(:ci_daily_build_group_report_result, project: project_1, date: 1.week.ago, group: group) }
+      let!(:coverage_2) { create(:ci_daily_build_group_report_result, project: project_1, date: 1.week.ago, group_name: 'karma', group: group) }
       let(:start_date) { 1.week.ago.to_date.to_s }
 
       it 'returns coverage from the start_date' do

ee/spec/requests/api/graphql/group_query_spec.rb

@@ -372,12 +372,12 @@ RSpec.describe 'getting group information' do
     end
 
     context 'when default sorting' do
-      let_it_be(:cov_1) { create(:ci_daily_build_group_report_result, project: project_1, coverage: 77.0) }
-      let_it_be(:cov_2) { create(:ci_daily_build_group_report_result, project: project_2, coverage: 88.8, date: 1.week.ago) }
-      let_it_be(:cov_3) { create(:ci_daily_build_group_report_result, project: project_1, coverage: 66.6, date: 2.weeks.ago) }
-      let_it_be(:cov_4) { create(:ci_daily_build_group_report_result, project: project_2, coverage: 99.9, date: 3.weeks.ago) }
-      let_it_be(:cov_5) { create(:ci_daily_build_group_report_result, project: project_1, coverage: 44.4, date: 4.weeks.ago) }
-      let_it_be(:cov_6) { create(:ci_daily_build_group_report_result, project: project_1, coverage: 100.0, date: 6.weeks.ago) }
+      let_it_be(:cov_1) { create(:ci_daily_build_group_report_result, project: project_1, coverage: 77.0, group: group) }
+      let_it_be(:cov_2) { create(:ci_daily_build_group_report_result, project: project_2, coverage: 88.8, date: 1.week.ago, group: group) }
+      let_it_be(:cov_3) { create(:ci_daily_build_group_report_result, project: project_1, coverage: 66.6, date: 2.weeks.ago, group: group) }
+      let_it_be(:cov_4) { create(:ci_daily_build_group_report_result, project: project_2, coverage: 99.9, date: 3.weeks.ago, group: group) }
+      let_it_be(:cov_5) { create(:ci_daily_build_group_report_result, project: project_1, coverage: 44.4, date: 4.weeks.ago, group: group) }
+      let_it_be(:cov_6) { create(:ci_daily_build_group_report_result, project: project_1, coverage: 100.0, date: 6.weeks.ago, group: group) }
 
       let(:start_date) { 5.weeks.ago.to_date.to_s }
 

spec/models/ci/daily_build_group_report_result_spec.rb

@@ -162,39 +162,5 @@ RSpec.describe Ci::DailyBuildGroupReportResult do
         end
       end
     end
-
-    describe '.by_date' do
-      subject(:coverages) { described_class.by_date(start_date) }
-
-      let!(:coverage_1) { create(:ci_daily_build_group_report_result, date: 1.week.ago) }
-
-      context 'when project has several coverage' do
-        let!(:coverage_2) { create(:ci_daily_build_group_report_result, date: 2.weeks.ago) }
-        let(:start_date) { 1.week.ago.to_date.to_s }
-
-        it 'returns the coverage from the start_date' do
-          expect(coverages).to contain_exactly(coverage_1)
-        end
-      end
-
-      context 'when start_date is over 90 days' do
-        let!(:coverage_2) { create(:ci_daily_build_group_report_result, date: 90.days.ago) }
-        let!(:coverage_3) { create(:ci_daily_build_group_report_result, date: 91.days.ago) }
-        let(:start_date) { 1.year.ago.to_date.to_s }
-
-        it 'returns the coverage in the last 90 days' do
-          expect(coverages).to contain_exactly(coverage_1, coverage_2)
-        end
-      end
-
-      context 'when start_date is not a string' do
-        let!(:coverage_2) { create(:ci_daily_build_group_report_result, date: 90.days.ago) }
-        let(:start_date) { 1.week.ago }
-
-        it 'returns the coverage in the last 90 days' do
-          expect(coverages).to contain_exactly(coverage_1, coverage_2)
-        end
-      end
-    end
   end
 end