diff options
55 files changed, 2207 insertions, 5 deletions
@@ -320,6 +320,7 @@ group :test do gem 'webmock', '~> 1.21.0' gem 'test_after_commit', '~> 0.4.2' gem 'sham_rack', '~> 1.3.6' + gem 'timecop', '~> 0.8.0' end group :production do diff --git a/Gemfile.lock b/Gemfile.lock index 8e26429df14..bbaed6b6a47 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -980,6 +980,7 @@ DEPENDENCIES teaspoon-jasmine (~> 2.2.0) test_after_commit (~> 0.4.2) thin (~> 1.7.0) + timecop (~> 0.8.0) turbolinks (~> 2.5.0) u2f (~> 0.2.1) uglifier (~> 2.7.2) diff --git a/app/assets/javascripts/cycle-analytics.js.es6 b/app/assets/javascripts/cycle-analytics.js.es6 new file mode 100644 index 00000000000..acae5439fa2 --- /dev/null +++ b/app/assets/javascripts/cycle-analytics.js.es6 @@ -0,0 +1,92 @@ +((global) => { + + const COOKIE_NAME = 'cycle_analytics_help_dismissed'; + + gl.CycleAnalytics = class CycleAnalytics { + constructor() { + const that = this; + + this.isHelpDismissed = $.cookie(COOKIE_NAME); + this.vue = new Vue({ + el: '#cycle-analytics', + name: 'CycleAnalytics', + created: this.fetchData(), + data: this.decorateData({ isLoading: true }), + methods: { + dismissLanding() { + that.dismissLanding(); + } + } + }); + } + + fetchData(options) { + options = options || { startDate: 30 }; + + $.ajax({ + url: $('#cycle-analytics').data('request-path'), + method: 'GET', + dataType: 'json', + contentType: 'application/json', + data: { start_date: options.startDate } + }).done((data) => { + this.vue.$data = this.decorateData(data); + this.initDropdown(); + }) + .error((data) => { + this.handleError(data); + }) + .always(() => { + this.vue.isLoading = false; + }) + } + + decorateData(data) { + data.summary = data.summary || []; + data.stats = data.stats || []; + data.isHelpDismissed = this.isHelpDismissed; + data.isLoading = data.isLoading || false; + + data.summary.forEach((item) => { + item.value = item.value || '-'; + }); + + data.stats.forEach((item) => { + item.value = item.value || '-'; + }) + + return data; + } + + handleError(data) { + this.vue.$data = { + hasError: true, + isHelpDismissed: this.isHelpDismissed + }; + + new Flash('There was an error while fetching cycle analytics data.', 'alert'); + } + + dismissLanding() { + this.vue.isHelpDismissed = true; + $.cookie(COOKIE_NAME, true); + } + + initDropdown() { + const $dropdown = $('.js-ca-dropdown'); + const $label = $dropdown.find('.dropdown-label'); + + $dropdown.find('li a').off('click').on('click', (e) => { + e.preventDefault(); + const $target = $(e.currentTarget); + const value = $target.data('value'); + + $label.text($target.text().trim()); + this.vue.isLoading = true; + this.fetchData({ startDate: value }); + }) + } + + } + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 99b16f7d59b..ddf11ecf34c 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -94,6 +94,11 @@ break; case "projects:merge_requests:conflicts": window.mcui = new MergeConflictResolver() + break; + case 'projects:merge_requests:index': + shortcut_handler = new ShortcutsNavigation(); + Issuable.init(); + break; case 'dashboard:activity': new Activities(); break; @@ -185,6 +190,9 @@ new gl.ProtectedBranchCreate(); new gl.ProtectedBranchEditList(); break; + case 'projects:cycle_analytics:show': + new gl.CycleAnalytics(); + break; } switch (path.first()) { case 'admin': diff --git a/app/assets/stylesheets/pages/cycle_analytics.scss b/app/assets/stylesheets/pages/cycle_analytics.scss new file mode 100644 index 00000000000..d62b9b917ee --- /dev/null +++ b/app/assets/stylesheets/pages/cycle_analytics.scss @@ -0,0 +1,120 @@ +#cycle-analytics { + margin: 24px auto 0; + width: 800px; + position: relative; + + .panel { + + .content-block { + padding: 24px 0; + border-bottom: none; + position: relative; + } + + .column { + text-align: center; + + .header { + font-size: 30px; + line-height: 38px; + font-weight: normal; + margin: 0; + } + + .text { + color: $layout-link-gray; + margin: 0; + } + + &:last-child { + text-align: right; + } + } + + .dropdown { + position: relative; + top: 13px; + } + } + + .bordered-box { + border: 1px solid $border-color; + @include border-radius($border-radius-default); + position: relative; + } + + .content-list { + li { + padding: 18px $gl-padding $gl-padding; + + .container-fluid { + padding: 0; + } + } + + .title-col { + p { + margin: 0; + + &.title { + line-height: 19px; + font-size: 15px; + font-weight: 600; + } + &:text { + color: #8C8C8C; + } + } + } + + .value-col { + text-align: right; + + span { + line-height: 42px; + } + } + } + + .landing { + margin-bottom: $gl-padding; + overflow: hidden; + + .dismiss-icon { + position: absolute; + right: $gl-padding; + cursor: pointer; + } + + svg { + margin: 0 20px; + float: left; + width: 136px; + height: 136px; + } + + .inner-content { + width: 480px; + float: left; + + h4 { + color: $gl-text-color; + font-size: 17px; + } + + p { + color: #8C8C8C; + margin-bottom: $gl-padding; + } + } + } + + .fa-spinner { + font-size: 28px; + position: relative; + margin-left: -20px; + left: 50%; + margin-top: 36px; + } + +} diff --git a/app/controllers/projects/cycle_analytics_controller.rb b/app/controllers/projects/cycle_analytics_controller.rb new file mode 100644 index 00000000000..dd0acc57425 --- /dev/null +++ b/app/controllers/projects/cycle_analytics_controller.rb @@ -0,0 +1,30 @@ +class Projects::CycleAnalyticsController < Projects::ApplicationController + include CycleAnalyticsHelper + + before_action :authorize_read_cycle_analytics! + + def show + @cycle_analytics = CycleAnalytics.new(@project, from: parse_start_date) + + respond_to do |format| + format.html + format.json { render json: cycle_analytics_json(@cycle_analytics) } + end + end + + private + + def parse_start_date + case cycle_analytics_params[:start_date] + when '30' then 30.days.ago + when '90' then 90.days.ago + else 90.days.ago + end + end + + def cycle_analytics_params + return {} unless params[:cycle_analytics].present? + + { start_date: params[:cycle_analytics][:start_date] } + end +end diff --git a/app/helpers/cycle_analytics_helper.rb b/app/helpers/cycle_analytics_helper.rb new file mode 100644 index 00000000000..f6f654a639e --- /dev/null +++ b/app/helpers/cycle_analytics_helper.rb @@ -0,0 +1,35 @@ +module CycleAnalyticsHelper + include ActionView::Helpers::DateHelper + + def cycle_analytics_json(cycle_analytics) + cycle_analytics_view_data = [[:issue, "Issue", "Time before an issue gets scheduled"], + [:plan, "Plan", "Time before an issue starts implementation"], + [:code, "Code", "Time until first merge request"], + [:test, "Test", "Total test time for all commits/merges"], + [:review, "Review", "Time between merge request creation and merge/close"], + [:staging, "Staging", "From merge request merge until deploy to production"], + [:production, "Production", "From issue creation until deploy to production"]] + + stats = cycle_analytics_view_data.reduce([]) do |stats, (stage_method, stage_text, stage_description)| + value = cycle_analytics.send(stage_method).presence + + stats << { + title: stage_text, + description: stage_description, + value: value && !value.zero? ? distance_of_time_in_words(value) : nil + } + stats + end + + summary = [ + { title: "New Issues", value: cycle_analytics.summary.new_issues }, + { title: "Commits", value: cycle_analytics.summary.commits }, + { title: "Deploys", value: cycle_analytics.summary.deploys } + ] + + { + summary: summary, + stats: stats + } + end +end diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index a322a90cc4e..5b71113feb9 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -46,6 +46,10 @@ module GitlabRoutingHelper namespace_project_environments_path(project.namespace, project, *args) end + def project_cycle_analytics_path(project, *args) + namespace_project_cycle_analytics_path(project.namespace, project, *args) + end + def project_builds_path(project, *args) namespace_project_builds_path(project.namespace, project, *args) end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 70647b8532b..cbb4f60cd32 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -56,6 +56,18 @@ module Ci pipeline.finished_at = Time.now end + after_transition [:created, :pending] => :running do |pipeline| + pipeline.merge_requests.each do |merge_request| + merge_request.metrics.record_latest_build_start_time!(pipeline.started_at) if merge_request.metrics.present? + end + end + + after_transition any => [:success] do |pipeline| + pipeline.merge_requests.each do |merge_request| + merge_request.metrics.record_latest_build_finish_time!(pipeline.finished_at) if merge_request.metrics.present? + end + end + before_transition do |pipeline| pipeline.update_duration end @@ -277,6 +289,13 @@ module Ci project.execute_services(data, :pipeline_hooks) end + # Merge requests for which the current pipeline is running against + # the merge request's latest commit. + def merge_requests + project.merge_requests.where(source_branch: self.ref). + select { |merge_request| merge_request.pipeline.try(:id) == self.id } + end + private def pipeline_data diff --git a/app/models/concerns/database_median.rb b/app/models/concerns/database_median.rb new file mode 100644 index 00000000000..19a495a05b8 --- /dev/null +++ b/app/models/concerns/database_median.rb @@ -0,0 +1,76 @@ +# https://www.periscopedata.com/blog/medians-in-sql.html +module DatabaseMedian + extend ActiveSupport::Concern + + def median_datetime(arel_table, query_so_far, column_sym) + case ActiveRecord::Base.connection.adapter_name + when 'PostgreSQL' + pg_median_datetime(arel_table, query_so_far, column_sym) + when 'Mysql2' + mysql_median_datetime(arel_table, query_so_far, column_sym) + else + raise NotImplementedError, "We haven't implemented a database median strategy for your database type." + end + end + + def mysql_median_datetime(arel_table, query_so_far, column_sym) + query = arel_table. + from(arel_table.project(Arel.sql('*')).order(arel_table[column_sym]).as(arel_table.table_name)). + project(Arel::Nodes::NamedFunction.new("AVG", [arel_table[column_sym]]).as('median')). + where(Arel::Nodes::Between.new(Arel.sql("(select @row_id := @row_id + 1)"), + Arel::Nodes::And.new([Arel.sql('@ct/2.0'), + Arel.sql('@ct/2.0 + 1')]))). + # Disallow negative values + where(arel_table[column_sym].gteq(0)) + + [Arel.sql("CREATE TEMPORARY TABLE IF NOT EXISTS #{query_so_far.to_sql}"), + Arel.sql("set @ct := (select count(1) from #{arel_table.table_name});"), + Arel.sql("set @row_id := 0;"), + query, + Arel.sql("DROP TEMPORARY TABLE IF EXISTS #{arel_table.table_name};")] + end + + def pg_median_datetime(arel_table, query_so_far, column_sym) + # Create a CTE with the column we're operating on, row number (after sorting by the column + # we're operating on), and count of the table we're operating on (duplicated across) all rows + # of the CTE. For example, if we're looking to find the median of the `projects.star_count` + # column, the CTE might look like this: + # + # star_count | row_id | ct + # ------------+--------+---- + # 5 | 1 | 3 + # 9 | 2 | 3 + # 15 | 3 | 3 + cte_table = Arel::Table.new("ordered_records") + cte = Arel::Nodes::As.new(cte_table, + arel_table. + project(arel_table[column_sym].as(column_sym.to_s), + Arel::Nodes::Over.new(Arel::Nodes::NamedFunction.new("row_number", []), + Arel::Nodes::Window.new.order(arel_table[column_sym])).as('row_id'), + arel_table.project("COUNT(1)").as('ct')). + # Disallow negative values + where(arel_table[column_sym].gteq(zero_interval))) + + # From the CTE, select either the middle row or the middle two rows (this is accomplished + # by 'where cte.row_id between cte.ct / 2.0 AND cte.ct / 2.0 + 1'). Find the average of the + # selected rows, and this is the median value. + cte_table.project(Arel::Nodes::NamedFunction.new("AVG", + [extract_epoch(cte_table[column_sym])], + "median")). + where(Arel::Nodes::Between.new(cte_table[:row_id], + Arel::Nodes::And.new([(cte_table[:ct] / Arel.sql('2.0')), + (cte_table[:ct] / Arel.sql('2.0') + 1)]))). + with(query_so_far, cte) + end + + private + + def extract_epoch(arel_attribute) + Arel.sql("EXTRACT(EPOCH FROM \"#{arel_attribute.relation.name}\".\"#{arel_attribute.name}\")") + end + + # Need to cast '0' to an INTERVAL before we can check if the interval is positive + def zero_interval + Arel::Nodes::NamedFunction.new("CAST", [Arel.sql("'0' AS INTERVAL")]) + end +end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 22231b2e0f0..6ed9216d666 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -28,6 +28,7 @@ module Issuable loaded? && to_a.all? { |note| note.association(:award_emoji).loaded? } end end + has_many :label_links, as: :target, dependent: :destroy has_many :labels, through: :label_links has_many :todos, as: :target, dependent: :destroy diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb new file mode 100644 index 00000000000..ef3f1bbcab5 --- /dev/null +++ b/app/models/cycle_analytics.rb @@ -0,0 +1,130 @@ +class CycleAnalytics + include DatabaseMedian + + def initialize(project, from:) + @project = project + @from = from + @summary = Summary.new(project, from: from) + end + + def summary + @summary + end + + def issue + calculate_metric!(:issue, + TableReferences.issues[:created_at], + [TableReferences.issue_metrics[:first_associated_with_milestone_at], + TableReferences.issue_metrics[:first_added_to_board_at]]) + end + + def plan + calculate_metric!(:plan, + [TableReferences.issue_metrics[:first_associated_with_milestone_at], + TableReferences.issue_metrics[:first_added_to_board_at]], + TableReferences.issue_metrics[:first_mentioned_in_commit_at]) + end + + def code + calculate_metric!(:code, + TableReferences.issue_metrics[:first_mentioned_in_commit_at], + TableReferences.merge_requests[:created_at]) + end + + def test + calculate_metric!(:test, + TableReferences.merge_request_metrics[:latest_build_started_at], + TableReferences.merge_request_metrics[:latest_build_finished_at]) + end + + def review + calculate_metric!(:review, + TableReferences.merge_requests[:created_at], + TableReferences.merge_request_metrics[:merged_at]) + end + + def staging + calculate_metric!(:staging, + TableReferences.merge_request_metrics[:merged_at], + TableReferences.merge_request_metrics[:first_deployed_to_production_at]) + end + + def production + calculate_metric!(:production, + TableReferences.issues[:created_at], + TableReferences.merge_request_metrics[:first_deployed_to_production_at]) + end + + private + + def calculate_metric!(name, start_time_attrs, end_time_attrs) + cte_table = Arel::Table.new("cte_table_for_#{name}") + + # Add a `SELECT` for (end_time - start-time), and add an alias for it. + query = Arel::Nodes::As.new(cte_table, subtract_datetimes(base_query, end_time_attrs, start_time_attrs, name.to_s)) + queries = Array.wrap(median_datetime(cte_table, query, name)) + results = queries.map { |query| run_query(query) } + extract_median(results).presence + end + + # Join table with a row for every <issue,merge_request> pair (where the merge request + # closes the given issue) with issue and merge request metrics included. The metrics + # are loaded with an inner join, so issues / merge requests without metrics are + # automatically excluded. + def base_query + arel_table = TableReferences.merge_requests_closing_issues + + # Load issues + query = arel_table.join(TableReferences.issues).on(TableReferences.issues[:id].eq(arel_table[:issue_id])). + join(TableReferences.issue_metrics).on(TableReferences.issues[:id].eq(TableReferences.issue_metrics[:issue_id])). + where(TableReferences.issues[:project_id].eq(@project.id)). + where(TableReferences.issues[:deleted_at].eq(nil)). + where(TableReferences.issues[:created_at].gteq(@from)) + + # Load merge_requests + query = query.join(TableReferences.merge_requests, Arel::Nodes::OuterJoin).on(TableReferences.merge_requests[:id].eq(arel_table[:merge_request_id])). + join(TableReferences.merge_request_metrics).on(TableReferences.merge_requests[:id].eq(TableReferences.merge_request_metrics[:merge_request_id])) + + # Limit to merge requests that have been deployed to production after `@from` + query.where(TableReferences.merge_request_metrics[:first_deployed_to_production_at].gteq(@from)) + end + + # Note: We use COALESCE to pick up the first non-null column for end_time / start_time. + def subtract_datetimes(query_so_far, end_time_attrs, start_time_attrs, as) + diff_fn = case ActiveRecord::Base.connection.adapter_name + when 'PostgreSQL' + Arel::Nodes::Subtraction.new( + Arel::Nodes::NamedFunction.new("COALESCE", Array.wrap(end_time_attrs)), + Arel::Nodes::NamedFunction.new("COALESCE", Array.wrap(start_time_attrs))) + when 'Mysql2' + Arel::Nodes::NamedFunction.new( + "TIMESTAMPDIFF", + [Arel.sql('second'), + Arel::Nodes::NamedFunction.new("COALESCE", Array.wrap(start_time_attrs)), + Arel::Nodes::NamedFunction.new("COALESCE", Array.wrap(end_time_attrs))]) + else + raise NotImplementedError, "Cycle analytics doesn't support your database type." + end + + query_so_far.project(diff_fn.as(as)) + end + + def run_query(query) + if query.is_a? String + ActiveRecord::Base.connection.execute query + else + ActiveRecord::Base.connection.execute query.to_sql + end + end + + def extract_median(results) + result = results.compact.first + + case ActiveRecord::Base.connection.adapter_name + when 'PostgreSQL' + result.first['median'].to_f + when 'Mysql2' + result.to_a.flatten.first + end + end +end diff --git a/app/models/cycle_analytics/queries.rb b/app/models/cycle_analytics/queries.rb new file mode 100644 index 00000000000..7bdf1fb6290 --- /dev/null +++ b/app/models/cycle_analytics/queries.rb @@ -0,0 +1,80 @@ +class CycleAnalytics + class Queries + def initialize(project) + @project = project + end + + def issues(options = {}) + @issues_data ||= + begin + issues_query(options).preload(:metrics).map { |issue| { issue: issue } } + end + end + + def merge_requests_closing_issues(options = {}) + @merge_requests_closing_issues_data ||= + begin + merge_requests_closing_issues = MergeRequestsClosingIssues.where(issue: issues_query(options)).preload(issue: [:metrics], merge_request: [:metrics]) + + merge_requests_closing_issues.map do |record| + { issue: record.issue, merge_request: record.merge_request } + end + end + end + + def issue_first_associated_with_milestone_at + lambda do |data_point| + issue = data_point[:issue] + issue.metrics.first_associated_with_milestone_at if issue.metrics.present? + end + end + + def issue_first_added_to_list_label_at + lambda do |data_point| + issue = data_point[:issue] + issue.metrics.first_added_to_board_at if issue.metrics.present? + end + end + + def issue_first_mentioned_in_commit_at + lambda do |data_point| + issue = data_point[:issue] + issue.metrics.first_mentioned_in_commit_at if issue.metrics.present? + end + end + + def merge_request_merged_at + lambda do |data_point| + merge_request = data_point[:merge_request] + merge_request.metrics.merged_at if merge_request.metrics.present? + end + end + + def merge_request_build_started_at + lambda do |data_point| + merge_request = data_point[:merge_request] + merge_request.metrics.latest_build_started_at if merge_request.metrics.present? + end + end + + def merge_request_build_finished_at + lambda do |data_point| + merge_request = data_point[:merge_request] + merge_request.metrics.latest_build_finished_at if merge_request.metrics.present? + end + end + + def merge_request_deployed_to_production_at + lambda do |data_point| + merge_request = data_point[:merge_request] + merge_request.metrics.first_deployed_to_production_at if merge_request.metrics.present? + end + end + + private + + def issues_query(created_after:) + @project.issues.where("created_at >= ?", created_after) + end + end +end diff --git a/app/models/cycle_analytics/summary.rb b/app/models/cycle_analytics/summary.rb new file mode 100644 index 00000000000..04a90a8da49 --- /dev/null +++ b/app/models/cycle_analytics/summary.rb @@ -0,0 +1,24 @@ +class CycleAnalytics + class Summary + def initialize(project, from:) + @project = project + @from = from + end + + def new_issues + @project.issues.where("created_at > ?", @from).count + end + + def commits + repository = @project.repository.raw_repository + + if @project.default_branch + repository.log(ref: @project.default_branch, after: @from).count + end + end + + def deploys + @project.deployments.where("created_at > ?", @from).count + end + end +end diff --git a/app/models/cycle_analytics/table_references.rb b/app/models/cycle_analytics/table_references.rb new file mode 100644 index 00000000000..f276723ee0e --- /dev/null +++ b/app/models/cycle_analytics/table_references.rb @@ -0,0 +1,25 @@ +class CycleAnalytics + module TableReferences + class << self + def issues + Issue.arel_table + end + + def issue_metrics + Issue::Metrics.arel_table + end + + def merge_requests + MergeRequest.arel_table + end + + def merge_request_metrics + MergeRequest::Metrics.arel_table + end + + def merge_requests_closing_issues + MergeRequestsClosingIssues.arel_table + end + end + end +end diff --git a/app/models/issue.rb b/app/models/issue.rb index 788611305fe..2403748d017 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -23,6 +23,11 @@ class Issue < ActiveRecord::Base has_many :events, as: :target, dependent: :destroy + has_one :metrics, dependent: :destroy + + has_many :merge_requests_closing_issues, class_name: MergeRequestsClosingIssues + has_many :closed_by_merge_requests, through: :merge_requests_closing_issues, source: :merge_request + validates :project, presence: true scope :cared, ->(user) { where(assignee_id: user) } @@ -36,6 +41,8 @@ class Issue < ActiveRecord::Base scope :order_due_date_asc, -> { reorder('issues.due_date IS NULL, issues.due_date ASC') } scope :order_due_date_desc, -> { reorder('issues.due_date IS NULL, issues.due_date DESC') } + after_save :record_metrics + attr_spammable :title, spam_title: true attr_spammable :description, spam_description: true @@ -195,7 +202,7 @@ 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? + return [] if !open? ext = all_references(current_user) @@ -270,4 +277,9 @@ class Issue < ActiveRecord::Base def check_for_spam? project.public? end + + def record_metrics + metrics = Metrics.find_or_create_by(issue_id: self.id) + metrics.record! + end end diff --git a/app/models/issue/metrics.rb b/app/models/issue/metrics.rb new file mode 100644 index 00000000000..3c1fcea9e6f --- /dev/null +++ b/app/models/issue/metrics.rb @@ -0,0 +1,25 @@ +class Issue::Metrics < ActiveRecord::Base + belongs_to :issue + + def record! + if issue.milestone_id.present? && self.first_associated_with_milestone_at.blank? + self.first_associated_with_milestone_at = Time.now + end + + if issue_assigned_to_list_label? && self.first_added_to_board_at.blank? + self.first_added_to_board_at = Time.now + end + + self.save if self.changed? + end + + def record_commit_mention!(commit_time) + self.update(first_mentioned_in_commit_at: commit_time) if self.first_mentioned_in_commit_at.blank? + end + + private + + def issue_assigned_to_list_label? + issue.labels.any? { |label| label.lists.present? } + end +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 75f48fd4ba5..e7af37a6c0a 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -13,9 +13,13 @@ 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 + has_many :merge_requests_closing_issues, class_name: MergeRequestsClosingIssues + has_many :issues_closed, through: :merge_requests_closing_issues, source: :issue + serialize :merge_params, Hash after_create :ensure_merge_request_diff, unless: :importing? @@ -31,6 +35,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 @@ -501,6 +507,19 @@ class MergeRequest < ActiveRecord::Base target_project end + # If the merge request closes any issues, save this information in the + # `MergeRequestsClosingIssues` model. This is a performance optimization. + # Calculating this information for a number of merge requests requires + # running `ReferenceExtractor` on each of them separately. + def cache_merge_request_closes_issues!(current_user = self.author) + transaction do + self.merge_requests_closing_issues.destroy_all + closes_issues(current_user).each do |issue| + MergeRequestsClosingIssues.create!(merge_request: self, issue: issue) + end + end + end + def closes_issue?(issue) closes_issues.include?(issue) end @@ -822,4 +841,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..8c42c830fac --- /dev/null +++ b/app/models/merge_request/metrics.rb @@ -0,0 +1,23 @@ +class MergeRequest::Metrics < ActiveRecord::Base + belongs_to :merge_request + + def record! + if merge_request.merged? && self.merged_at.blank? + self.merged_at = Time.now + end + + self.save if self.changed? + end + + def record_production_deploy!(deploy_time) + self.update(first_deployed_to_production_at: deploy_time) if self.first_deployed_to_production_at.blank? + end + + def record_latest_build_start_time!(start_time) + self.update(latest_build_started_at: start_time, latest_build_finished_at: nil) + end + + def record_latest_build_finish_time!(finish_time) + self.update(latest_build_finished_at: finish_time) + end +end diff --git a/app/models/merge_requests_closing_issues.rb b/app/models/merge_requests_closing_issues.rb new file mode 100644 index 00000000000..cd49076002d --- /dev/null +++ b/app/models/merge_requests_closing_issues.rb @@ -0,0 +1,9 @@ +class MergeRequestsClosingIssues < ActiveRecord::Base + belongs_to :merge_request + belongs_to :issue + + validates_uniqueness_of :merge_request_id, scope: :issue_id + + validates_presence_of :merge_request + validates_presence_of :issue +end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 00c4c7b1440..ae77b2f0506 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -107,6 +107,7 @@ class ProjectPolicy < BasePolicy can! :admin_pipeline can! :admin_environment can! :admin_deployment + can! :read_cycle_analytics end def public_access! diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb index e6667132e27..aad5cc8a15b 100644 --- a/app/services/create_deployment_service.rb +++ b/app/services/create_deployment_service.rb @@ -4,7 +4,7 @@ class CreateDeploymentService < BaseService def execute(deployable = nil) environment = find_or_create_environment - project.deployments.create( + deployment = project.deployments.create( environment: environment, ref: params[:ref], tag: params[:tag], @@ -12,6 +12,36 @@ class CreateDeploymentService < BaseService user: current_user, deployable: deployable ) + + update_merge_request_metrics(deployment, environment) + + deployment + end + + private + + def update_merge_request_metrics(deployment, environment) + if environment.name == "production" + query = project.merge_requests.joins("LEFT OUTER JOIN merge_request_metrics ON merge_request_metrics.merge_request_id = merge_requests.id"). + where(target_branch: params[:ref], "merge_request_metrics.first_deployed_to_production_at" => nil) + + previous_deployment = previous_deployment_for_ref(deployment) + merge_requests_deployed_to_production_for_first_time = if previous_deployment + query.where("merge_request_metrics.merged_at < ? AND merge_request_metrics.merged_at > ?", deployment.created_at, previous_deployment.created_at) + else + query.where("merge_request_metrics.merged_at < ?", deployment.created_at) + end + + merge_requests_deployed_to_production_for_first_time.each { |merge_request| merge_request.metrics.record_production_deploy!(deployment.created_at) } + end + end + + def previous_deployment_for_ref(current_deployment) + @previous_deployment_for_ref ||= + project.deployments.joins(:environment). + where("environments.name": params[:environment], ref: params[:ref]). + where.not(id: current_deployment.id). + first end private diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 948041063c0..cde993221ff 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -134,6 +134,7 @@ class GitPushService < BaseService end commit.create_cross_references!(authors[commit], closed_issues) + update_issue_metrics(commit, authors) end end @@ -186,4 +187,9 @@ class GitPushService < BaseService def branch_name @branch_name ||= Gitlab::Git.ref_name(params[:ref]) end + + def update_issue_metrics(commit, authors) + mentioned_issues = commit.all_references(authors[commit]).issues + mentioned_issues.each { |issue| issue.metrics.record_commit_mention!(commit.committed_date) if issue.metrics.present? } + end end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index ba424b09463..9704a7050eb 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -35,6 +35,18 @@ module MergeRequests end end + def create(merge_request) + merge_request = super(merge_request) + merge_request.cache_merge_request_closes_issues!(current_user) + merge_request + end + + def update(merge_request) + merge_request = super(merge_request) + merge_request.cache_merge_request_closes_issues!(current_user) + merge_request + end + private def filter_params diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 5cedd6f11d9..cd4dd967404 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -13,6 +13,7 @@ module MergeRequests reload_merge_requests reset_merge_when_build_succeeds mark_pending_todos_done + cache_merge_requests_closing_issues # Leave a system note if a branch was deleted/added if branch_added? || branch_removed? @@ -141,6 +142,14 @@ module MergeRequests end end + # If the merge requests closes any issues, save this information in the + # `MergeRequestsClosingIssues` model (as a performance optimization). + def cache_merge_requests_closing_issues + merge_requests_for_source_branch.each do |merge_request| + merge_request.cache_merge_request_closes_issues!(@current_user) + end + end + def filter_merge_requests(merge_requests) merge_requests.uniq.select(&:source_project) end diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 8e4937b7aa0..e44a2bfed9d 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -47,7 +47,7 @@ Repository - if project_nav_tab? :pipelines - = nav_link(controller: [:pipelines, :builds, :environments]) do + = nav_link(controller: [:pipelines, :builds, :environments, :cycle_analytics]) do = link_to project_pipelines_path(@project), title: 'Pipelines', class: 'shortcuts-pipelines' do %span Pipelines diff --git a/app/views/projects/cycle_analytics/show.html.haml b/app/views/projects/cycle_analytics/show.html.haml new file mode 100644 index 00000000000..f0b95fc4b94 --- /dev/null +++ b/app/views/projects/cycle_analytics/show.html.haml @@ -0,0 +1,55 @@ += render 'projects/pipelines/head' + +#cycle-analytics{"v-cloak" => "true", data: { request_path: "#{project_cycle_analytics_path(@project)}"}} + + .bordered-box.landing.content-block{"v-if" => "!isHelpDismissed"} + = icon('times', class: 'dismiss-icon', "@click": "dismissLanding()") + = custom_icon('icon_cycle_analytics_splash') + .inner-content + %h4 + Introducing Cycle Analytics + %p + Cycle Analytics gives an overview on how much time it takes to go from idea to production in your project. + + = button_tag 'Read more', class: 'btn' + + = icon("spinner spin", "v-show" => "isLoading") + + .wrapper{"v-show" => "!isLoading && !hasError"} + .panel.panel-default + .panel-heading + Pipeline Health + + .content-block + .container-fluid + .row + .col-xs-3.column{"v-for" => "item in summary"} + %h3.header {{item.value}} + %p.text {{item.title}} + + .col-xs-3.column + .dropdown.inline.js-ca-dropdown + %button.dropdown-menu-toggle{"data-toggle" => "dropdown", :type => "button"} + %span.dropdown-label Last 30 days + %i.fa.fa-chevron-down + %ul.dropdown-menu.dropdown-menu-align-right + %li + %a{'href' => "#", 'data-value' => '30'} + Last 30 days + %li + %a{'href' => "#", 'data-value' => '90'} + Last 90 days + + .bordered-box + %ul.content-list + %li{"v-for" => "item in stats"} + .container-fluid + .row + .col-xs-10.title-col + %p.title + {{item.title}} + %p.text + {{item.description}} + .col-xs-2.value-col + %span + {{item.value}} diff --git a/app/views/projects/pipelines/_head.html.haml b/app/views/projects/pipelines/_head.html.haml index f611ddc8f5f..fa1470f5fbc 100644 --- a/app/views/projects/pipelines/_head.html.haml +++ b/app/views/projects/pipelines/_head.html.haml @@ -19,3 +19,8 @@ = link_to project_environments_path(@project), title: 'Environments', class: 'shortcuts-environments' do %span Environments + + = nav_link(controller: %w(cycle_analytics)) do + = link_to project_cycle_analytics_path(@project), title: 'Cycle Analytics' do + %span + Cycle Analytics diff --git a/app/views/shared/icons/_icon_cycle_analytics_splash.svg b/app/views/shared/icons/_icon_cycle_analytics_splash.svg new file mode 100644 index 00000000000..f7af1b6fea6 --- /dev/null +++ b/app/views/shared/icons/_icon_cycle_analytics_splash.svg @@ -0,0 +1 @@ +<svg width="167" height="166" viewBox="261 77 167 166" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><defs><path id="a" d="M18 84h34v9H18z"/><mask id="h" x="0" y="0" width="34" height="9" fill="#fff"><use xlink:href="#a"/></mask><path d="M26 95c0 4.97 4.03 9 9 9s9-4.03 9-9" id="b"/><mask id="i" x="0" y="0" width="18" height="9" fill="#fff"><use xlink:href="#b"/></mask><path d="M42 48l25.564 18.26C68.91 67.22 70 69.34 70 70.994v22.004c0 2.21-1.456 2.962-3.253 1.678L42 77V48z" id="c"/><mask id="j" x="0" y="0" width="28" height="47.303" fill="#fff"><use xlink:href="#c"/></mask><path d="M0 48l25.564 18.26C26.91 67.22 28 69.34 28 70.994v22.004c0 2.21-1.456 2.962-3.253 1.678L0 77V48z" id="d"/><mask id="k" x="0" y="0" width="28" height="47.303" fill="#fff"><use xlink:href="#d"/></mask><path d="M52.994 86C59.156 79.382 62 67.656 62 49.725 62 19.235 38.032 1.18 38.032 1.18c-1.675-1.434-4.408-1.466-6.064 0C31.968 1.18 8 19.234 8 49.724 8 67.655 10.844 79.382 17.006 86h35.988z" id="e"/><mask id="l" x="0" y="0" width="54" height="85.908" fill="#fff"><use xlink:href="#e"/></mask><circle id="f" cx="35" cy="43" r="13"/><mask id="m" x="0" y="0" width="26" height="26" fill="#fff"><use xlink:href="#f"/></mask><circle id="g" cx="35" cy="43" r="8"/><mask id="n" x="0" y="0" width="16" height="16" fill="#fff"><use xlink:href="#g"/></mask></defs><g fill="none" fill-rule="evenodd" transform="translate(262 77)"><g transform="translate(47 6)"><path d="M51 132c0 2.21 1.79 4 4 4s4-1.79 4-4M11 122c0 2.21 1.79 4 4 4s4-1.79 4-4" stroke="#E5E5E5" stroke-width="2" stroke-linecap="round"/><rect fill="#E5E5E5" x="50" y="107" width="2" height="6" rx="1"/><rect fill="#E5E5E5" x="50" y="117" width="2" height="6" rx="1"/><rect fill="#E5E5E5" x="34" y="107" width="2" height="6" rx="1"/><rect fill="#E5E5E5" x="42" y="107" width="2" height="6" rx="1"/><rect fill="#E5E5E5" x="34" y="117" width="2" height="6" rx="1"/><rect fill="#E5E5E5" x="34" y="127" width="2" height="6" rx="1"/><rect fill="#E5E5E5" x="42" y="117" width="2" height="30" rx="1"/><rect fill="#E5E5E5" x="26" y="107" width="2" height="27" rx="1"/><rect fill="#E5E5E5" x="50" y="127" width="2" height="6" rx="1"/><rect fill="#E5E5E5" x="18" y="107" width="2" height="6" rx="1"/><rect fill="#E5E5E5" x="18" y="117" width="2" height="6" rx="1"/><use stroke="#E5E5E5" mask="url(#h)" stroke-width="4" xlink:href="#a"/><use stroke="#E5E5E5" mask="url(#i)" stroke-width="4" xlink:href="#b"/><use stroke="#E5E5E5" mask="url(#j)" stroke-width="4" xlink:href="#c"/><use stroke="#E5E5E5" mask="url(#k)" stroke-width="4" transform="matrix(-1 0 0 1 28 0)" xlink:href="#d"/><use stroke="#E5E5E5" mask="url(#l)" stroke-width="4" fill="#FFF" xlink:href="#e"/><use stroke="#E5E5E5" mask="url(#m)" stroke-width="4" xlink:href="#f"/><use stroke="#E5E5E5" mask="url(#n)" stroke-width="4" xlink:href="#g"/><path d="M43 146c0 3.59 2.91 6.5 6.5 6.5s6.5-2.91 6.5-6.5M14 133c0 3.59 2.91 6.5 6.5 6.5s6.5-2.91 6.5-6.5" stroke="#E5E5E5" stroke-width="2" stroke-linecap="round"/></g><circle stroke="#E5E5E5" stroke-width="2" cx="138.5" cy="115.5" r="6.5"/><circle stroke="#E5E5E5" stroke-width="2" cx="33.5" cy="28.5" r="6.5"/><circle stroke="#E5E5E5" stroke-width="2" cx="142" cy="52" r="4"/><circle stroke="#E5E5E5" stroke-width="2" cx="4" cy="83" r="4"/></g></svg>
\ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 068c92d1400..33b070188dc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -780,6 +780,8 @@ Rails.application.routes.draw do resources :environments + resource :cycle_analytics + resources :builds, only: [:index, :show], constraints: { id: /\d+/ } do collection do post :cancel_all diff --git a/db/fixtures/development/17_cycle_analytics.rb b/db/fixtures/development/17_cycle_analytics.rb new file mode 100644 index 00000000000..f51d28d9ebf --- /dev/null +++ b/db/fixtures/development/17_cycle_analytics.rb @@ -0,0 +1,246 @@ +require 'sidekiq/testing' +require './spec/support/test_env' + +class Gitlab::Seeder::CycleAnalytics + def initialize(project, perf: false) + @project = project + @user = User.find(1) + @issue_count = perf ? 1000 : 5 + stub_git_pre_receive! + end + + # The GitLab API needn't be running for the fixtures to be + # created. Since we're performing a number of git actions + # here (like creating a branch or committing a file), we need + # to disable the `pre_receive` hook in order to remove this + # dependency on the GitLab API. + def stub_git_pre_receive! + GitHooksService.class_eval do + def run_hook(name) + [true, ''] + end + end + end + + def seed_metrics! + @issue_count.times do |index| + # Issue + Timecop.travel 5.days.from_now + title = "#{FFaker::Product.brand}-#{FFaker::Product.brand}-#{rand(1000)}" + issue = Issue.create(project: @project, title: title, author: @user) + issue_metrics = issue.metrics + + # Milestones / Labels + Timecop.travel 5.days.from_now + if index.even? + issue_metrics.first_associated_with_milestone_at = rand(6..12).hours.from_now + else + issue_metrics.first_added_to_board_at = rand(6..12).hours.from_now + end + + # Commit + Timecop.travel 5.days.from_now + issue_metrics.first_mentioned_in_commit_at = rand(6..12).hours.from_now + + # MR + Timecop.travel 5.days.from_now + branch_name = "#{FFaker::Product.brand}-#{FFaker::Product.brand}-#{rand(1000)}" + @project.repository.add_branch(@user, branch_name, 'master') + merge_request = MergeRequest.create(target_project: @project, source_project: @project, source_branch: branch_name, target_branch: 'master', title: branch_name, author: @user) + merge_request_metrics = merge_request.metrics + + # MR closing issues + Timecop.travel 5.days.from_now + MergeRequestsClosingIssues.create!(issue: issue, merge_request: merge_request) + + # Merge + Timecop.travel 5.days.from_now + merge_request_metrics.merged_at = rand(6..12).hours.from_now + + # Start build + Timecop.travel 5.days.from_now + merge_request_metrics.latest_build_started_at = rand(6..12).hours.from_now + + # Finish build + Timecop.travel 5.days.from_now + merge_request_metrics.latest_build_finished_at = rand(6..12).hours.from_now + + # Deploy to production + Timecop.travel 5.days.from_now + merge_request_metrics.first_deployed_to_production_at = rand(6..12).hours.from_now + + issue_metrics.save! + merge_request_metrics.save! + + print '.' + end + end + + def seed! + Sidekiq::Testing.inline! do + issues = create_issues + puts '.' + + # Stage 1 + Timecop.travel 5.days.from_now + add_milestones_and_list_labels(issues) + print '.' + + # Stage 2 + Timecop.travel 5.days.from_now + branches = mention_in_commits(issues) + print '.' + + # Stage 3 + Timecop.travel 5.days.from_now + merge_requests = create_merge_requests_closing_issues(issues, branches) + print '.' + + # Stage 4 + Timecop.travel 5.days.from_now + run_builds(merge_requests) + print '.' + + # Stage 5 + Timecop.travel 5.days.from_now + merge_merge_requests(merge_requests) + print '.' + + # Stage 6 / 7 + Timecop.travel 5.days.from_now + deploy_to_production(merge_requests) + print '.' + end + + print '.' + end + + private + + def create_issues + Array.new(@issue_count) do + issue_params = { + title: "Cycle Analytics: #{FFaker::Lorem.sentence(6)}", + description: FFaker::Lorem.sentence, + state: 'opened', + assignee: @project.team.users.sample + } + + Issues::CreateService.new(@project, @project.team.users.sample, issue_params).execute + end + end + + def add_milestones_and_list_labels(issues) + issues.shuffle.map.with_index do |issue, index| + Timecop.travel 12.hours.from_now + + if index.even? + issue.update(milestone: @project.milestones.sample) + else + label_name = "#{FFaker::Product.brand}-#{FFaker::Product.brand}-#{rand(1000)}" + list_label = FactoryGirl.create(:label, title: label_name, project: issue.project) + FactoryGirl.create(:list, board: FactoryGirl.create(:board, project: issue.project), label: list_label) + issue.update(labels: [list_label]) + end + + issue + end + end + + def mention_in_commits(issues) + issues.map do |issue| + Timecop.travel 12.hours.from_now + + branch_name = filename = "#{FFaker::Product.brand}-#{FFaker::Product.brand}-#{rand(1000)}" + + issue.project.repository.add_branch(@user, branch_name, 'master') + + options = { + committer: issue.project.repository.user_to_committer(@user), + author: issue.project.repository.user_to_committer(@user), + commit: { message: "Commit for ##{issue.iid}", branch: branch_name, update_ref: true }, + file: { content: "content", path: filename, update: false } + } + + commit_sha = Gitlab::Git::Blob.commit(issue.project.repository, options) + issue.project.repository.commit(commit_sha) + + + GitPushService.new(issue.project, + @user, + oldrev: issue.project.repository.commit("master").sha, + newrev: commit_sha, + ref: 'refs/heads/master').execute + + branch_name + end + end + + def create_merge_requests_closing_issues(issues, branches) + issues.zip(branches).map do |issue, branch| + Timecop.travel 12.hours.from_now + + opts = { + title: 'Cycle Analytics merge_request', + description: "Fixes #{issue.to_reference}", + source_branch: branch, + target_branch: 'master' + } + + MergeRequests::CreateService.new(issue.project, @user, opts).execute + end + end + + def run_builds(merge_requests) + merge_requests.each do |merge_request| + Timecop.travel 12.hours.from_now + + service = Ci::CreatePipelineService.new(merge_request.project, + @user, + ref: "refs/heads/#{merge_request.source_branch}") + pipeline = service.execute(ignore_skip_ci: true, save_on_errors: false) + + pipeline.run! + Timecop.travel rand(1..6).hours.from_now + pipeline.succeed! + end + end + + def merge_merge_requests(merge_requests) + merge_requests.each do |merge_request| + Timecop.travel 12.hours.from_now + + MergeRequests::MergeService.new(merge_request.project, @user).execute(merge_request) + end + end + + def deploy_to_production(merge_requests) + merge_requests.each do |merge_request| + Timecop.travel 12.hours.from_now + + CreateDeploymentService.new(merge_request.project, @user, { + environment: 'production', + ref: 'master', + tag: false, + sha: @project.repository.commit('master').sha + }).execute + end + end +end + +Gitlab::Seeder.quiet do + if ENV['SEED_CYCLE_ANALYTICS'] + Project.all.each do |project| + seeder = Gitlab::Seeder::CycleAnalytics.new(project) + seeder.seed! + end + elsif ENV['CYCLE_ANALYTICS_PERF_TEST'] + seeder = Gitlab::Seeder::CycleAnalytics.new(Project.order(:id).first, perf: true) + seeder.seed! + elsif ENV['CYCLE_ANALYTICS_POPULATE_METRICS_DIRECTLY'] + seeder = Gitlab::Seeder::CycleAnalytics.new(Project.order(:id).first, perf: true) + seeder.seed_metrics! + else + puts "Not running the cycle analytics seed file. Use the `SEED_CYCLE_ANALYTICS` environment variable to enable it." + end +end diff --git a/db/migrate/20160824124900_add_table_issue_metrics.rb b/db/migrate/20160824124900_add_table_issue_metrics.rb new file mode 100644 index 00000000000..04dff26043d --- /dev/null +++ b/db/migrate/20160824124900_add_table_issue_metrics.rb @@ -0,0 +1,37 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddTableIssueMetrics < 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 :issue_metrics do |t| + t.references :issue, index: { name: "index_issue_metrics" }, foreign_key: true, null: false + + t.datetime 'first_mentioned_in_commit_at' + t.datetime 'first_associated_with_milestone_at' + t.datetime 'first_added_to_board_at' + + t.timestamps null: false + end + end +end 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..fb5494c5e8b --- /dev/null +++ b/db/migrate/20160825052008_add_table_merge_request_metrics.rb @@ -0,0 +1,38 @@ +# 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 'latest_build_started_at' + t.datetime 'latest_build_finished_at' + t.datetime 'first_deployed_to_production_at' + t.datetime 'merged_at' + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20160915042921_create_merge_requests_closing_issues.rb b/db/migrate/20160915042921_create_merge_requests_closing_issues.rb new file mode 100644 index 00000000000..02c28cbe0dd --- /dev/null +++ b/db/migrate/20160915042921_create_merge_requests_closing_issues.rb @@ -0,0 +1,34 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CreateMergeRequestsClosingIssues < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = true + + # 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 = 'Adding foreign keys' + + # 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_requests_closing_issues do |t| + t.references :merge_request, foreign_key: true, index: true, null: false + t.references :issue, foreign_key: true, index: true, null: false + + t.timestamps null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 3567908de03..adb7d2dc91e 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: 20160913212128) do +ActiveRecord::Schema.define(version: 20160915081353) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -439,6 +439,17 @@ ActiveRecord::Schema.define(version: 20160913212128) do add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree + create_table "issue_metrics", force: :cascade do |t| + t.integer "issue_id", null: false + t.datetime "first_associated_with_milestone_at" + t.datetime "first_added_to_board_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.datetime "first_mentioned_in_commit_at" + end + + add_index "issue_metrics", ["issue_id"], name: "index_issue_metrics", using: :btree + create_table "issues", force: :cascade do |t| t.string "title" t.integer "assignee_id" @@ -581,6 +592,21 @@ ActiveRecord::Schema.define(version: 20160913212128) 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 "merged_at" + t.datetime "first_closed_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.datetime "first_deployed_to_production_at" + t.datetime "latest_build_started_at" + t.datetime "latest_build_finished_at" + 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 @@ -622,6 +648,13 @@ ActiveRecord::Schema.define(version: 20160913212128) do add_index "merge_requests", ["title"], name: "index_merge_requests_on_title", using: :btree add_index "merge_requests", ["title"], name: "index_merge_requests_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"} + create_table "merge_requests_closing_issues", force: :cascade do |t| + t.integer "merge_request_id", null: false + t.integer "issue_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "milestones", force: :cascade do |t| t.string "title", null: false t.integer "project_id", null: false @@ -1147,8 +1180,12 @@ ActiveRecord::Schema.define(version: 20160913212128) do add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree add_foreign_key "boards", "projects" + 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 "merge_requests_closing_issues", "issues" + add_foreign_key "merge_requests_closing_issues", "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" diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb index 82591604fcb..6f24bf58d14 100644 --- a/spec/factories/deployments.rb +++ b/spec/factories/deployments.rb @@ -3,11 +3,12 @@ FactoryGirl.define do sha '97de212e80737a608d939f648d959671fb0a0142' ref 'master' tag false + project nil environment factory: :environment after(:build) do |deployment, evaluator| - deployment.project = deployment.environment.project + deployment.project ||= deployment.environment.project end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index f1857f846dc..0b989b98b61 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -187,6 +187,37 @@ describe Ci::Pipeline, models: true do end end + describe "merge request metrics" do + let(:project) { FactoryGirl.create :project } + let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) } + let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) } + + context 'when transitioning to running' do + it 'records the build start time' do + time = Time.now + Timecop.freeze(time) { build.run } + + expect(merge_request.metrics.latest_build_started_at).to eq(time) + end + + it 'clears the build end time' do + build.run + + expect(merge_request.metrics.latest_build_finished_at).to be_nil + end + end + + context 'when transitioning to success' do + it 'records the build end time' do + build.run + time = Time.now + Timecop.freeze(time) { build.success } + + expect(merge_request.metrics.latest_build_finished_at).to eq(time) + end + end + end + def create_build(name, queued_at = current, started_from = 0) create(:ci_build, name: name, @@ -468,4 +499,28 @@ describe Ci::Pipeline, models: true do stage_idx: stage_idx) end end + + describe "#merge_requests" do + let(:project) { FactoryGirl.create :project } + let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) } + + it "returns merge requests whose `diff_head_sha` matches the pipeline's SHA" do + merge_request = create(:merge_request, source_project: project, source_branch: pipeline.ref) + + expect(pipeline.merge_requests).to eq([merge_request]) + end + + it "doesn't return merge requests whose source branch doesn't match the pipeline's ref" do + create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') + + expect(pipeline.merge_requests).to be_empty + end + + it "doesn't return merge requests whose `diff_head_sha` doesn't match the pipeline's SHA" do + create(:merge_request, source_project: project, source_branch: pipeline.ref) + allow_any_instance_of(MergeRequest).to receive(:diff_head_sha) { '97de212e80737a608d939f648d959671fb0a0142b' } + + expect(pipeline.merge_requests).to be_empty + end + end end diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb new file mode 100644 index 00000000000..e81c78df8f0 --- /dev/null +++ b/spec/models/cycle_analytics/code_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe 'CycleAnalytics#code', feature: true do + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec(phase: :code, + data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } }, + start_time_conditions: [["issue mentioned in a commit", -> (context, data) { context.create_commit_referencing_issue(data[:issue]) }]], + end_time_conditions: [["merge request that closes issue is created", -> (context, data) { context.create_merge_request_closing_issue(data[:issue]) }]], + post_fn: -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end) + + context "when a regular merge request (that doesn't close the issue) is created" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + + create_commit_referencing_issue(issue) + create_merge_request_closing_issue(issue, message: "Closes nothing") + + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.code).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb new file mode 100644 index 00000000000..8d7d03193f0 --- /dev/null +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe 'CycleAnalytics#issue', models: true do + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec(phase: :issue, + data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } }, + start_time_conditions: [["issue created", -> (context, data) { data[:issue].save }]], + end_time_conditions: [["issue associated with a milestone", -> (context, data) { data[:issue].update(milestone: context.create(:milestone, project: context.project)) if data[:issue].persisted? }], + ["list label added to issue", -> (context, data) { data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) if data[:issue].persisted? }]], + post_fn: -> (context, data) do + if data[:issue].persisted? + context.create_merge_request_closing_issue(data[:issue].reload) + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end + end) + + context "when a regular label (instead of a list label) is added to the issue" do + it "returns nil" do + 5.times do + regular_label = create(:label) + issue = create(:issue, project: project) + issue.update(label_ids: [regular_label.id]) + + create_merge_request_closing_issue(issue) + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.issue).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb new file mode 100644 index 00000000000..aa779e5050f --- /dev/null +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe 'CycleAnalytics#plan', feature: true do + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec(phase: :plan, + data_fn: -> (context) { { issue: context.create(:issue, project: context.project), + branch_name: context.random_git_name } }, + start_time_conditions: [["issue associated with a milestone", -> (context, data) { data[:issue].update(milestone: context.create(:milestone, project: context.project)) }], + ["list label added to issue", -> (context, data) { data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) }]], + end_time_conditions: [["issue mentioned in a commit", -> (context, data) { context.create_commit_referencing_issue(data[:issue], branch_name: data[:branch_name]) }]], + post_fn: -> (context, data) do + context.create_merge_request_closing_issue(data[:issue], source_branch: data[:branch_name]) + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end) + + context "when a regular label (instead of a list label) is added to the issue" do + it "returns nil" do + branch_name = random_git_name + label = create(:label) + issue = create(:issue, project: project) + issue.update(label_ids: [label.id]) + create_commit_referencing_issue(issue, branch_name: branch_name) + + create_merge_request_closing_issue(issue, source_branch: branch_name) + merge_merge_requests_closing_issue(issue) + deploy_master + + expect(subject.issue).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb new file mode 100644 index 00000000000..703f8d5f782 --- /dev/null +++ b/spec/models/cycle_analytics/production_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe 'CycleAnalytics#production', feature: true do + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec(phase: :production, + data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } }, + start_time_conditions: [["issue is created", -> (context, data) { data[:issue].save }]], + before_end_fn: lambda do |context, data| + context.create_merge_request_closing_issue(data[:issue]) + context.merge_merge_requests_closing_issue(data[:issue]) + end, + end_time_conditions: + [["merge request that closes issue is deployed to production", -> (context, data) { context.deploy_master }], + ["production deploy happens after merge request is merged (along with other changes)", + lambda do |context, data| + # Make other changes on master + sha = context.project.repository.commit_file(context.user, context.random_git_name, "content", "commit message", 'master', false) + context.project.repository.commit(sha) + + context.deploy_master + end]]) + + context "when a regular merge request (that doesn't close the issue) is merged and deployed" do + it "returns nil" do + 5.times do + merge_request = create(:merge_request) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master + end + + expect(subject.production).to be_nil + end + end + + context "when the deployment happens to a non-production environment" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master(environment: 'staging') + end + + expect(subject.production).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb new file mode 100644 index 00000000000..100ce11299a --- /dev/null +++ b/spec/models/cycle_analytics/review_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe 'CycleAnalytics#review', feature: true do + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec(phase: :review, + data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } }, + start_time_conditions: [["merge request that closes issue is created", -> (context, data) { context.create_merge_request_closing_issue(data[:issue]) }]], + end_time_conditions: [["merge request that closes issue is merged", -> (context, data) { context.merge_merge_requests_closing_issue(data[:issue]) }]], + post_fn: -> (context, data) { context.deploy_master }) + + context "when a regular merge request (that doesn't close the issue) is created and merged" do + it "returns nil" do + 5.times do + MergeRequests::MergeService.new(project, user).execute(create(:merge_request)) + + deploy_master + end + + expect(subject.review).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb new file mode 100644 index 00000000000..105f566f41c --- /dev/null +++ b/spec/models/cycle_analytics/staging_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe 'CycleAnalytics#staging', feature: true do + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec(phase: :staging, + data_fn: lambda do |context| + issue = context.create(:issue, project: context.project) + { issue: issue, merge_request: context.create_merge_request_closing_issue(issue) } + end, + start_time_conditions: [["merge request that closes issue is merged", -> (context, data) { context.merge_merge_requests_closing_issue(data[:issue])} ]], + end_time_conditions: [["merge request that closes issue is deployed to production", -> (context, data) { context.deploy_master }], + ["production deploy happens after merge request is merged (along with other changes)", + lambda do |context, data| + # Make other changes on master + sha = context.project.repository.commit_file(context.user, context.random_git_name, "content", "commit message", 'master', false) + context.project.repository.commit(sha) + + context.deploy_master + end]]) + + context "when a regular merge request (that doesn't close the issue) is merged and deployed" do + it "returns nil" do + 5.times do + merge_request = create(:merge_request) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master + end + + expect(subject.staging).to be_nil + end + end + + context "when the deployment happens to a non-production environment" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master(environment: 'staging') + end + + expect(subject.staging).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/summary_spec.rb b/spec/models/cycle_analytics/summary_spec.rb new file mode 100644 index 00000000000..743bc2da33f --- /dev/null +++ b/spec/models/cycle_analytics/summary_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe CycleAnalytics::Summary, models: true do + let(:project) { create(:project) } + let(:from) { Time.now } + let(:user) { create(:user, :admin) } + subject { described_class.new(project, from: from) } + + describe "#new_issues" do + it "finds the number of issues created after the 'from date'" do + Timecop.freeze(5.days.ago) { create(:issue, project: project) } + Timecop.freeze(5.days.from_now) { create(:issue, project: project) } + + expect(subject.new_issues).to eq(1) + end + + it "doesn't find issues from other projects" do + Timecop.freeze(5.days.from_now) { create(:issue, project: create(:project)) } + + expect(subject.new_issues).to eq(0) + end + end + + describe "#commits" do + it "finds the number of commits created after the 'from date'" do + Timecop.freeze(5.days.ago) { create_commit("Test message", project, user, 'master') } + Timecop.freeze(5.days.from_now) { create_commit("Test message", project, user, 'master') } + + expect(subject.commits).to eq(1) + end + + it "doesn't find commits from other projects" do + Timecop.freeze(5.days.from_now) { create_commit("Test message", create(:project), user, 'master') } + + expect(subject.commits).to eq(0) + end + end + + describe "#deploys" do + it "finds the number of deploys made created after the 'from date'" do + Timecop.freeze(5.days.ago) { create(:deployment, project: project) } + Timecop.freeze(5.days.from_now) { create(:deployment, project: project) } + + expect(subject.deploys).to eq(1) + end + + it "doesn't find commits from other projects" do + Timecop.freeze(5.days.from_now) { create(:deployment, project: create(:project)) } + + expect(subject.deploys).to eq(0) + end + end +end diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb new file mode 100644 index 00000000000..79edc29c173 --- /dev/null +++ b/spec/models/cycle_analytics/test_spec.rb @@ -0,0 +1,93 @@ +require 'spec_helper' + +describe 'CycleAnalytics#test', feature: true do + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec(phase: :test, + data_fn: lambda do |context| + issue = context.create(:issue, project: context.project) + merge_request = context.create_merge_request_closing_issue(issue) + { + pipeline: context.create(:ci_pipeline, ref: merge_request.source_branch, sha: merge_request.diff_head_sha, project: context.project), + issue: issue + } + end, + start_time_conditions: [["pipeline is started", -> (context, data) { data[:pipeline].run! }]], + end_time_conditions: [["pipeline is finished", -> (context, data) { data[:pipeline].succeed! }]], + post_fn: -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end) + + context "when the pipeline is for a regular merge request (that doesn't close an issue)" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha) + + pipeline.run! + pipeline.succeed! + + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.test).to be_nil + end + end + + context "when the pipeline is not for a merge request" do + it "returns nil" do + 5.times do + pipeline = create(:ci_pipeline, ref: "refs/heads/master", sha: project.repository.commit('master').sha) + + pipeline.run! + pipeline.succeed! + + deploy_master + end + + expect(subject.test).to be_nil + end + end + + context "when the pipeline is dropped (failed)" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha) + + pipeline.run! + pipeline.drop! + + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.test).to be_nil + end + end + + context "when the pipeline is cancelled" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha) + + pipeline.run! + pipeline.cancel! + + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.test).to be_nil + end + end +end diff --git a/spec/models/issue/metrics_spec.rb b/spec/models/issue/metrics_spec.rb new file mode 100644 index 00000000000..df977486943 --- /dev/null +++ b/spec/models/issue/metrics_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Issue::Metrics, models: true do + let(:project) { create(:project) } + + subject { create(:issue, project: project) } + + describe "when recording the default set of issue metrics on issue save" do + context "milestones" do + it "records the first time an issue is associated with a milestone" do + time = Time.now + Timecop.freeze(time) { subject.update(milestone: create(:milestone)) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_associated_with_milestone_at).to eq(time) + end + + it "does not record the second time an issue is associated with a milestone" do + time = Time.now + Timecop.freeze(time) { subject.update(milestone: create(:milestone)) } + Timecop.freeze(time + 2.hours) { subject.update(milestone: nil) } + Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone)) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_associated_with_milestone_at).to eq(time) + end + end + + context "list labels" do + it "records the first time an issue is associated with a list label" do + list_label = create(:label, lists: [create(:list)]) + time = Time.now + Timecop.freeze(time) { subject.update(label_ids: [list_label.id]) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_added_to_board_at).to eq(time) + end + + it "does not record the second time an issue is associated with a list label" do + time = Time.now + first_list_label = create(:label, lists: [create(:list)]) + Timecop.freeze(time) { subject.update(label_ids: [first_list_label.id]) } + second_list_label = create(:label, lists: [create(:list)]) + Timecop.freeze(time + 5.hours) { subject.update(label_ids: [second_list_label.id]) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_added_to_board_at).to eq(time) + end + end + end +end diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb new file mode 100644 index 00000000000..718b50642ad --- /dev/null +++ b/spec/models/merge_request/metrics_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe MergeRequest::Metrics, models: true do + let(:project) { create(:project) } + + subject { create(:merge_request, source_project: project) } + + describe "when recording the default set of metrics on merge request save" do + it "records the merge time" do + time = Time.now + Timecop.freeze(time) { subject.mark_as_merged } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.merged_at).to eq(time) + end + end +end diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 41b897f36cd..f80f8953486 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -169,4 +169,80 @@ describe CreateDeploymentService, services: true do end end end + + describe "merge request metrics" do + let(:params) do + { + environment: 'production', + ref: 'master', + tag: false, + sha: '97de212e80737a608d939f648d959671fb0a0142b', + } + end + + let(:merge_request) { create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: project) } + + context "while updating the 'first_deployed_to_production_at' time" do + before { merge_request.mark_as_merged } + + context "for merge requests merged before the current deploy" do + it "sets the time if the deploy's environment is 'production'" do + time = Time.now + Timecop.freeze(time) { service.execute } + + expect(merge_request.metrics.first_deployed_to_production_at).to eq(time) + end + + it "doesn't set the time if the deploy's environment is not 'production'" do + staging_params = params.merge(environment: 'staging') + service = described_class.new(project, user, staging_params) + service.execute + + expect(merge_request.metrics.first_deployed_to_production_at).to be_nil + end + + it 'does not raise errors if the merge request does not have a metrics record' do + merge_request.metrics.destroy + + expect(merge_request.reload.metrics).to be_nil + expect { service.execute }.not_to raise_error + end + end + + context "for merge requests merged before the previous deploy" do + context "if the 'first_deployed_to_production_at' time is already set" do + it "does not overwrite the older 'first_deployed_to_production_at' time" do + # Previous deploy + time = Time.now + Timecop.freeze(time) { service.execute } + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to eq(time) + + # Current deploy + service = described_class.new(project, user, params) + Timecop.freeze(time + 12.hours) { service.execute } + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to eq(time) + end + end + + context "if the 'first_deployed_to_production_at' time is not already set" do + it "does not overwrite the older 'first_deployed_to_production_at' time" do + # Previous deploy + time = Time.now + Timecop.freeze(time) { service.execute } + merge_request.metrics.update(first_deployed_to_production_at: nil) + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + + # Current deploy + service = described_class.new(project, user, params) + Timecop.freeze(time + 12.hours) { service.execute } + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + end + end + end + end + end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 22724434a7f..f2ef9f3dd81 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -339,6 +339,43 @@ describe GitPushService, services: true do end end + describe "issue metrics" do + let(:issue) { create :issue, project: project } + let(:commit_author) { create :user } + let(:commit) { project.commit } + let(:commit_time) { Time.now } + + before do + project.team << [commit_author, :developer] + project.team << [user, :developer] + + allow(commit).to receive_messages( + safe_message: "this commit \n mentions #{issue.to_reference}", + references: [issue], + author_name: commit_author.name, + author_email: commit_author.email, + committed_date: commit_time + ) + + allow(project.repository).to receive(:commits_between).and_return([commit]) + end + + context "while saving the 'first_mentioned_in_commit_at' metric for an issue" do + it 'sets the metric for referenced issues' do + execute_service(project, user, @oldrev, @newrev, @ref) + + expect(issue.metrics.first_mentioned_in_commit_at).to eq(commit_time) + end + + it 'does not set the metric for non-referenced issues' do + non_referenced_issue = create(:issue, project: project) + execute_service(project, user, @oldrev, @newrev, @ref) + + expect(non_referenced_issue.metrics.first_mentioned_in_commit_at).to be_nil + end + end + end + describe "closing issues from pushed commits containing a closing reference" do let(:issue) { create :issue, project: project } let(:other_issue) { create :issue, project: project } diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index c1e4f8bd96b..a0614abcf62 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -83,5 +83,33 @@ describe MergeRequests::CreateService, services: true do } end end + + context 'while saving references to issues that the created merge request closes' do + let(:first_issue) { create(:issue, project: project) } + let(:second_issue) { create(:issue, project: project) } + + let(:opts) do + { + title: 'Awesome merge_request', + source_branch: 'feature', + target_branch: 'master', + force_remove_source_branch: '1' + } + end + + before do + project.team << [user, :master] + project.team << [assignee, :developer] + end + + it 'creates a `MergeRequestsClosingIssues` record for each issue' do + issue_closing_opts = opts.merge(description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}") + service = described_class.new(project, user, issue_closing_opts) + allow(service).to receive(:execute_hooks) + merge_request = service.execute + + expect(merge_request.issues_closed).to match_array([first_issue, second_issue]) + end + end end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index fff86480c6d..7ecad8d6bad 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -174,6 +174,36 @@ describe MergeRequests::RefreshService, services: true do end end + context 'push commits closing issues' do + let(:issue) { create :issue, project: @project } + let(:commit_author) { create :user } + let(:commit) { project.commit } + let!(:merge_request) { create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: @project) } + + before do + project.team << [commit_author, :developer] + project.team << [user, :developer] + + allow(commit).to receive_messages( + safe_message: "Closes #{issue.to_reference}", + references: [issue], + author_name: commit_author.name, + author_email: commit_author.email, + committed_date: Time.now + ) + + allow_any_instance_of(MergeRequest).to receive(:commits).and_return([commit]) + end + + it 'creates a `MergeRequestsClosingIssues` record for each closed issue' do + refresh_service = service.new(@project, @user) + allow(refresh_service).to receive(:execute_hooks) + refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature') + + expect(merge_request.reload.issues_closed).to eq([issue]) + end + end + def reload_mrs @merge_request.reload @fork_merge_request.reload diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 6dfeb581975..cfd506707de 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -263,5 +263,39 @@ describe MergeRequests::UpdateService, services: true do end end end + + context 'while saving references to issues that the updated merge request closes' do + let(:first_issue) { create(:issue, project: project) } + let(:second_issue) { create(:issue, project: project) } + + it 'creates a `MergeRequestsClosingIssues` record for each issue' do + issue_closing_opts = { description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}" } + service = described_class.new(project, user, issue_closing_opts) + allow(service).to receive(:execute_hooks) + service.execute(merge_request) + + expect(merge_request.reload.issues_closed).to match_array([first_issue, second_issue]) + end + end + + it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do + opts = { + title: 'Awesome merge_request', + description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}", + source_branch: 'feature', + target_branch: 'master', + force_remove_source_branch: '1' + } + + merge_request = MergeRequests::CreateService.new(project, user, opts).execute + + expect(merge_request.issues_closed).to match_array([first_issue, second_issue]) + + service = described_class.new(project, user, description: "not closing any issues") + allow(service).to receive(:execute_hooks) + service.execute(merge_request.reload) + + expect(merge_request.reload.issues_closed).to be_empty + end end end diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb new file mode 100644 index 00000000000..e8e760a6187 --- /dev/null +++ b/spec/support/cycle_analytics_helpers.rb @@ -0,0 +1,64 @@ +module CycleAnalyticsHelpers + def create_commit_referencing_issue(issue, branch_name: random_git_name) + project.repository.add_branch(user, branch_name, 'master') + create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name) + end + + def create_commit(message, project, user, branch_name) + filename = random_git_name + oldrev = project.repository.commit(branch_name).sha + + options = { + committer: project.repository.user_to_committer(user), + author: project.repository.user_to_committer(user), + commit: { message: message, branch: branch_name, update_ref: true }, + file: { content: "content", path: filename, update: false } + } + + commit_sha = Gitlab::Git::Blob.commit(project.repository, options) + project.repository.commit(commit_sha) + + GitPushService.new(project, + user, + oldrev: oldrev, + newrev: commit_sha, + ref: 'refs/heads/master').execute + end + + def create_merge_request_closing_issue(issue, message: nil, source_branch: nil) + if !source_branch || project.repository.commit(source_branch).blank? + source_branch = random_git_name + project.repository.add_branch(user, source_branch, 'master') + end + + sha = project.repository.commit_file(user, random_git_name, "content", "commit message", source_branch, false) + project.repository.commit(sha) + + opts = { + title: 'Awesome merge_request', + description: message || "Fixes #{issue.to_reference}", + source_branch: source_branch, + target_branch: 'master' + } + + MergeRequests::CreateService.new(project, user, opts).execute + end + + def merge_merge_requests_closing_issue(issue) + merge_requests = issue.closed_by_merge_requests + merge_requests.each { |merge_request| MergeRequests::MergeService.new(project, user).execute(merge_request) } + end + + def deploy_master(environment: 'production') + CreateDeploymentService.new(project, user, { + environment: environment, + ref: 'master', + tag: false, + sha: project.repository.commit('master').sha + }).execute + end +end + +RSpec.configure do |config| + config.include CycleAnalyticsHelpers +end diff --git a/spec/support/cycle_analytics_helpers/test_generation.rb b/spec/support/cycle_analytics_helpers/test_generation.rb new file mode 100644 index 00000000000..4d5ce86691f --- /dev/null +++ b/spec/support/cycle_analytics_helpers/test_generation.rb @@ -0,0 +1,165 @@ +# rubocop:disable Metrics/AbcSize + +# Note: The ABC size is large here because we have a method generating test cases with +# multiple nested contexts. This shouldn't count as a violation. + +module CycleAnalyticsHelpers + module TestGeneration + # Generate the most common set of specs that all cycle analytics phases need to have. + # + # Arguments: + # + # phase: Which phase are we testing? Will call `CycleAnalytics.new.send(phase)` for the final assertion + # data_fn: A function that returns a hash, constituting initial data for the test case + # start_time_conditions: An array of `conditions`. Each condition is an tuple of `condition_name` and `condition_fn`. `condition_fn` is called with + # `context` (no lexical scope, so need to do `context.create` for factories, for example) and `data` (from the `data_fn`). + # Each `condition_fn` is expected to implement a case which consitutes the start of the given cycle analytics phase. + # end_time_conditions: An array of `conditions`. Each condition is an tuple of `condition_name` and `condition_fn`. `condition_fn` is called with + # `context` (no lexical scope, so need to do `context.create` for factories, for example) and `data` (from the `data_fn`). + # Each `condition_fn` is expected to implement a case which consitutes the end of the given cycle analytics phase. + # before_end_fn: This function is run before calling the end time conditions. Used for setup that needs to be run between the start and end conditions. + # post_fn: Code that needs to be run after running the end time conditions. + + def generate_cycle_analytics_spec(phase:, data_fn:, start_time_conditions:, end_time_conditions:, before_end_fn: nil, post_fn: nil) + combinations_of_start_time_conditions = (1..start_time_conditions.size).flat_map { |size| start_time_conditions.combination(size).to_a } + combinations_of_end_time_conditions = (1..end_time_conditions.size).flat_map { |size| end_time_conditions.combination(size).to_a } + + scenarios = combinations_of_start_time_conditions.product(combinations_of_end_time_conditions) + scenarios.each do |start_time_conditions, end_time_conditions| + context "start condition: #{start_time_conditions.map(&:first).to_sentence}" do + context "end condition: #{end_time_conditions.map(&:first).to_sentence}" do + it "finds the median of available durations between the two conditions" do + time_differences = Array.new(5) do |index| + data = data_fn[self] + start_time = (index * 10).days.from_now + end_time = start_time + rand(1..5).days + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + + # Run `before_end_fn` at the midpoint between `start_time` and `end_time` + Timecop.freeze(start_time + (end_time - start_time) / 2) { before_end_fn[self, data] } if before_end_fn + + end_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(end_time) { condition_fn[self, data] } + end + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + + end_time - start_time + end + + median_time_difference = time_differences.sort[2] + expect(subject.send(phase)).to be_within(5).of(median_time_difference) + end + + context "when the data belongs to another project" do + let(:other_project) { create(:project) } + + it "returns nil" do + # Use a stub to "trick" the data/condition functions + # into using another project. This saves us from having to + # define separate data/condition functions for this particular + # test case. + allow(self).to receive(:project) { other_project } + + 5.times do + data = data_fn[self] + start_time = Time.now + end_time = rand(1..10).days.from_now + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + + end_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(end_time) { condition_fn[self, data] } + end + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + end + + # Turn off the stub before checking assertions + allow(self).to receive(:project).and_call_original + + expect(subject.send(phase)).to be_nil + end + end + + context "when the end condition happens before the start condition" do + it 'returns nil' do + data = data_fn[self] + start_time = Time.now + end_time = start_time + rand(1..5).days + + # Run `before_end_fn` at the midpoint between `start_time` and `end_time` + Timecop.freeze(start_time + (end_time - start_time) / 2) { before_end_fn[self, data] } if before_end_fn + + end_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(end_time) { condition_fn[self, data] } + end + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + + expect(subject.send(phase)).to be_nil + end + end + end + end + + context "start condition NOT PRESENT: #{start_time_conditions.map(&:first).to_sentence}" do + context "end condition: #{end_time_conditions.map(&:first).to_sentence}" do + it "returns nil" do + 5.times do + data = data_fn[self] + end_time = rand(1..10).days.from_now + + end_time_conditions.each_with_index do |(condition_name, condition_fn), index| + Timecop.freeze(end_time + index.days) { condition_fn[self, data] } + end + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + end + + expect(subject.send(phase)).to be_nil + end + end + end + + context "start condition: #{start_time_conditions.map(&:first).to_sentence}" do + context "end condition NOT PRESENT: #{end_time_conditions.map(&:first).to_sentence}" do + it "returns nil" do + 5.times do + data = data_fn[self] + start_time = Time.now + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + + post_fn[self, data] if post_fn + end + + expect(subject.send(phase)).to be_nil + end + end + end + end + + context "when none of the start / end conditions are matched" do + it "returns nil" do + expect(subject.send(phase)).to be_nil + end + end + end + end +end + +RSpec.configure do |config| + config.extend CycleAnalyticsHelpers::TestGeneration +end diff --git a/spec/support/git_helpers.rb b/spec/support/git_helpers.rb new file mode 100644 index 00000000000..93422390ef7 --- /dev/null +++ b/spec/support/git_helpers.rb @@ -0,0 +1,9 @@ +module GitHelpers + def random_git_name + "#{FFaker::Product.brand}-#{FFaker::Product.brand}-#{rand(1000)}" + end +end + +RSpec.configure do |config| + config.include GitHelpers +end |
