Merge branch '222312-update-usage-ping-data-to-track-mrs-with-added-rules' into 'master'

Resolve "Update usage ping data to track MRs with added rules"

See merge request gitlab-org/gitlab!36222

doc/development/telemetry/usage_ping.md

@@ -671,6 +671,7 @@ appear to be associated to any of the services running, since they all appear to
 | `suggestions`                                                       | `usage_activity_by_stage`            | `create`      |                  | EE      |                                                                            |
 | `approval_project_rules`                                  | `usage_activity_by_stage`            | `create`      |                  | EE      | Number of project approval rules                                           |
 | `approval_project_rules_with_target_branch`               | `usage_activity_by_stage`            | `create`      |                  | EE      | Number of project approval rules with not default target branch            |
+| `merge_requests_with_added_rules`                         | `usage_activity_by_stage`            | `create`      |                  | EE      | Merge Requests with added rules                                            |
 | `clusters`                                                | `usage_activity_by_stage`            | `monitor`     |                  | CE+EE   |                                                                            |
 | `clusters_applications_prometheus`                        | `usage_activity_by_stage`            | `monitor`     |                  | CE+EE   |                                                                            |
 | `operations_dashboard_default_dashboard`                  | `usage_activity_by_stage`            | `monitor`     |                  | CE+EE   |                                                                            |

ee/app/models/approval_merge_request_rule.rb

@@ -25,6 +25,7 @@ class ApprovalMergeRequestRule < ApplicationRecord
   end
   scope :code_owner_approval_optional, -> { code_owner.where(approvals_required: 0) }
   scope :code_owner_approval_required, -> { code_owner.where('approvals_required > 0') }
+  scope :with_added_approval_rules, -> { left_outer_joins(:approval_merge_request_rule_source).where(approval_merge_request_rule_sources: { approval_merge_request_rule_id: nil }) }
 
   validates :name, uniqueness: { scope: [:merge_request_id, :rule_type, :section] }
   validates :rule_type, uniqueness: { scope: :merge_request_id, message: proc { _('any-approver for the merge request already exists') } }, if: :any_approver?

ee/changelogs/unreleased/222312-update-usage-ping-data-to-track-mrs-with-added-rules.yml

+---
+title: Update Usage Ping data to track Merge Requests with added rules
+merge_request: 36222
+author:
+type: added

ee/lib/ee/gitlab/usage_data.rb

@@ -156,6 +156,12 @@ module EE
         end
 
         override :system_usage_data
