Merge branch '12079-productivity-analytics-part-1' into 'master'

Add productivity analytics data gathering

Closes #12079

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

app/models/merge_request_diff.rb

@@ -197,7 +197,7 @@ class MergeRequestDiff < ApplicationRecord
 
   def lines_count
     strong_memoize(:lines_count) do
-      diffs.diff_files.sum(&:line_count)
+      raw_diffs(limits: false).line_count
     end
   end
 
@@ -222,6 +222,10 @@ class MergeRequestDiff < ApplicationRecord
     commits.last
   end
 
+  def last_commit
+    commits.first
+  end
+
   def base_commit
     return unless base_commit_sha
 

app/services/merge_request_metrics_service.rb

@@ -19,3 +19,5 @@ class MergeRequestMetricsService
     update!(latest_closed_by_id: nil, latest_closed_at: nil)
   end
 end
+
+MergeRequestMetricsService.prepend_if_ee('EE::MergeRequestMetricsService')

ee/app/models/ee/note.rb

@@ -12,6 +12,7 @@ module EE
       belongs_to :review, inverse_of: :notes
 
       scope :searchable, -> { where(system: false).includes(:noteable) }
+      scope :by_humans, -> { user.joins(:author).merge(::User.humans) }
 
       after_commit :notify_after_create, on: :create
       after_commit :notify_after_destroy, on: :destroy

ee/app/models/ee/user.rb

@@ -72,6 +72,9 @@ module EE
         joins(:identities).where(identities: { provider: provider })
       end
 
+      scope :bots, -> { where.not(bot_type: nil) }
+      scope :humans, -> { where(bot_type: nil) }
+
       accepts_nested_attributes_for :namespace
 
       enum roadmap_layout: { weeks: 1, months: 4, quarters: 12 }

ee/app/services/ee/merge_request_metrics_service.rb

+# frozen_string_literal: true
+
+module EE
+  module MergeRequestMetricsService
+    extend ::Gitlab::Utils::Override
+
+    delegate :merge_request, to: :@merge_request_metrics
+
+    override :merge
+    def merge(event)
+      data = {
+        merged_by_id: event.author_id,
+        merged_at: event.created_at
+      }.merge(productivity_calculator.productivity_data)
+
+      update!(data)
+    end
+
+    private
+
+    def productivity_calculator
+      @productivity_calculator ||= Analytics::ProductivityCalculator.new(merge_request)
+    end
+  end
+end

ee/lib/analytics/productivity_calculator.rb

+# frozen_string_literal: true
+
+module Analytics
+  class ProductivityCalculator
+    def initialize(merge_request)
+      @merge_request = merge_request
+    end
+
+    def productivity_data
+      {
+        first_comment_at: first_comment_at,
+        first_commit_at: first_commit_at,
+        last_commit_at: last_commit_at,
+        commits_count: commits_count,
+        diff_size: diff_size,
+        modified_paths_size: modified_paths_size
+      }
+    end
+
+    private
+
+    attr_reader :merge_request
+
+    delegate :commits_count, :merge_request_diff, to: :merge_request
+
+    def diff_size
+      merge_request_diff.lines_count
+    end
+
+    def first_comment_at
+      merge_request.notes.by_humans.fresh.first&.created_at
+    end
+
+    def first_commit_at
+      merge_request_diff&.first_commit&.authored_date
+    end
+
+    def last_commit_at
+      merge_request_diff&.last_commit&.committed_date
+    end
+
+    def modified_paths_size
+      merge_request.modified_paths.size
+    end
+  end
+end

ee/spec/factories/users.rb

@@ -9,6 +9,10 @@ FactoryBot.modify do
     trait :group_managed do
       association :managing_group, factory: :group
     end
+
+    trait :bot do
+      bot_type { User.bot_types[:support_bot] }
+    end
   end
 
   factory :omniauth_user do

ee/spec/lib/analytics/productivity_calculator_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Analytics::ProductivityCalculator do
+  subject { described_class.new(merge_request) }
+  let(:merge_request) { create(:merge_request_with_diff_notes, :merged, :with_diffs, created_at: 31.days.ago) }
+
+  describe '#productivity_data' do
+    it 'calculates productivity data' do
+      expected_data = {
+        first_comment_at: merge_request.notes.order(created_at: :asc).first.created_at,
+        first_commit_at: merge_request.first_commit.authored_date,
+        last_commit_at: merge_request.merge_request_diff.last_commit.committed_date,
+        commits_count: merge_request.commits_count,
+        diff_size: merge_request.merge_request_diff.lines_count,
+        modified_paths_size: merge_request.modified_paths.size
+      }
+
+      expect(subject.productivity_data).to eq(expected_data)
+    end
+  end
+end

ee/spec/models/ee/note_spec.rb

@@ -80,4 +80,14 @@ describe Note do
       expect(note).to be_for_design
     end
   end
+
+  describe '.by_humans' do
+    it 'return human notes only' do
+      user_note = create(:note)
+      create(:system_note)
+      create(:note, author: create(:user, :bot))
+
+      expect(described_class.by_humans).to match_array([user_note])
+    end
+  end
 end

ee/spec/models/user_spec.rb

@@ -71,6 +71,16 @@ describe User do
         expect(user_ids).not_to include(group_guest_user)
       end
     end
