summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJames Lopez <james@jameslopez.es>2016-12-01 11:21:24 +0100
committerJames Lopez <james@jameslopez.es>2017-01-17 11:32:55 +0100
commitb8056669849729cab5700466a7fae6dc6b2743b2 (patch)
tree788c5a8bf535b48d3e1b8593f6be8a45205e317e
parent8183e848648bc737e4a09f76f4f55ee1cf106b26 (diff)
downloadgitlab-ce-b8056669849729cab5700466a7fae6dc6b2743b2.tar.gz
refactor cycle analytics - updated based on MR feedback
-rw-r--r--app/controllers/projects/cycle_analytics/events_controller.rb16
-rw-r--r--app/controllers/projects/cycle_analytics_controller.rb4
-rw-r--r--app/models/cycle_analytics.rb12
-rw-r--r--app/serializers/analytics_stage_entity.rb4
-rw-r--r--lib/gitlab/cycle_analytics/base_event.rb10
-rw-r--r--lib/gitlab/cycle_analytics/base_stage.rb19
-rw-r--r--lib/gitlab/cycle_analytics/class_name_util.rb13
-rw-r--r--lib/gitlab/cycle_analytics/code_stage.rb12
-rw-r--r--lib/gitlab/cycle_analytics/event.rb9
-rw-r--r--lib/gitlab/cycle_analytics/issue_stage.rb14
-rw-r--r--lib/gitlab/cycle_analytics/metrics_fetcher.rb2
-rw-r--r--lib/gitlab/cycle_analytics/plan_stage.rb14
-rw-r--r--lib/gitlab/cycle_analytics/production_stage.rb12
-rw-r--r--lib/gitlab/cycle_analytics/review_stage.rb12
-rw-r--r--lib/gitlab/cycle_analytics/stage.rb9
-rw-r--r--lib/gitlab/cycle_analytics/staging_stage.rb12
-rw-r--r--lib/gitlab/cycle_analytics/test_stage.rb12
-rw-r--r--spec/lib/gitlab/cycle_analytics/events_spec.rb2
-rw-r--r--spec/lib/gitlab/cycle_analytics/shared_stage_spec.rb2
-rw-r--r--spec/serializers/analytics_stage_serializer_spec.rb2
20 files changed, 107 insertions, 85 deletions
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