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, 22 insertions, 15 deletions

diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index 15f349c89bb..07d7e19e70d 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -43,23 +43,30 @@ class Deployment < ActiveRecord::Base
     project.repository.is_ancestor?(commit.id, sha)
   end
 
-  def update_merge_request_metrics
-    if environment.update_merge_request_metrics?
-      merge_requests = project.merge_requests.
-                       joins(:metrics).
-                       where(target_branch: self.ref, merge_request_metrics: { first_deployed_to_production_at: nil }).
-                       where("merge_request_metrics.merged_at <= ?", self.created_at)
-
-      if previous_deployment
-        merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.created_at)
-      end
+  def update_merge_request_metrics!
+    return unless environment.update_merge_request_metrics?
+
+    merge_requests = project.merge_requests.
+                     joins(:metrics).
+                     where(target_branch: self.ref, merge_request_metrics: { first_deployed_to_production_at: nil }).
+                     where("merge_request_metrics.merged_at <= ?", self.created_at)
 
-      # Need to use `map` instead of `select` because MySQL doesn't allow `SELECT`ing from the same table
-      # that we're updating.
-      MergeRequest::Metrics.
-        where(merge_request_id: merge_requests.map(&:id), first_deployed_to_production_at: nil).
-        update_all(first_deployed_to_production_at: self.created_at)
+    if previous_deployment
+      merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.created_at)
     end
+
+    # Need to use `map` instead of `select` because MySQL doesn't allow `SELECT`ing from the same table
+    # that we're updating.
+    merge_request_ids =
+      if Gitlab::Database.postgresql?
+        merge_requests.select(:id)
+      elsif Gitlab::Database.mysql?
+        merge_requests.map(&:id)
+      end
+
+    MergeRequest::Metrics.
+      where(merge_request_id: merge_request_ids, first_deployed_to_production_at: nil).
+      update_all(first_deployed_to_production_at: self.created_at)
   end
 
   def previous_deployment