+
+    describe 'bots & humans' do
+      it 'returns corresponding users' do
+        human = create(:user)
+        bot = create(:user, :bot)
+
+        expect(described_class.humans).to match_array([human])
+        expect(described_class.bots).to match_array([bot])
+      end
+    end
   end
 
   describe '.find_by_smartcard_identity' do

ee/spec/services/ee/merge_request_metrics_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EE::MergeRequestMetricsService do
+  subject { MergeRequestMetricsService.new(merge_request.metrics) }
+
+  describe '#merge' do
+    let(:merge_request) { create(:merge_request, :merged) }
+    let(:expected_commit_count) { 21 }
+
+    it 'saves metrics with productivity_data' do
+      allow(merge_request).to receive(:commits_count).and_return(expected_commit_count)
+
+      expect do
+        subject.merge(instance_double('Event', author_id: merge_request.author.id, created_at: Time.now))
+        merge_request.metrics.reload
+      end.to change { merge_request.metrics.commits_count }.to(expected_commit_count)
+    end
+  end
+end

lib/gitlab/git/diff_collection.rb

@@ -81,6 +81,12 @@ module Gitlab
         end
       end
 
+      def line_count
+        populate!
+
+        @line_count
+      end
+
       def decorate!
         collection = each_with_index do |element, i|
           @array[i] = yield(element)

spec/lib/gitlab/git/diff_collection_spec.rb

@@ -74,6 +74,11 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           end
         end
 
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq file_count * line_count }
+        end
+
         context 'when limiting is disabled' do
           let(:limits) { false }
 
@@ -100,6 +105,11 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
               expect(subject.size).to eq(3)
             end
           end
+
+          describe '#line_count' do
+            subject { super().line_count }
+            it { is_expected.to eq file_count * line_count }
+          end
         end
       end
 
@@ -120,6 +130,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           subject { super().real_size }
           it { is_expected.to eq('0+') }
         end
+
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq 1000 }
+        end
+
         it { expect(subject.size).to eq(0) }
 
         context 'when limiting is disabled' do
@@ -139,6 +155,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
             subject { super().real_size }
             it { is_expected.to eq('3') }
           end
+
+          describe '#line_count' do
+            subject { super().line_count }
+            it { is_expected.to eq file_count * line_count }
+          end
+
           it { expect(subject.size).to eq(3) }
         end
       end
@@ -164,6 +186,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           subject { super().real_size }
           it { is_expected.to eq('10+') }
         end
+
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq 10 }
+        end
+
         it { expect(subject.size).to eq(10) }
 
         context 'when limiting is disabled' do
@@ -183,6 +211,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
             subject { super().real_size }
             it { is_expected.to eq('11') }
           end
+
+          describe '#line_count' do
+            subject { super().line_count }
+            it { is_expected.to eq file_count * line_count }
+          end
+
           it { expect(subject.size).to eq(11) }
         end
       end
@@ -204,6 +238,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           subject { super().real_size }
           it { is_expected.to eq('3+') }
         end
+
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq 120 }
+        end
+
         it { expect(subject.size).to eq(3) }
 
         context 'when limiting is disabled' do
@@ -223,6 +263,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
             subject { super().real_size }
             it { is_expected.to eq('11') }
           end
+
+          describe '#line_count' do
+            subject { super().line_count }
+            it { is_expected.to eq file_count * line_count }
+          end
+
           it { expect(subject.size).to eq(11) }
         end
       end
@@ -248,6 +294,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           subject { super().real_size }
           it { is_expected.to eq('10') }
         end
+
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq file_count * line_count }
+        end
+
         it { expect(subject.size).to eq(10) }
       end
     end
@@ -270,6 +322,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
         subject { super().real_size }
         it { is_expected.to eq('9+') }
       end
+
+      describe '#line_count' do
+        subject { super().line_count }
+        it { is_expected.to eq file_count * line_count }
+      end
+
       it { expect(subject.size).to eq(9) }
 
       context 'when limiting is disabled' do
@@ -289,6 +347,12 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
           subject { super().real_size }
           it { is_expected.to eq('10') }
         end
+
+        describe '#line_count' do
+          subject { super().line_count }
+          it { is_expected.to eq file_count * line_count }
+        end
+
         it { expect(subject.size).to eq(10) }
       end
     end
@@ -316,6 +380,11 @@ describe Gitlab::Git::DiffCollection, :seed_helper do
       subject { super().real_size }
       it { is_expected.to eq('0')}
     end
+
+    describe '#line_count' do
+      subject { super().line_count }
+      it { is_expected.to eq 0 }
+    end
   end
 
   describe '#each' do

spec/models/merge_request_diff_spec.rb

@@ -400,6 +400,18 @@ describe MergeRequestDiff do
     end
   end
 
+  describe '#first_commit' do
+    it 'returns first commit' do
+      expect(diff_with_commits.first_commit.sha).to eq(diff_with_commits.merge_request_diff_commits.last.sha)
+    end
+  end
+
+  describe '#last_commit' do
+    it 'returns last commit' do
+      expect(diff_with_commits.last_commit.sha).to eq(diff_with_commits.merge_request_diff_commits.first.sha)
+    end
+  end
+
   describe '#commits_by_shas' do
     let(:commit_shas) { diff_with_commits.commit_shas }
 
@@ -489,7 +501,7 @@ describe MergeRequestDiff do
     subject { diff_with_commits }
 
     it 'returns sum of all changed lines count in diff files' do
-      expect(subject.lines_count).to eq 109
+      expect(subject.lines_count).to eq 189
     end
   end
 end