+        # Rubocop's Metrics/AbcSize metric is disabled for this method as Rubocop
+        # determines this method to be too complex while there's no way to make it
+        # less "complex" without introducing extra methods (which actually will
+        # make things _more_ complex).
+        #
+        # rubocop: disable Metrics/AbcSize
         def system_usage_data
           super.tap do |usage_data|
             usage_data[:counts].merge!(
@@ -172,6 +178,10 @@ module EE
                 ldap_users: count(::User.ldap, 'users.id'),
                 pod_logs_usages_total: redis_usage_data { ::Gitlab::UsageCounters::PodLogs.usage_totals[:total] },
                 projects_enforcing_code_owner_approval: count(::Project.without_deleted.non_archived.requiring_code_owner_approval),
+                merge_requests_with_added_rules: distinct_count(::ApprovalMergeRequestRule.with_added_approval_rules,
+                                                                :merge_request_id,
+                                                                start: approval_merge_request_rule_minimum_id,
+                                                                finish: approval_merge_request_rule_maximum_id),
                 merge_requests_with_optional_codeowners: distinct_count(::ApprovalMergeRequestRule.code_owner_approval_optional, :merge_request_id),
                 merge_requests_with_required_codeowners: distinct_count(::ApprovalMergeRequestRule.code_owner_approval_required, :merge_request_id),
                 projects_mirrored_with_pipelines_enabled: count(::Project.mirrored_with_enabled_pipelines),
@@ -220,6 +230,10 @@ module EE
         def usage_activity_by_stage_create(time_period)
           super.merge({
             projects_enforcing_code_owner_approval: distinct_count(::Project.requiring_code_owner_approval.where(time_period), :creator_id),
+            merge_requests_with_added_rules: distinct_count(::ApprovalMergeRequestRule.where(time_period).with_added_approval_rules,
+                                                            :merge_request_id,
+                                                            start: approval_merge_request_rule_minimum_id,
+                                                            finish: approval_merge_request_rule_maximum_id),
             merge_requests_with_optional_codeowners: distinct_count(::ApprovalMergeRequestRule.code_owner_approval_optional.where(time_period), :merge_request_id),
             merge_requests_with_required_codeowners: distinct_count(::ApprovalMergeRequestRule.code_owner_approval_required.where(time_period), :merge_request_id),
             projects_imported_from_github: distinct_count(::Project.github_imported.where(time_period), :creator_id),
@@ -334,6 +348,18 @@ module EE
 
         private
 
+        def approval_merge_request_rule_minimum_id
+          strong_memoize(:approval_merge_request_rule_minimum_id) do
+            ::ApprovalMergeRequestRule.minimum(:id)
+          end
+        end
+
+        def approval_merge_request_rule_maximum_id
+          strong_memoize(:approval_merge_request_rule_maximum_id) do
+            ::ApprovalMergeRequestRule.maximum(:id)
+          end
+        end
+
         def distinct_count_service_desk_enabled_projects(time_period)
           project_creator_id_start = user_minimum_id
           project_creator_id_finish = user_maximum_id

ee/spec/factories/approval_rules.rb

@@ -6,6 +6,11 @@ FactoryBot.define do
     sequence(:name) { |n| "#{ApprovalRuleLike::DEFAULT_NAME}-#{n}" }
   end
 
+  factory :approval_merge_request_rule_source do
+    approval_merge_request_rule
+    approval_project_rule
+  end
+
   factory :code_owner_rule, parent: :approval_merge_request_rule do
     merge_request
     rule_type { :code_owner }

ee/spec/lib/ee/gitlab/usage_data_spec.rb

@@ -312,6 +312,9 @@ RSpec.describe Gitlab::UsageData do
             project = create(:project, :repository_private, :github_imported,
                               :test_repo, creator: user)
             merge_request = create(:merge_request, source_project: project)
+            project_rule = create(:approval_project_rule, project: project)
+            merge_rule = create(:approval_merge_request_rule, merge_request: merge_request)
+            create(:approval_merge_request_rule_source, approval_merge_request_rule: merge_rule, approval_project_rule: project_rule)
             create(:project, creator: user)
             create(:project, creator: user, disable_overriding_approvers_per_merge_request: true)
             create(:project, creator: user, disable_overriding_approvers_per_merge_request: false)
@@ -321,14 +324,16 @@ RSpec.describe Gitlab::UsageData do
             create(:suggestion, note: create(:note, project: project))
             create(:code_owner_rule, merge_request: merge_request, approvals_required: 3)
             create(:code_owner_rule, merge_request: merge_request, approvals_required: 7)
+            create(:approval_merge_request_rule, merge_request: merge_request)
             create_list(:code_owner_rule, 3, approvals_required: 2)
             create_list(:code_owner_rule, 2)
           end
 
           expect(described_class.uncached_data[:usage_activity_by_stage][:create]).to include(
-            approval_project_rules: 4,
+            approval_project_rules: 6,
             approval_project_rules_with_target_branch: 2,
             projects_enforcing_code_owner_approval: 0,
+            merge_requests_with_added_rules: 12,
             merge_requests_with_optional_codeowners: 4,
             merge_requests_with_required_codeowners: 8,
             projects_imported_from_github: 2,
@@ -337,9 +342,10 @@ RSpec.describe Gitlab::UsageData do
             suggestions: 2
           )
           expect(described_class.uncached_data[:usage_activity_by_stage_monthly][:create]).to include(
-            approval_project_rules: 4,
+            approval_project_rules: 6,
             approval_project_rules_with_target_branch: 2,
             projects_enforcing_code_owner_approval: 0,
+            merge_requests_with_added_rules: 6,
             merge_requests_with_optional_codeowners: 2,
             merge_requests_with_required_codeowners: 4,
             projects_imported_from_github: 1,

lib/gitlab/usage_data.rb

@@ -680,6 +680,8 @@ module Gitlab
         clear_memoization(:unique_visit_service)
         clear_memoization(:deployment_minimum_id)
         clear_memoization(:deployment_maximum_id)
+        clear_memoization(:approval_merge_request_rule_minimum_id)
+        clear_memoization(:approval_merge_request_rule_maximum_id)
       end
 
       # rubocop: disable CodeReuse/ActiveRecord

spec/lib/gitlab/usage_data_spec.rb

@@ -20,7 +20,9 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
       it 'clears memoized values' do
         values = %i(issue_minimum_id issue_maximum_id
                     user_minimum_id user_maximum_id unique_visit_service
-                    deployment_minimum_id deployment_maximum_id)
+                    deployment_minimum_id deployment_maximum_id
+                    approval_merge_request_rule_minimum_id
+                    approval_merge_request_rule_maximum_id)
         values.each do |key|
           expect(described_class).to receive(:clear_memoization).with(key)
         end