refactor cycle analytics - updated based on MR feedback

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

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

app/models/cycle_analytics.rb

 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

app/serializers/analytics_stage_entity.rb

 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|

lib/gitlab/cycle_analytics/base_event.rb

@@ -2,13 +2,13 @@ module Gitlab
   module CycleAnalytics
     class BaseEvent
       include MetricsTables
-      attr_reader :stage, :start_time_attrs, :end_time_attrs, :projections, :query
-      def initialize(fetcher:, stage:, options:)
+      include ClassNameUtil
+      attr_reader :start_time_attrs, :end_time_attrs, :projections, :query
+      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!

lib/gitlab/cycle_analytics/base_stage.rb

 module Gitlab
   module CycleAnalytics
     class BaseStage
-      attr_reader :stage, :description
-      def initialize(project:, options:, stage:)
+      include ClassNameUtil
+      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

lib/gitlab/cycle_analytics/class_name_util.rb

+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

lib/gitlab/cycle_analytics/code_stage.rb

 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

lib/gitlab/cycle_analytics/event.rb

+module Gitlab
+  module CycleAnalytics
+    module Event
+      def self.[](stage_name)
+        const_get("::Gitlab::CycleAnalytics::#{stage_name.to_s.camelize}Event")
+      end
+    end
+  end
+end

lib/gitlab/cycle_analytics/issue_stage.rb

 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

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).

lib/gitlab/cycle_analytics/plan_stage.rb

 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

lib/gitlab/cycle_analytics/production_stage.rb

 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

lib/gitlab/cycle_analytics/review_stage.rb

 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

lib/gitlab/cycle_analytics/stage.rb

+module Gitlab
+  module CycleAnalytics
+    module Stage
+      def self.[](stage_name)
+        const_get("::Gitlab::CycleAnalytics::#{stage_name.to_s.camelize}Stage")
+      end
+    end
+  end
+end

lib/gitlab/cycle_analytics/staging_stage.rb

 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

lib/gitlab/cycle_analytics/test_stage.rb

 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

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

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

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