diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb
index 68efa826b5a6d719ae8489ce72f65020125dee46..16072c35b64ec3d4b2c94eb96eb589c7199d1644 100644
--- a/app/models/cycle_analytics.rb
+++ b/app/models/cycle_analytics.rb
@@ -15,11 +15,18 @@ class CycleAnalytics
def code
issues = Issue.all.to_a
start_time_fn = -> (merge_request) { merge_request.created_at }
- calculate_metric(issues.map(&:closed_by_merge_requests).flatten,
+ calculate_metric(issues.map { |issue| issue.closed_by_merge_requests(nil, check_if_open: false) }.flatten,
start_time_fn,
Queries::mr_wip_flag_removed_or_assigned_to_user_other_than_author_time)
end
+ def review
+ issues = Issue.all.to_a
+ calculate_metric(issues.map { |issue| issue.closed_by_merge_requests(nil, check_if_open: false) }.flatten,
+ Queries::mr_wip_flag_removed_or_assigned_to_user_other_than_author_time,
+ Queries::mr_first_closed_or_merged_at)
+ end
+
private
def calculate_metric(data, start_time_fn, end_time_fn)
diff --git a/app/models/cycle_analytics/queries.rb b/app/models/cycle_analytics/queries.rb
index ec0311b91b5b7c9c94e97a9e042bc0e0d373150b..41081a4e08c77d3ab4f1b4462bb1ea5429a86d7f 100644
--- a/app/models/cycle_analytics/queries.rb
+++ b/app/models/cycle_analytics/queries.rb
@@ -3,13 +3,24 @@ class CycleAnalytics
class << self
def issue_first_associated_with_milestone_or_first_added_to_list_label_time
lambda do |issue|
- issue.metrics.first_associated_with_milestone_at.presence || issue.metrics.first_added_to_board_at.presence
+ if issue.metrics.present?
+ issue.metrics.first_associated_with_milestone_at.presence ||
+ issue.metrics.first_added_to_board_at.presence
+ end
+ end
+ end
+
+ def mr_first_closed_or_merged_at
+ lambda do |merge_request|
+ if merge_request.metrics.present?
+ merge_request.metrics.merged_at.presence || merge_request.metrics.first_closed_at.presence
+ end
end
end
def issue_closing_merge_request_opened_time
lambda do |issue|
- merge_requests = issue.closed_by_merge_requests
+ merge_requests = issue.closed_by_merge_requests(nil, check_if_open: false)
merge_requests.map(&:created_at).min if merge_requests.present?
end
end
@@ -17,7 +28,8 @@ class CycleAnalytics
def mr_wip_flag_removed_or_assigned_to_user_other_than_author_time
lambda do |merge_request|
if merge_request.metrics.present?
- merge_request.metrics.wip_flag_first_removed_at || merge_request.metrics.first_assigned_to_user_other_than_author
+ merge_request.metrics.wip_flag_first_removed_at.presence ||
+ merge_request.metrics.first_assigned_to_user_other_than_author.presence
end
end
end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 1aa0cef884e6aa2ba623474fa23dcaaa299faefe..cec5e456c94591eef34156ed8b0296c80423a2ac 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -198,8 +198,8 @@ class Issue < ActiveRecord::Base
# From all notes on this issue, we'll select the system notes about linked
# merge requests. Of those, the MRs closing `self` are returned.
- def closed_by_merge_requests(current_user = nil)
- return [] unless open?
+ def closed_by_merge_requests(current_user = nil, check_if_open: true)
+ return [] if !open? && check_if_open
ext = all_references(current_user)
@@ -207,7 +207,11 @@ class Issue < ActiveRecord::Base
note.all_references(current_user, extractor: ext)
end
- ext.merge_requests.select { |mr| mr.open? && mr.closes_issue?(self) }
+ if check_if_open
+ ext.merge_requests.select { |mr| mr.open? && mr.closes_issue?(self) }
+ else
+ ext.merge_requests.select { |mr| mr.closes_issue?(self) }
+ end
end
def moved?
diff --git a/app/models/merge_request/metrics.rb b/app/models/merge_request/metrics.rb
index 7f7cf95e388ecc75441253b8326e66b1a2b3d173..a0bc41c9cc16abf235fb9731e96e7f0cd9ede5e9 100644
--- a/app/models/merge_request/metrics.rb
+++ b/app/models/merge_request/metrics.rb
@@ -10,6 +10,14 @@ class MergeRequest::Metrics < ActiveRecord::Base
self.first_assigned_to_user_other_than_author = Time.now
end
+ if merge_request.merged? && self.merged_at.blank?
+ self.merged_at = Time.now
+ end
+
+ if merge_request.closed? && self.first_closed_at.blank?
+ self.first_closed_at = Time.now
+ end
+
self.save if self.changed?
end
end
diff --git a/app/views/projects/cycle_analytics/show.html.haml b/app/views/projects/cycle_analytics/show.html.haml
index 0774c54ddab1954558e1fff5ada958faa7c16a2e..271aae5d5fd50f6a47af52061b5ece845963e564 100644
--- a/app/views/projects/cycle_analytics/show.html.haml
+++ b/app/views/projects/cycle_analytics/show.html.haml
@@ -19,3 +19,10 @@
= distance_of_time_in_words code
- else
= "<Not enough data>"
+
+ %li.list-group-item
+ Review:
+ - if review = @cycle_analytics.review.presence
+ = distance_of_time_in_words review
+ - else
+ = "<Not enough data>"
diff --git a/db/migrate/20160825052008_add_table_merge_request_metrics.rb b/db/migrate/20160825052008_add_table_merge_request_metrics.rb
index 5745175ec252d5585b9000d278ed1115a7265f94..809804cbfea527151f8473b3084a46660f070333 100644
--- a/db/migrate/20160825052008_add_table_merge_request_metrics.rb
+++ b/db/migrate/20160825052008_add_table_merge_request_metrics.rb
@@ -29,6 +29,8 @@ class AddTableMergeRequestMetrics < ActiveRecord::Migration
t.datetime 'wip_flag_first_removed_at'
t.datetime 'first_assigned_to_user_other_than_author'
+ t.datetime 'merged_at'
+ t.datetime 'first_closed_at'
t.timestamps null: false
end
diff --git a/db/schema.rb b/db/schema.rb
index e2004dcd4bc0aca62b7c1a3c2b9cbe390279316c..2daa78ad1e16d360da4c1f30c65067b276533502 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -609,6 +609,8 @@ ActiveRecord::Schema.define(version: 20160825052008) do
t.integer "merge_request_id", null: false
t.datetime "wip_flag_first_removed_at"
t.datetime "first_assigned_to_user_other_than_author"
+ t.datetime "merged_at"
+ t.datetime "first_closed_at"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end