From 3268e3779166c7ddf47ecc0d78397cd75cf2f0e8 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 18 Nov 2016 16:38:02 +0100 Subject: [PATCH 01/25] WIP - started refactoring cycle analytics median stuff into stages --- app/models/cycle_analytics.rb | 47 +++---------------- lib/gitlab/cycle_analytics/base_event.rb | 8 ++-- lib/gitlab/cycle_analytics/base_stage.rb | 24 ++++++++++ lib/gitlab/cycle_analytics/code_event.rb | 1 - lib/gitlab/cycle_analytics/code_stage.rb | 11 +++++ lib/gitlab/cycle_analytics/events_query.rb | 9 +--- lib/gitlab/cycle_analytics/issue_event.rb | 1 - lib/gitlab/cycle_analytics/issue_stage.rb | 12 +++++ lib/gitlab/cycle_analytics/metrics_fetcher.rb | 3 +- lib/gitlab/cycle_analytics/plan_event.rb | 1 - lib/gitlab/cycle_analytics/plan_stage.rb | 12 +++++ .../cycle_analytics/production_event.rb | 1 - .../cycle_analytics/production_stage.rb | 11 +++++ lib/gitlab/cycle_analytics/review_event.rb | 1 - lib/gitlab/cycle_analytics/review_stage.rb | 11 +++++ lib/gitlab/cycle_analytics/staging_event.rb | 1 - lib/gitlab/cycle_analytics/staging_stage.rb | 11 +++++ lib/gitlab/cycle_analytics/test_event.rb | 1 - lib/gitlab/cycle_analytics/test_stage.rb | 11 +++++ 19 files changed, 118 insertions(+), 59 deletions(-) create mode 100644 lib/gitlab/cycle_analytics/base_stage.rb create mode 100644 lib/gitlab/cycle_analytics/code_stage.rb create mode 100644 lib/gitlab/cycle_analytics/issue_stage.rb create mode 100644 lib/gitlab/cycle_analytics/plan_stage.rb create mode 100644 lib/gitlab/cycle_analytics/production_stage.rb create mode 100644 lib/gitlab/cycle_analytics/review_stage.rb create mode 100644 lib/gitlab/cycle_analytics/staging_stage.rb create mode 100644 lib/gitlab/cycle_analytics/test_stage.rb diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index ba4ee6fcf9d..5e33273c9ba 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -1,17 +1,17 @@ class CycleAnalytics STAGES = %i[issue plan code test review staging production].freeze - def initialize(project, current_user, from:) + def initialize(project, from:) @project = project - @current_user = current_user - @from = from - @fetcher = Gitlab::CycleAnalytics::MetricsFetcher.new(project: project, from: from, branch: nil) + @options = options end def summary - @summary ||= Summary.new(@project, @current_user, from: @from) + @summary ||= Summary.new(@project, from: @options[:from]) end + def method_missing(method_sym, *arguments, &block) + classify_stage(method_sym).new(project: @project, options: @options, stage: method_sym) def permissions(user:) Gitlab::CycleAnalytics::Permissions.get(user: user, project: @project) end @@ -23,40 +23,7 @@ class CycleAnalytics Issue::Metrics.arel_table[:first_added_to_board_at]]) end - def plan - @fetcher.calculate_metric(:plan, - [Issue::Metrics.arel_table[:first_associated_with_milestone_at], - Issue::Metrics.arel_table[:first_added_to_board_at]], - Issue::Metrics.arel_table[:first_mentioned_in_commit_at]) - end - - def code - @fetcher.calculate_metric(:code, - Issue::Metrics.arel_table[:first_mentioned_in_commit_at], - MergeRequest.arel_table[:created_at]) - end - - def test - @fetcher.calculate_metric(:test, - MergeRequest::Metrics.arel_table[:latest_build_started_at], - MergeRequest::Metrics.arel_table[:latest_build_finished_at]) - end - - def review - @fetcher.calculate_metric(:review, - MergeRequest.arel_table[:created_at], - MergeRequest::Metrics.arel_table[:merged_at]) - end - - def staging - @fetcher.calculate_metric(:staging, - MergeRequest::Metrics.arel_table[:merged_at], - MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) - end - - def production - @fetcher.calculate_metric(:production, - Issue.arel_table[:created_at], - MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) + def classify_stage(method_sym) + "Gitlab::CycleAnalytics::#{method_sym.to_s.capitalize}Stage".constantize end end diff --git a/lib/gitlab/cycle_analytics/base_event.rb b/lib/gitlab/cycle_analytics/base_event.rb index 53a148ad703..c87841c119a 100644 --- a/lib/gitlab/cycle_analytics/base_event.rb +++ b/lib/gitlab/cycle_analytics/base_event.rb @@ -5,10 +5,10 @@ module Gitlab attr_reader :stage, :start_time_attrs, :end_time_attrs, :projections, :query - def initialize(project:, options:) - @query = EventsQuery.new(project: project, options: options) - @project = project - @options = options + def initialize(fetcher:, stage:) + @query = EventsQuery.new(fetcher: fetcher) + @project = fetcher.project + @stage = stage end def fetch diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb new file mode 100644 index 00000000000..70f1e1018c9 --- /dev/null +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -0,0 +1,24 @@ +module Gitlab + module CycleAnalytics + class BaseStage + def initialize(project:, options:, stage: stage) + @project = project + @options = options + @fetcher = Gitlab::CycleAnalytics::MetricsFetcher.new(project: project, + from: options[:from], + branch: options[:branch]) + @stage = stage + end + + def events + event_class.new(fetcher: @fetcher, stage: @stage).fetch + end + + private + + def event_class + "Gitlab::CycleAnalytics::#{@stage.to_s.capitalize}Event".constantize + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/code_event.rb b/lib/gitlab/cycle_analytics/code_event.rb index 2afdf0b8518..68251630e08 100644 --- a/lib/gitlab/cycle_analytics/code_event.rb +++ b/lib/gitlab/cycle_analytics/code_event.rb @@ -4,7 +4,6 @@ module Gitlab include MergeRequestAllowed def initialize(*args) - @stage = :code @start_time_attrs = issue_metrics_table[:first_mentioned_in_commit_at] @end_time_attrs = mr_table[:created_at] @projections = [mr_table[:title], diff --git a/lib/gitlab/cycle_analytics/code_stage.rb b/lib/gitlab/cycle_analytics/code_stage.rb new file mode 100644 index 00000000000..9d28393ce53 --- /dev/null +++ b/lib/gitlab/cycle_analytics/code_stage.rb @@ -0,0 +1,11 @@ +module Gitlab + module CycleAnalytics + class CodeStage < BaseStage + def median + @fetcher.calculate_metric(:code, + Issue::Metrics.arel_table[:first_mentioned_in_commit_at], + MergeRequest.arel_table[:created_at]) + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/events_query.rb b/lib/gitlab/cycle_analytics/events_query.rb index 2418832ccc2..e2b79384c9b 100644 --- a/lib/gitlab/cycle_analytics/events_query.rb +++ b/lib/gitlab/cycle_analytics/events_query.rb @@ -1,13 +1,8 @@ module Gitlab module CycleAnalytics class EventsQuery - attr_reader :project - - def initialize(project:, options: {}) - @project = project - @from = options[:from] - @branch = options[:branch] - @fetcher = Gitlab::CycleAnalytics::MetricsFetcher.new(project: project, from: @from, branch: @branch) + def initialize(fetcher:) + @fetcher = fetcher end def execute(stage_class) diff --git a/lib/gitlab/cycle_analytics/issue_event.rb b/lib/gitlab/cycle_analytics/issue_event.rb index 705b7e5ce24..76e8decf36e 100644 --- a/lib/gitlab/cycle_analytics/issue_event.rb +++ b/lib/gitlab/cycle_analytics/issue_event.rb @@ -4,7 +4,6 @@ module Gitlab include IssueAllowed def initialize(*args) - @stage = :issue @start_time_attrs = issue_table[:created_at] @end_time_attrs = [issue_metrics_table[:first_associated_with_milestone_at], issue_metrics_table[:first_added_to_board_at]] diff --git a/lib/gitlab/cycle_analytics/issue_stage.rb b/lib/gitlab/cycle_analytics/issue_stage.rb new file mode 100644 index 00000000000..6793cc77976 --- /dev/null +++ b/lib/gitlab/cycle_analytics/issue_stage.rb @@ -0,0 +1,12 @@ +module Gitlab + module CycleAnalytics + class IssueStage < BaseStage + def median + @fetcher.calculate_metric(:issue, + Issue.arel_table[:created_at], + [Issue::Metrics.arel_table[:first_associated_with_milestone_at], + Issue::Metrics.arel_table[:first_added_to_board_at]]) + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/metrics_fetcher.rb b/lib/gitlab/cycle_analytics/metrics_fetcher.rb index b71e8735e27..51835bbde24 100644 --- a/lib/gitlab/cycle_analytics/metrics_fetcher.rb +++ b/lib/gitlab/cycle_analytics/metrics_fetcher.rb @@ -5,10 +5,11 @@ module Gitlab include Gitlab::Database::DateTime include MetricsTables + attr_reader :project + DEPLOYMENT_METRIC_STAGES = %i[production staging] def initialize(project:, from:, branch:) - @project = project @project = project @from = from @branch = branch diff --git a/lib/gitlab/cycle_analytics/plan_event.rb b/lib/gitlab/cycle_analytics/plan_event.rb index 7c3f0e9989f..4b06143495b 100644 --- a/lib/gitlab/cycle_analytics/plan_event.rb +++ b/lib/gitlab/cycle_analytics/plan_event.rb @@ -2,7 +2,6 @@ module Gitlab module CycleAnalytics class PlanEvent < BaseEvent def initialize(*args) - @stage = :plan @start_time_attrs = issue_metrics_table[:first_associated_with_milestone_at] @end_time_attrs = [issue_metrics_table[:first_added_to_board_at], issue_metrics_table[:first_mentioned_in_commit_at]] diff --git a/lib/gitlab/cycle_analytics/plan_stage.rb b/lib/gitlab/cycle_analytics/plan_stage.rb new file mode 100644 index 00000000000..772237087c0 --- /dev/null +++ b/lib/gitlab/cycle_analytics/plan_stage.rb @@ -0,0 +1,12 @@ +module Gitlab + module CycleAnalytics + class PlanStage < BaseStage + def median + @fetcher.calculate_metric(:plan, + [Issue::Metrics.arel_table[:first_associated_with_milestone_at], + Issue::Metrics.arel_table[:first_added_to_board_at]], + Issue::Metrics.arel_table[:first_mentioned_in_commit_at]) + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/production_event.rb b/lib/gitlab/cycle_analytics/production_event.rb index 4868c3c6237..c03cd4f4909 100644 --- a/lib/gitlab/cycle_analytics/production_event.rb +++ b/lib/gitlab/cycle_analytics/production_event.rb @@ -4,7 +4,6 @@ module Gitlab include IssueAllowed def initialize(*args) - @stage = :production @start_time_attrs = issue_table[:created_at] @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at] @projections = [issue_table[:title], diff --git a/lib/gitlab/cycle_analytics/production_stage.rb b/lib/gitlab/cycle_analytics/production_stage.rb new file mode 100644 index 00000000000..2fb087a8cac --- /dev/null +++ b/lib/gitlab/cycle_analytics/production_stage.rb @@ -0,0 +1,11 @@ +module Gitlab + module CycleAnalytics + class ProductionStage < BaseStage + def median + @fetcher.calculate_metric(:production, + Issue.arel_table[:created_at], + MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/review_event.rb b/lib/gitlab/cycle_analytics/review_event.rb index b394a02cc52..3f9ffa9657b 100644 --- a/lib/gitlab/cycle_analytics/review_event.rb +++ b/lib/gitlab/cycle_analytics/review_event.rb @@ -4,7 +4,6 @@ module Gitlab include MergeRequestAllowed def initialize(*args) - @stage = :review @start_time_attrs = mr_table[:created_at] @end_time_attrs = mr_metrics_table[:merged_at] @projections = [mr_table[:title], diff --git a/lib/gitlab/cycle_analytics/review_stage.rb b/lib/gitlab/cycle_analytics/review_stage.rb new file mode 100644 index 00000000000..ec9f07319e8 --- /dev/null +++ b/lib/gitlab/cycle_analytics/review_stage.rb @@ -0,0 +1,11 @@ +module Gitlab + module CycleAnalytics + class ReviewStage < BaseStage + def median + @fetcher.calculate_metric(:review, + MergeRequest.arel_table[:created_at], + MergeRequest::Metrics.arel_table[:merged_at]) + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/staging_event.rb b/lib/gitlab/cycle_analytics/staging_event.rb index a1f30b716f6..eae18b447f0 100644 --- a/lib/gitlab/cycle_analytics/staging_event.rb +++ b/lib/gitlab/cycle_analytics/staging_event.rb @@ -2,7 +2,6 @@ module Gitlab module CycleAnalytics class StagingEvent < BaseEvent def initialize(*args) - @stage = :staging @start_time_attrs = mr_metrics_table[:merged_at] @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at] @projections = [build_table[:id]] diff --git a/lib/gitlab/cycle_analytics/staging_stage.rb b/lib/gitlab/cycle_analytics/staging_stage.rb new file mode 100644 index 00000000000..9c67a2aa6fe --- /dev/null +++ b/lib/gitlab/cycle_analytics/staging_stage.rb @@ -0,0 +1,11 @@ +module Gitlab + module CycleAnalytics + class StagingStage < BaseStage + def median + @fetcher.calculate_metric(:staging, + MergeRequest::Metrics.arel_table[:merged_at], + MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/test_event.rb b/lib/gitlab/cycle_analytics/test_event.rb index d553d0b5aec..d0736672adf 100644 --- a/lib/gitlab/cycle_analytics/test_event.rb +++ b/lib/gitlab/cycle_analytics/test_event.rb @@ -4,7 +4,6 @@ module Gitlab def initialize(*args) super(*args) - @stage = :test @start_time_attrs = mr_metrics_table[:latest_build_started_at] @end_time_attrs = mr_metrics_table[:latest_build_finished_at] end diff --git a/lib/gitlab/cycle_analytics/test_stage.rb b/lib/gitlab/cycle_analytics/test_stage.rb new file mode 100644 index 00000000000..6bedfdbba61 --- /dev/null +++ b/lib/gitlab/cycle_analytics/test_stage.rb @@ -0,0 +1,11 @@ +module Gitlab + module CycleAnalytics + class TestStage < BaseStage + def median + @fetcher.calculate_metric(:test, + MergeRequest::Metrics.arel_table[:latest_build_started_at], + MergeRequest::Metrics.arel_table[:latest_build_finished_at]) + end + end + end +end From a998276223510ceee67f686dfc3ef536c0252c5a Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 21 Nov 2016 17:15:25 +0100 Subject: [PATCH 02/25] added analytics stage serializer and moved some info to the stage classes from the controller --- app/serializers/analytics_stage_entity.rb | 12 ++++++++++++ app/serializers/analytics_stage_serializer.rb | 3 +++ lib/gitlab/cycle_analytics/base_stage.rb | 2 ++ lib/gitlab/cycle_analytics/code_stage.rb | 6 ++++++ lib/gitlab/cycle_analytics/issue_stage.rb | 6 ++++++ lib/gitlab/cycle_analytics/plan_stage.rb | 6 ++++++ lib/gitlab/cycle_analytics/production_stage.rb | 6 ++++++ lib/gitlab/cycle_analytics/review_stage.rb | 6 ++++++ lib/gitlab/cycle_analytics/staging_stage.rb | 6 ++++++ lib/gitlab/cycle_analytics/test_stage.rb | 6 ++++++ 10 files changed, 59 insertions(+) create mode 100644 app/serializers/analytics_stage_entity.rb create mode 100644 app/serializers/analytics_stage_serializer.rb diff --git a/app/serializers/analytics_stage_entity.rb b/app/serializers/analytics_stage_entity.rb new file mode 100644 index 00000000000..72a587c8c1d --- /dev/null +++ b/app/serializers/analytics_stage_entity.rb @@ -0,0 +1,12 @@ +class AnalyticsStageEntity < Grape::Entity + include EntityDateHelper + + expose :stage, as: :title do |object| + object[:stage].to_s.capitalize + end + expose :description + + expose :median, as: :value do |stage| + stage[:median] && !stage[:median].zero? ? distance_of_time_in_words(stage[:median]) : nil + end +end diff --git a/app/serializers/analytics_stage_serializer.rb b/app/serializers/analytics_stage_serializer.rb new file mode 100644 index 00000000000..613cf6874d8 --- /dev/null +++ b/app/serializers/analytics_stage_serializer.rb @@ -0,0 +1,3 @@ +class AnalyticsStageSerializer < BaseSerializer + entity AnalyticsStageEntity +end diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb index 70f1e1018c9..49d1e6304a9 100644 --- a/lib/gitlab/cycle_analytics/base_stage.rb +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -1,6 +1,8 @@ module Gitlab module CycleAnalytics class BaseStage + attr_reader :stage, :description + def initialize(project:, options:, stage: stage) @project = project @options = options diff --git a/lib/gitlab/cycle_analytics/code_stage.rb b/lib/gitlab/cycle_analytics/code_stage.rb index 9d28393ce53..f72989c9a72 100644 --- a/lib/gitlab/cycle_analytics/code_stage.rb +++ b/lib/gitlab/cycle_analytics/code_stage.rb @@ -1,6 +1,12 @@ module Gitlab module CycleAnalytics class CodeStage < BaseStage + def initialize(*args) + super(*args) + + @description = "Time until first merge request" + end + def median @fetcher.calculate_metric(:code, Issue::Metrics.arel_table[:first_mentioned_in_commit_at], diff --git a/lib/gitlab/cycle_analytics/issue_stage.rb b/lib/gitlab/cycle_analytics/issue_stage.rb index 6793cc77976..a2ada238cd2 100644 --- a/lib/gitlab/cycle_analytics/issue_stage.rb +++ b/lib/gitlab/cycle_analytics/issue_stage.rb @@ -1,6 +1,12 @@ module Gitlab module CycleAnalytics class IssueStage < BaseStage + def initialize(*args) + super(*args) + + @description = "Time before an issue gets scheduled" + end + def median @fetcher.calculate_metric(:issue, Issue.arel_table[:created_at], diff --git a/lib/gitlab/cycle_analytics/plan_stage.rb b/lib/gitlab/cycle_analytics/plan_stage.rb index 772237087c0..c836068c4ef 100644 --- a/lib/gitlab/cycle_analytics/plan_stage.rb +++ b/lib/gitlab/cycle_analytics/plan_stage.rb @@ -1,6 +1,12 @@ module Gitlab module CycleAnalytics class PlanStage < BaseStage + def initialize(*args) + super(*args) + + @description = "Time before an issue starts implementation" + end + def median @fetcher.calculate_metric(:plan, [Issue::Metrics.arel_table[:first_associated_with_milestone_at], diff --git a/lib/gitlab/cycle_analytics/production_stage.rb b/lib/gitlab/cycle_analytics/production_stage.rb index 2fb087a8cac..d46d37e1acc 100644 --- a/lib/gitlab/cycle_analytics/production_stage.rb +++ b/lib/gitlab/cycle_analytics/production_stage.rb @@ -1,6 +1,12 @@ module Gitlab module CycleAnalytics class ProductionStage < BaseStage + def initialize(*args) + super(*args) + + @description = "From issue creation until deploy to production" + end + def median @fetcher.calculate_metric(:production, Issue.arel_table[:created_at], diff --git a/lib/gitlab/cycle_analytics/review_stage.rb b/lib/gitlab/cycle_analytics/review_stage.rb index ec9f07319e8..4159ba5d70d 100644 --- a/lib/gitlab/cycle_analytics/review_stage.rb +++ b/lib/gitlab/cycle_analytics/review_stage.rb @@ -1,6 +1,12 @@ module Gitlab module CycleAnalytics class ReviewStage < BaseStage + def initialize(*args) + super(*args) + + @description = "Time between merge request creation and merge/close" + end + def median @fetcher.calculate_metric(:review, MergeRequest.arel_table[:created_at], diff --git a/lib/gitlab/cycle_analytics/staging_stage.rb b/lib/gitlab/cycle_analytics/staging_stage.rb index 9c67a2aa6fe..cb4398f15ac 100644 --- a/lib/gitlab/cycle_analytics/staging_stage.rb +++ b/lib/gitlab/cycle_analytics/staging_stage.rb @@ -1,6 +1,12 @@ module Gitlab module CycleAnalytics class StagingStage < BaseStage + def initialize(*args) + super(*args) + + @description = "From merge request merge until deploy to production" + end + def median @fetcher.calculate_metric(:staging, MergeRequest::Metrics.arel_table[:merged_at], diff --git a/lib/gitlab/cycle_analytics/test_stage.rb b/lib/gitlab/cycle_analytics/test_stage.rb index 6bedfdbba61..3ab93bebd87 100644 --- a/lib/gitlab/cycle_analytics/test_stage.rb +++ b/lib/gitlab/cycle_analytics/test_stage.rb @@ -1,6 +1,12 @@ module Gitlab module CycleAnalytics class TestStage < BaseStage + def initialize(*args) + super(*args) + + @description = "Total test time for all commits/merges" + end + def median @fetcher.calculate_metric(:test, MergeRequest::Metrics.arel_table[:latest_build_started_at], From dc6ea14b0d11a5e73e81c95ef723f0c1af69215b Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 22 Nov 2016 10:33:19 +0100 Subject: [PATCH 03/25] fixed stage entity and added missing stage specs --- app/serializers/analytics_stage_entity.rb | 4 +-- lib/gitlab/cycle_analytics/base_stage.rb | 6 +++- .../gitlab/cycle_analytics/code_stage_spec.rb | 8 +++++ .../cycle_analytics/issue_stage_spec.rb | 8 +++++ .../gitlab/cycle_analytics/plan_stage_spec.rb | 8 +++++ .../cycle_analytics/production_stage_spec.rb | 8 +++++ .../cycle_analytics/review_stage_spec.rb | 8 +++++ .../cycle_analytics/shared_stage_spec.rb | 30 +++++++++++++++++++ .../cycle_analytics/staging_stage_spec.rb | 8 +++++ .../gitlab/cycle_analytics/test_stage_spec.rb | 8 +++++ 10 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 spec/lib/gitlab/cycle_analytics/code_stage_spec.rb create mode 100644 spec/lib/gitlab/cycle_analytics/issue_stage_spec.rb create mode 100644 spec/lib/gitlab/cycle_analytics/plan_stage_spec.rb create mode 100644 spec/lib/gitlab/cycle_analytics/production_stage_spec.rb create mode 100644 spec/lib/gitlab/cycle_analytics/review_stage_spec.rb create mode 100644 spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb create mode 100644 spec/lib/gitlab/cycle_analytics/staging_stage_spec.rb create mode 100644 spec/lib/gitlab/cycle_analytics/test_stage_spec.rb diff --git a/app/serializers/analytics_stage_entity.rb b/app/serializers/analytics_stage_entity.rb index 72a587c8c1d..d454a4937f4 100644 --- a/app/serializers/analytics_stage_entity.rb +++ b/app/serializers/analytics_stage_entity.rb @@ -2,11 +2,11 @@ class AnalyticsStageEntity < Grape::Entity include EntityDateHelper expose :stage, as: :title do |object| - object[:stage].to_s.capitalize + object.stage.to_s.capitalize end expose :description expose :median, as: :value do |stage| - stage[:median] && !stage[:median].zero? ? distance_of_time_in_words(stage[:median]) : nil + stage.median && !stage.median.zero? ? distance_of_time_in_words(stage.median) : nil end end diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb index 49d1e6304a9..27971bfc093 100644 --- a/lib/gitlab/cycle_analytics/base_stage.rb +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -3,7 +3,7 @@ module Gitlab class BaseStage attr_reader :stage, :description - def initialize(project:, options:, stage: stage) + def initialize(project:, options:, stage:) @project = project @options = options @fetcher = Gitlab::CycleAnalytics::MetricsFetcher.new(project: project, @@ -16,6 +16,10 @@ module Gitlab event_class.new(fetcher: @fetcher, stage: @stage).fetch end + def median_data + AnalyticsStageSerializer.new.represent(self).as_json + end + private def event_class diff --git a/spec/lib/gitlab/cycle_analytics/code_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/code_stage_spec.rb new file mode 100644 index 00000000000..e8fc67acf05 --- /dev/null +++ b/spec/lib/gitlab/cycle_analytics/code_stage_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' +require 'lib/gitlab/cycle_analytics/shared_stage_spec' + +describe Gitlab::CycleAnalytics::CodeStage do + let(:stage_name) { :code } + + it_behaves_like 'base stage' +end diff --git a/spec/lib/gitlab/cycle_analytics/issue_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/issue_stage_spec.rb new file mode 100644 index 00000000000..3127f01989d --- /dev/null +++ b/spec/lib/gitlab/cycle_analytics/issue_stage_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' +require 'lib/gitlab/cycle_analytics/shared_stage_spec' + +describe Gitlab::CycleAnalytics::IssueStage do + let(:stage_name) { :issue } + + it_behaves_like 'base stage' +end diff --git a/spec/lib/gitlab/cycle_analytics/plan_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/plan_stage_spec.rb new file mode 100644 index 00000000000..4c715921ad6 --- /dev/null +++ b/spec/lib/gitlab/cycle_analytics/plan_stage_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' +require 'lib/gitlab/cycle_analytics/shared_stage_spec' + +describe Gitlab::CycleAnalytics::PlanStage do + let(:stage_name) { :plan } + + it_behaves_like 'base stage' +end diff --git a/spec/lib/gitlab/cycle_analytics/production_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/production_stage_spec.rb new file mode 100644 index 00000000000..916684b81eb --- /dev/null +++ b/spec/lib/gitlab/cycle_analytics/production_stage_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' +require 'lib/gitlab/cycle_analytics/shared_stage_spec' + +describe Gitlab::CycleAnalytics::ProductionStage do + let(:stage_name) { :production } + + it_behaves_like 'base stage' +end diff --git a/spec/lib/gitlab/cycle_analytics/review_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/review_stage_spec.rb new file mode 100644 index 00000000000..1412c8dfa08 --- /dev/null +++ b/spec/lib/gitlab/cycle_analytics/review_stage_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' +require 'lib/gitlab/cycle_analytics/shared_stage_spec' + +describe Gitlab::CycleAnalytics::ReviewStage do + let(:stage_name) { :review } + + it_behaves_like 'base stage' +end diff --git a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb new file mode 100644 index 00000000000..dd1ef4fc129 --- /dev/null +++ b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +shared_examples 'base stage' do + let(:stage) { described_class.new(project: double, options: {}, stage: stage_name) } + + before do + allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:calculate_metric).and_return(1.12) + allow_any_instance_of(Gitlab::CycleAnalytics::BaseEvent).to receive(:event_result).and_return({}) + end + + it 'has the median data value' do + expect(stage.median_data[:value]).not_to be_nil + end + + it 'has the median data stage' do + expect(stage.median_data[:title]).not_to be_nil + end + + it 'has the median data description' do + expect(stage.median_data[:description]).not_to be_nil + end + + it 'has the stage' do + expect(stage.stage).to eq(stage_name) + end + + it 'has the events' do + expect(stage.events).not_to be_nil + end +end diff --git a/spec/lib/gitlab/cycle_analytics/staging_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/staging_stage_spec.rb new file mode 100644 index 00000000000..8154b3ac701 --- /dev/null +++ b/spec/lib/gitlab/cycle_analytics/staging_stage_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' +require 'lib/gitlab/cycle_analytics/shared_stage_spec' + +describe Gitlab::CycleAnalytics::StagingStage do + let(:stage_name) { :staging } + + it_behaves_like 'base stage' +end diff --git a/spec/lib/gitlab/cycle_analytics/test_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/test_stage_spec.rb new file mode 100644 index 00000000000..eacde22cd56 --- /dev/null +++ b/spec/lib/gitlab/cycle_analytics/test_stage_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' +require 'lib/gitlab/cycle_analytics/shared_stage_spec' + +describe Gitlab::CycleAnalytics::TestStage do + let(:stage_name) { :test } + + it_behaves_like 'base stage' +end From fc6f8f20562ad761c034ffff076d329a3e9e8f4d Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 22 Nov 2016 11:46:02 +0100 Subject: [PATCH 04/25] added new summary serializers and refactor all of the summary stuff into separate logical classes --- .../projects/cycle_analytics_controller.rb | 50 ++----------------- app/models/cycle_analytics.rb | 2 +- app/models/cycle_analytics/summary.rb | 43 ---------------- app/serializers/analytics_summary_entity.rb | 7 +++ .../analytics_summary_serializer.rb | 3 ++ lib/gitlab/cycle_analytics/summary.rb | 36 +++++++++++++ lib/gitlab/cycle_analytics/summary/base.rb | 20 ++++++++ lib/gitlab/cycle_analytics/summary/commit.rb | 36 +++++++++++++ lib/gitlab/cycle_analytics/summary/deploy.rb | 11 ++++ lib/gitlab/cycle_analytics/summary/issue.rb | 15 ++++++ .../analytics_stage_serializer_spec.rb | 24 +++++++++ .../analytics_summary_serializer_spec.rb | 24 +++++++++ 12 files changed, 182 insertions(+), 89 deletions(-) create mode 100644 app/serializers/analytics_summary_entity.rb create mode 100644 app/serializers/analytics_summary_serializer.rb create mode 100644 lib/gitlab/cycle_analytics/summary.rb create mode 100644 lib/gitlab/cycle_analytics/summary/base.rb create mode 100644 lib/gitlab/cycle_analytics/summary/commit.rb create mode 100644 lib/gitlab/cycle_analytics/summary/deploy.rb create mode 100644 lib/gitlab/cycle_analytics/summary/issue.rb create mode 100644 spec/serializers/analytics_stage_serializer_spec.rb create mode 100644 spec/serializers/analytics_summary_serializer_spec.rb diff --git a/app/controllers/projects/cycle_analytics_controller.rb b/app/controllers/projects/cycle_analytics_controller.rb index ac639ef015b..73fe3c9c4c9 100644 --- a/app/controllers/projects/cycle_analytics_controller.rb +++ b/app/controllers/projects/cycle_analytics_controller.rb @@ -8,10 +8,6 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController def show @cycle_analytics = ::CycleAnalytics.new(@project, current_user, from: start_date(cycle_analytics_params)) - stats_values, cycle_analytics_json = generate_cycle_analytics_data - - @cycle_analytics_no_data = stats_values.blank? - respond_to do |format| format.html format.json { render json: cycle_analytics_json } @@ -26,47 +22,11 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController { start_date: params[:cycle_analytics][:start_date] } end - def generate_cycle_analytics_data - stats_values = [] - - cycle_analytics_view_data = [[:issue, "Issue", "Related Issues", "Time before an issue gets scheduled"], - [:plan, "Plan", "Related Commits", "Time before an issue starts implementation"], - [:code, "Code", "Related Merge Requests", "Time spent coding"], - [:test, "Test", "Relative Builds Trigger by Commits", "The time taken to build and test the application"], - [:review, "Review", "Relative Merged Requests", "The time taken to review the code"], - [:staging, "Staging", "Relative Deployed Builds", "The time taken in staging"], - [:production, "Production", "Related Issues", "The total time taken from idea to production"]] - - stats = cycle_analytics_view_data.reduce([]) do |stats, (stage_method, stage_text, stage_legend, stage_description)| - value = @cycle_analytics.send(stage_method).presence - - stats_values << value.abs if value - - stats << { - title: stage_text, - description: stage_description, - legend: stage_legend, - value: value && !value.zero? ? distance_of_time_in_words(value) : nil - } - - stats - end - - issues = @cycle_analytics.summary.new_issues - commits = @cycle_analytics.summary.commits - deploys = @cycle_analytics.summary.deploys - - summary = [ - { title: "New Issue".pluralize(issues), value: issues }, - { title: "Commit".pluralize(commits), value: commits }, - { title: "Deploy".pluralize(deploys), value: deploys } - ] - - cycle_analytics_hash = { summary: summary, - stats: stats, - permissions: @cycle_analytics.permissions(user: current_user) + def cycle_analytics_json + { + summary: @cycle_analytics.summary, + stats: nil, # TODO + permissions: @cycle_analytics.permissions(user: current_user)# TODO } - - [stats_values, cycle_analytics_hash] end end diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index 5e33273c9ba..e0f9690f1f4 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -7,7 +7,7 @@ class CycleAnalytics end def summary - @summary ||= Summary.new(@project, from: @options[:from]) + @summary ||= Gitlab::CycleAnalytics::Summary.new(@project, from: @options[:from]).data end def method_missing(method_sym, *arguments, &block) diff --git a/app/models/cycle_analytics/summary.rb b/app/models/cycle_analytics/summary.rb index c9910d8cd09..e69de29bb2d 100644 --- a/app/models/cycle_analytics/summary.rb +++ b/app/models/cycle_analytics/summary.rb @@ -1,43 +0,0 @@ -class CycleAnalytics - class Summary - def initialize(project, current_user, from:) - @project = project - @current_user = current_user - @from = from - end - - def new_issues - IssuesFinder.new(@current_user, project_id: @project.id).execute.created_after(@from).count - end - - def commits - ref = @project.default_branch.presence - count_commits_for(ref) - end - - def deploys - @project.deployments.where("created_at > ?", @from).count - end - - private - - # Don't use the `Gitlab::Git::Repository#log` method, because it enforces - # a limit. Since we need a commit count, we _can't_ enforce a limit, so - # the easiest way forward is to replicate the relevant portions of the - # `log` function here. - def count_commits_for(ref) - return unless ref - - repository = @project.repository.raw_repository - sha = @project.repository.commit(ref).sha - - cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{repository.path} log) - cmd << '--format=%H' - cmd << "--after=#{@from.iso8601}" - cmd << sha - - raw_output = IO.popen(cmd) { |io| io.read } - raw_output.lines.count - end - end -end diff --git a/app/serializers/analytics_summary_entity.rb b/app/serializers/analytics_summary_entity.rb new file mode 100644 index 00000000000..91803ec07f5 --- /dev/null +++ b/app/serializers/analytics_summary_entity.rb @@ -0,0 +1,7 @@ +class AnalyticsSummaryEntity < Grape::Entity + expose :value, safe: true + + expose :title do |object| + object.title.pluralize(object.value) + end +end diff --git a/app/serializers/analytics_summary_serializer.rb b/app/serializers/analytics_summary_serializer.rb new file mode 100644 index 00000000000..c87a24aa47c --- /dev/null +++ b/app/serializers/analytics_summary_serializer.rb @@ -0,0 +1,3 @@ +class AnalyticsSummarySerializer < BaseSerializer + entity AnalyticsSummaryEntity +end diff --git a/lib/gitlab/cycle_analytics/summary.rb b/lib/gitlab/cycle_analytics/summary.rb new file mode 100644 index 00000000000..7d172855a94 --- /dev/null +++ b/lib/gitlab/cycle_analytics/summary.rb @@ -0,0 +1,36 @@ +module Gitlab + module CycleAnalytics + module Summary + extend self + + def initialize(project, from:) + @project = project + @from = from + end + + def data + [serialize(issue), + serialize(commit), + serialize(deploy)] + end + + private + + def serialize(summary_object) + AnalyticsSummarySerializer.new.represent(summary_object).as_json + end + + def issue + Summary::Issue.new(project: @project, from: @from) + end + + def deploy + Summary::Deploy.new(project: @project, from: @from) + end + + def commit + Summary::Commit.new(project: @project, from: @from) + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/summary/base.rb b/lib/gitlab/cycle_analytics/summary/base.rb new file mode 100644 index 00000000000..1bc4ff00b99 --- /dev/null +++ b/lib/gitlab/cycle_analytics/summary/base.rb @@ -0,0 +1,20 @@ +module Gitlab + module CycleAnalytics + module Summary + class Base + def initialize(project:, from:) + @project = project + @from = from + end + + def title + self.name + end + + def value + raise NotImplementedError.new("Expected #{self.name} to implement value") + end + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/summary/commit.rb b/lib/gitlab/cycle_analytics/summary/commit.rb new file mode 100644 index 00000000000..ec3c067c0be --- /dev/null +++ b/lib/gitlab/cycle_analytics/summary/commit.rb @@ -0,0 +1,36 @@ +module Gitlab + module CycleAnalytics + module Summary + class Commit < Base + def value + @value ||= count_commits + end + + private + + # Don't use the `Gitlab::Git::Repository#log` method, because it enforces + # a limit. Since we need a commit count, we _can't_ enforce a limit, so + # the easiest way forward is to replicate the relevant portions of the + # `log` function here. + def count_commits + return unless ref + + repository = @project.repository.raw_repository + sha = @project.repository.commit(ref).sha + + cmd = %W(git --git-dir=#{repository.path} log) + cmd << '--format=%H' + cmd << "--after=#{@from.iso8601}" + cmd << sha + + raw_output = IO.popen(cmd) { |io| io.read } + raw_output.lines.count + end + + def ref + @ref ||= @project.default_branch.presence + end + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/summary/deploy.rb b/lib/gitlab/cycle_analytics/summary/deploy.rb new file mode 100644 index 00000000000..06032e9200e --- /dev/null +++ b/lib/gitlab/cycle_analytics/summary/deploy.rb @@ -0,0 +1,11 @@ +module Gitlab + module CycleAnalytics + module Summary + class Deploy < Base + def value + @value ||= @project.deployments.where("created_at > ?", @from).count + end + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/summary/issue.rb b/lib/gitlab/cycle_analytics/summary/issue.rb new file mode 100644 index 00000000000..7d62164aae3 --- /dev/null +++ b/lib/gitlab/cycle_analytics/summary/issue.rb @@ -0,0 +1,15 @@ +module Gitlab + module CycleAnalytics + module Summary + class Issue < Base + def title + 'New Issue' + end + + def value + @value ||= @project.issues.created_after(@from).count + end + end + end + end +end diff --git a/spec/serializers/analytics_stage_serializer_spec.rb b/spec/serializers/analytics_stage_serializer_spec.rb new file mode 100644 index 00000000000..0f2d534e714 --- /dev/null +++ b/spec/serializers/analytics_stage_serializer_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe AnalyticsStageSerializer do + let(:serializer) do + described_class + .new.represent(resource) + end + + let(:json) { serializer.as_json } + let(:resource) { Gitlab::CycleAnalytics::CodeStage.new(project: double, options: {}, stage: :code) } + + before do + allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:calculate_metric).and_return(1.12) + allow_any_instance_of(Gitlab::CycleAnalytics::BaseEvent).to receive(:event_result).and_return({}) + end + + it 'it generates payload for single object' do + expect(json).to be_an_instance_of Hash + end + + it 'contains important elements of AnalyticsStage' do + expect(json).to include(:title, :description, :value) + end +end diff --git a/spec/serializers/analytics_summary_serializer_spec.rb b/spec/serializers/analytics_summary_serializer_spec.rb new file mode 100644 index 00000000000..e08e3f88710 --- /dev/null +++ b/spec/serializers/analytics_summary_serializer_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe AnalyticsSummarySerializer do + let(:serializer) do + described_class + .new.represent(resource) + end + + let(:json) { serializer.as_json } + let(:project) { create(:empty_project) } + let(:resource) { Gitlab::CycleAnalytics::Summary::Issue.new(project: double, from: 1.day.ago) } + + before do + allow_any_instance_of(Gitlab::CycleAnalytics::Summary::Issue).to receive(:value).and_return(1.12) + end + + it 'it generates payload for single object' do + expect(json).to be_an_instance_of Hash + end + + it 'contains important elements of AnalyticsStage' do + expect(json).to include(:title, :value) + end +end From 8639ea1b0315045c0e4a5ad8d6419903507850c3 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 22 Nov 2016 12:01:21 +0100 Subject: [PATCH 05/25] fix bad merge --- app/models/cycle_analytics.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index e0f9690f1f4..9681d34f2d1 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -12,15 +12,17 @@ class CycleAnalytics def method_missing(method_sym, *arguments, &block) classify_stage(method_sym).new(project: @project, options: @options, stage: method_sym) + end + def permissions(user:) Gitlab::CycleAnalytics::Permissions.get(user: user, project: @project) end def issue @fetcher.calculate_metric(:issue, - Issue.arel_table[:created_at], - [Issue::Metrics.arel_table[:first_associated_with_milestone_at], - Issue::Metrics.arel_table[:first_added_to_board_at]]) + Issue.arel_table[:created_at], + [Issue::Metrics.arel_table[:first_associated_with_milestone_at], + Issue::Metrics.arel_table[:first_added_to_board_at]]) end def classify_stage(method_sym) From 02e1e4819234662faddd7d8eb5c54d9bfdf9e7e6 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 22 Nov 2016 14:29:25 +0100 Subject: [PATCH 06/25] more refactoring and fixing old specs --- .../concerns/cycle_analytics_params.rb | 4 +++ .../cycle_analytics/events_controller.rb | 4 --- .../projects/cycle_analytics_controller.rb | 8 +++--- app/models/cycle_analytics.rb | 26 ++++++++++++------- lib/gitlab/cycle_analytics/summary.rb | 18 +++---------- spec/models/cycle_analytics/code_spec.rb | 18 ++++++------- spec/models/cycle_analytics/issue_spec.rb | 2 +- spec/models/cycle_analytics/plan_spec.rb | 2 +- .../models/cycle_analytics/production_spec.rb | 2 +- spec/models/cycle_analytics/review_spec.rb | 2 +- spec/models/cycle_analytics/staging_spec.rb | 2 +- spec/models/cycle_analytics/summary_spec.rb | 2 +- spec/models/cycle_analytics/test_spec.rb | 2 +- .../test_generation.rb | 7 ++++- 14 files changed, 50 insertions(+), 49 deletions(-) diff --git a/app/controllers/concerns/cycle_analytics_params.rb b/app/controllers/concerns/cycle_analytics_params.rb index 2aaf8f2b451..91456561a17 100644 --- a/app/controllers/concerns/cycle_analytics_params.rb +++ b/app/controllers/concerns/cycle_analytics_params.rb @@ -1,6 +1,10 @@ module CycleAnalyticsParams extend ActiveSupport::Concern + def options + @options ||= { from: start_date(events_params), current_user: current_user } + end + def start_date(params) params[:start_date] == '30' ? 30.days.ago : 90.days.ago end diff --git a/app/controllers/projects/cycle_analytics/events_controller.rb b/app/controllers/projects/cycle_analytics/events_controller.rb index 13b3eec761f..5e9524b15db 100644 --- a/app/controllers/projects/cycle_analytics/events_controller.rb +++ b/app/controllers/projects/cycle_analytics/events_controller.rb @@ -51,10 +51,6 @@ module Projects @events ||= Gitlab::CycleAnalytics::Events.new(project: project, options: options) end - def options - @options ||= { from: start_date(events_params), current_user: current_user } - end - def events_params return {} unless params[:events].present? diff --git a/app/controllers/projects/cycle_analytics_controller.rb b/app/controllers/projects/cycle_analytics_controller.rb index 73fe3c9c4c9..93dbe2819e7 100644 --- a/app/controllers/projects/cycle_analytics_controller.rb +++ b/app/controllers/projects/cycle_analytics_controller.rb @@ -6,7 +6,9 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController before_action :authorize_read_cycle_analytics! def show - @cycle_analytics = ::CycleAnalytics.new(@project, current_user, from: start_date(cycle_analytics_params)) + @cycle_analytics = ::CycleAnalytics.new(@project, options: options) + + @cycle_analytics_no_data = @cycle_analytics.no_stats? respond_to do |format| format.html @@ -25,8 +27,8 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController def cycle_analytics_json { summary: @cycle_analytics.summary, - stats: nil, # TODO - permissions: @cycle_analytics.permissions(user: current_user)# TODO + stats: @cycle_analytics.stats, + permissions: @cycle_analytics.permissions(user: current_user) } end end diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index 9681d34f2d1..00e9f7c7d5c 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -1,7 +1,7 @@ class CycleAnalytics STAGES = %i[issue plan code test review staging production].freeze - def initialize(project, from:) + def initialize(project, options:) @project = project @options = options end @@ -10,22 +10,28 @@ class CycleAnalytics @summary ||= Gitlab::CycleAnalytics::Summary.new(@project, from: @options[:from]).data end - def method_missing(method_sym, *arguments, &block) - classify_stage(method_sym).new(project: @project, options: @options, stage: method_sym) + def stats + @stats ||= stats_per_stage + end + + def no_stats? + stats.map(&:value).compact.empty? end def permissions(user:) Gitlab::CycleAnalytics::Permissions.get(user: user, project: @project) end - def issue - @fetcher.calculate_metric(:issue, - Issue.arel_table[:created_at], - [Issue::Metrics.arel_table[:first_associated_with_milestone_at], - Issue::Metrics.arel_table[:first_added_to_board_at]]) + private + + def stats_per_stage + STAGES.map do |stage_name| + classify_stage(method_sym).new(project: @project, options: @options, stage: stage_name).median_data + end end - def classify_stage(method_sym) - "Gitlab::CycleAnalytics::#{method_sym.to_s.capitalize}Stage".constantize + def classify_stage(stage_name) + "Gitlab::CycleAnalytics::#{stage_name.to_s.capitalize}Stage".constantize end + end diff --git a/lib/gitlab/cycle_analytics/summary.rb b/lib/gitlab/cycle_analytics/summary.rb index 7d172855a94..5f0103c9d5a 100644 --- a/lib/gitlab/cycle_analytics/summary.rb +++ b/lib/gitlab/cycle_analytics/summary.rb @@ -9,9 +9,9 @@ module Gitlab end def data - [serialize(issue), - serialize(commit), - serialize(deploy)] + [serialize(Summary::Issue.new(project: @project, from: @from)), + serialize(Summary::Commit.new(project: @project, from: @from)), + serialize(Summary::Deploy.new(project: @project, from: @from))] end private @@ -19,18 +19,6 @@ module Gitlab def serialize(summary_object) AnalyticsSummarySerializer.new.represent(summary_object).as_json end - - def issue - Summary::Issue.new(project: @project, from: @from) - end - - def deploy - Summary::Deploy.new(project: @project, from: @from) - end - - def commit - Summary::Commit.new(project: @project, from: @from) - end end end end diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb index 7771785ead3..4838b57e353 100644 --- a/spec/models/cycle_analytics/code_spec.rb +++ b/spec/models/cycle_analytics/code_spec.rb @@ -6,7 +6,7 @@ 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, user, from: from_date) } + subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } context 'with deployment' do generate_cycle_analytics_spec( @@ -16,10 +16,10 @@ describe 'CycleAnalytics#code', feature: true do -> (context, data) do context.create_commit_referencing_issue(data[:issue]) end]], - end_time_conditions: [["merge request that closes issue is created", - -> (context, data) do - context.create_merge_request_closing_issue(data[:issue]) - end]], + end_time_conditions: [["merge request that closes issue is created", + -> (context, data) do + context.create_merge_request_closing_issue(data[:issue]) + end]], post_fn: -> (context, data) do context.merge_merge_requests_closing_issue(data[:issue]) context.deploy_master @@ -50,10 +50,10 @@ describe 'CycleAnalytics#code', feature: true do -> (context, data) do context.create_commit_referencing_issue(data[:issue]) end]], - end_time_conditions: [["merge request that closes issue is created", - -> (context, data) do - context.create_merge_request_closing_issue(data[:issue]) - end]], + end_time_conditions: [["merge request that closes issue is created", + -> (context, data) do + context.create_merge_request_closing_issue(data[:issue]) + end]], post_fn: -> (context, data) do context.merge_merge_requests_closing_issue(data[:issue]) end) diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb index 5ed3d37f2fb..ce6e99bbec9 100644 --- a/spec/models/cycle_analytics/issue_spec.rb +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -6,7 +6,7 @@ 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, user, from: from_date) } + subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :issue, diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb index baf3e3241a1..bd5a6a77b7a 100644 --- a/spec/models/cycle_analytics/plan_spec.rb +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -6,7 +6,7 @@ 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, user, from: from_date) } + subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :plan, diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb index 21b9c6e7150..653e203b491 100644 --- a/spec/models/cycle_analytics/production_spec.rb +++ b/spec/models/cycle_analytics/production_spec.rb @@ -6,7 +6,7 @@ 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, user, from: from_date) } + subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :production, diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb index 158621d59a4..219cd4c0212 100644 --- a/spec/models/cycle_analytics/review_spec.rb +++ b/spec/models/cycle_analytics/review_spec.rb @@ -6,7 +6,7 @@ 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, user, from: from_date) } + subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :review, diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb index dad653964b7..8dffb6b8fe1 100644 --- a/spec/models/cycle_analytics/staging_spec.rb +++ b/spec/models/cycle_analytics/staging_spec.rb @@ -6,7 +6,7 @@ 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, user, from: from_date) } + subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :staging, diff --git a/spec/models/cycle_analytics/summary_spec.rb b/spec/models/cycle_analytics/summary_spec.rb index 725bc68b25f..1a54c57a278 100644 --- a/spec/models/cycle_analytics/summary_spec.rb +++ b/spec/models/cycle_analytics/summary_spec.rb @@ -4,7 +4,7 @@ describe CycleAnalytics::Summary, models: true do let(:project) { create(:project) } let(:from) { Time.now } let(:user) { create(:user, :admin) } - subject { described_class.new(project, user, from: from) } + subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } describe "#new_issues" do it "finds the number of issues created after the 'from date'" do diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb index 2313724e8f3..ac1304beca8 100644 --- a/spec/models/cycle_analytics/test_spec.rb +++ b/spec/models/cycle_analytics/test_spec.rb @@ -6,7 +6,7 @@ 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, user, from: from_date) } + subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } generate_cycle_analytics_spec( phase: :test, diff --git a/spec/support/cycle_analytics_helpers/test_generation.rb b/spec/support/cycle_analytics_helpers/test_generation.rb index 8e19a6c92e2..ab624161616 100644 --- a/spec/support/cycle_analytics_helpers/test_generation.rb +++ b/spec/support/cycle_analytics_helpers/test_generation.rb @@ -1,8 +1,13 @@ +class CycleAnalyticsTest < CycleAnalytics + def method_missing(method_sym, *arguments, &block) + classify_stage(method_sym).new(project: @project, options: @options, stage: method_sym).median + end +end + # 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. From a67311cb4c9f54af43d300fde5240f9a370193d1 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 23 Nov 2016 11:28:28 +0100 Subject: [PATCH 07/25] Fix other spec failures --- .../concerns/cycle_analytics_params.rb | 4 +- .../cycle_analytics/events_controller.rb | 26 ++-- .../projects/cycle_analytics_controller.rb | 4 +- app/models/cycle_analytics.rb | 11 +- lib/gitlab/cycle_analytics/base_event.rb | 3 +- lib/gitlab/cycle_analytics/base_stage.rb | 2 +- lib/gitlab/cycle_analytics/events.rb | 38 ----- .../{summary.rb => stage_summary.rb} | 4 +- lib/gitlab/cycle_analytics/summary/base.rb | 2 +- .../gitlab/cycle_analytics/code_event_spec.rb | 2 + .../lib/gitlab/cycle_analytics/events_spec.rb | 137 ++++++++++-------- .../cycle_analytics/issue_event_spec.rb | 2 + .../gitlab/cycle_analytics/plan_event_spec.rb | 2 + .../cycle_analytics/production_event_spec.rb | 2 + .../cycle_analytics/review_event_spec.rb | 2 + .../cycle_analytics/shared_event_spec.rb | 8 +- .../cycle_analytics/staging_event_spec.rb | 2 + .../gitlab/cycle_analytics/test_event_spec.rb | 2 + spec/models/cycle_analytics/summary_spec.rb | 2 +- .../analytics_summary_serializer_spec.rb | 4 +- 20 files changed, 128 insertions(+), 131 deletions(-) delete mode 100644 lib/gitlab/cycle_analytics/events.rb rename lib/gitlab/cycle_analytics/{summary.rb => stage_summary.rb} (93%) diff --git a/app/controllers/concerns/cycle_analytics_params.rb b/app/controllers/concerns/cycle_analytics_params.rb index 91456561a17..52e06f4945a 100644 --- a/app/controllers/concerns/cycle_analytics_params.rb +++ b/app/controllers/concerns/cycle_analytics_params.rb @@ -1,8 +1,8 @@ module CycleAnalyticsParams extend ActiveSupport::Concern - def options - @options ||= { from: start_date(events_params), current_user: current_user } + def options(params) + @options ||= { from: start_date(params), current_user: current_user } end def start_date(params) diff --git a/app/controllers/projects/cycle_analytics/events_controller.rb b/app/controllers/projects/cycle_analytics/events_controller.rb index 5e9524b15db..e571e1dfce2 100644 --- a/app/controllers/projects/cycle_analytics/events_controller.rb +++ b/app/controllers/projects/cycle_analytics/events_controller.rb @@ -9,46 +9,46 @@ module Projects before_action :authorize_read_merge_request!, only: [:code, :review] def issue - render_events(events.issue_events) + render_events(cycle_analytics.events_for(:issue)) end def plan - render_events(events.plan_events) + render_events(cycle_analytics.events_for(:plan)) end def code - render_events(events.code_events) + render_events(cycle_analytics.events_for(:code)) end def test - options[:branch] = events_params[:branch_name] + options(events_params)[:branch] = events_params[:branch_name] - render_events(events.test_events) + render_events(cycle_analytics.events_for(:test)) end def review - render_events(events.review_events) + render_events(cycle_analytics.events_for(:review)) end def staging - render_events(events.staging_events) + render_events(cycle_analytics.events_for(:staging)) end def production - render_events(events.production_events) + render_events(cycle_analytics.events_for(:production)) end private - - def render_events(events_list) + + def render_events(events) respond_to do |format| format.html - format.json { render json: { events: events_list } } + format.json { render json: { events: events } } end end - def events - @events ||= Gitlab::CycleAnalytics::Events.new(project: project, options: options) + def cycle_analytics + @cycle_analytics ||= ::CycleAnalytics.new(project, options: options(events_params)) end def events_params diff --git a/app/controllers/projects/cycle_analytics_controller.rb b/app/controllers/projects/cycle_analytics_controller.rb index 93dbe2819e7..cf53d0a1919 100644 --- a/app/controllers/projects/cycle_analytics_controller.rb +++ b/app/controllers/projects/cycle_analytics_controller.rb @@ -6,7 +6,7 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController before_action :authorize_read_cycle_analytics! def show - @cycle_analytics = ::CycleAnalytics.new(@project, options: options) + @cycle_analytics = ::CycleAnalytics.new(@project, options: options(cycle_analytics_params)) @cycle_analytics_no_data = @cycle_analytics.no_stats? @@ -21,7 +21,7 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController def cycle_analytics_params return {} unless params[:cycle_analytics].present? - { start_date: params[:cycle_analytics][:start_date] } + params[:cycle_analytics].slice(:start_date) end def cycle_analytics_json diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index 00e9f7c7d5c..5bcc6fa1954 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -7,7 +7,7 @@ class CycleAnalytics end def summary - @summary ||= Gitlab::CycleAnalytics::Summary.new(@project, from: @options[:from]).data + @summary ||= ::Gitlab::CycleAnalytics::StageSummary.new(@project, from: @options[:from]).data end def stats @@ -15,23 +15,26 @@ class CycleAnalytics end def no_stats? - stats.map(&:value).compact.empty? + stats.map { |hash| hash[:value] }.compact.empty? end def permissions(user:) Gitlab::CycleAnalytics::Permissions.get(user: user, project: @project) end + def events_for(stage) + classify_stage(stage).new(project: @project, options: @options, stage: stage).events + end + private def stats_per_stage STAGES.map do |stage_name| - classify_stage(method_sym).new(project: @project, options: @options, stage: stage_name).median_data + classify_stage(stage_name).new(project: @project, options: @options, stage: stage_name).median_data end end def classify_stage(stage_name) "Gitlab::CycleAnalytics::#{stage_name.to_s.capitalize}Stage".constantize end - end diff --git a/lib/gitlab/cycle_analytics/base_event.rb b/lib/gitlab/cycle_analytics/base_event.rb index c87841c119a..d540cb6549c 100644 --- a/lib/gitlab/cycle_analytics/base_event.rb +++ b/lib/gitlab/cycle_analytics/base_event.rb @@ -5,10 +5,11 @@ module Gitlab attr_reader :stage, :start_time_attrs, :end_time_attrs, :projections, :query - def initialize(fetcher:, stage:) + def initialize(fetcher:, stage:, options:) @query = EventsQuery.new(fetcher: fetcher) @project = fetcher.project @stage = stage + @options = options end def fetch diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb index 27971bfc093..162ebf18c77 100644 --- a/lib/gitlab/cycle_analytics/base_stage.rb +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -13,7 +13,7 @@ module Gitlab end def events - event_class.new(fetcher: @fetcher, stage: @stage).fetch + event_class.new(fetcher: @fetcher, stage: @stage, options: @options).fetch end def median_data diff --git a/lib/gitlab/cycle_analytics/events.rb b/lib/gitlab/cycle_analytics/events.rb deleted file mode 100644 index 2d703d76cbb..00000000000 --- a/lib/gitlab/cycle_analytics/events.rb +++ /dev/null @@ -1,38 +0,0 @@ -module Gitlab - module CycleAnalytics - class Events - def initialize(project:, options:) - @project = project - @options = options - end - - def issue_events - IssueEvent.new(project: @project, options: @options).fetch - end - - def plan_events - PlanEvent.new(project: @project, options: @options).fetch - end - - def code_events - CodeEvent.new(project: @project, options: @options).fetch - end - - def test_events - TestEvent.new(project: @project, options: @options).fetch - end - - def review_events - ReviewEvent.new(project: @project, options: @options).fetch - end - - def staging_events - StagingEvent.new(project: @project, options: @options).fetch - end - - def production_events - ProductionEvent.new(project: @project, options: @options).fetch - end - end - end -end diff --git a/lib/gitlab/cycle_analytics/summary.rb b/lib/gitlab/cycle_analytics/stage_summary.rb similarity index 93% rename from lib/gitlab/cycle_analytics/summary.rb rename to lib/gitlab/cycle_analytics/stage_summary.rb index 5f0103c9d5a..dd9e4ac2813 100644 --- a/lib/gitlab/cycle_analytics/summary.rb +++ b/lib/gitlab/cycle_analytics/stage_summary.rb @@ -1,8 +1,6 @@ module Gitlab module CycleAnalytics - module Summary - extend self - + class StageSummary def initialize(project, from:) @project = project @from = from diff --git a/lib/gitlab/cycle_analytics/summary/base.rb b/lib/gitlab/cycle_analytics/summary/base.rb index 1bc4ff00b99..43fa3795e5c 100644 --- a/lib/gitlab/cycle_analytics/summary/base.rb +++ b/lib/gitlab/cycle_analytics/summary/base.rb @@ -8,7 +8,7 @@ module Gitlab end def title - self.name + self.class.name.demodulize end def value diff --git a/spec/lib/gitlab/cycle_analytics/code_event_spec.rb b/spec/lib/gitlab/cycle_analytics/code_event_spec.rb index 43f42d1bde8..0673906e678 100644 --- a/spec/lib/gitlab/cycle_analytics/code_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/code_event_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' describe Gitlab::CycleAnalytics::CodeEvent do + let(:stage_name) { :code } + it_behaves_like 'default query config' do it 'does not have the default order' do expect(event.order).not_to eq(event.start_time_attrs) diff --git a/spec/lib/gitlab/cycle_analytics/events_spec.rb b/spec/lib/gitlab/cycle_analytics/events_spec.rb index 6062e7af4f5..1258f4ed450 100644 --- a/spec/lib/gitlab/cycle_analytics/events_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/events_spec.rb @@ -1,12 +1,14 @@ require 'spec_helper' -describe Gitlab::CycleAnalytics::Events do +describe 'cycle analytics events' do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } let!(:context) { create(:issue, project: project, created_at: 2.days.ago) } - subject { described_class.new(project: project, options: { from: from_date, current_user: user }) } + let(:events) do + CycleAnalytics.new(project, options: { from: from_date, current_user: user }).events_for(stage) + end before do allow_any_instance_of(Gitlab::ReferenceExtractor).to receive(:issues).and_return([context]) @@ -15,104 +17,112 @@ describe Gitlab::CycleAnalytics::Events do end describe '#issue_events' do + let(:stage) { :issue } + it 'has the total time' do - expect(subject.issue_events.first[:total_time]).not_to be_empty + expect(events.first[:total_time]).not_to be_empty end it 'has a title' do - expect(subject.issue_events.first[:title]).to eq(context.title) + expect(events.first[:title]).to eq(context.title) end it 'has the URL' do - expect(subject.issue_events.first[:url]).not_to be_nil + expect(events.first[:url]).not_to be_nil end it 'has an iid' do - expect(subject.issue_events.first[:iid]).to eq(context.iid.to_s) + expect(events.first[:iid]).to eq(context.iid.to_s) end it 'has a created_at timestamp' do - expect(subject.issue_events.first[:created_at]).to end_with('ago') + expect(events.first[:created_at]).to end_with('ago') end it "has the author's URL" do - expect(subject.issue_events.first[:author][:web_url]).not_to be_nil + expect(events.first[:author][:web_url]).not_to be_nil end it "has the author's avatar URL" do - expect(subject.issue_events.first[:author][:avatar_url]).not_to be_nil + expect(events.first[:author][:avatar_url]).not_to be_nil end it "has the author's name" do - expect(subject.issue_events.first[:author][:name]).to eq(context.author.name) + expect(events.first[:author][:name]).to eq(context.author.name) end end describe '#plan_events' do + let(:stage) { :plan } + it 'has a title' do - expect(subject.plan_events.first[:title]).not_to be_nil + expect(events.first[:title]).not_to be_nil end it 'has a sha short ID' do - expect(subject.plan_events.first[:short_sha]).not_to be_nil + expect(events.first[:short_sha]).not_to be_nil end it 'has the URL' do - expect(subject.plan_events.first[:commit_url]).not_to be_nil + expect(events.first[:commit_url]).not_to be_nil end it 'has the total time' do - expect(subject.plan_events.first[:total_time]).not_to be_empty + expect(events.first[:total_time]).not_to be_empty end it "has the author's URL" do - expect(subject.plan_events.first[:author][:web_url]).not_to be_nil + expect(events.first[:author][:web_url]).not_to be_nil end it "has the author's avatar URL" do - expect(subject.plan_events.first[:author][:avatar_url]).not_to be_nil + expect(events.first[:author][:avatar_url]).not_to be_nil end it "has the author's name" do - expect(subject.plan_events.first[:author][:name]).not_to be_nil + expect(events.first[:author][:name]).not_to be_nil end end describe '#code_events' do + let(:stage) { :code } + before do create_commit_referencing_issue(context) end it 'has the total time' do - expect(subject.code_events.first[:total_time]).not_to be_empty + expect(events.first[:total_time]).not_to be_empty end it 'has a title' do - expect(subject.code_events.first[:title]).to eq('Awesome merge_request') + expect(events.first[:title]).to eq('Awesome merge_request') end it 'has an iid' do - expect(subject.code_events.first[:iid]).to eq(context.iid.to_s) + expect(events.first[:iid]).to eq(context.iid.to_s) end it 'has a created_at timestamp' do - expect(subject.code_events.first[:created_at]).to end_with('ago') + expect(events.first[:created_at]).to end_with('ago') end it "has the author's URL" do - expect(subject.code_events.first[:author][:web_url]).not_to be_nil + expect(events.first[:author][:web_url]).not_to be_nil end it "has the author's avatar URL" do - expect(subject.code_events.first[:author][:avatar_url]).not_to be_nil + expect(events.first[:author][:avatar_url]).not_to be_nil end it "has the author's name" do - expect(subject.code_events.first[:author][:name]).to eq(MergeRequest.first.author.name) + expect(events.first[:author][:name]).to eq(MergeRequest.first.author.name) end end describe '#test_events' do + let(:stage) { :test } + let(:merge_request) { MergeRequest.first } let!(:pipeline) do create(:ci_pipeline, @@ -130,83 +140,85 @@ describe Gitlab::CycleAnalytics::Events do end it 'has the name' do - expect(subject.test_events.first[:name]).not_to be_nil + expect(events.first[:name]).not_to be_nil end it 'has the ID' do - expect(subject.test_events.first[:id]).not_to be_nil + expect(events.first[:id]).not_to be_nil end it 'has the URL' do - expect(subject.test_events.first[:url]).not_to be_nil + expect(events.first[:url]).not_to be_nil end it 'has the branch name' do - expect(subject.test_events.first[:branch]).not_to be_nil + expect(events.first[:branch]).not_to be_nil end it 'has the branch URL' do - expect(subject.test_events.first[:branch][:url]).not_to be_nil + expect(events.first[:branch][:url]).not_to be_nil end it 'has the short SHA' do - expect(subject.test_events.first[:short_sha]).not_to be_nil + expect(events.first[:short_sha]).not_to be_nil end it 'has the commit URL' do - expect(subject.test_events.first[:commit_url]).not_to be_nil + expect(events.first[:commit_url]).not_to be_nil end it 'has the date' do - expect(subject.test_events.first[:date]).not_to be_nil + expect(events.first[:date]).not_to be_nil end it 'has the total time' do - expect(subject.test_events.first[:total_time]).not_to be_empty + expect(events.first[:total_time]).not_to be_empty end end describe '#review_events' do + let(:stage) { :review } let!(:context) { create(:issue, project: project, created_at: 2.days.ago) } it 'has the total time' do - expect(subject.review_events.first[:total_time]).not_to be_empty + expect(events.first[:total_time]).not_to be_empty end it 'has a title' do - expect(subject.review_events.first[:title]).to eq('Awesome merge_request') + expect(events.first[:title]).to eq('Awesome merge_request') end it 'has an iid' do - expect(subject.review_events.first[:iid]).to eq(context.iid.to_s) + expect(events.first[:iid]).to eq(context.iid.to_s) end it 'has the URL' do - expect(subject.review_events.first[:url]).not_to be_nil + expect(events.first[:url]).not_to be_nil end it 'has a state' do - expect(subject.review_events.first[:state]).not_to be_nil + expect(events.first[:state]).not_to be_nil end it 'has a created_at timestamp' do - expect(subject.review_events.first[:created_at]).not_to be_nil + expect(events.first[:created_at]).not_to be_nil end it "has the author's URL" do - expect(subject.review_events.first[:author][:web_url]).not_to be_nil + expect(events.first[:author][:web_url]).not_to be_nil end it "has the author's avatar URL" do - expect(subject.review_events.first[:author][:avatar_url]).not_to be_nil + expect(events.first[:author][:avatar_url]).not_to be_nil end it "has the author's name" do - expect(subject.review_events.first[:author][:name]).to eq(MergeRequest.first.author.name) + expect(events.first[:author][:name]).to eq(MergeRequest.first.author.name) end end describe '#staging_events' do + let(:stage) { :staging } let(:merge_request) { MergeRequest.first } let!(:pipeline) do create(:ci_pipeline, @@ -227,55 +239,56 @@ describe Gitlab::CycleAnalytics::Events do end it 'has the name' do - expect(subject.staging_events.first[:name]).not_to be_nil + expect(events.first[:name]).not_to be_nil end it 'has the ID' do - expect(subject.staging_events.first[:id]).not_to be_nil + expect(events.first[:id]).not_to be_nil end it 'has the URL' do - expect(subject.staging_events.first[:url]).not_to be_nil + expect(events.first[:url]).not_to be_nil end it 'has the branch name' do - expect(subject.staging_events.first[:branch]).not_to be_nil + expect(events.first[:branch]).not_to be_nil end it 'has the branch URL' do - expect(subject.staging_events.first[:branch][:url]).not_to be_nil + expect(events.first[:branch][:url]).not_to be_nil end it 'has the short SHA' do - expect(subject.staging_events.first[:short_sha]).not_to be_nil + expect(events.first[:short_sha]).not_to be_nil end it 'has the commit URL' do - expect(subject.staging_events.first[:commit_url]).not_to be_nil + expect(events.first[:commit_url]).not_to be_nil end it 'has the date' do - expect(subject.staging_events.first[:date]).not_to be_nil + expect(events.first[:date]).not_to be_nil end it 'has the total time' do - expect(subject.staging_events.first[:total_time]).not_to be_empty + expect(events.first[:total_time]).not_to be_empty end it "has the author's URL" do - expect(subject.staging_events.first[:author][:web_url]).not_to be_nil + expect(events.first[:author][:web_url]).not_to be_nil end it "has the author's avatar URL" do - expect(subject.staging_events.first[:author][:avatar_url]).not_to be_nil + expect(events.first[:author][:avatar_url]).not_to be_nil end it "has the author's name" do - expect(subject.staging_events.first[:author][:name]).to eq(MergeRequest.first.author.name) + expect(events.first[:author][:name]).to eq(MergeRequest.first.author.name) end end describe '#production_events' do + let(:stage) { :production } let!(:context) { create(:issue, project: project, created_at: 2.days.ago) } before do @@ -284,35 +297,35 @@ describe Gitlab::CycleAnalytics::Events do end it 'has the total time' do - expect(subject.production_events.first[:total_time]).not_to be_empty + expect(events.first[:total_time]).not_to be_empty end it 'has a title' do - expect(subject.production_events.first[:title]).to eq(context.title) + expect(events.first[:title]).to eq(context.title) end it 'has the URL' do - expect(subject.production_events.first[:url]).not_to be_nil + expect(events.first[:url]).not_to be_nil end it 'has an iid' do - expect(subject.production_events.first[:iid]).to eq(context.iid.to_s) + expect(events.first[:iid]).to eq(context.iid.to_s) end it 'has a created_at timestamp' do - expect(subject.production_events.first[:created_at]).to end_with('ago') + expect(events.first[:created_at]).to end_with('ago') end it "has the author's URL" do - expect(subject.production_events.first[:author][:web_url]).not_to be_nil + expect(events.first[:author][:web_url]).not_to be_nil end it "has the author's avatar URL" do - expect(subject.production_events.first[:author][:avatar_url]).not_to be_nil + expect(events.first[:author][:avatar_url]).not_to be_nil end it "has the author's name" do - expect(subject.production_events.first[:author][:name]).to eq(context.author.name) + expect(events.first[:author][:name]).to eq(context.author.name) end end diff --git a/spec/lib/gitlab/cycle_analytics/issue_event_spec.rb b/spec/lib/gitlab/cycle_analytics/issue_event_spec.rb index 1c5c308da7d..7967d3727db 100644 --- a/spec/lib/gitlab/cycle_analytics/issue_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/issue_event_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' describe Gitlab::CycleAnalytics::IssueEvent do + let(:stage_name) { :issue } + it_behaves_like 'default query config' do it 'has the default order' do expect(event.order).to eq(event.start_time_attrs) diff --git a/spec/lib/gitlab/cycle_analytics/plan_event_spec.rb b/spec/lib/gitlab/cycle_analytics/plan_event_spec.rb index 4a5604115ec..5c4b8b343bd 100644 --- a/spec/lib/gitlab/cycle_analytics/plan_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/plan_event_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' describe Gitlab::CycleAnalytics::PlanEvent do + let(:stage_name) { :plan } + it_behaves_like 'default query config' do it 'has the default order' do expect(event.order).to eq(event.start_time_attrs) diff --git a/spec/lib/gitlab/cycle_analytics/production_event_spec.rb b/spec/lib/gitlab/cycle_analytics/production_event_spec.rb index ac17e3b4287..99ed9a0ab5c 100644 --- a/spec/lib/gitlab/cycle_analytics/production_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/production_event_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' describe Gitlab::CycleAnalytics::ProductionEvent do + let(:stage_name) { :production } + it_behaves_like 'default query config' do it 'has the default order' do expect(event.order).to eq(event.start_time_attrs) diff --git a/spec/lib/gitlab/cycle_analytics/review_event_spec.rb b/spec/lib/gitlab/cycle_analytics/review_event_spec.rb index 1ff53aa0227..efc40d4ca4a 100644 --- a/spec/lib/gitlab/cycle_analytics/review_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/review_event_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' describe Gitlab::CycleAnalytics::ReviewEvent do + let(:stage_name) { :review } + it_behaves_like 'default query config' do it 'has the default order' do expect(event.order).to eq(event.start_time_attrs) diff --git a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb index 7019e4c3351..0b0ea662b74 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb @@ -1,7 +1,13 @@ require 'spec_helper' shared_examples 'default query config' do - let(:event) { described_class.new(project: double, options: {}) } + let(:fetcher) do + Gitlab::CycleAnalytics::MetricsFetcher.new(project: create(:empty_project), + from: 1.day.ago, + branch: nil) + end + + let(:event) { described_class.new(fetcher: fetcher, stage: stage_name, options: {}) } it 'has the start attributes' do expect(event.start_time_attrs).not_to be_nil diff --git a/spec/lib/gitlab/cycle_analytics/staging_event_spec.rb b/spec/lib/gitlab/cycle_analytics/staging_event_spec.rb index 4862d4765f2..b7ab477067c 100644 --- a/spec/lib/gitlab/cycle_analytics/staging_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/staging_event_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' describe Gitlab::CycleAnalytics::StagingEvent do + let(:stage_name) { :staging } + it_behaves_like 'default query config' do it 'does not have the default order' do expect(event.order).not_to eq(event.start_time_attrs) diff --git a/spec/lib/gitlab/cycle_analytics/test_event_spec.rb b/spec/lib/gitlab/cycle_analytics/test_event_spec.rb index e249db69fc6..a4fc8963e5b 100644 --- a/spec/lib/gitlab/cycle_analytics/test_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/test_event_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' describe Gitlab::CycleAnalytics::TestEvent do + let(:stage_name) { :test } + it_behaves_like 'default query config' do it 'does not have the default order' do expect(event.order).not_to eq(event.start_time_attrs) diff --git a/spec/models/cycle_analytics/summary_spec.rb b/spec/models/cycle_analytics/summary_spec.rb index 1a54c57a278..a8c1c4b9c5e 100644 --- a/spec/models/cycle_analytics/summary_spec.rb +++ b/spec/models/cycle_analytics/summary_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe CycleAnalytics::Summary, models: true do +describe Gitlab::CycleAnalytics::StageSummary, models: true do let(:project) { create(:project) } let(:from) { Time.now } let(:user) { create(:user, :admin) } diff --git a/spec/serializers/analytics_summary_serializer_spec.rb b/spec/serializers/analytics_summary_serializer_spec.rb index e08e3f88710..fe551734bc1 100644 --- a/spec/serializers/analytics_summary_serializer_spec.rb +++ b/spec/serializers/analytics_summary_serializer_spec.rb @@ -8,10 +8,10 @@ describe AnalyticsSummarySerializer do let(:json) { serializer.as_json } let(:project) { create(:empty_project) } - let(:resource) { Gitlab::CycleAnalytics::Summary::Issue.new(project: double, from: 1.day.ago) } + let(:resource) { Gitlab::CycleAnalytics::StageSummary::Issue.new(project: double, from: 1.day.ago) } before do - allow_any_instance_of(Gitlab::CycleAnalytics::Summary::Issue).to receive(:value).and_return(1.12) + allow_any_instance_of(Gitlab::CycleAnalytics::StageSummary::Issue).to receive(:value).and_return(1.12) end it 'it generates payload for single object' do From e4e313fccab6816fb2e52ebdf83807fba4a52051 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 23 Nov 2016 13:04:06 +0100 Subject: [PATCH 08/25] Fix other spec failures --- .../cycle_analytics/stage_summary_spec.rb} | 16 ++++++++-------- .../analytics_summary_serializer_spec.rb | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) rename spec/{models/cycle_analytics/summary_spec.rb => lib/gitlab/cycle_analytics/stage_summary_spec.rb} (81%) diff --git a/spec/models/cycle_analytics/summary_spec.rb b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb similarity index 81% rename from spec/models/cycle_analytics/summary_spec.rb rename to spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb index a8c1c4b9c5e..77dbf1c79a5 100644 --- a/spec/models/cycle_analytics/summary_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb @@ -4,20 +4,20 @@ describe Gitlab::CycleAnalytics::StageSummary, models: true do let(:project) { create(:project) } let(:from) { Time.now } let(:user) { create(:user, :admin) } - subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } + subject { described_class.new(project, from: Time.now).data } 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) + expect(subject.first[:value]).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) + expect(subject.first[:value]).to eq(0) end end @@ -26,19 +26,19 @@ describe Gitlab::CycleAnalytics::StageSummary, models: true 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) + expect(subject.second[:value]).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) + expect(subject.second[:value]).to eq(0) end it "finds a large (> 100) snumber of commits if present" do Timecop.freeze(5.days.from_now) { create_commit("Test message", project, user, 'master', count: 100) } - expect(subject.commits).to eq(100) + expect(subject.second[:value]).to eq(100) end end @@ -47,13 +47,13 @@ describe Gitlab::CycleAnalytics::StageSummary, models: true 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) + expect(subject.third[:value]).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) + expect(subject.third[:value]).to eq(0) end end end diff --git a/spec/serializers/analytics_summary_serializer_spec.rb b/spec/serializers/analytics_summary_serializer_spec.rb index fe551734bc1..e08e3f88710 100644 --- a/spec/serializers/analytics_summary_serializer_spec.rb +++ b/spec/serializers/analytics_summary_serializer_spec.rb @@ -8,10 +8,10 @@ describe AnalyticsSummarySerializer do let(:json) { serializer.as_json } let(:project) { create(:empty_project) } - let(:resource) { Gitlab::CycleAnalytics::StageSummary::Issue.new(project: double, from: 1.day.ago) } + let(:resource) { Gitlab::CycleAnalytics::Summary::Issue.new(project: double, from: 1.day.ago) } before do - allow_any_instance_of(Gitlab::CycleAnalytics::StageSummary::Issue).to receive(:value).and_return(1.12) + allow_any_instance_of(Gitlab::CycleAnalytics::Summary::Issue).to receive(:value).and_return(1.12) end it 'it generates payload for single object' do From 8183e848648bc737e4a09f76f4f55ee1cf106b26 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 23 Nov 2016 15:35:47 +0100 Subject: [PATCH 09/25] fix tricky test failure to do with private method --- .../cycle_analytics_helpers/test_generation.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/support/cycle_analytics_helpers/test_generation.rb b/spec/support/cycle_analytics_helpers/test_generation.rb index ab624161616..dc866b11429 100644 --- a/spec/support/cycle_analytics_helpers/test_generation.rb +++ b/spec/support/cycle_analytics_helpers/test_generation.rb @@ -56,7 +56,7 @@ module CycleAnalyticsHelpers end median_time_difference = time_differences.sort[2] - expect(subject.send(phase)).to be_within(5).of(median_time_difference) + expect(subject.public_send(phase)).to be_within(5).of(median_time_difference) end context "when the data belongs to another project" do @@ -88,7 +88,7 @@ module CycleAnalyticsHelpers # Turn off the stub before checking assertions allow(self).to receive(:project).and_call_original - expect(subject.send(phase)).to be_nil + expect(subject.public_send(phase)).to be_nil end end @@ -111,7 +111,7 @@ module CycleAnalyticsHelpers Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn - expect(subject.send(phase)).to be_nil + expect(subject.public_send(phase)).to be_nil end end end @@ -131,7 +131,7 @@ module CycleAnalyticsHelpers Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn end - expect(subject.send(phase)).to be_nil + expect(subject.public_send(phase)).to be_nil end end end @@ -150,7 +150,7 @@ module CycleAnalyticsHelpers post_fn[self, data] if post_fn end - expect(subject.send(phase)).to be_nil + expect(subject.public_send(phase)).to be_nil end end end @@ -158,7 +158,7 @@ module CycleAnalyticsHelpers context "when none of the start / end conditions are matched" do it "returns nil" do - expect(subject.send(phase)).to be_nil + expect(subject.public_send(phase)).to be_nil end end end From b8056669849729cab5700466a7fae6dc6b2743b2 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 1 Dec 2016 11:21:24 +0100 Subject: [PATCH 10/25] refactor cycle analytics - updated based on MR feedback --- .../cycle_analytics/events_controller.rb | 16 ++++++++-------- .../projects/cycle_analytics_controller.rb | 4 ++-- app/models/cycle_analytics.rb | 12 ++++-------- app/serializers/analytics_stage_entity.rb | 4 +--- lib/gitlab/cycle_analytics/base_event.rb | 10 +++++++--- lib/gitlab/cycle_analytics/base_stage.rb | 19 +++++++++++++------ lib/gitlab/cycle_analytics/class_name_util.rb | 13 +++++++++++++ lib/gitlab/cycle_analytics/code_stage.rb | 12 +++++------- lib/gitlab/cycle_analytics/event.rb | 9 +++++++++ lib/gitlab/cycle_analytics/issue_stage.rb | 14 ++++++-------- lib/gitlab/cycle_analytics/metrics_fetcher.rb | 2 +- lib/gitlab/cycle_analytics/plan_stage.rb | 14 ++++++-------- .../cycle_analytics/production_stage.rb | 12 +++++------- lib/gitlab/cycle_analytics/review_stage.rb | 12 +++++------- lib/gitlab/cycle_analytics/stage.rb | 9 +++++++++ lib/gitlab/cycle_analytics/staging_stage.rb | 12 +++++------- lib/gitlab/cycle_analytics/test_stage.rb | 12 +++++------- .../lib/gitlab/cycle_analytics/events_spec.rb | 2 +- .../cycle_analytics/shared_stage_spec.rb | 2 +- .../analytics_stage_serializer_spec.rb | 2 +- 20 files changed, 107 insertions(+), 85 deletions(-) create mode 100644 lib/gitlab/cycle_analytics/class_name_util.rb create mode 100644 lib/gitlab/cycle_analytics/event.rb create mode 100644 lib/gitlab/cycle_analytics/stage.rb diff --git a/app/controllers/projects/cycle_analytics/events_controller.rb b/app/controllers/projects/cycle_analytics/events_controller.rb index e571e1dfce2..d4969c66467 100644 --- a/app/controllers/projects/cycle_analytics/events_controller.rb +++ b/app/controllers/projects/cycle_analytics/events_controller.rb @@ -9,33 +9,33 @@ module Projects before_action :authorize_read_merge_request!, only: [:code, :review] def issue - render_events(cycle_analytics.events_for(:issue)) + render_events(cycle_analytics[:issue].events) end def plan - render_events(cycle_analytics.events_for(:plan)) + render_events(cycle_analytics[:plan].events) end def code - render_events(cycle_analytics.events_for(:code)) + render_events(cycle_analytics[:code].events) end def test options(events_params)[:branch] = events_params[:branch_name] - render_events(cycle_analytics.events_for(:test)) + render_events(cycle_analytics[:test].events) end def review - render_events(cycle_analytics.events_for(:review)) + render_events(cycle_analytics[:review].events) end def staging - render_events(cycle_analytics.events_for(:staging)) + render_events(cycle_analytics[:staging].events) end def production - render_events(cycle_analytics.events_for(:production)) + render_events(cycle_analytics[:production].events) end private @@ -54,7 +54,7 @@ module Projects def events_params return {} unless params[:events].present? - params[:events].slice(:start_date, :branch_name) + params[:events].permit(:start_date, :branch_name) end end end diff --git a/app/controllers/projects/cycle_analytics_controller.rb b/app/controllers/projects/cycle_analytics_controller.rb index cf53d0a1919..88ac3ad046b 100644 --- a/app/controllers/projects/cycle_analytics_controller.rb +++ b/app/controllers/projects/cycle_analytics_controller.rb @@ -6,7 +6,7 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController before_action :authorize_read_cycle_analytics! def show - @cycle_analytics = ::CycleAnalytics.new(@project, options: options(cycle_analytics_params)) + @cycle_analytics = ::CycleAnalytics.new(@project, options(cycle_analytics_params)) @cycle_analytics_no_data = @cycle_analytics.no_stats? @@ -21,7 +21,7 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController def cycle_analytics_params return {} unless params[:cycle_analytics].present? - params[:cycle_analytics].slice(:start_date) + params[:cycle_analytics].permit(:start_date) end def cycle_analytics_json diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index 5bcc6fa1954..c6862c9733d 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -1,7 +1,7 @@ class CycleAnalytics STAGES = %i[issue plan code test review staging production].freeze - def initialize(project, options:) + def initialize(project, options) @project = project @options = options end @@ -22,19 +22,15 @@ class CycleAnalytics Gitlab::CycleAnalytics::Permissions.get(user: user, project: @project) end - def events_for(stage) - classify_stage(stage).new(project: @project, options: @options, stage: stage).events + def [](stage_name) + Gitlab::CycleAnalytics::Stage[stage_name].new(project: @project, options: @options) end private def stats_per_stage STAGES.map do |stage_name| - classify_stage(stage_name).new(project: @project, options: @options, stage: stage_name).median_data + Gitlab::CycleAnalytics::Stage[stage_name].new(project: @project, options: @options).median_data end end - - def classify_stage(stage_name) - "Gitlab::CycleAnalytics::#{stage_name.to_s.capitalize}Stage".constantize - end end diff --git a/app/serializers/analytics_stage_entity.rb b/app/serializers/analytics_stage_entity.rb index d454a4937f4..a559d0850c4 100644 --- a/app/serializers/analytics_stage_entity.rb +++ b/app/serializers/analytics_stage_entity.rb @@ -1,9 +1,7 @@ class AnalyticsStageEntity < Grape::Entity include EntityDateHelper - expose :stage, as: :title do |object| - object.stage.to_s.capitalize - end + expose :title expose :description expose :median, as: :value do |stage| diff --git a/lib/gitlab/cycle_analytics/base_event.rb b/lib/gitlab/cycle_analytics/base_event.rb index d540cb6549c..eac807af037 100644 --- a/lib/gitlab/cycle_analytics/base_event.rb +++ b/lib/gitlab/cycle_analytics/base_event.rb @@ -2,13 +2,13 @@ module Gitlab module CycleAnalytics class BaseEvent include MetricsTables + include ClassNameUtil - attr_reader :stage, :start_time_attrs, :end_time_attrs, :projections, :query + attr_reader :start_time_attrs, :end_time_attrs, :projections, :query - def initialize(fetcher:, stage:, options:) + def initialize(fetcher:, options:) @query = EventsQuery.new(fetcher: fetcher) @project = fetcher.project - @stage = stage @options = options end @@ -26,6 +26,10 @@ module Gitlab @order || @start_time_attrs end + def stage + class_name_for('Event') + end + private def update_author! diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb index 162ebf18c77..551318f536a 100644 --- a/lib/gitlab/cycle_analytics/base_stage.rb +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -1,29 +1,36 @@ module Gitlab module CycleAnalytics class BaseStage - attr_reader :stage, :description + include ClassNameUtil - def initialize(project:, options:, stage:) + def initialize(project:, options:) @project = project @options = options @fetcher = Gitlab::CycleAnalytics::MetricsFetcher.new(project: project, from: options[:from], branch: options[:branch]) - @stage = stage end def events - event_class.new(fetcher: @fetcher, stage: @stage, options: @options).fetch + Gitlab::CycleAnalytics::Event[stage].new(fetcher: @fetcher, options: @options).fetch end def median_data AnalyticsStageSerializer.new.represent(self).as_json end + def title + stage.to_s.capitalize + end + + def median + raise NotImplementedError.new("Expected #{self.name} to implement median") + end + private - def event_class - "Gitlab::CycleAnalytics::#{@stage.to_s.capitalize}Event".constantize + def stage + class_name_for('Stage') end end end diff --git a/lib/gitlab/cycle_analytics/class_name_util.rb b/lib/gitlab/cycle_analytics/class_name_util.rb new file mode 100644 index 00000000000..aac8d888077 --- /dev/null +++ b/lib/gitlab/cycle_analytics/class_name_util.rb @@ -0,0 +1,13 @@ +module Gitlab + module CycleAnalytics + module ClassNameUtil + def class_name_for(type) + class_name.split(type).first.to_sym + end + + def class_name + self.class.name.demodulize + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/code_stage.rb b/lib/gitlab/cycle_analytics/code_stage.rb index f72989c9a72..778ce7b5435 100644 --- a/lib/gitlab/cycle_analytics/code_stage.rb +++ b/lib/gitlab/cycle_analytics/code_stage.rb @@ -1,16 +1,14 @@ module Gitlab module CycleAnalytics class CodeStage < BaseStage - def initialize(*args) - super(*args) - - @description = "Time until first merge request" + def description + "Time until first merge request" end def median - @fetcher.calculate_metric(:code, - Issue::Metrics.arel_table[:first_mentioned_in_commit_at], - MergeRequest.arel_table[:created_at]) + @fetcher.median(:code, + Issue::Metrics.arel_table[:first_mentioned_in_commit_at], + MergeRequest.arel_table[:created_at]) end end end diff --git a/lib/gitlab/cycle_analytics/event.rb b/lib/gitlab/cycle_analytics/event.rb new file mode 100644 index 00000000000..62fac89a0d5 --- /dev/null +++ b/lib/gitlab/cycle_analytics/event.rb @@ -0,0 +1,9 @@ +module Gitlab + module CycleAnalytics + module Event + def self.[](stage_name) + const_get("::Gitlab::CycleAnalytics::#{stage_name.to_s.camelize}Event") + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/issue_stage.rb b/lib/gitlab/cycle_analytics/issue_stage.rb index a2ada238cd2..c317872fb1d 100644 --- a/lib/gitlab/cycle_analytics/issue_stage.rb +++ b/lib/gitlab/cycle_analytics/issue_stage.rb @@ -1,17 +1,15 @@ module Gitlab module CycleAnalytics class IssueStage < BaseStage - def initialize(*args) - super(*args) - - @description = "Time before an issue gets scheduled" + def description + "Time before an issue gets scheduled" end def median - @fetcher.calculate_metric(:issue, - Issue.arel_table[:created_at], - [Issue::Metrics.arel_table[:first_associated_with_milestone_at], - Issue::Metrics.arel_table[:first_added_to_board_at]]) + @fetcher.median(:issue, + Issue.arel_table[:created_at], + [Issue::Metrics.arel_table[:first_associated_with_milestone_at], + Issue::Metrics.arel_table[:first_added_to_board_at]]) end end end diff --git a/lib/gitlab/cycle_analytics/metrics_fetcher.rb b/lib/gitlab/cycle_analytics/metrics_fetcher.rb index 51835bbde24..bd68a0980ca 100644 --- a/lib/gitlab/cycle_analytics/metrics_fetcher.rb +++ b/lib/gitlab/cycle_analytics/metrics_fetcher.rb @@ -15,7 +15,7 @@ module Gitlab @branch = branch end - def calculate_metric(name, start_time_attrs, end_time_attrs) + def median(name, start_time_attrs, end_time_attrs) cte_table = Arel::Table.new("cte_table_for_#{name}") # Build a `SELECT` query. We find the first of the `end_time_attrs` that isn't `NULL` (call this end_time). diff --git a/lib/gitlab/cycle_analytics/plan_stage.rb b/lib/gitlab/cycle_analytics/plan_stage.rb index c836068c4ef..5e6dd30d9e3 100644 --- a/lib/gitlab/cycle_analytics/plan_stage.rb +++ b/lib/gitlab/cycle_analytics/plan_stage.rb @@ -1,17 +1,15 @@ module Gitlab module CycleAnalytics class PlanStage < BaseStage - def initialize(*args) - super(*args) - - @description = "Time before an issue starts implementation" + def description + "Time before an issue starts implementation" end def median - @fetcher.calculate_metric(:plan, - [Issue::Metrics.arel_table[:first_associated_with_milestone_at], - Issue::Metrics.arel_table[:first_added_to_board_at]], - Issue::Metrics.arel_table[:first_mentioned_in_commit_at]) + @fetcher.median(:plan, + [Issue::Metrics.arel_table[:first_associated_with_milestone_at], + Issue::Metrics.arel_table[:first_added_to_board_at]], + Issue::Metrics.arel_table[:first_mentioned_in_commit_at]) end end end diff --git a/lib/gitlab/cycle_analytics/production_stage.rb b/lib/gitlab/cycle_analytics/production_stage.rb index d46d37e1acc..acd2f7b2b3b 100644 --- a/lib/gitlab/cycle_analytics/production_stage.rb +++ b/lib/gitlab/cycle_analytics/production_stage.rb @@ -1,16 +1,14 @@ module Gitlab module CycleAnalytics class ProductionStage < BaseStage - def initialize(*args) - super(*args) - - @description = "From issue creation until deploy to production" + def description + "From issue creation until deploy to production" end def median - @fetcher.calculate_metric(:production, - Issue.arel_table[:created_at], - MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) + @fetcher.median(:production, + Issue.arel_table[:created_at], + MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) end end end diff --git a/lib/gitlab/cycle_analytics/review_stage.rb b/lib/gitlab/cycle_analytics/review_stage.rb index 4159ba5d70d..c7b5e34e16a 100644 --- a/lib/gitlab/cycle_analytics/review_stage.rb +++ b/lib/gitlab/cycle_analytics/review_stage.rb @@ -1,16 +1,14 @@ module Gitlab module CycleAnalytics class ReviewStage < BaseStage - def initialize(*args) - super(*args) - - @description = "Time between merge request creation and merge/close" + def description + "Time between merge request creation and merge/close" end def median - @fetcher.calculate_metric(:review, - MergeRequest.arel_table[:created_at], - MergeRequest::Metrics.arel_table[:merged_at]) + @fetcher.median(:review, + MergeRequest.arel_table[:created_at], + MergeRequest::Metrics.arel_table[:merged_at]) end end end diff --git a/lib/gitlab/cycle_analytics/stage.rb b/lib/gitlab/cycle_analytics/stage.rb new file mode 100644 index 00000000000..acf746db6cd --- /dev/null +++ b/lib/gitlab/cycle_analytics/stage.rb @@ -0,0 +1,9 @@ +module Gitlab + module CycleAnalytics + module Stage + def self.[](stage_name) + const_get("::Gitlab::CycleAnalytics::#{stage_name.to_s.camelize}Stage") + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/staging_stage.rb b/lib/gitlab/cycle_analytics/staging_stage.rb index cb4398f15ac..b715a9453c7 100644 --- a/lib/gitlab/cycle_analytics/staging_stage.rb +++ b/lib/gitlab/cycle_analytics/staging_stage.rb @@ -1,16 +1,14 @@ module Gitlab module CycleAnalytics class StagingStage < BaseStage - def initialize(*args) - super(*args) - - @description = "From merge request merge until deploy to production" + def description + "From merge request merge until deploy to production" end def median - @fetcher.calculate_metric(:staging, - MergeRequest::Metrics.arel_table[:merged_at], - MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) + @fetcher.median(:staging, + MergeRequest::Metrics.arel_table[:merged_at], + MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) end end end diff --git a/lib/gitlab/cycle_analytics/test_stage.rb b/lib/gitlab/cycle_analytics/test_stage.rb index 3ab93bebd87..58f72bb405e 100644 --- a/lib/gitlab/cycle_analytics/test_stage.rb +++ b/lib/gitlab/cycle_analytics/test_stage.rb @@ -1,16 +1,14 @@ module Gitlab module CycleAnalytics class TestStage < BaseStage - def initialize(*args) - super(*args) - - @description = "Total test time for all commits/merges" + def description + "Total test time for all commits/merges" end def median - @fetcher.calculate_metric(:test, - MergeRequest::Metrics.arel_table[:latest_build_started_at], - MergeRequest::Metrics.arel_table[:latest_build_finished_at]) + @fetcher.median(:test, + MergeRequest::Metrics.arel_table[:latest_build_started_at], + MergeRequest::Metrics.arel_table[:latest_build_finished_at]) end end end diff --git a/spec/lib/gitlab/cycle_analytics/events_spec.rb b/spec/lib/gitlab/cycle_analytics/events_spec.rb index 1258f4ed450..9d2ba481919 100644 --- a/spec/lib/gitlab/cycle_analytics/events_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/events_spec.rb @@ -7,7 +7,7 @@ describe 'cycle analytics events' do let!(:context) { create(:issue, project: project, created_at: 2.days.ago) } let(:events) do - CycleAnalytics.new(project, options: { from: from_date, current_user: user }).events_for(stage) + CycleAnalytics.new(project, { from: from_date, current_user: user })[stage].events end before do diff --git a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb index dd1ef4fc129..f4189d3c7fc 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb @@ -4,7 +4,7 @@ shared_examples 'base stage' do let(:stage) { described_class.new(project: double, options: {}, stage: stage_name) } before do - allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:calculate_metric).and_return(1.12) + allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:median).and_return(1.12) allow_any_instance_of(Gitlab::CycleAnalytics::BaseEvent).to receive(:event_result).and_return({}) end diff --git a/spec/serializers/analytics_stage_serializer_spec.rb b/spec/serializers/analytics_stage_serializer_spec.rb index 0f2d534e714..47c537fcf84 100644 --- a/spec/serializers/analytics_stage_serializer_spec.rb +++ b/spec/serializers/analytics_stage_serializer_spec.rb @@ -10,7 +10,7 @@ describe AnalyticsStageSerializer do let(:resource) { Gitlab::CycleAnalytics::CodeStage.new(project: double, options: {}, stage: :code) } before do - allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:calculate_metric).and_return(1.12) + allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:median).and_return(1.12) allow_any_instance_of(Gitlab::CycleAnalytics::BaseEvent).to receive(:event_result).and_return({}) end From 69ecd951a9e0acbc31f0a4b9b02e8a1981ceff1e Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 1 Dec 2016 12:44:35 +0100 Subject: [PATCH 11/25] refactor fetcher and fixed specs --- .../cycle_analytics/events_controller.rb | 2 +- lib/gitlab/cycle_analytics/base_event.rb | 4 +-- lib/gitlab/cycle_analytics/events_query.rb | 32 ------------------- lib/gitlab/cycle_analytics/metrics_fetcher.rb | 15 +++++++++ lib/gitlab/database/median.rb | 5 +++ spec/models/cycle_analytics/code_spec.rb | 6 ++-- spec/models/cycle_analytics/issue_spec.rb | 4 +-- spec/models/cycle_analytics/plan_spec.rb | 4 +-- .../models/cycle_analytics/production_spec.rb | 6 ++-- spec/models/cycle_analytics/review_spec.rb | 4 +-- spec/models/cycle_analytics/staging_spec.rb | 6 ++-- spec/models/cycle_analytics/test_spec.rb | 10 +++--- .../test_generation.rb | 18 ++++------- 13 files changed, 49 insertions(+), 67 deletions(-) delete mode 100644 lib/gitlab/cycle_analytics/events_query.rb diff --git a/app/controllers/projects/cycle_analytics/events_controller.rb b/app/controllers/projects/cycle_analytics/events_controller.rb index d4969c66467..b69d46f2c41 100644 --- a/app/controllers/projects/cycle_analytics/events_controller.rb +++ b/app/controllers/projects/cycle_analytics/events_controller.rb @@ -48,7 +48,7 @@ module Projects end def cycle_analytics - @cycle_analytics ||= ::CycleAnalytics.new(project, options: options(events_params)) + @cycle_analytics ||= ::CycleAnalytics.new(project, options(events_params)) end def events_params diff --git a/lib/gitlab/cycle_analytics/base_event.rb b/lib/gitlab/cycle_analytics/base_event.rb index eac807af037..2ce9c34d8e8 100644 --- a/lib/gitlab/cycle_analytics/base_event.rb +++ b/lib/gitlab/cycle_analytics/base_event.rb @@ -7,7 +7,7 @@ module Gitlab attr_reader :start_time_attrs, :end_time_attrs, :projections, :query def initialize(fetcher:, options:) - @query = EventsQuery.new(fetcher: fetcher) + @fetcher = fetcher @project = fetcher.project @options = options end @@ -39,7 +39,7 @@ module Gitlab end def event_result - @event_result ||= @query.execute(self).to_a + @event_result ||= @fetcher.events(self).to_a end def serialize(_event) diff --git a/lib/gitlab/cycle_analytics/events_query.rb b/lib/gitlab/cycle_analytics/events_query.rb deleted file mode 100644 index e2b79384c9b..00000000000 --- a/lib/gitlab/cycle_analytics/events_query.rb +++ /dev/null @@ -1,32 +0,0 @@ -module Gitlab - module CycleAnalytics - class EventsQuery - def initialize(fetcher:) - @fetcher = fetcher - end - - def execute(stage_class) - @stage_class = stage_class - - ActiveRecord::Base.connection.exec_query(query.to_sql) - end - - private - - def query - base_query = @fetcher.base_query_for(@stage_class.stage) - diff_fn = @fetcher.subtract_datetimes_diff(base_query, @stage_class.start_time_attrs, @stage_class.end_time_attrs) - - @stage_class.custom_query(base_query) - - base_query.project(extract_epoch(diff_fn).as('total_time'), *@stage_class.projections).order(@stage_class.order.desc) - end - - def extract_epoch(arel_attribute) - return arel_attribute unless Gitlab::Database.postgresql? - - Arel.sql(%Q{EXTRACT(EPOCH FROM (#{arel_attribute.to_sql}))}) - end - end - end -end diff --git a/lib/gitlab/cycle_analytics/metrics_fetcher.rb b/lib/gitlab/cycle_analytics/metrics_fetcher.rb index bd68a0980ca..0542fbfb38d 100644 --- a/lib/gitlab/cycle_analytics/metrics_fetcher.rb +++ b/lib/gitlab/cycle_analytics/metrics_fetcher.rb @@ -29,6 +29,21 @@ module Gitlab median_datetime(cte_table, interval_query, name) end + def events(stage_class) + ActiveRecord::Base.connection.exec_query(events_query(stage_class).to_sql) + end + + private + + def events_query(stage_class) + base_query = base_query_for(stage_class.stage) + diff_fn = subtract_datetimes_diff(base_query, stage_class.start_time_attrs, stage_class.end_time_attrs) + + stage_class.custom_query(base_query) + + base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *stage_class.projections).order(stage_class.order.desc) + end + # Join table with a row for every 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 diff --git a/lib/gitlab/database/median.rb b/lib/gitlab/database/median.rb index 1444d25ebc7..08607c27c09 100644 --- a/lib/gitlab/database/median.rb +++ b/lib/gitlab/database/median.rb @@ -103,6 +103,11 @@ module Gitlab Arel.sql(%Q{EXTRACT(EPOCH FROM "#{arel_attribute.relation.name}"."#{arel_attribute.name}")}) end + def extract_diff_epoch(diff) + return diff unless Gitlab::Database.postgresql? + + Arel.sql(%Q{EXTRACT(EPOCH FROM (#{diff.to_sql}))}) + 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")]) diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb index 4838b57e353..70f985afefb 100644 --- a/spec/models/cycle_analytics/code_spec.rb +++ b/spec/models/cycle_analytics/code_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#code', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } + subject { CycleAnalytics.new(project, from: from_date) } context 'with deployment' do generate_cycle_analytics_spec( @@ -37,7 +37,7 @@ describe 'CycleAnalytics#code', feature: true do deploy_master end - expect(subject.code).to be_nil + expect(subject[:code].median).to be_nil end end end @@ -69,7 +69,7 @@ describe 'CycleAnalytics#code', feature: true do merge_merge_requests_closing_issue(issue) end - expect(subject.code).to be_nil + expect(subject[:code].median).to be_nil end end end diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb index ce6e99bbec9..e4b6a8f4518 100644 --- a/spec/models/cycle_analytics/issue_spec.rb +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#issue', models: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } + subject { CycleAnalytics.new(project, from: from_date) } generate_cycle_analytics_spec( phase: :issue, @@ -42,7 +42,7 @@ describe 'CycleAnalytics#issue', models: true do merge_merge_requests_closing_issue(issue) end - expect(subject.issue).to be_nil + expect(subject[:issue].median).to be_nil end end end diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb index bd5a6a77b7a..dc5b04852d6 100644 --- a/spec/models/cycle_analytics/plan_spec.rb +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#plan', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } + subject { CycleAnalytics.new(project, from: from_date) } generate_cycle_analytics_spec( phase: :plan, @@ -44,7 +44,7 @@ describe 'CycleAnalytics#plan', feature: true do create_merge_request_closing_issue(issue, source_branch: branch_name) merge_merge_requests_closing_issue(issue) - expect(subject.issue).to be_nil + expect(subject[:issue].median).to be_nil end end end diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb index 653e203b491..5e99188f318 100644 --- a/spec/models/cycle_analytics/production_spec.rb +++ b/spec/models/cycle_analytics/production_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#production', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } + subject { CycleAnalytics.new(project, from: from_date) } generate_cycle_analytics_spec( phase: :production, @@ -35,7 +35,7 @@ describe 'CycleAnalytics#production', feature: true do deploy_master end - expect(subject.production).to be_nil + expect(subject[:production].median).to be_nil end end @@ -48,7 +48,7 @@ describe 'CycleAnalytics#production', feature: true do deploy_master(environment: 'staging') end - expect(subject.production).to be_nil + expect(subject[:production].median).to be_nil end end end diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb index 219cd4c0212..45baa5f7006 100644 --- a/spec/models/cycle_analytics/review_spec.rb +++ b/spec/models/cycle_analytics/review_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#review', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } + subject { CycleAnalytics.new(project, from: from_date) } generate_cycle_analytics_spec( phase: :review, @@ -27,7 +27,7 @@ describe 'CycleAnalytics#review', feature: true do MergeRequests::MergeService.new(project, user).execute(create(:merge_request)) end - expect(subject.review).to be_nil + expect(subject[:review].median).to be_nil end end end diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb index 8dffb6b8fe1..77625aad580 100644 --- a/spec/models/cycle_analytics/staging_spec.rb +++ b/spec/models/cycle_analytics/staging_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#staging', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } + subject { CycleAnalytics.new(project, from: from_date) } generate_cycle_analytics_spec( phase: :staging, @@ -45,7 +45,7 @@ describe 'CycleAnalytics#staging', feature: true do deploy_master end - expect(subject.staging).to be_nil + expect(subject[:staging].median).to be_nil end end @@ -58,7 +58,7 @@ describe 'CycleAnalytics#staging', feature: true do deploy_master(environment: 'staging') end - expect(subject.staging).to be_nil + expect(subject[:staging].median).to be_nil end end end diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb index ac1304beca8..27a117d2d76 100644 --- a/spec/models/cycle_analytics/test_spec.rb +++ b/spec/models/cycle_analytics/test_spec.rb @@ -6,7 +6,7 @@ describe 'CycleAnalytics#test', feature: true do let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } - subject { CycleAnalyticsTest.new(project, options: { from: from_date }) } + subject { CycleAnalytics.new(project, from: from_date) } generate_cycle_analytics_spec( phase: :test, @@ -35,7 +35,7 @@ describe 'CycleAnalytics#test', feature: true do merge_merge_requests_closing_issue(issue) end - expect(subject.test).to be_nil + expect(subject[:test].median).to be_nil end end @@ -48,7 +48,7 @@ describe 'CycleAnalytics#test', feature: true do pipeline.succeed! end - expect(subject.test).to be_nil + expect(subject[:test].median).to be_nil end end @@ -65,7 +65,7 @@ describe 'CycleAnalytics#test', feature: true do merge_merge_requests_closing_issue(issue) end - expect(subject.test).to be_nil + expect(subject[:test].median).to be_nil end end @@ -82,7 +82,7 @@ describe 'CycleAnalytics#test', feature: true do merge_merge_requests_closing_issue(issue) end - expect(subject.test).to be_nil + expect(subject[:test].median).to be_nil end end end diff --git a/spec/support/cycle_analytics_helpers/test_generation.rb b/spec/support/cycle_analytics_helpers/test_generation.rb index dc866b11429..35b40d73191 100644 --- a/spec/support/cycle_analytics_helpers/test_generation.rb +++ b/spec/support/cycle_analytics_helpers/test_generation.rb @@ -1,9 +1,3 @@ -class CycleAnalyticsTest < CycleAnalytics - def method_missing(method_sym, *arguments, &block) - classify_stage(method_sym).new(project: @project, options: @options, stage: method_sym).median - end -end - # rubocop:disable Metrics/AbcSize # Note: The ABC size is large here because we have a method generating test cases with @@ -56,7 +50,7 @@ module CycleAnalyticsHelpers end median_time_difference = time_differences.sort[2] - expect(subject.public_send(phase)).to be_within(5).of(median_time_difference) + expect(subject[phase].median).to be_within(5).of(median_time_difference) end context "when the data belongs to another project" do @@ -88,7 +82,7 @@ module CycleAnalyticsHelpers # Turn off the stub before checking assertions allow(self).to receive(:project).and_call_original - expect(subject.public_send(phase)).to be_nil + expect(subject[phase].median).to be_nil end end @@ -111,7 +105,7 @@ module CycleAnalyticsHelpers Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn - expect(subject.public_send(phase)).to be_nil + expect(subject[phase].median).to be_nil end end end @@ -131,7 +125,7 @@ module CycleAnalyticsHelpers Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn end - expect(subject.public_send(phase)).to be_nil + expect(subject[phase].median).to be_nil end end end @@ -150,7 +144,7 @@ module CycleAnalyticsHelpers post_fn[self, data] if post_fn end - expect(subject.public_send(phase)).to be_nil + expect(subject[phase].median).to be_nil end end end @@ -158,7 +152,7 @@ module CycleAnalyticsHelpers context "when none of the start / end conditions are matched" do it "returns nil" do - expect(subject.public_send(phase)).to be_nil + expect(subject[phase].median).to be_nil end end end From 58dddcdfed6212046447de8b6d304ffd463d0350 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 1 Dec 2016 13:28:24 +0100 Subject: [PATCH 12/25] few fixes after merge --- app/models/cycle_analytics.rb | 4 +++- lib/gitlab/cycle_analytics/stage_summary.rb | 5 +++-- lib/gitlab/cycle_analytics/summary/issue.rb | 8 +++++++- spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb | 4 ++-- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index c6862c9733d..57082ae6087 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -7,7 +7,9 @@ class CycleAnalytics end def summary - @summary ||= ::Gitlab::CycleAnalytics::StageSummary.new(@project, from: @options[:from]).data + @summary ||= ::Gitlab::CycleAnalytics::StageSummary.new(@project, + from: @options[:from], + current_user: @options[:current_user]).data end def stats diff --git a/lib/gitlab/cycle_analytics/stage_summary.rb b/lib/gitlab/cycle_analytics/stage_summary.rb index dd9e4ac2813..b34baf5b081 100644 --- a/lib/gitlab/cycle_analytics/stage_summary.rb +++ b/lib/gitlab/cycle_analytics/stage_summary.rb @@ -1,13 +1,14 @@ module Gitlab module CycleAnalytics class StageSummary - def initialize(project, from:) + def initialize(project, from:, current_user:) @project = project @from = from + @current_user = current_user end def data - [serialize(Summary::Issue.new(project: @project, from: @from)), + [serialize(Summary::Issue.new(project: @project, from: @from, current_user: @current_user)), serialize(Summary::Commit.new(project: @project, from: @from)), serialize(Summary::Deploy.new(project: @project, from: @from))] end diff --git a/lib/gitlab/cycle_analytics/summary/issue.rb b/lib/gitlab/cycle_analytics/summary/issue.rb index 7d62164aae3..008468f24b9 100644 --- a/lib/gitlab/cycle_analytics/summary/issue.rb +++ b/lib/gitlab/cycle_analytics/summary/issue.rb @@ -2,12 +2,18 @@ module Gitlab module CycleAnalytics module Summary class Issue < Base + def initialize(project:, from:, current_user:) + @project = project + @from = from + @current_user = current_user + end + def title 'New Issue' end def value - @value ||= @project.issues.created_after(@from).count + @value ||= IssuesFinder.new(@current_user, project_id: @project.id).execute.created_after(@from).count end end end diff --git a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb index 77dbf1c79a5..fb6b6c4a8d2 100644 --- a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' describe Gitlab::CycleAnalytics::StageSummary, models: true do let(:project) { create(:project) } - let(:from) { Time.now } + let(:from) { 1.day.ago } let(:user) { create(:user, :admin) } - subject { described_class.new(project, from: Time.now).data } + subject { described_class.new(project, from: Time.now, current_user: user).data } describe "#new_issues" do it "finds the number of issues created after the 'from date'" do From daa4f3ded718f4144877b7f0402bd495151c28de Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 1 Dec 2016 15:40:46 +0100 Subject: [PATCH 13/25] fix spec failures after merge --- spec/lib/gitlab/cycle_analytics/plan_event_spec.rb | 2 +- spec/lib/gitlab/cycle_analytics/shared_event_spec.rb | 2 +- spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb | 6 +++--- spec/serializers/analytics_stage_serializer_spec.rb | 4 ++-- spec/serializers/analytics_summary_serializer_spec.rb | 9 +++++++-- 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/spec/lib/gitlab/cycle_analytics/plan_event_spec.rb b/spec/lib/gitlab/cycle_analytics/plan_event_spec.rb index 5c4b8b343bd..df407e51c64 100644 --- a/spec/lib/gitlab/cycle_analytics/plan_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/plan_event_spec.rb @@ -11,7 +11,7 @@ describe Gitlab::CycleAnalytics::PlanEvent do context 'no commits' do it 'does not blow up if there are no commits' do - allow_any_instance_of(Gitlab::CycleAnalytics::EventsQuery).to receive(:execute).and_return([{}]) + allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:events).and_return([{}]) expect { event.fetch }.not_to raise_error end diff --git a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb index 0b0ea662b74..5e1c7531fb5 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb @@ -7,7 +7,7 @@ shared_examples 'default query config' do branch: nil) end - let(:event) { described_class.new(fetcher: fetcher, stage: stage_name, options: {}) } + let(:event) { described_class.new(fetcher: fetcher, options: {}) } it 'has the start attributes' do expect(event.start_time_attrs).not_to be_nil diff --git a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb index f4189d3c7fc..cfb5dc12ff1 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' shared_examples 'base stage' do - let(:stage) { described_class.new(project: double, options: {}, stage: stage_name) } + let(:stage) { described_class.new(project: double, options: {}) } before do allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:median).and_return(1.12) @@ -20,8 +20,8 @@ shared_examples 'base stage' do expect(stage.median_data[:description]).not_to be_nil end - it 'has the stage' do - expect(stage.stage).to eq(stage_name) + it 'has the title' do + expect(stage.title).to eq(stage_name.to_s.capitalize) end it 'has the events' do diff --git a/spec/serializers/analytics_stage_serializer_spec.rb b/spec/serializers/analytics_stage_serializer_spec.rb index 47c537fcf84..3627a21230f 100644 --- a/spec/serializers/analytics_stage_serializer_spec.rb +++ b/spec/serializers/analytics_stage_serializer_spec.rb @@ -7,7 +7,7 @@ describe AnalyticsStageSerializer do end let(:json) { serializer.as_json } - let(:resource) { Gitlab::CycleAnalytics::CodeStage.new(project: double, options: {}, stage: :code) } + let(:resource) { Gitlab::CycleAnalytics::CodeStage.new(project: double, options: {}) } before do allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:median).and_return(1.12) @@ -15,7 +15,7 @@ describe AnalyticsStageSerializer do end it 'it generates payload for single object' do - expect(json).to be_an_instance_of Hash + expect(json).to be_kind_of Hash end it 'contains important elements of AnalyticsStage' do diff --git a/spec/serializers/analytics_summary_serializer_spec.rb b/spec/serializers/analytics_summary_serializer_spec.rb index e08e3f88710..7a84c8b0b40 100644 --- a/spec/serializers/analytics_summary_serializer_spec.rb +++ b/spec/serializers/analytics_summary_serializer_spec.rb @@ -8,14 +8,19 @@ describe AnalyticsSummarySerializer do let(:json) { serializer.as_json } let(:project) { create(:empty_project) } - let(:resource) { Gitlab::CycleAnalytics::Summary::Issue.new(project: double, from: 1.day.ago) } + let(:user) { create(:user) } + let(:resource) do + Gitlab::CycleAnalytics::Summary::Issue.new(project: double, + from: 1.day.ago, + current_user: user) + end before do allow_any_instance_of(Gitlab::CycleAnalytics::Summary::Issue).to receive(:value).and_return(1.12) end it 'it generates payload for single object' do - expect(json).to be_an_instance_of Hash + expect(json).to be_kind_of Hash end it 'contains important elements of AnalyticsStage' do From b214be493d9f179d4a929ee32d94a336da7b38f1 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 2 Dec 2016 17:09:29 +0100 Subject: [PATCH 14/25] big refactor based on MR feedback --- app/models/cycle_analytics.rb | 2 +- .../{base_event.rb => base_event_fetcher.rb} | 12 +++------ lib/gitlab/cycle_analytics/base_stage.rb | 13 +++++++--- lib/gitlab/cycle_analytics/class_name_util.rb | 13 ---------- ...{review_event.rb => code_event_fetcher.rb} | 7 +++--- lib/gitlab/cycle_analytics/code_stage.rb | 17 ++++++++----- lib/gitlab/cycle_analytics/event.rb | 2 +- ...uction_event.rb => issue_event_fetcher.rb} | 4 +-- lib/gitlab/cycle_analytics/issue_stage.rb | 19 ++++++++------ lib/gitlab/cycle_analytics/metrics_fetcher.rb | 25 +++++++++++-------- .../{plan_event.rb => plan_event_fetcher.rb} | 5 +--- lib/gitlab/cycle_analytics/plan_stage.rb | 19 ++++++++------ ...e_event.rb => production_event_fetcher.rb} | 5 +--- .../cycle_analytics/production_stage.rb | 17 ++++++++----- ...{code_event.rb => review_event_fetcher.rb} | 7 +----- lib/gitlab/cycle_analytics/review_stage.rb | 17 ++++++++----- lib/gitlab/cycle_analytics/stage.rb | 2 +- ...ging_event.rb => staging_event_fetcher.rb} | 4 +-- lib/gitlab/cycle_analytics/staging_stage.rb | 17 ++++++++----- lib/gitlab/cycle_analytics/test_event.rb | 12 --------- .../cycle_analytics/test_event_fetcher.rb | 6 +++++ lib/gitlab/cycle_analytics/test_stage.rb | 17 ++++++++----- ...ent_spec.rb => code_event_fetcher_spec.rb} | 2 +- ...nt_spec.rb => issue_event_fetcher_spec.rb} | 2 +- ...ent_spec.rb => plan_event_fetcher_spec.rb} | 2 +- ...ec.rb => production_event_fetcher_spec.rb} | 2 +- ...t_spec.rb => review_event_fetcher_spec.rb} | 2 +- .../cycle_analytics/shared_event_spec.rb | 5 ++-- .../cycle_analytics/shared_stage_spec.rb | 2 +- ..._spec.rb => staging_event_fetcher_spec.rb} | 2 +- ...ent_spec.rb => test_event_fetcher_spec.rb} | 2 +- .../analytics_stage_serializer_spec.rb | 2 +- 32 files changed, 136 insertions(+), 129 deletions(-) rename lib/gitlab/cycle_analytics/{base_event.rb => base_event_fetcher.rb} (83%) delete mode 100644 lib/gitlab/cycle_analytics/class_name_util.rb rename lib/gitlab/cycle_analytics/{review_event.rb => code_event_fetcher.rb} (79%) rename lib/gitlab/cycle_analytics/{production_event.rb => issue_event_fetcher.rb} (75%) rename lib/gitlab/cycle_analytics/{plan_event.rb => plan_event_fetcher.rb} (81%) rename lib/gitlab/cycle_analytics/{issue_event.rb => production_event_fetcher.rb} (67%) rename lib/gitlab/cycle_analytics/{code_event.rb => review_event_fetcher.rb} (71%) rename lib/gitlab/cycle_analytics/{staging_event.rb => staging_event_fetcher.rb} (77%) delete mode 100644 lib/gitlab/cycle_analytics/test_event.rb create mode 100644 lib/gitlab/cycle_analytics/test_event_fetcher.rb rename spec/lib/gitlab/cycle_analytics/{code_event_spec.rb => code_event_fetcher_spec.rb} (83%) rename spec/lib/gitlab/cycle_analytics/{issue_event_spec.rb => issue_event_fetcher_spec.rb} (82%) rename spec/lib/gitlab/cycle_analytics/{plan_event_spec.rb => plan_event_fetcher_spec.rb} (90%) rename spec/lib/gitlab/cycle_analytics/{production_event_spec.rb => production_event_fetcher_spec.rb} (81%) rename spec/lib/gitlab/cycle_analytics/{review_event_spec.rb => review_event_fetcher_spec.rb} (82%) rename spec/lib/gitlab/cycle_analytics/{staging_event_spec.rb => staging_event_fetcher_spec.rb} (83%) rename spec/lib/gitlab/cycle_analytics/{test_event_spec.rb => test_event_fetcher_spec.rb} (83%) diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index 57082ae6087..78aebd828d7 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -32,7 +32,7 @@ class CycleAnalytics def stats_per_stage STAGES.map do |stage_name| - Gitlab::CycleAnalytics::Stage[stage_name].new(project: @project, options: @options).median_data + self[stage_name].new(project: @project, options: @options).median_data end end end diff --git a/lib/gitlab/cycle_analytics/base_event.rb b/lib/gitlab/cycle_analytics/base_event_fetcher.rb similarity index 83% rename from lib/gitlab/cycle_analytics/base_event.rb rename to lib/gitlab/cycle_analytics/base_event_fetcher.rb index 2ce9c34d8e8..0d851f81b1d 100644 --- a/lib/gitlab/cycle_analytics/base_event.rb +++ b/lib/gitlab/cycle_analytics/base_event_fetcher.rb @@ -1,15 +1,15 @@ module Gitlab module CycleAnalytics - class BaseEvent + class BaseEventFetcher include MetricsTables - include ClassNameUtil - attr_reader :start_time_attrs, :end_time_attrs, :projections, :query + attr_reader :projections, :query, :stage - def initialize(fetcher:, options:) + def initialize(fetcher:, options:, stage:) @fetcher = fetcher @project = fetcher.project @options = options + @stage = stage end def fetch @@ -26,10 +26,6 @@ module Gitlab @order || @start_time_attrs end - def stage - class_name_for('Event') - end - private def update_author! diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb index 551318f536a..f3b8bb6e1d3 100644 --- a/lib/gitlab/cycle_analytics/base_stage.rb +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -1,18 +1,23 @@ module Gitlab module CycleAnalytics class BaseStage - include ClassNameUtil + attr_accessor :start_time_attrs, :end_time_attrs def initialize(project:, options:) @project = project @options = options @fetcher = Gitlab::CycleAnalytics::MetricsFetcher.new(project: project, from: options[:from], - branch: options[:branch]) + branch: options[:branch], + stage: self) + end + + def event + @event ||= Gitlab::CycleAnalytics::Event[stage].new(fetcher: @fetcher, options: @options) end def events - Gitlab::CycleAnalytics::Event[stage].new(fetcher: @fetcher, options: @options).fetch + event.fetch end def median_data @@ -24,7 +29,7 @@ module Gitlab end def median - raise NotImplementedError.new("Expected #{self.name} to implement median") + @fetcher.median end private diff --git a/lib/gitlab/cycle_analytics/class_name_util.rb b/lib/gitlab/cycle_analytics/class_name_util.rb deleted file mode 100644 index aac8d888077..00000000000 --- a/lib/gitlab/cycle_analytics/class_name_util.rb +++ /dev/null @@ -1,13 +0,0 @@ -module Gitlab - module CycleAnalytics - module ClassNameUtil - def class_name_for(type) - class_name.split(type).first.to_sym - end - - def class_name - self.class.name.demodulize - end - end - end -end diff --git a/lib/gitlab/cycle_analytics/review_event.rb b/lib/gitlab/cycle_analytics/code_event_fetcher.rb similarity index 79% rename from lib/gitlab/cycle_analytics/review_event.rb rename to lib/gitlab/cycle_analytics/code_event_fetcher.rb index 3f9ffa9657b..5245b9ca8fc 100644 --- a/lib/gitlab/cycle_analytics/review_event.rb +++ b/lib/gitlab/cycle_analytics/code_event_fetcher.rb @@ -1,21 +1,22 @@ module Gitlab module CycleAnalytics - class ReviewEvent < BaseEvent + class CodeEventFetcher < BaseEventFetcher include MergeRequestAllowed def initialize(*args) - @start_time_attrs = mr_table[:created_at] - @end_time_attrs = mr_metrics_table[:merged_at] @projections = [mr_table[:title], mr_table[:iid], mr_table[:id], mr_table[:created_at], mr_table[:state], mr_table[:author_id]] + @order = mr_table[:created_at] super(*args) end + private + def serialize(event) AnalyticsMergeRequestSerializer.new(project: @project).represent(event).as_json end diff --git a/lib/gitlab/cycle_analytics/code_stage.rb b/lib/gitlab/cycle_analytics/code_stage.rb index 778ce7b5435..977d0d0210c 100644 --- a/lib/gitlab/cycle_analytics/code_stage.rb +++ b/lib/gitlab/cycle_analytics/code_stage.rb @@ -1,14 +1,19 @@ module Gitlab module CycleAnalytics class CodeStage < BaseStage - def description - "Time until first merge request" + def initialize(*args) + @start_time_attrs = issue_metrics_table[:first_mentioned_in_commit_at] + @end_time_attrs = mr_table[:created_at] + + super(*args) end - def median - @fetcher.median(:code, - Issue::Metrics.arel_table[:first_mentioned_in_commit_at], - MergeRequest.arel_table[:created_at]) + def stage + :code + end + + def description + "Time until first merge request" end end end diff --git a/lib/gitlab/cycle_analytics/event.rb b/lib/gitlab/cycle_analytics/event.rb index 62fac89a0d5..bb3a5722a0f 100644 --- a/lib/gitlab/cycle_analytics/event.rb +++ b/lib/gitlab/cycle_analytics/event.rb @@ -2,7 +2,7 @@ module Gitlab module CycleAnalytics module Event def self.[](stage_name) - const_get("::Gitlab::CycleAnalytics::#{stage_name.to_s.camelize}Event") + CycleAnalytics.const_get("#{stage_name.to_s.camelize}Event") end end end diff --git a/lib/gitlab/cycle_analytics/production_event.rb b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb similarity index 75% rename from lib/gitlab/cycle_analytics/production_event.rb rename to lib/gitlab/cycle_analytics/issue_event_fetcher.rb index c03cd4f4909..0d8da99455e 100644 --- a/lib/gitlab/cycle_analytics/production_event.rb +++ b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb @@ -1,11 +1,9 @@ module Gitlab module CycleAnalytics - class ProductionEvent < BaseEvent + class IssueEventFetcher < BaseEventFetcher include IssueAllowed def initialize(*args) - @start_time_attrs = issue_table[:created_at] - @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at] @projections = [issue_table[:title], issue_table[:iid], issue_table[:id], diff --git a/lib/gitlab/cycle_analytics/issue_stage.rb b/lib/gitlab/cycle_analytics/issue_stage.rb index c317872fb1d..14e72c7ea48 100644 --- a/lib/gitlab/cycle_analytics/issue_stage.rb +++ b/lib/gitlab/cycle_analytics/issue_stage.rb @@ -1,15 +1,20 @@ module Gitlab module CycleAnalytics class IssueStage < BaseStage - def description - "Time before an issue gets scheduled" + def initialize(*args) + @start_time_attrs = issue_table[:created_at] + @end_time_attrs = [issue_metrics_table[:first_associated_with_milestone_at], + issue_metrics_table[:first_added_to_board_at]] + + super(*args) end - def median - @fetcher.median(:issue, - Issue.arel_table[:created_at], - [Issue::Metrics.arel_table[:first_associated_with_milestone_at], - Issue::Metrics.arel_table[:first_added_to_board_at]]) + def stage + :issue + end + + def description + "Time before an issue gets scheduled" end end end diff --git a/lib/gitlab/cycle_analytics/metrics_fetcher.rb b/lib/gitlab/cycle_analytics/metrics_fetcher.rb index 0542fbfb38d..865abd0fa6c 100644 --- a/lib/gitlab/cycle_analytics/metrics_fetcher.rb +++ b/lib/gitlab/cycle_analytics/metrics_fetcher.rb @@ -9,14 +9,15 @@ module Gitlab DEPLOYMENT_METRIC_STAGES = %i[production staging] - def initialize(project:, from:, branch:) + def initialize(project:, from:, branch:, stage:) @project = project @from = from @branch = branch + @stage = stage end - def median(name, start_time_attrs, end_time_attrs) - cte_table = Arel::Table.new("cte_table_for_#{name}") + def median + cte_table = Arel::Table.new("cte_table_for_#{@stage.stage}") # Build a `SELECT` query. We find the first of the `end_time_attrs` that isn't `NULL` (call this end_time). # Next, we find the first of the start_time_attrs that isn't `NULL` (call this start_time). @@ -24,24 +25,26 @@ module Gitlab # cycle analytics stage. interval_query = Arel::Nodes::As.new( cte_table, - subtract_datetimes(base_query_for(name), start_time_attrs, end_time_attrs, name.to_s)) + subtract_datetimes(base_query_for(name), @stage.start_time_attrs, @stage.end_time_attrs, @stage.stage.to_s)) median_datetime(cte_table, interval_query, name) end - def events(stage_class) - ActiveRecord::Base.connection.exec_query(events_query(stage_class).to_sql) + def events + ActiveRecord::Base.connection.exec_query(events_query.to_sql) end private - def events_query(stage_class) - base_query = base_query_for(stage_class.stage) - diff_fn = subtract_datetimes_diff(base_query, stage_class.start_time_attrs, stage_class.end_time_attrs) + def events_query + base_query = base_query_for(@stage.stage) + event = @stage.event - stage_class.custom_query(base_query) + diff_fn = subtract_datetimes_diff(base_query, @stage.start_time_attrs, @stage.end_time_attrs) - base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *stage_class.projections).order(stage_class.order.desc) + event_instance.custom_query(base_query) + + base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *event.projections).order(event.order.desc) end # Join table with a row for every pair (where the merge request diff --git a/lib/gitlab/cycle_analytics/plan_event.rb b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb similarity index 81% rename from lib/gitlab/cycle_analytics/plan_event.rb rename to lib/gitlab/cycle_analytics/plan_event_fetcher.rb index 4b06143495b..3e23c5644d3 100644 --- a/lib/gitlab/cycle_analytics/plan_event.rb +++ b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb @@ -1,10 +1,7 @@ module Gitlab module CycleAnalytics - class PlanEvent < BaseEvent + class PlanEventFetcher < BaseEventFetcher def initialize(*args) - @start_time_attrs = issue_metrics_table[:first_associated_with_milestone_at] - @end_time_attrs = [issue_metrics_table[:first_added_to_board_at], - issue_metrics_table[:first_mentioned_in_commit_at]] @projections = [mr_diff_table[:st_commits].as('commits'), issue_metrics_table[:first_mentioned_in_commit_at]] diff --git a/lib/gitlab/cycle_analytics/plan_stage.rb b/lib/gitlab/cycle_analytics/plan_stage.rb index 5e6dd30d9e3..de2d5aaeb23 100644 --- a/lib/gitlab/cycle_analytics/plan_stage.rb +++ b/lib/gitlab/cycle_analytics/plan_stage.rb @@ -1,15 +1,20 @@ module Gitlab module CycleAnalytics class PlanStage < BaseStage - def description - "Time before an issue starts implementation" + def initialize(*args) + @start_time_attrs = [issue_metrics_table[:first_associated_with_milestone_at], + issue_metrics_table[:first_added_to_board_at]] + @end_time_attrs = issue_metrics_table[:first_mentioned_in_commit_at] + + super(*args) end - def median - @fetcher.median(:plan, - [Issue::Metrics.arel_table[:first_associated_with_milestone_at], - Issue::Metrics.arel_table[:first_added_to_board_at]], - Issue::Metrics.arel_table[:first_mentioned_in_commit_at]) + def stage + :code + end + + def description + "Time before an issue starts implementation" end end end diff --git a/lib/gitlab/cycle_analytics/issue_event.rb b/lib/gitlab/cycle_analytics/production_event_fetcher.rb similarity index 67% rename from lib/gitlab/cycle_analytics/issue_event.rb rename to lib/gitlab/cycle_analytics/production_event_fetcher.rb index 76e8decf36e..b7eff7d22f4 100644 --- a/lib/gitlab/cycle_analytics/issue_event.rb +++ b/lib/gitlab/cycle_analytics/production_event_fetcher.rb @@ -1,12 +1,9 @@ module Gitlab module CycleAnalytics - class IssueEvent < BaseEvent + class ProductionEventFetcher < BaseEventFetcher include IssueAllowed def initialize(*args) - @start_time_attrs = issue_table[:created_at] - @end_time_attrs = [issue_metrics_table[:first_associated_with_milestone_at], - issue_metrics_table[:first_added_to_board_at]] @projections = [issue_table[:title], issue_table[:iid], issue_table[:id], diff --git a/lib/gitlab/cycle_analytics/production_stage.rb b/lib/gitlab/cycle_analytics/production_stage.rb index acd2f7b2b3b..104c6d3fd30 100644 --- a/lib/gitlab/cycle_analytics/production_stage.rb +++ b/lib/gitlab/cycle_analytics/production_stage.rb @@ -1,14 +1,19 @@ module Gitlab module CycleAnalytics class ProductionStage < BaseStage - def description - "From issue creation until deploy to production" + def initialize(*args) + @start_time_attrs = issue_table[:created_at] + @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at] + + super(*args) end - def median - @fetcher.median(:production, - Issue.arel_table[:created_at], - MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) + def stage + :production + end + + def description + "From issue creation until deploy to production" end end end diff --git a/lib/gitlab/cycle_analytics/code_event.rb b/lib/gitlab/cycle_analytics/review_event_fetcher.rb similarity index 71% rename from lib/gitlab/cycle_analytics/code_event.rb rename to lib/gitlab/cycle_analytics/review_event_fetcher.rb index 68251630e08..4df0bd06393 100644 --- a/lib/gitlab/cycle_analytics/code_event.rb +++ b/lib/gitlab/cycle_analytics/review_event_fetcher.rb @@ -1,24 +1,19 @@ module Gitlab module CycleAnalytics - class CodeEvent < BaseEvent + class ReviewEventFetcher < BaseEventFetcher include MergeRequestAllowed def initialize(*args) - @start_time_attrs = issue_metrics_table[:first_mentioned_in_commit_at] - @end_time_attrs = mr_table[:created_at] @projections = [mr_table[:title], mr_table[:iid], mr_table[:id], mr_table[:created_at], mr_table[:state], mr_table[:author_id]] - @order = mr_table[:created_at] super(*args) end - private - def serialize(event) AnalyticsMergeRequestSerializer.new(project: @project).represent(event).as_json end diff --git a/lib/gitlab/cycle_analytics/review_stage.rb b/lib/gitlab/cycle_analytics/review_stage.rb index c7b5e34e16a..c7bbd29693b 100644 --- a/lib/gitlab/cycle_analytics/review_stage.rb +++ b/lib/gitlab/cycle_analytics/review_stage.rb @@ -1,14 +1,19 @@ module Gitlab module CycleAnalytics class ReviewStage < BaseStage - def description - "Time between merge request creation and merge/close" + def initialize(*args) + @start_time_attrs = mr_table[:created_at] + @end_time_attrs = mr_metrics_table[:merged_at] + + super(*args) end - def median - @fetcher.median(:review, - MergeRequest.arel_table[:created_at], - MergeRequest::Metrics.arel_table[:merged_at]) + def stage + :review + end + + def description + "Time between merge request creation and merge/close" end end end diff --git a/lib/gitlab/cycle_analytics/stage.rb b/lib/gitlab/cycle_analytics/stage.rb index acf746db6cd..28e0455df59 100644 --- a/lib/gitlab/cycle_analytics/stage.rb +++ b/lib/gitlab/cycle_analytics/stage.rb @@ -2,7 +2,7 @@ module Gitlab module CycleAnalytics module Stage def self.[](stage_name) - const_get("::Gitlab::CycleAnalytics::#{stage_name.to_s.camelize}Stage") + CycleAnalytics.const_get("#{stage_name.to_s.camelize}Stage") end end end diff --git a/lib/gitlab/cycle_analytics/staging_event.rb b/lib/gitlab/cycle_analytics/staging_event_fetcher.rb similarity index 77% rename from lib/gitlab/cycle_analytics/staging_event.rb rename to lib/gitlab/cycle_analytics/staging_event_fetcher.rb index eae18b447f0..ea98e211ad6 100644 --- a/lib/gitlab/cycle_analytics/staging_event.rb +++ b/lib/gitlab/cycle_analytics/staging_event_fetcher.rb @@ -1,9 +1,7 @@ module Gitlab module CycleAnalytics - class StagingEvent < BaseEvent + class StagingEventFetcher < BaseEventFetcher def initialize(*args) - @start_time_attrs = mr_metrics_table[:merged_at] - @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at] @projections = [build_table[:id]] @order = build_table[:created_at] diff --git a/lib/gitlab/cycle_analytics/staging_stage.rb b/lib/gitlab/cycle_analytics/staging_stage.rb index b715a9453c7..079b26760bb 100644 --- a/lib/gitlab/cycle_analytics/staging_stage.rb +++ b/lib/gitlab/cycle_analytics/staging_stage.rb @@ -1,14 +1,19 @@ module Gitlab module CycleAnalytics class StagingStage < BaseStage - def description - "From merge request merge until deploy to production" + def initialize(*args) + @start_time_attrs = mr_metrics_table[:merged_at] + @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at] + + super(*args) end - def median - @fetcher.median(:staging, - MergeRequest::Metrics.arel_table[:merged_at], - MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) + def stage + :staging + end + + def description + "From merge request merge until deploy to production" end end end diff --git a/lib/gitlab/cycle_analytics/test_event.rb b/lib/gitlab/cycle_analytics/test_event.rb deleted file mode 100644 index d0736672adf..00000000000 --- a/lib/gitlab/cycle_analytics/test_event.rb +++ /dev/null @@ -1,12 +0,0 @@ -module Gitlab - module CycleAnalytics - class TestEvent < StagingEvent - def initialize(*args) - super(*args) - - @start_time_attrs = mr_metrics_table[:latest_build_started_at] - @end_time_attrs = mr_metrics_table[:latest_build_finished_at] - end - end - end -end diff --git a/lib/gitlab/cycle_analytics/test_event_fetcher.rb b/lib/gitlab/cycle_analytics/test_event_fetcher.rb new file mode 100644 index 00000000000..a2589c6601a --- /dev/null +++ b/lib/gitlab/cycle_analytics/test_event_fetcher.rb @@ -0,0 +1,6 @@ +module Gitlab + module CycleAnalytics + class TestEventFetcher < StagingEventFetcher + end + end +end diff --git a/lib/gitlab/cycle_analytics/test_stage.rb b/lib/gitlab/cycle_analytics/test_stage.rb index 58f72bb405e..a105e5f2b1f 100644 --- a/lib/gitlab/cycle_analytics/test_stage.rb +++ b/lib/gitlab/cycle_analytics/test_stage.rb @@ -1,14 +1,19 @@ module Gitlab module CycleAnalytics class TestStage < BaseStage - def description - "Total test time for all commits/merges" + def initialize(*args) + @start_time_attrs = mr_metrics_table[:latest_build_started_at] + @end_time_attrs = mr_metrics_table[:latest_build_finished_at] + + super(*args) end - def median - @fetcher.median(:test, - MergeRequest::Metrics.arel_table[:latest_build_started_at], - MergeRequest::Metrics.arel_table[:latest_build_finished_at]) + def stage + :test + end + + def description + "Total test time for all commits/merges" end end end diff --git a/spec/lib/gitlab/cycle_analytics/code_event_spec.rb b/spec/lib/gitlab/cycle_analytics/code_event_fetcher_spec.rb similarity index 83% rename from spec/lib/gitlab/cycle_analytics/code_event_spec.rb rename to spec/lib/gitlab/cycle_analytics/code_event_fetcher_spec.rb index 0673906e678..abfd60d7f6a 100644 --- a/spec/lib/gitlab/cycle_analytics/code_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/code_event_fetcher_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' -describe Gitlab::CycleAnalytics::CodeEvent do +describe Gitlab::CycleAnalytics::CodeEventFetcher do let(:stage_name) { :code } it_behaves_like 'default query config' do diff --git a/spec/lib/gitlab/cycle_analytics/issue_event_spec.rb b/spec/lib/gitlab/cycle_analytics/issue_event_fetcher_spec.rb similarity index 82% rename from spec/lib/gitlab/cycle_analytics/issue_event_spec.rb rename to spec/lib/gitlab/cycle_analytics/issue_event_fetcher_spec.rb index 7967d3727db..f4d995d072f 100644 --- a/spec/lib/gitlab/cycle_analytics/issue_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/issue_event_fetcher_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' -describe Gitlab::CycleAnalytics::IssueEvent do +describe Gitlab::CycleAnalytics::IssueEventFetcher do let(:stage_name) { :issue } it_behaves_like 'default query config' do diff --git a/spec/lib/gitlab/cycle_analytics/plan_event_spec.rb b/spec/lib/gitlab/cycle_analytics/plan_event_fetcher_spec.rb similarity index 90% rename from spec/lib/gitlab/cycle_analytics/plan_event_spec.rb rename to spec/lib/gitlab/cycle_analytics/plan_event_fetcher_spec.rb index df407e51c64..679779de51e 100644 --- a/spec/lib/gitlab/cycle_analytics/plan_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/plan_event_fetcher_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' -describe Gitlab::CycleAnalytics::PlanEvent do +describe Gitlab::CycleAnalytics::PlanEventFetcher do let(:stage_name) { :plan } it_behaves_like 'default query config' do diff --git a/spec/lib/gitlab/cycle_analytics/production_event_spec.rb b/spec/lib/gitlab/cycle_analytics/production_event_fetcher_spec.rb similarity index 81% rename from spec/lib/gitlab/cycle_analytics/production_event_spec.rb rename to spec/lib/gitlab/cycle_analytics/production_event_fetcher_spec.rb index 99ed9a0ab5c..a9126b8fa1c 100644 --- a/spec/lib/gitlab/cycle_analytics/production_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/production_event_fetcher_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' -describe Gitlab::CycleAnalytics::ProductionEvent do +describe Gitlab::CycleAnalytics::ProductionEventFetcher do let(:stage_name) { :production } it_behaves_like 'default query config' do diff --git a/spec/lib/gitlab/cycle_analytics/review_event_spec.rb b/spec/lib/gitlab/cycle_analytics/review_event_fetcher_spec.rb similarity index 82% rename from spec/lib/gitlab/cycle_analytics/review_event_spec.rb rename to spec/lib/gitlab/cycle_analytics/review_event_fetcher_spec.rb index efc40d4ca4a..c3e66dcb861 100644 --- a/spec/lib/gitlab/cycle_analytics/review_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/review_event_fetcher_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' -describe Gitlab::CycleAnalytics::ReviewEvent do +describe Gitlab::CycleAnalytics::ReviewEventFetcher do let(:stage_name) { :review } it_behaves_like 'default query config' do diff --git a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb index 5e1c7531fb5..60ec87255c8 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb @@ -4,10 +4,11 @@ shared_examples 'default query config' do let(:fetcher) do Gitlab::CycleAnalytics::MetricsFetcher.new(project: create(:empty_project), from: 1.day.ago, - branch: nil) + branch: nil, + stage: stage_name) end - let(:event) { described_class.new(fetcher: fetcher, options: {}) } + let(:event) { described_class.new(fetcher: fetcher, options: {}, stage: stage_name) } it 'has the start attributes' do expect(event.start_time_attrs).not_to be_nil diff --git a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb index cfb5dc12ff1..8cc7875258e 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb @@ -5,7 +5,7 @@ shared_examples 'base stage' do before do allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:median).and_return(1.12) - allow_any_instance_of(Gitlab::CycleAnalytics::BaseEvent).to receive(:event_result).and_return({}) + allow_any_instance_of(Gitlab::CycleAnalytics::BaseEventFetcher).to receive(:event_result).and_return({}) end it 'has the median data value' do diff --git a/spec/lib/gitlab/cycle_analytics/staging_event_spec.rb b/spec/lib/gitlab/cycle_analytics/staging_event_fetcher_spec.rb similarity index 83% rename from spec/lib/gitlab/cycle_analytics/staging_event_spec.rb rename to spec/lib/gitlab/cycle_analytics/staging_event_fetcher_spec.rb index b7ab477067c..8338e17b96d 100644 --- a/spec/lib/gitlab/cycle_analytics/staging_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/staging_event_fetcher_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' -describe Gitlab::CycleAnalytics::StagingEvent do +describe Gitlab::CycleAnalytics::StagingEventFetcher do let(:stage_name) { :staging } it_behaves_like 'default query config' do diff --git a/spec/lib/gitlab/cycle_analytics/test_event_spec.rb b/spec/lib/gitlab/cycle_analytics/test_event_fetcher_spec.rb similarity index 83% rename from spec/lib/gitlab/cycle_analytics/test_event_spec.rb rename to spec/lib/gitlab/cycle_analytics/test_event_fetcher_spec.rb index a4fc8963e5b..9d4f7667f1d 100644 --- a/spec/lib/gitlab/cycle_analytics/test_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/test_event_fetcher_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' require 'lib/gitlab/cycle_analytics/shared_event_spec' -describe Gitlab::CycleAnalytics::TestEvent do +describe Gitlab::CycleAnalytics::TestEventFetcher do let(:stage_name) { :test } it_behaves_like 'default query config' do diff --git a/spec/serializers/analytics_stage_serializer_spec.rb b/spec/serializers/analytics_stage_serializer_spec.rb index 3627a21230f..5597fbed151 100644 --- a/spec/serializers/analytics_stage_serializer_spec.rb +++ b/spec/serializers/analytics_stage_serializer_spec.rb @@ -11,7 +11,7 @@ describe AnalyticsStageSerializer do before do allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:median).and_return(1.12) - allow_any_instance_of(Gitlab::CycleAnalytics::BaseEvent).to receive(:event_result).and_return({}) + allow_any_instance_of(Gitlab::CycleAnalytics::BaseEventFetcher).to receive(:event_result).and_return({}) end it 'it generates payload for single object' do From 3f681f4cefb5eda594acaab2eaf1be18ebd9066c Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 5 Dec 2016 09:47:10 +0100 Subject: [PATCH 15/25] fix specs, refactor missing bits from events stuff --- lib/gitlab/cycle_analytics/base_event_fetcher.rb | 8 ++------ lib/gitlab/cycle_analytics/base_stage.rb | 4 +++- lib/gitlab/cycle_analytics/event.rb | 2 +- lib/gitlab/cycle_analytics/metrics_fetcher.rb | 9 ++++++--- .../gitlab/cycle_analytics/code_event_fetcher_spec.rb | 4 ++-- .../gitlab/cycle_analytics/issue_event_fetcher_spec.rb | 6 +----- .../gitlab/cycle_analytics/plan_event_fetcher_spec.rb | 4 ---- .../cycle_analytics/production_event_fetcher_spec.rb | 6 +----- .../gitlab/cycle_analytics/review_event_fetcher_spec.rb | 6 +----- spec/lib/gitlab/cycle_analytics/shared_event_spec.rb | 8 -------- spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb | 8 ++++++++ .../gitlab/cycle_analytics/staging_event_fetcher_spec.rb | 4 ++-- .../gitlab/cycle_analytics/test_event_fetcher_spec.rb | 4 ++-- 13 files changed, 29 insertions(+), 44 deletions(-) diff --git a/lib/gitlab/cycle_analytics/base_event_fetcher.rb b/lib/gitlab/cycle_analytics/base_event_fetcher.rb index 0d851f81b1d..d4b2d665e59 100644 --- a/lib/gitlab/cycle_analytics/base_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/base_event_fetcher.rb @@ -3,7 +3,7 @@ module Gitlab class BaseEventFetcher include MetricsTables - attr_reader :projections, :query, :stage + attr_reader :projections, :query, :stage, :order def initialize(fetcher:, options:, stage:) @fetcher = fetcher @@ -22,10 +22,6 @@ module Gitlab def custom_query(_base_query); end - def order - @order || @start_time_attrs - end - private def update_author! @@ -35,7 +31,7 @@ module Gitlab end def event_result - @event_result ||= @fetcher.events(self).to_a + @event_result ||= @fetcher.events.to_a end def serialize(_event) diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb index f3b8bb6e1d3..f81a41bccb6 100644 --- a/lib/gitlab/cycle_analytics/base_stage.rb +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -1,6 +1,8 @@ module Gitlab module CycleAnalytics class BaseStage + include MetricsTables + attr_accessor :start_time_attrs, :end_time_attrs def initialize(project:, options:) @@ -13,7 +15,7 @@ module Gitlab end def event - @event ||= Gitlab::CycleAnalytics::Event[stage].new(fetcher: @fetcher, options: @options) + @event ||= Gitlab::CycleAnalytics::Event[stage].new(fetcher: @fetcher, options: @options, stage: stage) end def events diff --git a/lib/gitlab/cycle_analytics/event.rb b/lib/gitlab/cycle_analytics/event.rb index bb3a5722a0f..1ba7bc08ee5 100644 --- a/lib/gitlab/cycle_analytics/event.rb +++ b/lib/gitlab/cycle_analytics/event.rb @@ -2,7 +2,7 @@ module Gitlab module CycleAnalytics module Event def self.[](stage_name) - CycleAnalytics.const_get("#{stage_name.to_s.camelize}Event") + CycleAnalytics.const_get("#{stage_name.to_s.camelize}EventFetcher") end end end diff --git a/lib/gitlab/cycle_analytics/metrics_fetcher.rb b/lib/gitlab/cycle_analytics/metrics_fetcher.rb index 865abd0fa6c..dd291840ecd 100644 --- a/lib/gitlab/cycle_analytics/metrics_fetcher.rb +++ b/lib/gitlab/cycle_analytics/metrics_fetcher.rb @@ -38,13 +38,16 @@ module Gitlab def events_query base_query = base_query_for(@stage.stage) - event = @stage.event diff_fn = subtract_datetimes_diff(base_query, @stage.start_time_attrs, @stage.end_time_attrs) - event_instance.custom_query(base_query) + @stage.event.custom_query(base_query) - base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *event.projections).order(event.order.desc) + base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *@stage.event.projections).order(order.desc) + end + + def order + @stage.event.order || @stage.start_time_attrs.is_a?(Array) ? @stage.start_time_attrs.first : @stage.start_time_attrs end # Join table with a row for every pair (where the merge request diff --git a/spec/lib/gitlab/cycle_analytics/code_event_fetcher_spec.rb b/spec/lib/gitlab/cycle_analytics/code_event_fetcher_spec.rb index abfd60d7f6a..0267e8c2f69 100644 --- a/spec/lib/gitlab/cycle_analytics/code_event_fetcher_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/code_event_fetcher_spec.rb @@ -5,8 +5,8 @@ describe Gitlab::CycleAnalytics::CodeEventFetcher do let(:stage_name) { :code } it_behaves_like 'default query config' do - it 'does not have the default order' do - expect(event.order).not_to eq(event.start_time_attrs) + it 'has a default order' do + expect(event.order).not_to be_nil end end end diff --git a/spec/lib/gitlab/cycle_analytics/issue_event_fetcher_spec.rb b/spec/lib/gitlab/cycle_analytics/issue_event_fetcher_spec.rb index f4d995d072f..fd9fa2fee49 100644 --- a/spec/lib/gitlab/cycle_analytics/issue_event_fetcher_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/issue_event_fetcher_spec.rb @@ -4,9 +4,5 @@ require 'lib/gitlab/cycle_analytics/shared_event_spec' describe Gitlab::CycleAnalytics::IssueEventFetcher do let(:stage_name) { :issue } - it_behaves_like 'default query config' do - it 'has the default order' do - expect(event.order).to eq(event.start_time_attrs) - end - end + it_behaves_like 'default query config' end diff --git a/spec/lib/gitlab/cycle_analytics/plan_event_fetcher_spec.rb b/spec/lib/gitlab/cycle_analytics/plan_event_fetcher_spec.rb index 679779de51e..1c3c1728fc6 100644 --- a/spec/lib/gitlab/cycle_analytics/plan_event_fetcher_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/plan_event_fetcher_spec.rb @@ -5,10 +5,6 @@ describe Gitlab::CycleAnalytics::PlanEventFetcher do let(:stage_name) { :plan } it_behaves_like 'default query config' do - it 'has the default order' do - expect(event.order).to eq(event.start_time_attrs) - end - context 'no commits' do it 'does not blow up if there are no commits' do allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:events).and_return([{}]) diff --git a/spec/lib/gitlab/cycle_analytics/production_event_fetcher_spec.rb b/spec/lib/gitlab/cycle_analytics/production_event_fetcher_spec.rb index a9126b8fa1c..74001181305 100644 --- a/spec/lib/gitlab/cycle_analytics/production_event_fetcher_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/production_event_fetcher_spec.rb @@ -4,9 +4,5 @@ require 'lib/gitlab/cycle_analytics/shared_event_spec' describe Gitlab::CycleAnalytics::ProductionEventFetcher do let(:stage_name) { :production } - it_behaves_like 'default query config' do - it 'has the default order' do - expect(event.order).to eq(event.start_time_attrs) - end - end + it_behaves_like 'default query config' end diff --git a/spec/lib/gitlab/cycle_analytics/review_event_fetcher_spec.rb b/spec/lib/gitlab/cycle_analytics/review_event_fetcher_spec.rb index c3e66dcb861..4f67c95ed4c 100644 --- a/spec/lib/gitlab/cycle_analytics/review_event_fetcher_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/review_event_fetcher_spec.rb @@ -4,9 +4,5 @@ require 'lib/gitlab/cycle_analytics/shared_event_spec' describe Gitlab::CycleAnalytics::ReviewEventFetcher do let(:stage_name) { :review } - it_behaves_like 'default query config' do - it 'has the default order' do - expect(event.order).to eq(event.start_time_attrs) - end - end + it_behaves_like 'default query config' end diff --git a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb index 60ec87255c8..725f9a558f5 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb @@ -10,18 +10,10 @@ shared_examples 'default query config' do let(:event) { described_class.new(fetcher: fetcher, options: {}, stage: stage_name) } - it 'has the start attributes' do - expect(event.start_time_attrs).not_to be_nil - end - it 'has the stage attribute' do expect(event.stage).not_to be_nil end - it 'has the end attributes' do - expect(event.end_time_attrs).not_to be_nil - end - it 'has the projection attributes' do expect(event.projections).not_to be_nil end diff --git a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb index 8cc7875258e..c88e3e22f5c 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb @@ -8,6 +8,14 @@ shared_examples 'base stage' do allow_any_instance_of(Gitlab::CycleAnalytics::BaseEventFetcher).to receive(:event_result).and_return({}) end + it 'has the start attributes' do + expect(stage.start_time_attrs).not_to be_nil + end + + it 'has the end attributes' do + expect(stage.end_time_attrs).not_to be_nil + end + it 'has the median data value' do expect(stage.median_data[:value]).not_to be_nil end diff --git a/spec/lib/gitlab/cycle_analytics/staging_event_fetcher_spec.rb b/spec/lib/gitlab/cycle_analytics/staging_event_fetcher_spec.rb index 8338e17b96d..bbc82496340 100644 --- a/spec/lib/gitlab/cycle_analytics/staging_event_fetcher_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/staging_event_fetcher_spec.rb @@ -5,8 +5,8 @@ describe Gitlab::CycleAnalytics::StagingEventFetcher do let(:stage_name) { :staging } it_behaves_like 'default query config' do - it 'does not have the default order' do - expect(event.order).not_to eq(event.start_time_attrs) + it 'has a default order' do + expect(event.order).not_to be_nil end end end diff --git a/spec/lib/gitlab/cycle_analytics/test_event_fetcher_spec.rb b/spec/lib/gitlab/cycle_analytics/test_event_fetcher_spec.rb index 9d4f7667f1d..6639fa54e0e 100644 --- a/spec/lib/gitlab/cycle_analytics/test_event_fetcher_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/test_event_fetcher_spec.rb @@ -5,8 +5,8 @@ describe Gitlab::CycleAnalytics::TestEventFetcher do let(:stage_name) { :test } it_behaves_like 'default query config' do - it 'does not have the default order' do - expect(event.order).not_to eq(event.start_time_attrs) + it 'has a default order' do + expect(event.order).not_to be_nil end end end From 099aa124ebefac0ab490ab8e28294e0ea78279de Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 5 Dec 2016 10:45:07 +0100 Subject: [PATCH 16/25] fix plan stage issue and some spec failures --- lib/gitlab/cycle_analytics/base_stage.rb | 4 +++- lib/gitlab/cycle_analytics/metrics_fetcher.rb | 6 +++++- lib/gitlab/cycle_analytics/plan_stage.rb | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb index f81a41bccb6..c2605364ff0 100644 --- a/lib/gitlab/cycle_analytics/base_stage.rb +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -15,7 +15,9 @@ module Gitlab end def event - @event ||= Gitlab::CycleAnalytics::Event[stage].new(fetcher: @fetcher, options: @options, stage: stage) + @event ||= Gitlab::CycleAnalytics::Event[stage].new(fetcher: @fetcher, + options: @options, + stage: stage) end def events diff --git a/lib/gitlab/cycle_analytics/metrics_fetcher.rb b/lib/gitlab/cycle_analytics/metrics_fetcher.rb index dd291840ecd..559dbc0e8fc 100644 --- a/lib/gitlab/cycle_analytics/metrics_fetcher.rb +++ b/lib/gitlab/cycle_analytics/metrics_fetcher.rb @@ -47,7 +47,11 @@ module Gitlab end def order - @stage.event.order || @stage.start_time_attrs.is_a?(Array) ? @stage.start_time_attrs.first : @stage.start_time_attrs + @stage.event.order || default_order + end + + def default_order + @stage.start_time_attrs.is_a?(Array) ? @stage.start_time_attrs.first : @stage.start_time_attrs end # Join table with a row for every pair (where the merge request diff --git a/lib/gitlab/cycle_analytics/plan_stage.rb b/lib/gitlab/cycle_analytics/plan_stage.rb index de2d5aaeb23..f8c9b9c4495 100644 --- a/lib/gitlab/cycle_analytics/plan_stage.rb +++ b/lib/gitlab/cycle_analytics/plan_stage.rb @@ -10,7 +10,7 @@ module Gitlab end def stage - :code + :plan end def description From 056b0f199388ffd4d3156c59a344f284131f7cc6 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 5 Dec 2016 12:07:31 +0100 Subject: [PATCH 17/25] fix missing refactor in metrics fetcher --- lib/gitlab/cycle_analytics/metrics_fetcher.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/cycle_analytics/metrics_fetcher.rb b/lib/gitlab/cycle_analytics/metrics_fetcher.rb index 559dbc0e8fc..4115c092c0d 100644 --- a/lib/gitlab/cycle_analytics/metrics_fetcher.rb +++ b/lib/gitlab/cycle_analytics/metrics_fetcher.rb @@ -25,9 +25,9 @@ module Gitlab # cycle analytics stage. interval_query = Arel::Nodes::As.new( cte_table, - subtract_datetimes(base_query_for(name), @stage.start_time_attrs, @stage.end_time_attrs, @stage.stage.to_s)) + subtract_datetimes(base_query_for(@stage.stage), @stage.start_time_attrs, @stage.end_time_attrs, @stage.stage.to_s)) - median_datetime(cte_table, interval_query, name) + median_datetime(cte_table, interval_query, @stage.stage) end def events From bbb9f840824aec2467ed2580cae4d9b42b85a7d4 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 5 Dec 2016 13:28:53 +0100 Subject: [PATCH 18/25] a few more fixes --- app/models/cycle_analytics.rb | 2 +- .../cycle_analytics/issue_event_fetcher.rb | 17 ----------------- .../cycle_analytics/production_event_fetcher.rb | 2 +- 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index 78aebd828d7..37ac56310ef 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -32,7 +32,7 @@ class CycleAnalytics def stats_per_stage STAGES.map do |stage_name| - self[stage_name].new(project: @project, options: @options).median_data + self[stage_name].median_data end end end diff --git a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb index 0d8da99455e..69ba7c3cc9c 100644 --- a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb @@ -1,23 +1,6 @@ module Gitlab module CycleAnalytics class IssueEventFetcher < BaseEventFetcher - include IssueAllowed - - def initialize(*args) - @projections = [issue_table[:title], - issue_table[:iid], - issue_table[:id], - issue_table[:created_at], - issue_table[:author_id]] - - super(*args) - end - - private - - def serialize(event) - AnalyticsIssueSerializer.new(project: @project).represent(event).as_json - end end end end diff --git a/lib/gitlab/cycle_analytics/production_event_fetcher.rb b/lib/gitlab/cycle_analytics/production_event_fetcher.rb index b7eff7d22f4..882e780874f 100644 --- a/lib/gitlab/cycle_analytics/production_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/production_event_fetcher.rb @@ -1,6 +1,6 @@ module Gitlab module CycleAnalytics - class ProductionEventFetcher < BaseEventFetcher + class ProductionEventFetcher < IssueEventFetcher include IssueAllowed def initialize(*args) From 834bcacbaec837d8ec0a269f111bca769843bcb4 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 7 Dec 2016 09:25:11 +0100 Subject: [PATCH 19/25] fix refactor of production event fetcher --- .../cycle_analytics/issue_event_fetcher.rb | 17 +++++++++++++++++ .../cycle_analytics/production_event_fetcher.rb | 17 ----------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb index 69ba7c3cc9c..0d8da99455e 100644 --- a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb @@ -1,6 +1,23 @@ module Gitlab module CycleAnalytics class IssueEventFetcher < BaseEventFetcher + include IssueAllowed + + def initialize(*args) + @projections = [issue_table[:title], + issue_table[:iid], + issue_table[:id], + issue_table[:created_at], + issue_table[:author_id]] + + super(*args) + end + + private + + def serialize(event) + AnalyticsIssueSerializer.new(project: @project).represent(event).as_json + end end end end diff --git a/lib/gitlab/cycle_analytics/production_event_fetcher.rb b/lib/gitlab/cycle_analytics/production_event_fetcher.rb index 882e780874f..0fa2e87f673 100644 --- a/lib/gitlab/cycle_analytics/production_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/production_event_fetcher.rb @@ -1,23 +1,6 @@ module Gitlab module CycleAnalytics class ProductionEventFetcher < IssueEventFetcher - include IssueAllowed - - def initialize(*args) - @projections = [issue_table[:title], - issue_table[:iid], - issue_table[:id], - issue_table[:created_at], - issue_table[:author_id]] - - super(*args) - end - - private - - def serialize(event) - AnalyticsIssueSerializer.new(project: @project).represent(event).as_json - end end end end From 982d5a050667c517bbc996a08ca0922f2c5fbfb4 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 9 Dec 2016 12:41:15 +0100 Subject: [PATCH 20/25] refactored metrics fetcher - merged into stage and events --- .../cycle_analytics/base_event_fetcher.rb | 27 ++++-- lib/gitlab/cycle_analytics/base_query.rb | 31 +++++++ lib/gitlab/cycle_analytics/base_stage.rb | 36 +++++--- lib/gitlab/cycle_analytics/code_stage.rb | 2 +- lib/gitlab/cycle_analytics/issue_stage.rb | 2 +- lib/gitlab/cycle_analytics/metrics_fetcher.rb | 86 ------------------- .../cycle_analytics/plan_event_fetcher.rb | 4 +- lib/gitlab/cycle_analytics/plan_stage.rb | 2 +- .../cycle_analytics/production_helper.rb | 9 ++ .../cycle_analytics/production_stage.rb | 9 +- lib/gitlab/cycle_analytics/review_stage.rb | 2 +- .../cycle_analytics/staging_event_fetcher.rb | 4 +- lib/gitlab/cycle_analytics/staging_stage.rb | 4 +- lib/gitlab/cycle_analytics/summary/commit.rb | 7 +- lib/gitlab/cycle_analytics/test_stage.rb | 10 ++- .../cycle_analytics/shared_event_spec.rb | 5 +- 16 files changed, 119 insertions(+), 121 deletions(-) create mode 100644 lib/gitlab/cycle_analytics/base_query.rb delete mode 100644 lib/gitlab/cycle_analytics/metrics_fetcher.rb create mode 100644 lib/gitlab/cycle_analytics/production_helper.rb diff --git a/lib/gitlab/cycle_analytics/base_event_fetcher.rb b/lib/gitlab/cycle_analytics/base_event_fetcher.rb index d4b2d665e59..8b4ccfd5363 100644 --- a/lib/gitlab/cycle_analytics/base_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/base_event_fetcher.rb @@ -1,15 +1,14 @@ module Gitlab module CycleAnalytics class BaseEventFetcher - include MetricsTables + include BaseQuery attr_reader :projections, :query, :stage, :order - def initialize(fetcher:, options:, stage:) - @fetcher = fetcher - @project = fetcher.project - @options = options + def initialize(project:, stage:, options:) + @project = project @stage = stage + @options = options end def fetch @@ -20,8 +19,6 @@ module Gitlab end.compact end - def custom_query(_base_query); end - private def update_author! @@ -31,7 +28,21 @@ module Gitlab end def event_result - @event_result ||= @fetcher.events.to_a + @event_result ||= ActiveRecord::Base.connection.exec_query(events_query.to_sql).to_a + end + + def events_query + diff_fn = subtract_datetimes_diff(base_query, @options[:start_time_attrs], @options[:end_time_attrs]) + + base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *projections).order(order.desc) + end + + def order + @order || default_order + end + + def default_order + @options[:start_time_attrs].is_a?(Array) ? @options[:start_time_attrs].first : @options[:start_time_attrs] end def serialize(_event) diff --git a/lib/gitlab/cycle_analytics/base_query.rb b/lib/gitlab/cycle_analytics/base_query.rb new file mode 100644 index 00000000000..d560dca45c8 --- /dev/null +++ b/lib/gitlab/cycle_analytics/base_query.rb @@ -0,0 +1,31 @@ +module Gitlab + module CycleAnalytics + module BaseQuery + include MetricsTables + include Gitlab::Database::Median + include Gitlab::Database::DateTime + + private + + def base_query + @base_query ||= stage_query + end + + def stage_query + query = mr_closing_issues_table.join(issue_table).on(issue_table[:id].eq(mr_closing_issues_table[:issue_id])). + join(issue_metrics_table).on(issue_table[:id].eq(issue_metrics_table[:issue_id])). + where(issue_table[:project_id].eq(@project.id)). + where(issue_table[:deleted_at].eq(nil)). + where(issue_table[:created_at].gteq(@options[:from])) + + # Load merge_requests + query = query.join(mr_table, Arel::Nodes::OuterJoin). + on(mr_table[:id].eq(mr_closing_issues_table[:merge_request_id])). + join(mr_metrics_table). + on(mr_table[:id].eq(mr_metrics_table[:merge_request_id])) + + query + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb index c2605364ff0..afec16d1818 100644 --- a/lib/gitlab/cycle_analytics/base_stage.rb +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -1,23 +1,17 @@ module Gitlab module CycleAnalytics class BaseStage - include MetricsTables - - attr_accessor :start_time_attrs, :end_time_attrs + include BaseQuery def initialize(project:, options:) @project = project @options = options - @fetcher = Gitlab::CycleAnalytics::MetricsFetcher.new(project: project, - from: options[:from], - branch: options[:branch], - stage: self) end def event - @event ||= Gitlab::CycleAnalytics::Event[stage].new(fetcher: @fetcher, - options: @options, - stage: stage) + @event ||= Gitlab::CycleAnalytics::Event[name].new(project: @project, + stage: name, + options: event_options) end def events @@ -29,17 +23,31 @@ module Gitlab end def title - stage.to_s.capitalize + name.to_s.capitalize end def median - @fetcher.median + cte_table = Arel::Table.new("cte_table_for_#{name}") + + # Build a `SELECT` query. We find the first of the `end_time_attrs` that isn't `NULL` (call this end_time). + # Next, we find the first of the start_time_attrs that isn't `NULL` (call this start_time). + # We compute the (end_time - start_time) interval, and give it an alias based on the current + # cycle analytics stage. + interval_query = Arel::Nodes::As.new( + cte_table, + subtract_datetimes(base_query, @start_time_attrs, @end_time_attrs, name.to_s)) + + median_datetime(cte_table, interval_query, name) + end + + def name + raise NotImplementedError.new("Expected #{self.name} to implement name") end private - def stage - class_name_for('Stage') + def event_options + @options.merge(start_time_attrs: @start_time_attrs, end_time_attrs: @end_time_attrs) end end end diff --git a/lib/gitlab/cycle_analytics/code_stage.rb b/lib/gitlab/cycle_analytics/code_stage.rb index 977d0d0210c..111c0e99633 100644 --- a/lib/gitlab/cycle_analytics/code_stage.rb +++ b/lib/gitlab/cycle_analytics/code_stage.rb @@ -8,7 +8,7 @@ module Gitlab super(*args) end - def stage + def name :code end diff --git a/lib/gitlab/cycle_analytics/issue_stage.rb b/lib/gitlab/cycle_analytics/issue_stage.rb index 14e72c7ea48..d320458d7fd 100644 --- a/lib/gitlab/cycle_analytics/issue_stage.rb +++ b/lib/gitlab/cycle_analytics/issue_stage.rb @@ -9,7 +9,7 @@ module Gitlab super(*args) end - def stage + def name :issue end diff --git a/lib/gitlab/cycle_analytics/metrics_fetcher.rb b/lib/gitlab/cycle_analytics/metrics_fetcher.rb deleted file mode 100644 index 4115c092c0d..00000000000 --- a/lib/gitlab/cycle_analytics/metrics_fetcher.rb +++ /dev/null @@ -1,86 +0,0 @@ -module Gitlab - module CycleAnalytics - class MetricsFetcher - include Gitlab::Database::Median - include Gitlab::Database::DateTime - include MetricsTables - - attr_reader :project - - DEPLOYMENT_METRIC_STAGES = %i[production staging] - - def initialize(project:, from:, branch:, stage:) - @project = project - @from = from - @branch = branch - @stage = stage - end - - def median - cte_table = Arel::Table.new("cte_table_for_#{@stage.stage}") - - # Build a `SELECT` query. We find the first of the `end_time_attrs` that isn't `NULL` (call this end_time). - # Next, we find the first of the start_time_attrs that isn't `NULL` (call this start_time). - # We compute the (end_time - start_time) interval, and give it an alias based on the current - # cycle analytics stage. - interval_query = Arel::Nodes::As.new( - cte_table, - subtract_datetimes(base_query_for(@stage.stage), @stage.start_time_attrs, @stage.end_time_attrs, @stage.stage.to_s)) - - median_datetime(cte_table, interval_query, @stage.stage) - end - - def events - ActiveRecord::Base.connection.exec_query(events_query.to_sql) - end - - private - - def events_query - base_query = base_query_for(@stage.stage) - - diff_fn = subtract_datetimes_diff(base_query, @stage.start_time_attrs, @stage.end_time_attrs) - - @stage.event.custom_query(base_query) - - base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *@stage.event.projections).order(order.desc) - end - - def order - @stage.event.order || default_order - end - - def default_order - @stage.start_time_attrs.is_a?(Array) ? @stage.start_time_attrs.first : @stage.start_time_attrs - end - - # Join table with a row for every 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_for(name) - # Load issues - query = mr_closing_issues_table.join(issue_table).on(issue_table[:id].eq(mr_closing_issues_table[:issue_id])). - join(issue_metrics_table).on(issue_table[:id].eq(issue_metrics_table[:issue_id])). - where(issue_table[:project_id].eq(@project.id)). - where(issue_table[:deleted_at].eq(nil)). - where(issue_table[:created_at].gteq(@from)) - - query = query.where(build_table[:ref].eq(@branch)) if name == :test && @branch - - # Load merge_requests - query = query.join(mr_table, Arel::Nodes::OuterJoin). - on(mr_table[:id].eq(mr_closing_issues_table[:merge_request_id])). - join(mr_metrics_table). - on(mr_table[:id].eq(mr_metrics_table[:merge_request_id])) - - if DEPLOYMENT_METRIC_STAGES.include?(name) - # Limit to merge requests that have been deployed to production after `@from` - query.where(mr_metrics_table[:first_deployed_to_production_at].gteq(@from)) - end - - query - end - end - end -end diff --git a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb index 3e23c5644d3..88a8710dbe6 100644 --- a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb @@ -8,8 +8,10 @@ module Gitlab super(*args) end - def custom_query(base_query) + def events_query base_query.join(mr_diff_table).on(mr_diff_table[:merge_request_id].eq(mr_table[:id])) + + super end private diff --git a/lib/gitlab/cycle_analytics/plan_stage.rb b/lib/gitlab/cycle_analytics/plan_stage.rb index f8c9b9c4495..a7164e5c5b7 100644 --- a/lib/gitlab/cycle_analytics/plan_stage.rb +++ b/lib/gitlab/cycle_analytics/plan_stage.rb @@ -9,7 +9,7 @@ module Gitlab super(*args) end - def stage + def name :plan end diff --git a/lib/gitlab/cycle_analytics/production_helper.rb b/lib/gitlab/cycle_analytics/production_helper.rb new file mode 100644 index 00000000000..d693443bfa4 --- /dev/null +++ b/lib/gitlab/cycle_analytics/production_helper.rb @@ -0,0 +1,9 @@ +module Gitlab + module CycleAnalytics + module ProductionHelper + def stage_query + super.where(mr_metrics_table[:first_deployed_to_production_at].gteq(@options[:from])) + end + end + end +end diff --git a/lib/gitlab/cycle_analytics/production_stage.rb b/lib/gitlab/cycle_analytics/production_stage.rb index 104c6d3fd30..eb221c68324 100644 --- a/lib/gitlab/cycle_analytics/production_stage.rb +++ b/lib/gitlab/cycle_analytics/production_stage.rb @@ -1,6 +1,8 @@ module Gitlab module CycleAnalytics class ProductionStage < BaseStage + include ProductionHelper + def initialize(*args) @start_time_attrs = issue_table[:created_at] @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at] @@ -8,13 +10,18 @@ module Gitlab super(*args) end - def stage + def name :production end def description "From issue creation until deploy to production" end + + def query + # Limit to merge requests that have been deployed to production after `@from` + query.where(mr_metrics_table[:first_deployed_to_production_at].gteq(@from)) + end end end end diff --git a/lib/gitlab/cycle_analytics/review_stage.rb b/lib/gitlab/cycle_analytics/review_stage.rb index c7bbd29693b..72ce1ed1e16 100644 --- a/lib/gitlab/cycle_analytics/review_stage.rb +++ b/lib/gitlab/cycle_analytics/review_stage.rb @@ -8,7 +8,7 @@ module Gitlab super(*args) end - def stage + def name :review end diff --git a/lib/gitlab/cycle_analytics/staging_event_fetcher.rb b/lib/gitlab/cycle_analytics/staging_event_fetcher.rb index ea98e211ad6..a34731a5fcd 100644 --- a/lib/gitlab/cycle_analytics/staging_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/staging_event_fetcher.rb @@ -14,8 +14,10 @@ module Gitlab super end - def custom_query(base_query) + def events_query base_query.join(build_table).on(mr_metrics_table[:pipeline_id].eq(build_table[:commit_id])) + + super end private diff --git a/lib/gitlab/cycle_analytics/staging_stage.rb b/lib/gitlab/cycle_analytics/staging_stage.rb index 079b26760bb..398c1b5989a 100644 --- a/lib/gitlab/cycle_analytics/staging_stage.rb +++ b/lib/gitlab/cycle_analytics/staging_stage.rb @@ -1,6 +1,8 @@ module Gitlab module CycleAnalytics class StagingStage < BaseStage + include ProductionHelper + def initialize(*args) @start_time_attrs = mr_metrics_table[:merged_at] @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at] @@ -8,7 +10,7 @@ module Gitlab super(*args) end - def stage + def name :staging end diff --git a/lib/gitlab/cycle_analytics/summary/commit.rb b/lib/gitlab/cycle_analytics/summary/commit.rb index ec3c067c0be..61a50762164 100644 --- a/lib/gitlab/cycle_analytics/summary/commit.rb +++ b/lib/gitlab/cycle_analytics/summary/commit.rb @@ -23,8 +23,11 @@ module Gitlab cmd << "--after=#{@from.iso8601}" cmd << sha - raw_output = IO.popen(cmd) { |io| io.read } - raw_output.lines.count + output, status = Gitlab::Popen.popen(cmd) { |io| io.read } + + raise IOError, output unless status.zero? + + output.lines.count end def ref diff --git a/lib/gitlab/cycle_analytics/test_stage.rb b/lib/gitlab/cycle_analytics/test_stage.rb index a105e5f2b1f..7e59745ffef 100644 --- a/lib/gitlab/cycle_analytics/test_stage.rb +++ b/lib/gitlab/cycle_analytics/test_stage.rb @@ -8,13 +8,21 @@ module Gitlab super(*args) end - def stage + def name :test end def description "Total test time for all commits/merges" end + + def stage_query + if @options[:branch] + super.where(build_table[:ref].eq(@options[:branch])) + else + super + end + end end end end diff --git a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb index 725f9a558f5..03b013ffae8 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb @@ -8,10 +8,11 @@ shared_examples 'default query config' do stage: stage_name) end - let(:event) { described_class.new(fetcher: fetcher, options: {}, stage: stage_name) } + let(project) + let(:event) { described_class.new(project: project, stage: stage_name, options: {}) } it 'has the stage attribute' do - expect(event.stage).not_to be_nil + expect(event.name).not_to be_nil end it 'has the projection attributes' do From 30c6703f0afbf570ca3e3613a55afcbc7094c4eb Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 9 Dec 2016 15:23:09 +0100 Subject: [PATCH 21/25] fix specs --- lib/gitlab/cycle_analytics/base_event_fetcher.rb | 8 ++++---- lib/gitlab/cycle_analytics/summary/commit.rb | 2 +- .../cycle_analytics/plan_event_fetcher_spec.rb | 2 +- .../lib/gitlab/cycle_analytics/shared_event_spec.rb | 13 +++---------- .../lib/gitlab/cycle_analytics/shared_stage_spec.rb | 10 +--------- 5 files changed, 10 insertions(+), 25 deletions(-) diff --git a/lib/gitlab/cycle_analytics/base_event_fetcher.rb b/lib/gitlab/cycle_analytics/base_event_fetcher.rb index 8b4ccfd5363..8d10ddf15d7 100644 --- a/lib/gitlab/cycle_analytics/base_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/base_event_fetcher.rb @@ -19,6 +19,10 @@ module Gitlab end.compact end + def order + @order || default_order + end + private def update_author! @@ -37,10 +41,6 @@ module Gitlab base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *projections).order(order.desc) end - def order - @order || default_order - end - def default_order @options[:start_time_attrs].is_a?(Array) ? @options[:start_time_attrs].first : @options[:start_time_attrs] end diff --git a/lib/gitlab/cycle_analytics/summary/commit.rb b/lib/gitlab/cycle_analytics/summary/commit.rb index 61a50762164..7b8faa4d854 100644 --- a/lib/gitlab/cycle_analytics/summary/commit.rb +++ b/lib/gitlab/cycle_analytics/summary/commit.rb @@ -23,7 +23,7 @@ module Gitlab cmd << "--after=#{@from.iso8601}" cmd << sha - output, status = Gitlab::Popen.popen(cmd) { |io| io.read } + output, status = Gitlab::Popen.popen(cmd) raise IOError, output unless status.zero? diff --git a/spec/lib/gitlab/cycle_analytics/plan_event_fetcher_spec.rb b/spec/lib/gitlab/cycle_analytics/plan_event_fetcher_spec.rb index 1c3c1728fc6..2e5dc5b5547 100644 --- a/spec/lib/gitlab/cycle_analytics/plan_event_fetcher_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/plan_event_fetcher_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::CycleAnalytics::PlanEventFetcher do it_behaves_like 'default query config' do context 'no commits' do it 'does not blow up if there are no commits' do - allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:events).and_return([{}]) + allow(event).to receive(:event_result).and_return([{}]) expect { event.fetch }.not_to raise_error end diff --git a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb index 03b013ffae8..9c5e57342e9 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_event_spec.rb @@ -1,18 +1,11 @@ require 'spec_helper' shared_examples 'default query config' do - let(:fetcher) do - Gitlab::CycleAnalytics::MetricsFetcher.new(project: create(:empty_project), - from: 1.day.ago, - branch: nil, - stage: stage_name) - end - - let(project) - let(:event) { described_class.new(project: project, stage: stage_name, options: {}) } + let(:project) { create(:empty_project) } + let(:event) { described_class.new(project: project, stage: stage_name, options: { from: 1.day.ago }) } it 'has the stage attribute' do - expect(event.name).not_to be_nil + expect(event.stage).not_to be_nil end it 'has the projection attributes' do diff --git a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb index c88e3e22f5c..6f3883d80f8 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb @@ -4,18 +4,10 @@ shared_examples 'base stage' do let(:stage) { described_class.new(project: double, options: {}) } before do - allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:median).and_return(1.12) + allow(stage).to receive(:median).and_return(1.12) allow_any_instance_of(Gitlab::CycleAnalytics::BaseEventFetcher).to receive(:event_result).and_return({}) end - it 'has the start attributes' do - expect(stage.start_time_attrs).not_to be_nil - end - - it 'has the end attributes' do - expect(stage.end_time_attrs).not_to be_nil - end - it 'has the median data value' do expect(stage.median_data[:value]).not_to be_nil end From 1b220df56d935f6f9560bdb54f744f17ecfa035b Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 9 Dec 2016 15:28:08 +0100 Subject: [PATCH 22/25] fix bug retrieving medians --- lib/gitlab/cycle_analytics/base_stage.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb index afec16d1818..9143792e044 100644 --- a/lib/gitlab/cycle_analytics/base_stage.rb +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -35,7 +35,7 @@ module Gitlab # cycle analytics stage. interval_query = Arel::Nodes::As.new( cte_table, - subtract_datetimes(base_query, @start_time_attrs, @end_time_attrs, name.to_s)) + subtract_datetimes(base_query.dup, @start_time_attrs, @end_time_attrs, name.to_s)) median_datetime(cte_table, interval_query, name) end From 34875ce6b760980c1539c3ab88af0429e04d90bc Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 9 Dec 2016 16:38:59 +0100 Subject: [PATCH 23/25] fix serializer --- spec/serializers/analytics_stage_serializer_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/serializers/analytics_stage_serializer_spec.rb b/spec/serializers/analytics_stage_serializer_spec.rb index 5597fbed151..f9951826683 100644 --- a/spec/serializers/analytics_stage_serializer_spec.rb +++ b/spec/serializers/analytics_stage_serializer_spec.rb @@ -10,7 +10,7 @@ describe AnalyticsStageSerializer do let(:resource) { Gitlab::CycleAnalytics::CodeStage.new(project: double, options: {}) } before do - allow_any_instance_of(Gitlab::CycleAnalytics::MetricsFetcher).to receive(:median).and_return(1.12) + allow_any_instance_of(Gitlab::CycleAnalytics::BaseStage).to receive(:median).and_return(1.12) allow_any_instance_of(Gitlab::CycleAnalytics::BaseEventFetcher).to receive(:event_result).and_return({}) end From 150a448596dda076f0893facbd621429738aba92 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 12 Jan 2017 12:48:01 +0100 Subject: [PATCH 24/25] refactored a bunch of stuff based on feedback --- app/models/cycle_analytics.rb | 4 ++-- .../cycle_analytics/base_event_fetcher.rb | 2 +- lib/gitlab/cycle_analytics/base_stage.rb | 20 +++++++++---------- lib/gitlab/cycle_analytics/code_stage.rb | 9 +++++---- .../{event.rb => event_fetcher.rb} | 2 +- lib/gitlab/cycle_analytics/issue_stage.rb | 11 +++++----- lib/gitlab/cycle_analytics/plan_stage.rb | 11 +++++----- .../cycle_analytics/production_stage.rb | 9 +++++---- lib/gitlab/cycle_analytics/review_stage.rb | 9 +++++---- lib/gitlab/cycle_analytics/staging_stage.rb | 10 +++++----- lib/gitlab/cycle_analytics/test_stage.rb | 9 +++++---- .../cycle_analytics/shared_stage_spec.rb | 6 +++--- 12 files changed, 54 insertions(+), 48 deletions(-) rename lib/gitlab/cycle_analytics/{event.rb => event_fetcher.rb} (87%) diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index 37ac56310ef..054a6070bb8 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -17,7 +17,7 @@ class CycleAnalytics end def no_stats? - stats.map { |hash| hash[:value] }.compact.empty? + stats.all? { hash[:value].nil? } end def permissions(user:) @@ -32,7 +32,7 @@ class CycleAnalytics def stats_per_stage STAGES.map do |stage_name| - self[stage_name].median_data + self[stage_name].as_json end end end diff --git a/lib/gitlab/cycle_analytics/base_event_fetcher.rb b/lib/gitlab/cycle_analytics/base_event_fetcher.rb index 8d10ddf15d7..0d8791d396b 100644 --- a/lib/gitlab/cycle_analytics/base_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/base_event_fetcher.rb @@ -42,7 +42,7 @@ module Gitlab end def default_order - @options[:start_time_attrs].is_a?(Array) ? @options[:start_time_attrs].first : @options[:start_time_attrs] + [@options[:start_time_attrs]].flatten.first end def serialize(_event) diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb index 9143792e044..7ff15051558 100644 --- a/lib/gitlab/cycle_analytics/base_stage.rb +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -8,17 +8,11 @@ module Gitlab @options = options end - def event - @event ||= Gitlab::CycleAnalytics::Event[name].new(project: @project, - stage: name, - options: event_options) - end - def events - event.fetch + event_fetcher.fetch end - def median_data + def as_json AnalyticsStageSerializer.new.represent(self).as_json end @@ -35,7 +29,7 @@ module Gitlab # cycle analytics stage. interval_query = Arel::Nodes::As.new( cte_table, - subtract_datetimes(base_query.dup, @start_time_attrs, @end_time_attrs, name.to_s)) + subtract_datetimes(base_query.dup, start_time_attrs, end_time_attrs, name.to_s)) median_datetime(cte_table, interval_query, name) end @@ -46,8 +40,14 @@ module Gitlab private + def event_fetcher + @event_fetcher ||= Gitlab::CycleAnalytics::EventFetcher[name].new(project: @project, + stage: name, + options: event_options) + end + def event_options - @options.merge(start_time_attrs: @start_time_attrs, end_time_attrs: @end_time_attrs) + @options.merge(start_time_attrs: start_time_attrs, end_time_attrs: end_time_attrs) end end end diff --git a/lib/gitlab/cycle_analytics/code_stage.rb b/lib/gitlab/cycle_analytics/code_stage.rb index 111c0e99633..d1bc2055ba8 100644 --- a/lib/gitlab/cycle_analytics/code_stage.rb +++ b/lib/gitlab/cycle_analytics/code_stage.rb @@ -1,11 +1,12 @@ module Gitlab module CycleAnalytics class CodeStage < BaseStage - def initialize(*args) - @start_time_attrs = issue_metrics_table[:first_mentioned_in_commit_at] - @end_time_attrs = mr_table[:created_at] + def start_time_attrs + @start_time_attrs ||= issue_metrics_table[:first_mentioned_in_commit_at] + end - super(*args) + def end_time_attrs + @end_time_attrs ||= mr_table[:created_at] end def name diff --git a/lib/gitlab/cycle_analytics/event.rb b/lib/gitlab/cycle_analytics/event_fetcher.rb similarity index 87% rename from lib/gitlab/cycle_analytics/event.rb rename to lib/gitlab/cycle_analytics/event_fetcher.rb index 1ba7bc08ee5..50e126cf00b 100644 --- a/lib/gitlab/cycle_analytics/event.rb +++ b/lib/gitlab/cycle_analytics/event_fetcher.rb @@ -1,6 +1,6 @@ module Gitlab module CycleAnalytics - module Event + module EventFetcher def self.[](stage_name) CycleAnalytics.const_get("#{stage_name.to_s.camelize}EventFetcher") end diff --git a/lib/gitlab/cycle_analytics/issue_stage.rb b/lib/gitlab/cycle_analytics/issue_stage.rb index d320458d7fd..d2068fbc38f 100644 --- a/lib/gitlab/cycle_analytics/issue_stage.rb +++ b/lib/gitlab/cycle_analytics/issue_stage.rb @@ -1,12 +1,13 @@ module Gitlab module CycleAnalytics class IssueStage < BaseStage - def initialize(*args) - @start_time_attrs = issue_table[:created_at] - @end_time_attrs = [issue_metrics_table[:first_associated_with_milestone_at], - issue_metrics_table[:first_added_to_board_at]] + def start_time_attrs + @start_time_attrs ||= issue_table[:created_at] + end - super(*args) + def end_time_attrs + @end_time_attrs ||= [issue_metrics_table[:first_associated_with_milestone_at], + issue_metrics_table[:first_added_to_board_at]] end def name diff --git a/lib/gitlab/cycle_analytics/plan_stage.rb b/lib/gitlab/cycle_analytics/plan_stage.rb index a7164e5c5b7..3b4dfc6a30e 100644 --- a/lib/gitlab/cycle_analytics/plan_stage.rb +++ b/lib/gitlab/cycle_analytics/plan_stage.rb @@ -1,12 +1,13 @@ module Gitlab module CycleAnalytics class PlanStage < BaseStage - def initialize(*args) - @start_time_attrs = [issue_metrics_table[:first_associated_with_milestone_at], - issue_metrics_table[:first_added_to_board_at]] - @end_time_attrs = issue_metrics_table[:first_mentioned_in_commit_at] + def start_time_attrs + @start_time_attrs ||= [issue_metrics_table[:first_associated_with_milestone_at], + issue_metrics_table[:first_added_to_board_at]] + end - super(*args) + def end_time_attrs + @end_time_attrs ||= issue_metrics_table[:first_mentioned_in_commit_at] end def name diff --git a/lib/gitlab/cycle_analytics/production_stage.rb b/lib/gitlab/cycle_analytics/production_stage.rb index eb221c68324..2a6bcc80116 100644 --- a/lib/gitlab/cycle_analytics/production_stage.rb +++ b/lib/gitlab/cycle_analytics/production_stage.rb @@ -3,11 +3,12 @@ module Gitlab class ProductionStage < BaseStage include ProductionHelper - def initialize(*args) - @start_time_attrs = issue_table[:created_at] - @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at] + def start_time_attrs + @start_time_attrs ||= issue_table[:created_at] + end - super(*args) + def end_time_attrs + @end_time_attrs ||= mr_metrics_table[:first_deployed_to_production_at] end def name diff --git a/lib/gitlab/cycle_analytics/review_stage.rb b/lib/gitlab/cycle_analytics/review_stage.rb index 72ce1ed1e16..fbaa3010d81 100644 --- a/lib/gitlab/cycle_analytics/review_stage.rb +++ b/lib/gitlab/cycle_analytics/review_stage.rb @@ -1,11 +1,12 @@ module Gitlab module CycleAnalytics class ReviewStage < BaseStage - def initialize(*args) - @start_time_attrs = mr_table[:created_at] - @end_time_attrs = mr_metrics_table[:merged_at] + def start_time_attrs + @start_time_attrs ||= mr_table[:created_at] + end - super(*args) + def end_time_attrs + @end_time_attrs ||= mr_metrics_table[:merged_at] end def name diff --git a/lib/gitlab/cycle_analytics/staging_stage.rb b/lib/gitlab/cycle_analytics/staging_stage.rb index 398c1b5989a..945909a4d62 100644 --- a/lib/gitlab/cycle_analytics/staging_stage.rb +++ b/lib/gitlab/cycle_analytics/staging_stage.rb @@ -2,12 +2,12 @@ module Gitlab module CycleAnalytics class StagingStage < BaseStage include ProductionHelper + def start_time_attrs + @start_time_attrs ||= mr_metrics_table[:merged_at] + end - def initialize(*args) - @start_time_attrs = mr_metrics_table[:merged_at] - @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at] - - super(*args) + def end_time_attrs + @end_time_attrs ||= mr_metrics_table[:first_deployed_to_production_at] end def name diff --git a/lib/gitlab/cycle_analytics/test_stage.rb b/lib/gitlab/cycle_analytics/test_stage.rb index 7e59745ffef..0079d56e0e4 100644 --- a/lib/gitlab/cycle_analytics/test_stage.rb +++ b/lib/gitlab/cycle_analytics/test_stage.rb @@ -1,11 +1,12 @@ module Gitlab module CycleAnalytics class TestStage < BaseStage - def initialize(*args) - @start_time_attrs = mr_metrics_table[:latest_build_started_at] - @end_time_attrs = mr_metrics_table[:latest_build_finished_at] + def start_time_attrs + @start_time_attrs ||= mr_metrics_table[:latest_build_started_at] + end - super(*args) + def end_time_attrs + @end_time_attrs ||= mr_metrics_table[:latest_build_finished_at] end def name diff --git a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb index 6f3883d80f8..08425acbfc8 100644 --- a/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb @@ -9,15 +9,15 @@ shared_examples 'base stage' do end it 'has the median data value' do - expect(stage.median_data[:value]).not_to be_nil + expect(stage.as_json[:value]).not_to be_nil end it 'has the median data stage' do - expect(stage.median_data[:title]).not_to be_nil + expect(stage.as_json[:title]).not_to be_nil end it 'has the median data description' do - expect(stage.median_data[:description]).not_to be_nil + expect(stage.as_json[:description]).not_to be_nil end it 'has the title' do From 1d775d9712d0c493ae171e37fe507f5160cd7d0e Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 12 Jan 2017 14:32:30 +0100 Subject: [PATCH 25/25] fix spec --- app/models/cycle_analytics.rb | 2 +- lib/gitlab/cycle_analytics/base_stage.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index 054a6070bb8..d2e626c22e8 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -17,7 +17,7 @@ class CycleAnalytics end def no_stats? - stats.all? { hash[:value].nil? } + stats.all? { |hash| hash[:value].nil? } end def permissions(user:) diff --git a/lib/gitlab/cycle_analytics/base_stage.rb b/lib/gitlab/cycle_analytics/base_stage.rb index 7ff15051558..74bbcdcb3dd 100644 --- a/lib/gitlab/cycle_analytics/base_stage.rb +++ b/lib/gitlab/cycle_analytics/base_stage.rb @@ -42,8 +42,8 @@ module Gitlab def event_fetcher @event_fetcher ||= Gitlab::CycleAnalytics::EventFetcher[name].new(project: @project, - stage: name, - options: event_options) + stage: name, + options: event_options) end def event_options