Merge branch '39-count-unique-users-for-more-accurate-smau-reporting' into 'master'

New usage_activity_by_stage for UsageData - Manage

See merge request gitlab-org/gitlab-ee!14024

app/models/members/group_member.rb

@@ -15,8 +15,8 @@ class GroupMember < Member
   default_scope { where(source_type: SOURCE_TYPE) }
 
   scope :of_groups, ->(groups) { where(source_id: groups.select(:id)) }
-
   scope :count_users_by_group_id, -> { joins(:user).group(:source_id).count }
+  scope :of_ldap_type, -> { where(ldap: true) }
 
   after_create :update_two_factor_requirement, unless: :invite?
   after_destroy :update_two_factor_requirement, unless: :invite?

ee/app/models/concerns/ee/usage_statistics.rb

+# frozen_string_literal: true
+
+module EE
+  module UsageStatistics
+    extend ActiveSupport::Concern
+
+    class_methods do
+      def distinct_count_by(column = nil, fallback = -1)
+        distinct.count(column)
+      rescue ActiveRecord::StatementInvalid
+        fallback
+      end
+    end
+  end
+end

ee/app/models/ee/group_member.rb

@@ -6,6 +6,7 @@ module EE
 
     prepended do
       extend ::Gitlab::Utils::Override
+      include UsageStatistics
 
       validate :sso_enforcement, if: :group
       validate :group_domain_limitations, if: :group_has_domain_limitations?

ee/app/models/ldap_key.rb

 # frozen_string_literal: true
 
 class LDAPKey < Key
+  include EE::UsageStatistics # rubocop: disable Cop/InjectEnterpriseEditionModule
+
   def can_delete?
     false
   end

ee/lib/ee/gitlab/usage_data.rb

@@ -13,6 +13,13 @@ module EE
           super + [::Gitlab::UsageCounters::DesignsCounter]
         end
 
+        override :uncached_data
+        def uncached_data
+          return super unless ::Feature.enabled?(:usage_activity_by_stage, default_enabled: true)
+
+          super.merge(usage_activity_by_stage)
+        end
+
         override :features_usage_data
         def features_usage_data
           super.merge(features_usage_data_ee)
@@ -163,6 +170,23 @@ module EE
 
           count(::Issue.authored(::User.alert_bot))
         end
+
+        # Source: https://gitlab.com/gitlab-data/analytics/blob/master/transform/snowflake-dbt/data/ping_metrics_to_stage_mapping_data.csv
+        def usage_activity_by_stage
+          {
+            usage_activity_by_stage: {
+              manage: usage_activity_by_stage_manage
+            }
+          }
+        end
+
+        def usage_activity_by_stage_manage
+          {
+            groups: ::GroupMember.distinct_count_by(:user_id),
+            ldap_keys: ::LDAPKey.distinct_count_by(:user_id),
+            ldap_users: ::GroupMember.of_ldap_type.distinct_count_by(:user_id)
+          }
+        end
       end
     end
   end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::UsageData do
+  describe '.uncached_data' do
+    context 'when the :usage_activity_by_stage feaure is not enabled' do
+      before do
+        stub_feature_flags(usage_activity_by_stage: false)
+      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
+
+    context 'when the :usage_activity_by_stage feature is enabled' 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
+        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)
+        end
+      end
+    end
+  end
+end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EE::UsageStatistics do
+  describe '.distinct_count_by' do
+    let(:user_1) { create(:user) }
+    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)}
+
+      it 'returns a count of 1' do
+        expect(::GroupMember.distinct_count_by(:user_id)).to eq(1)
+      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)}
+
+      it 'returns a count of 2' do
+        expect(::GroupMember.distinct_count_by(:user_id)).to eq(2)
+      end
+    end
+
+    context 'the count query times out' do
+      before do
+        allow_any_instance_of(ActiveRecord::Relation)
+          .to receive(:count).and_raise(ActiveRecord::StatementInvalid.new(''))
+      end
+
+      it 'does not raise an error' do
+        expect { ::GroupMember.distinct_count_by(:user_id) }.not_to raise_error
+      end
+
+      it 'returns -1' do
+        expect(::GroupMember.distinct_count_by(:user_id)).to eq(-1)
+      end
+    end
+  end
+end

lib/gitlab/usage_data.rb

@@ -188,8 +188,8 @@ module Gitlab
         {} # augmented in EE
       end
 
-      def count(relation, fallback: -1)
-        relation.count
+      def count(relation, count_by: nil, fallback: -1)
+        count_by ? relation.count(count_by) : relation.count
       rescue ActiveRecord::StatementInvalid
         fallback
       end

spec/factories/group_members.rb

@@ -20,5 +20,9 @@ FactoryBot.define do
         "email#{n}@email.com"
       end
     end
+
+    trait(:ldap) do
+      ldap true
+    end
   end
 end

spec/lib/gitlab/usage_data_spec.rb

@@ -266,6 +266,12 @@ describe Gitlab::UsageData do
       expect(described_class.count(relation)).to eq(1)
     end
 
+    it 'returns the count for count_by when provided' do
+      allow(relation).to receive(:count).with(:creator_id).and_return(2)
+
+      expect(described_class.count(relation, count_by: :creator_id)).to eq(2)
+    end
+
     it 'returns the fallback value when counting fails' do
       allow(relation).to receive(:count).and_raise(ActiveRecord::StatementInvalid.new(''))
 

spec/models/members/group_member_spec.rb

@@ -3,19 +3,29 @@
 require 'spec_helper'
 
 describe GroupMember do
-  describe '.count_users_by_group_id' do
-    it 'counts users by group ID' do
-      user_1 = create(:user)
-      user_2 = create(:user)
-      group_1 = create(:group)
-      group_2 = create(:group)
-
-      group_1.add_owner(user_1)
-      group_1.add_owner(user_2)
-      group_2.add_owner(user_1)
-
-      expect(described_class.count_users_by_group_id).to eq(group_1.id => 2,
-                                                            group_2.id => 1)
+  context 'scopes' do
+    describe '.count_users_by_group_id' do
+      it 'counts users by group ID' do
+        user_1 = create(:user)
+        user_2 = create(:user)
+        group_1 = create(:group)
+        group_2 = create(:group)
+
+        group_1.add_owner(user_1)
+        group_1.add_owner(user_2)
+        group_2.add_owner(user_1)
+
+        expect(described_class.count_users_by_group_id).to eq(group_1.id => 2,
+                                                              group_2.id => 1)
+      end
+    end
+
+    describe '.of_ldap_type' do
+      it 'returns ldap type users' do
+        group_member = create(:group_member, :ldap)
+
+        expect(described_class.of_ldap_type).to eq([group_member])
+      end
     end
   end