Incorporate feedback fixes

Added usage_statistics concern with scope for models

ee/app/models/ee/epic.rb

@@ -13,6 +13,7 @@ module EE
       include Awardable
       include LabelEventable
       include RelativePositioning
+      include UsageStatistics
 
       enum state_id: { opened: 1, closed: 2 }
       alias_attribute :state, :state_id

ee/app/models/ee/issue.rb

@@ -12,9 +12,11 @@ module EE
       WEIGHT_NONE = 'None'.freeze
 
       include Elastic::ApplicationVersionedSearch
+      include UsageStatistics
 
       scope :order_weight_desc, -> { reorder ::Gitlab::Database.nulls_last_order('weight', 'DESC') }
       scope :order_weight_asc, -> { reorder ::Gitlab::Database.nulls_last_order('weight') }
+      scope :service_desk, -> { where(author: ::User.support_bot) }
 
       has_one :epic_issue
       has_one :epic, through: :epic_issue

ee/app/models/ee/list.rb

@@ -7,6 +7,8 @@ module EE
     # ActiveSupport::Concern does not prepend the ClassMethods,
     # so we cannot call `super` if we use it.
     def self.prepended(base)
+      base.include(UsageStatistics)
+
       class << base
         prepend ClassMethods
       end

ee/app/models/ee/note.rb

@@ -8,6 +8,7 @@ module EE
     prepended do
       include ::ObjectStorage::BackgroundMove
       include Elastic::ApplicationVersionedSearch
+      include UsageStatistics
 
       belongs_to :review, inverse_of: :notes
 

ee/app/models/ee/project.rb

@@ -21,6 +21,7 @@ module EE
       include InsightsFeature
       include Vulnerable
       include DeprecatedApprovalsBeforeMerge
