Implement a second round of review comments from @DouweM.

- Don't use `TableReferences` - using `.arel_table` is shorter!
- Move some database-related code to `Gitlab::Database`
- Remove the `MergeRequest#issues_closed` and
  `Issue#closed_by_merge_requests`  associations. They were either
  shadowing or were too similar to existing methods. They are not being
  used anywhere, so it's better to remove them to reduce confusion.
- Use Rails 3-style validations
- Index for `MergeRequest::Metrics#first_deployed_to_production_at`
- Only include `CycleAnalyticsHelpers::TestGeneration` for specs that
  need it.
- Other minor refactorings.

1 files changed, 29 insertions, 37 deletions

diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb
index 3ca160252ff..be295487fd2 100644
--- a/app/models/cycle_analytics.rb
+++ b/app/models/cycle_analytics.rb
@@ -5,55 +5,54 @@ class CycleAnalytics
   def initialize(project, from:)
     @project = project
     @from = from
-    @summary = Summary.new(project, from: from)
   end
 
   def summary
-    @summary
+    @summary ||= Summary.new(@project, from: @from)
   end
 
   def issue
     calculate_metric(:issue,
-                     TableReferences.issues[:created_at],
-                     [TableReferences.issue_metrics[:first_associated_with_milestone_at],
-                      TableReferences.issue_metrics[:first_added_to_board_at]])
+                     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 plan
     calculate_metric(:plan,
-                     [TableReferences.issue_metrics[:first_associated_with_milestone_at],
-                      TableReferences.issue_metrics[:first_added_to_board_at]],
-                     TableReferences.issue_metrics[:first_mentioned_in_commit_at])
+                     [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
     calculate_metric(:code,
-                     TableReferences.issue_metrics[:first_mentioned_in_commit_at],
-                     TableReferences.merge_requests[:created_at])
+                     Issue::Metrics.arel_table[:first_mentioned_in_commit_at],
+                     MergeRequest.arel_table[:created_at])
   end
 
   def test
     calculate_metric(:test,
-                     TableReferences.merge_request_metrics[:latest_build_started_at],
-                     TableReferences.merge_request_metrics[:latest_build_finished_at])
+                     MergeRequest::Metrics.arel_table[:latest_build_started_at],
+                     MergeRequest::Metrics.arel_table[:latest_build_finished_at])
   end
 
   def review
     calculate_metric(:review,
-                     TableReferences.merge_requests[:created_at],
-                     TableReferences.merge_request_metrics[:merged_at])
+                     MergeRequest.arel_table[:created_at],
+                     MergeRequest::Metrics.arel_table[:merged_at])
   end
 
   def staging
     calculate_metric(:staging,
-                     TableReferences.merge_request_metrics[:merged_at],
-                     TableReferences.merge_request_metrics[:first_deployed_to_production_at])
+                     MergeRequest::Metrics.arel_table[:merged_at],
+                     MergeRequest::Metrics.arel_table[:first_deployed_to_production_at])
   end
 
   def production
     calculate_metric(:production,
-                     TableReferences.issues[:created_at],
-                     TableReferences.merge_request_metrics[:first_deployed_to_production_at])
+                     Issue.arel_table[:created_at],
+                     MergeRequest::Metrics.arel_table[:first_deployed_to_production_at])
   end
 
   private
@@ -69,9 +68,7 @@ class CycleAnalytics
       cte_table,
       subtract_datetimes(base_query, end_time_attrs, start_time_attrs, name.to_s))
 
-    median_queries = Array.wrap(median_datetime(cte_table, interval_query, name))
-    results = median_queries.map { |query| run_query(query) }
-    extract_median(results).presence
+    median_datetime(cte_table, interval_query, name)
   end
 
   # Join table with a row for every <issue,merge_request> pair (where the merge request
@@ -79,27 +76,22 @@ class CycleAnalytics
   # are loaded with an inner join, so issues / merge requests without metrics are
   # automatically excluded.
   def base_query
-    arel_table = TableReferences.merge_requests_closing_issues
+    arel_table = MergeRequestsClosingIssues.arel_table
 
     # Load issues
-    query = arel_table.join(TableReferences.issues).on(TableReferences.issues[:id].eq(arel_table[:issue_id])).
-            join(TableReferences.issue_metrics).on(TableReferences.issues[:id].eq(TableReferences.issue_metrics[:issue_id])).
-            where(TableReferences.issues[:project_id].eq(@project.id)).
-            where(TableReferences.issues[:deleted_at].eq(nil)).
-            where(TableReferences.issues[:created_at].gteq(@from))
+    query = arel_table.join(Issue.arel_table).on(Issue.arel_table[:id].eq(arel_table[:issue_id])).
+            join(Issue::Metrics.arel_table).on(Issue.arel_table[:id].eq(Issue::Metrics.arel_table[:issue_id])).
+            where(Issue.arel_table[:project_id].eq(@project.id)).
+            where(Issue.arel_table[:deleted_at].eq(nil)).
+            where(Issue.arel_table[:created_at].gteq(@from))
 
     # Load merge_requests
-    query = query.join(TableReferences.merge_requests, Arel::Nodes::OuterJoin).
-            on(TableReferences.merge_requests[:id].eq(arel_table[:merge_request_id])).
-            join(TableReferences.merge_request_metrics).
-            on(TableReferences.merge_requests[:id].eq(TableReferences.merge_request_metrics[:merge_request_id]))
+    query = query.join(MergeRequest.arel_table, Arel::Nodes::OuterJoin).
+            on(MergeRequest.arel_table[:id].eq(arel_table[:merge_request_id])).
+            join(MergeRequest::Metrics.arel_table).
+            on(MergeRequest.arel_table[:id].eq(MergeRequest::Metrics.arel_table[:merge_request_id]))
 
     # Limit to merge requests that have been deployed to production after `@from`
-    query.where(TableReferences.merge_request_metrics[:first_deployed_to_production_at].gteq(@from))
-  end
-
-  def run_query(query)
-    query = query.to_sql unless query.is_a?(String)
-    ActiveRecord::Base.connection.execute(query)
+    query.where(MergeRequest::Metrics.arel_table[:first_deployed_to_production_at].gteq(@from))
   end
 end