Commit 4adf283e authored by Pavel Shutsin's avatar Pavel Shutsin

Add merge request metrics first_reassigned_at timestamp

It will be used at code review analytics
parent 92e160e6
...@@ -43,11 +43,7 @@ module MergeRequests ...@@ -43,11 +43,7 @@ module MergeRequests
abort_auto_merge(merge_request, 'target branch was changed') abort_auto_merge(merge_request, 'target branch was changed')
end end
if merge_request.assignees != old_assignees handle_assignees_change(merge_request, old_assignees) if merge_request.assignees != old_assignees
create_assignee_note(merge_request, old_assignees)
notification_service.async.reassigned_merge_request(merge_request, current_user, old_assignees)
todo_service.reassigned_issuable(merge_request, current_user, old_assignees)
end
if merge_request.previous_changes.include?('target_branch') || if merge_request.previous_changes.include?('target_branch') ||
merge_request.previous_changes.include?('source_branch') merge_request.previous_changes.include?('source_branch')
...@@ -120,6 +116,12 @@ module MergeRequests ...@@ -120,6 +116,12 @@ module MergeRequests
end end
end end
def handle_assignees_change(merge_request, old_assignees)
create_assignee_note(merge_request, old_assignees)
notification_service.async.reassigned_merge_request(merge_request, current_user, old_assignees)
todo_service.reassigned_issuable(merge_request, current_user, old_assignees)
end
def create_branch_change_note(issuable, branch_type, old_branch, new_branch) def create_branch_change_note(issuable, branch_type, old_branch, new_branch)
SystemNoteService.change_branch( SystemNoteService.change_branch(
issuable, issuable.project, current_user, branch_type, issuable, issuable.project, current_user, branch_type,
......
# frozen_string_literal: true
class AddMergeRequestMetricsFirstReassignedAt < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
with_lock_retries do
add_column :merge_request_metrics, :first_reassigned_at, :datetime_with_timezone
end
end
def down
with_lock_retries do
remove_column :merge_request_metrics, :first_reassigned_at, :datetime_with_timezone
end
end
end
# frozen_string_literal: true
class AddMergeRequestAssigneeCreatedAt < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
with_lock_retries do
add_column :merge_request_assignees, :created_at, :datetime_with_timezone
end
end
def down
with_lock_retries do
remove_column :merge_request_assignees, :created_at
end
end
end
...@@ -2450,6 +2450,7 @@ ActiveRecord::Schema.define(version: 2020_03_09_105539) do ...@@ -2450,6 +2450,7 @@ ActiveRecord::Schema.define(version: 2020_03_09_105539) do
create_table "merge_request_assignees", force: :cascade do |t| create_table "merge_request_assignees", force: :cascade do |t|
t.integer "user_id", null: false t.integer "user_id", null: false
t.integer "merge_request_id", null: false t.integer "merge_request_id", null: false
t.datetime_with_timezone "created_at"
t.index ["merge_request_id", "user_id"], name: "index_merge_request_assignees_on_merge_request_id_and_user_id", unique: true t.index ["merge_request_id", "user_id"], name: "index_merge_request_assignees_on_merge_request_id_and_user_id", unique: true
t.index ["merge_request_id"], name: "index_merge_request_assignees_on_merge_request_id" t.index ["merge_request_id"], name: "index_merge_request_assignees_on_merge_request_id"
t.index ["user_id"], name: "index_merge_request_assignees_on_user_id" t.index ["user_id"], name: "index_merge_request_assignees_on_user_id"
...@@ -2564,6 +2565,7 @@ ActiveRecord::Schema.define(version: 2020_03_09_105539) do ...@@ -2564,6 +2565,7 @@ ActiveRecord::Schema.define(version: 2020_03_09_105539) do
t.integer "modified_paths_size" t.integer "modified_paths_size"
t.integer "commits_count" t.integer "commits_count"
t.datetime_with_timezone "first_approved_at" t.datetime_with_timezone "first_approved_at"
t.datetime_with_timezone "first_reassigned_at"
t.index ["first_deployed_to_production_at"], name: "index_merge_request_metrics_on_first_deployed_to_production_at" t.index ["first_deployed_to_production_at"], name: "index_merge_request_metrics_on_first_deployed_to_production_at"
t.index ["latest_closed_at"], name: "index_merge_request_metrics_on_latest_closed_at", where: "(latest_closed_at IS NOT NULL)" t.index ["latest_closed_at"], name: "index_merge_request_metrics_on_latest_closed_at", where: "(latest_closed_at IS NOT NULL)"
t.index ["latest_closed_by_id"], name: "index_merge_request_metrics_on_latest_closed_by_id" t.index ["latest_closed_by_id"], name: "index_merge_request_metrics_on_latest_closed_by_id"
......
...@@ -46,6 +46,15 @@ module EE ...@@ -46,6 +46,15 @@ module EE
reset_approvals(merge_request) reset_approvals(merge_request)
end end
override :handle_assignees_change
def handle_assignees_change(merge_request, _old_assignees)
super
return unless merge_request.project.feature_available?(:code_review_analytics)
Analytics::RefreshReassignData.new(merge_request).execute_async
end
end end
end end
end end
---
title: Add code review reassign metric structure
merge_request: 26664
author:
type: added
...@@ -31,6 +31,15 @@ module Analytics ...@@ -31,6 +31,15 @@ module Analytics
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def first_reassigned_at
merge_request.merge_request_assignees
.where.not(assignee: merge_request.author)
.order(id: :asc).limit(1)
.pluck(:created_at).first
end
# rubocop: enable CodeReuse/ActiveRecord
private private
attr_reader :merge_request attr_reader :merge_request
......
...@@ -4,7 +4,7 @@ module Analytics ...@@ -4,7 +4,7 @@ module Analytics
class RefreshApprovalsData class RefreshApprovalsData
include MergeRequestMetricsRefresh include MergeRequestMetricsRefresh
# Override MergeRequestMetricsRefresh#initialize` to accept single MR only # Override `MergeRequestMetricsRefresh#initialize` to accept single MR only
def initialize(merge_request) def initialize(merge_request)
super super
end end
......
# frozen_string_literal: true
module Analytics
class RefreshReassignData
include MergeRequestMetricsRefresh
# Override `MergeRequestMetricsRefresh#initialize` to accept single MR only
def initialize(merge_request)
super
end
private
def metric_already_present?(metrics)
metrics.first_reassigned_at
end
def update_metric!(metrics)
metrics.update!(
first_reassigned_at: MergeRequestMetricsCalculator.new(metrics.merge_request).first_reassigned_at
)
end
end
end
...@@ -55,4 +55,14 @@ describe Analytics::MergeRequestMetricsCalculator do ...@@ -55,4 +55,14 @@ describe Analytics::MergeRequestMetricsCalculator do
expect(subject.first_approved_at).to be_like_time(1.day.ago) expect(subject.first_approved_at).to be_like_time(1.day.ago)
end end
end end
describe '#first_reassigned_at' do
it 'returns earliest non-author assignee creation timestamp' do
merge_request.merge_request_assignees.create(assignee: merge_request.author, created_at: 5.days.ago)
merge_request.merge_request_assignees.create(assignee: create(:user), created_at: 3.days.ago)
merge_request.merge_request_assignees.create(assignee: create(:user), created_at: 1.day.ago)
expect(subject.first_reassigned_at).to be_like_time(3.days.ago)
end
end
end end
# frozen_string_literal: true
require 'spec_helper'
describe Analytics::RefreshReassignData do
subject { described_class.new(merge_request) }
let(:merge_request) { create :merge_request }
describe '#execute' do
let(:calculated_value) { 2.days.ago.beginning_of_day }
include_examples 'common merge request metric refresh for', :first_reassigned_at
end
end
...@@ -215,5 +215,26 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -215,5 +215,26 @@ describe MergeRequests::UpdateService, :mailer do
update_merge_request({}) update_merge_request({})
end end
end end
context 'when reassigned' do
it 'schedules for analytics metric update' do
expect(Analytics::CodeReviewMetricsWorker)
.to receive(:perform_async).with('Analytics::RefreshReassignData', merge_request.id, {})
update_merge_request({ assignee_ids: [user2.id] })
end
context 'when code_review_analytics is not available' do
before do
stub_licensed_features(code_review_analytics: false)
end
it 'does not schedule for analytics metric update' do
expect(Analytics::CodeReviewMetricsWorker).not_to receive(:perform_async)
update_merge_request({ assignee_ids: [user2.id] })
end
end
end
end end
end end
...@@ -273,6 +273,7 @@ MergeRequest::Metrics: ...@@ -273,6 +273,7 @@ MergeRequest::Metrics:
- modified_paths_size - modified_paths_size
- commits_count - commits_count
- first_approved_at - first_approved_at
- first_reassigned_at
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