+      include UsageStatistics
 
       self.ignored_columns += %i[
         mirror_last_update_at
@@ -106,6 +107,11 @@ module EE
       scope :for_plan_name, -> (name) { joins(namespace: :plan).where(plans: { name: name }) }
       scope :requiring_code_owner_approval,
             -> { where(merge_requests_require_code_owner_approval: true) }
+      scope :with_active_services, -> { joins(:services).merge(::Service.active) }
+      scope :with_active_jira_services, -> { joins(:services).merge(::JiraService.active) }
+      scope :with_jira_dvcs_cloud, -> { joins(:feature_usage).merge(ProjectFeatureUsage.with_jira_dvcs_integration_enabled(cloud: true)) }
+      scope :with_jira_dvcs_server, -> { joins(:feature_usage).merge(ProjectFeatureUsage.with_jira_dvcs_integration_enabled(cloud: false)) }
+      scope :service_desk_enabled, -> { where(service_desk_enabled: true) }
 
       scope :with_security_reports_stored, -> { where('EXISTS (?)', ::Vulnerabilities::Occurrence.scoped_project.select(1)) }
       scope :with_security_reports, -> { where('EXISTS (?)', ::Ci::JobArtifact.security_reports.scoped_project.select(1)) }

ee/app/models/ee/todo.rb

@@ -2,8 +2,13 @@
 
 module EE
   module Todo
+    extend ActiveSupport::Concern
     extend ::Gitlab::Utils::Override
 
+    prepended do
+      include UsageStatistics
+    end
+
     override :parent
     def parent
       project || group

ee/lib/ee/gitlab/usage_data.rb

@@ -15,7 +15,11 @@ module EE
 
         override :uncached_data
         def uncached_data
-          return super unless ::Feature.enabled?(:usage_activity_by_stage, default_enabled: true)
+          # The `usage_activity_by_stage` queries are likely to time out on large instances, and are sure
+          # to time out on GitLab.com. Since we are mostly interested in gathering these statistics for
+          # self hosted instances, prevent them from running on GitLab.com and allow instance maintainers
+          # to disable them via a feature flag.
+          return super if ::Gitlab.com? || ::Feature.disabled?(:usage_activity_by_stage, default_enabled: true)
 
           super.merge(usage_activity_by_stage)
         end
@@ -179,11 +183,13 @@ module EE
         def usage_activity_by_stage
           {
             usage_activity_by_stage: {
-              manage: usage_activity_by_stage_manage
+              manage: usage_activity_by_stage_manage,
+              plan: usage_activity_by_stage_plan
             }
           }
         end
 
+        # Omitted because no user, creator or author associated: `campaigns_imported_from_github`, `ldap_group_links`
         def usage_activity_by_stage_manage
           {
             groups: ::GroupMember.distinct_count_by(:user_id),
@@ -191,6 +197,27 @@ module EE
             ldap_users: ::GroupMember.of_ldap_type.distinct_count_by(:user_id)
           }
         end
+
+        # Omitted because no user, creator or author associated: `boards`, `labels`, `milestones`, `uploads`
+        # Omitted because too expensive: `epics_deepest_relationship_level`
+        # Omitted because of encrypted properties: `projects_jira_cloud_active`, `projects_jira_server_active`
+        def usage_activity_by_stage_plan
+          {
+            assignee_lists: ::List.assignee.distinct_count_by(:user_id),
+            epics: ::Epic.distinct_count_by(:author_id),
+            issues: ::Issue.distinct_count_by(:author_id),
+            label_lists: ::List.label.distinct_count_by(:user_id),
+            milestone_lists: ::List.milestone.distinct_count_by(:user_id),
+            notes: ::Note.distinct_count_by(:author_id),
+            projects: ::Project.distinct_count_by(:creator_id),
+            projects_jira_active: ::Project.with_active_jira_services.distinct_count_by(:creator_id),
+            projects_jira_dvcs_cloud_active: ::Project.with_active_jira_services.with_jira_dvcs_cloud.distinct_count_by(:creator_id),
+            projects_jira_dvcs_server_active: ::Project.with_active_jira_services.with_jira_dvcs_server.distinct_count_by(:creator_id),
+            service_desk_enabled_projects: ::Project.with_active_services.service_desk_enabled.distinct_count_by(:creator_id),
+            service_desk_issues: ::Issue.service_desk.distinct_count_by,
+            todos: ::Todo.distinct_count_by(:author_id)
+          }
+        end
       end
     end
   end

ee/spec/factories/project_feature_usage.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :project_feature_usage do
+    project
+
+    trait :dvcs_cloud do
+      jira_dvcs_cloud_last_sync_at { Time.current }
+    end
+
+    trait :dvcs_server do
+      jira_dvcs_server_last_sync_at { Time.current }
+    end
+  end
+end

ee/spec/factories/projects.rb

@@ -65,5 +65,21 @@ FactoryBot.modify do
     trait :requiring_code_owner_approval do
       merge_requests_require_code_owner_approval true
     end
+
+    trait :jira_dvcs_cloud do
+      before(:create) do |project|
+        create(:project_feature_usage, :dvcs_cloud, project: project)
+      end
+    end
+
+    trait :jira_dvcs_server do
+      before(:create) do |project|
+        create(:project_feature_usage, :dvcs_server, project: project)
+      end
+    end
+
+    trait :service_desk_disabled do
+      service_desk_enabled nil
+    end
   end
 end

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

@@ -4,6 +4,26 @@ require 'spec_helper'
 
 describe Gitlab::UsageData do
   describe '.uncached_data' do
+    context 'when on Gitlab.com' do
+      before do
+        allow(Gitlab).to receive(:com?).and_return(true)
+      end
+
+      it 'does not include usage_activity_by_stage data' do
+        expect(described_class.uncached_data).not_to include(:usage_activity_by_stage)
+      end
+
+      context 'when feature is enabled' do
+        before do
+          stub_feature_flags(usage_activity_by_stage: true)
+        end
+
+        it 'does not include usage_activity_by_stage data' do
+          expect(described_class.uncached_data).not_to include(:usage_activity_by_stage)
+        end
+      end
+    end
+
     context 'when the :usage_activity_by_stage feaure is not enabled' do
       before do
         stub_feature_flags(usage_activity_by_stage: false)
@@ -14,19 +34,58 @@ describe Gitlab::UsageData do
       end
     end
 
-    context 'when the :usage_activity_by_stage feature is enabled' do
+    context 'when not on Gitlab.com' do
       it 'includes usage_activity_by_stage data' do
         expect(described_class.uncached_data).to include(:usage_activity_by_stage)
       end
 
-      context 'for manage' do
+      context 'for Manage' do
         it 'includes accurate usage_activity_by_stage data' do
           user = create(:user)
           create(:group_member, user: user)
           create(:key, type: 'LDAPKey', user: user)
           create(:group_member, ldap: true, user: user)
 
-          expect(described_class.uncached_data[:usage_activity_by_stage][:manage]).to eq(groups: 1, ldap_keys: 1, ldap_users: 1)
+          expect(described_class.uncached_data[:usage_activity_by_stage][:manage]).to eq(
+            groups: 1,
+            ldap_keys: 1,
+            ldap_users: 1
+          )
+        end
+      end
+
+      context 'for Plan' do
+        it 'includes accurate usage_activity_by_stage data' do
+          stub_licensed_features(board_assignee_lists: true, board_milestone_lists: true)
+
+          user    = create(:user)
+          project = create(:project, creator: user)
+          issue   = create(:issue, project: project, author: User.support_bot)
+          board   = create(:board, project: project)
+          create(:user_list, board: board, user: user)
+          create(:milestone_list, board: board, milestone: create(:milestone, project: project), user: user)
+          create(:list, board: board, label: create(:label, project: project), user: user)
+          create(:note, project: project, noteable: issue, author: user)
+          create(:epic, author: user)
+          create(:todo, project: project, target: issue, author: user)
+          create(:jira_service, :jira_cloud_service, active: true, project: create(:project, :jira_dvcs_cloud, creator: user))
+          create(:jira_service, active: true, project: create(:project, :jira_dvcs_server, creator: user))
+
+          expect(described_class.uncached_data[:usage_activity_by_stage][:plan]).to eq(
+            assignee_lists: 1,
+            epics: 1,
+            issues: 1,
+            label_lists: 1,
+            milestone_lists: 1,
+            notes: 1,
+            projects: 1,
+            projects_jira_active: 1,
+            projects_jira_dvcs_cloud_active: 1,
+            projects_jira_dvcs_server_active: 1,
+            service_desk_enabled_projects: 1,
+            service_desk_issues: 1,
+            todos: 1
+          )
         end
       end
     end

ee/spec/lib/gitlab/usage_data_spec.rb

@@ -236,6 +236,7 @@ describe Gitlab::UsageData do
     subject { described_class.data.dig(:counts, :incident_issues) }
 
     before do
+      ::User.support_bot # create the support bot user beforehand, because otherwise it is created when gathering usage data.
       create(:issue, project: project) # non incident issue
     end
 

ee/spec/models/concerns/ee/usage_statistics_spec.rb

@@ -8,16 +8,22 @@ describe EE::UsageStatistics do
     let(:user_2) { create(:user) }
 
     context 'two records created by the same user' do
-      let!(:models_created_by_user_1) { create_list(:group_member, 2, user: user_1)}
+      let!(:models_created_by_user_1) { create_list(:group_member, 2, user: user_1) }
 
       it 'returns a count of 1' do
         expect(::GroupMember.distinct_count_by(:user_id)).to eq(1)
       end
+
+      context 'when given no colum to count' do
+        it 'counts by :id and returns a count of 2' do
+          expect(::GroupMember.distinct_count_by).to eq(2)
+        end
+      end
     end
 
     context 'one record created by each user' do
-      let!(:model_created_by_user_1) { create(:group_member, user: user_1)}
-      let!(:model_created_by_user_2) { create(:group_member, user: user_2)}
+      let!(:model_created_by_user_1) { create(:group_member, user: user_1) }
+      let!(:model_created_by_user_2) { create(:group_member, user: user_2) }
 
       it 'returns a count of 2' do
         expect(::GroupMember.distinct_count_by(:user_id)).to eq(2)

ee/spec/models/issue_spec.rb

@@ -7,6 +7,18 @@ describe Issue do
 
   using RSpec::Parameterized::TableSyntax
 
+  context 'scopes' do
+    describe '.service_desk' do
+      it 'returns the service desk issue' do
+        service_desk_issue = create(:issue, author: ::User.support_bot)
+        regular_issue = create(:issue)
+
+        expect(described_class.service_desk).to include(service_desk_issue)
+        expect(described_class.service_desk).not_to include(regular_issue)
+      end
+    end
+  end
+
   describe 'validations' do
     subject { build(:issue) }
 

ee/spec/models/project_spec.rb

@@ -62,6 +62,56 @@ describe Project do
         expect(described_class.with_wiki_enabled).not_to include(project1)
       end
     end
+
+    describe '.with_active_services' do
+      it 'returns the correct project' do
+        active_service = create(:service, active: true)
+        inactive_service = create(:service, active: false)
+
+        expect(described_class.with_active_services).to include(active_service.project)
+        expect(described_class.with_active_services).not_to include(inactive_service.project)
+      end
+    end
+
+    describe '.with_active_jira_services' do
+      it 'returns the correct project' do
+        active_jira_service = create(:jira_service)
+        active_service = create(:service, active: true)
+
+        expect(described_class.with_active_jira_services).to include(active_jira_service.project)
+        expect(described_class.with_active_jira_services).not_to include(active_service.project)
+      end
+    end
+
+    describe '.service_desk_enabled' do
+      it 'returns the correct project' do
+        project_with_service_desk_enabled = create(:project)
+        project_with_service_desk_disabled = create(:project, :service_desk_disabled)
+
+        expect(described_class.service_desk_enabled).to include(project_with_service_desk_enabled)
+        expect(described_class.service_desk_enabled).not_to include(project_with_service_desk_disabled)
+      end
+    end
+
+    describe '.with_jira_dvcs_cloud' do
+      it 'returns the correct project' do
+        jira_dvcs_cloud_project = create(:project, :jira_dvcs_cloud)
+        jira_dvcs_server_project = create(:project, :jira_dvcs_server)
+
+        expect(described_class.with_jira_dvcs_cloud).to include(jira_dvcs_cloud_project)
+        expect(described_class.with_jira_dvcs_cloud).not_to include(jira_dvcs_server_project)
+      end
+    end
+
+    describe '.with_jira_dvcs_server' do
+      it 'returns the correct project' do
+        jira_dvcs_server_project = create(:project, :jira_dvcs_server)
+        jira_dvcs_cloud_project = create(:project, :jira_dvcs_cloud)
+
+        expect(described_class.with_jira_dvcs_server).to include(jira_dvcs_server_project)
+        expect(described_class.with_jira_dvcs_server).not_to include(jira_dvcs_cloud_project)
+      end
+    end
   end
 
   describe 'validations' do