diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2016-08-25 08:54:41 +0300 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2016-08-26 13:58:20 +0300 |
commit | 487906b3861068a8f81125814f919a07bfab8469 (patch) | |
tree | 13bb6d62ad6211c2f7075e0d1321f5405acbfeb0 | |
parent | f932bb8e41852d7bdcd66fe15a56277074df3aa3 (diff) |
Add the "Code" Cycle Analytics section.
1. Record the `wip_flag_first_removed_at` and
`first_assigned_to_user_other_than_author` metrics for a merge
request. Use a `merge_request_metrics` table, similar to the one for
`issues`. Metrics are recorded `after_save`.
2. Move larger queries to a `CycleAnalytics::Queries` module.
-rw-r--r-- | app/models/cycle_analytics.rb | 21 | ||||
-rw-r--r-- | app/models/cycle_analytics/queries.rb | 26 | ||||
-rw-r--r-- | app/models/merge_request.rb | 8 | ||||
-rw-r--r-- | app/models/merge_request/metrics.rb | 15 | ||||
-rw-r--r-- | app/views/projects/cycle_analytics/show.html.haml | 17 | ||||
-rw-r--r-- | db/migrate/20160825052008_add_table_merge_request_metrics.rb | 36 | ||||
-rw-r--r-- | db/schema.rb | 13 |
7 files changed, 124 insertions, 12 deletions
diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index d037b1fef12..68efa826b5a 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -2,20 +2,22 @@ class CycleAnalytics def issue issues = Issue.includes(:metrics).where("issue_metrics.id IS NOT NULL").references(:issue_metrics).to_a start_time_fn = -> (issue) { issue.created_at } - end_time_fn = -> (issue) { issue.metrics.first_associated_with_milestone_at.presence || issue.metrics.first_added_to_board_at.presence } - - calculate_metric(issues, start_time_fn, end_time_fn) + calculate_metric(issues, start_time_fn, Queries::issue_first_associated_with_milestone_or_first_added_to_list_label_time) end def plan issues = Issue.includes(:metrics).where("issue_metrics.id IS NOT NULL").references(:issue_metrics).to_a - start_time_fn = -> (issue) { issue.metrics.first_associated_with_milestone_at.presence || issue.metrics.first_added_to_board_at.presence } - end_time_fn = lambda do |issue| - merge_requests = issue.closed_by_merge_requests - merge_requests.map(&:created_at).min if merge_requests.present? - end + calculate_metric(issues, + Queries::issue_first_associated_with_milestone_or_first_added_to_list_label_time, + Queries::issue_closing_merge_request_opened_time) + end - calculate_metric(issues, start_time_fn, end_time_fn) + 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, + start_time_fn, + Queries::mr_wip_flag_removed_or_assigned_to_user_other_than_author_time) end private @@ -34,6 +36,7 @@ class CycleAnalytics end def median(coll) + return if coll.empty? size = coll.length (coll[size / 2] + coll[(size - 1) / 2]) / 2.0 end diff --git a/app/models/cycle_analytics/queries.rb b/app/models/cycle_analytics/queries.rb new file mode 100644 index 00000000000..ec0311b91b5 --- /dev/null +++ b/app/models/cycle_analytics/queries.rb @@ -0,0 +1,26 @@ +class CycleAnalytics + module Queries + 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 + end + end + + def issue_closing_merge_request_opened_time + lambda do |issue| + merge_requests = issue.closed_by_merge_requests + merge_requests.map(&:created_at).min if merge_requests.present? + end + end + + 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 + end + end + end + end + end +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 1d05e4a85d1..99a9f83cd50 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -13,6 +13,7 @@ class MergeRequest < ActiveRecord::Base has_many :merge_request_diffs, dependent: :destroy has_one :merge_request_diff, -> { order('merge_request_diffs.id DESC') } + has_one :metrics, dependent: :destroy has_many :events, as: :target, dependent: :destroy @@ -31,6 +32,8 @@ class MergeRequest < ActiveRecord::Base # when creating new merge request attr_accessor :can_be_created, :compare_commits, :compare + after_save :record_metrics + state_machine :state, initial: :opened do event :close do transition [:reopened, :opened] => :closed @@ -807,4 +810,9 @@ class MergeRequest < ActiveRecord::Base @conflicts_can_be_resolved_in_ui = false end end + + def record_metrics + metrics = Metrics.find_or_create_by(merge_request_id: self.id) + metrics.record! + end end diff --git a/app/models/merge_request/metrics.rb b/app/models/merge_request/metrics.rb new file mode 100644 index 00000000000..7f7cf95e388 --- /dev/null +++ b/app/models/merge_request/metrics.rb @@ -0,0 +1,15 @@ +class MergeRequest::Metrics < ActiveRecord::Base + belongs_to :merge_request + + def record! + if !merge_request.work_in_progress? && self.wip_flag_first_removed_at.blank? + self.wip_flag_first_removed_at = Time.now + end + + if merge_request.author_id != merge_request.assignee_id && self.first_assigned_to_user_other_than_author.blank? + self.first_assigned_to_user_other_than_author = 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 6aad68617fe..0774c54ddab 100644 --- a/app/views/projects/cycle_analytics/show.html.haml +++ b/app/views/projects/cycle_analytics/show.html.haml @@ -1,8 +1,21 @@ %ul.list-group %li.list-group-item Issue: - = distance_of_time_in_words @cycle_analytics.issue + - if issue = @cycle_analytics.issue + = distance_of_time_in_words issue + - else + <Not enough data> %li.list-group-item Plan: - = distance_of_time_in_words @cycle_analytics.plan + - if plan = @cycle_analytics.plan + = distance_of_time_in_words plan + - else + <Not enough data> + + %li.list-group-item + Code: + - if code = @cycle_analytics.code.presence + = distance_of_time_in_words code + - 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 new file mode 100644 index 00000000000..5745175ec25 --- /dev/null +++ b/db/migrate/20160825052008_add_table_merge_request_metrics.rb @@ -0,0 +1,36 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddTableMergeRequestMetrics < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + create_table :merge_request_metrics do |t| + t.references :merge_request, index: { name: "index_merge_request_metrics" }, foreign_key: true, null: false + + t.datetime 'wip_flag_first_removed_at' + t.datetime 'first_assigned_to_user_other_than_author' + + t.timestamps null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 2c580a164bd..e2004dcd4bc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160824124900) do +ActiveRecord::Schema.define(version: 20160825052008) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -605,6 +605,16 @@ ActiveRecord::Schema.define(version: 20160824124900) do add_index "merge_request_diffs", ["merge_request_id"], name: "index_merge_request_diffs_on_merge_request_id", using: :btree + create_table "merge_request_metrics", force: :cascade do |t| + 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 "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "merge_request_metrics", ["merge_request_id"], name: "index_merge_request_metrics", using: :btree + create_table "merge_requests", force: :cascade do |t| t.string "target_branch", null: false t.string "source_branch", null: false @@ -1163,6 +1173,7 @@ ActiveRecord::Schema.define(version: 20160824124900) do add_foreign_key "issue_metrics", "issues" add_foreign_key "lists", "boards" add_foreign_key "lists", "labels" + add_foreign_key "merge_request_metrics", "merge_requests" add_foreign_key "personal_access_tokens", "users" add_foreign_key "protected_branch_merge_access_levels", "protected_branches" add_foreign_key "protected_branch_push_access_levels", "protected_branches" |