Commit 5af52b2d authored by Adam Hegyi's avatar Adam Hegyi

Store added and removed lines in MR metrics

The change is begined a feature flag:
`store_merge_request_line_metrics`
parent f09786f1
---
title: Add added_lines and removed_lines columns to merge_request_metrics table
merge_request: 28658
author:
type: added
# frozen_string_literal: true
class AddLineMetricsToMrMetrics < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
with_lock_retries do
add_column :merge_request_metrics, :added_lines, :integer
add_column :merge_request_metrics, :removed_lines, :integer
end
end
def down
with_lock_retries do
remove_column :merge_request_metrics, :added_lines, :integer
remove_column :merge_request_metrics, :removed_lines, :integer
end
end
end
...@@ -3749,7 +3749,9 @@ CREATE TABLE public.merge_request_metrics ( ...@@ -3749,7 +3749,9 @@ CREATE TABLE public.merge_request_metrics (
modified_paths_size integer, modified_paths_size integer,
commits_count integer, commits_count integer,
first_approved_at timestamp with time zone, first_approved_at timestamp with time zone,
first_reassigned_at timestamp with time zone first_reassigned_at timestamp with time zone,
added_lines integer,
removed_lines integer
); );
CREATE SEQUENCE public.merge_request_metrics_id_seq CREATE SEQUENCE public.merge_request_metrics_id_seq
...@@ -12936,6 +12938,7 @@ COPY "schema_migrations" (version) FROM STDIN; ...@@ -12936,6 +12938,7 @@ COPY "schema_migrations" (version) FROM STDIN;
20200330123739 20200330123739
20200330132913 20200330132913
20200331220930 20200331220930
20200402123926
20200402135250 20200402135250
20200403184110 20200403184110
20200403185127 20200403185127
......
...@@ -11,7 +11,7 @@ module EE ...@@ -11,7 +11,7 @@ module EE
data = { data = {
merged_by_id: event.author_id, merged_by_id: event.author_id,
merged_at: event.created_at merged_at: event.created_at
}.merge(metrics_calculator.productivity_data) }.merge(metrics_calculator.productivity_data, metrics_calculator.line_counts_data)
update!(data) update!(data)
end end
......
...@@ -17,6 +17,17 @@ module Analytics ...@@ -17,6 +17,17 @@ module Analytics
} }
end end
# rubocop: disable CodeReuse/ActiveRecord
def line_counts_data
return {} if Feature.disabled?(:store_merge_request_line_metrics, merge_request.target_project)
{
added_lines: raw_diff_files.sum(&:added_lines),
removed_lines: raw_diff_files.sum(&:removed_lines)
}
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def first_comment_at def first_comment_at
merge_request.related_notes.by_humans merge_request.related_notes.by_humans
...@@ -61,5 +72,9 @@ module Analytics ...@@ -61,5 +72,9 @@ module Analytics
def modified_paths_size def modified_paths_size
merge_request.modified_paths.size merge_request.modified_paths.size
end end
def raw_diff_files
@raw_diff_files ||= merge_request_diff.diffs.raw_diff_files
end
end end
end end
...@@ -3,19 +3,49 @@ ...@@ -3,19 +3,49 @@
require 'spec_helper' require 'spec_helper'
describe EE::MergeRequestMetricsService do describe EE::MergeRequestMetricsService do
subject { MergeRequestMetricsService.new(merge_request.metrics) } subject do
service = MergeRequestMetricsService.new(merge_request.metrics)
service.merge(event)
service.merge_request.metrics.reload
service
end
describe '#merge' do describe '#merge' do
let(:merge_request) { create(:merge_request, :merged) } let(:merge_request) { create(:merge_request, :merged) }
let(:expected_commit_count) { 21 } let(:expected_commit_count) { 21 }
let(:event) { instance_double('Event', author_id: merge_request.author.id, created_at: Time.now) }
it 'saves metrics with productivity_data' do it 'saves metrics with productivity_data' do
allow(merge_request).to receive(:commits_count).and_return(expected_commit_count) allow(merge_request).to receive(:commits_count).and_return(expected_commit_count)
expect do expect { subject }.to change { merge_request.metrics.commits_count }.to(expected_commit_count)
subject.merge(instance_double('Event', author_id: merge_request.author.id, created_at: Time.now)) end
merge_request.metrics.reload
end.to change { merge_request.metrics.commits_count }.to(expected_commit_count) describe 'storing line counts' do
let(:expected_added_lines) { 118 }
let(:expected_removed_lines) { 9 }
it 'updates `added_lines`' do
expect { subject }.to change { merge_request.metrics.added_lines }.from(nil).to(expected_added_lines)
end
it 'updates `removed_lines`' do
expect { subject }.to change { merge_request.metrics.removed_lines }.from(nil).to(expected_removed_lines)
end
context 'when `store_merge_request_line_metrics` feature flag is disabled' do
before do
stub_feature_flags(store_merge_request_line_metrics: { enabled: false, thing: merge_request.target_project })
end
it 'does not update line counts' do
subject
expect(merge_request.metrics.added_lines).to be_nil
expect(merge_request.metrics.removed_lines).to be_nil
end
end
end end
end end
end end
...@@ -283,6 +283,8 @@ MergeRequest::Metrics: ...@@ -283,6 +283,8 @@ MergeRequest::Metrics:
- commits_count - commits_count
- first_approved_at - first_approved_at
- first_reassigned_at - first_reassigned_at
- added_lines
- removed_lines
Ci::Pipeline: Ci::Pipeline:
- id - id
- project_id - project_id
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment