Add finished_at

3 files changed, 23 insertions, 9 deletions

diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index 73689efaecc..fdc0438866b 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -50,6 +50,10 @@ class Deployment < ActiveRecord::Base
     event :cancel do
       transition any - [:canceled] => :canceled
     end
+
+    before_transition any => [:success, :failed, :canceled] do |deployment|
+      deployment.finished_at = Time.now
+    end
   end
 
   def update_status
@@ -116,15 +120,15 @@ class Deployment < ActiveRecord::Base
   end
 
   def update_merge_request_metrics!
-    return unless environment.update_merge_request_metrics?
+    return unless environment.update_merge_request_metrics? && success?
 
     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)
+                     .where("merge_request_metrics.merged_at <= ?", finished_at)
 
     if previous_deployment
-      merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.created_at)
+      merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.finished_at)
     end
 
     # Need to use `map` instead of `select` because MySQL doesn't allow `SELECT`ing from the same table
@@ -138,7 +142,7 @@ class Deployment < ActiveRecord::Base
 
     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)
+      .update_all(first_deployed_to_production_at: finished_at)
   end
 
   def previous_deployment
@@ -156,8 +160,16 @@ class Deployment < ActiveRecord::Base
     @stop_action ||= manual_actions.find_by(name: on_stop)
   end
 
+  def finished_at
+    return self.created_at if status.nil?
+
+    self.finished_at
+  end
+
   def formatted_deployment_time
-    created_at.to_time.in_time_zone.to_s(:medium)
+    return unless success?
+
+    finished_at.to_time.in_time_zone.to_s(:medium)
   end
 
   def has_metrics?
@@ -165,17 +177,17 @@ class Deployment < ActiveRecord::Base
   end
 
   def metrics
-    return {} unless has_metrics?
+    return {} unless has_metrics? && success?
 
     metrics = prometheus_adapter.query(:deployment, self)
-    metrics&.merge(deployment_time: created_at.to_i) || {}
+    metrics&.merge(deployment_time: finished_at.to_i) || {}
   end
 
   def additional_metrics
-    return {} unless has_metrics?
+    return {} unless has_metrics? && success?
 
     metrics = prometheus_adapter.query(:additional_metrics_deployment, self)
-    metrics&.merge(deployment_time: created_at.to_i) || {}
+    metrics&.merge(deployment_time: finished_at.to_i) || {}
   end
 
   private
diff --git a/db/migrate/20181016141739_add_status_to_deployments.rb b/db/migrate/20181016141739_add_status_to_deployments.rb
index 2cf4501c9e3..eda27068552 100644
--- a/db/migrate/20181016141739_add_status_to_deployments.rb
+++ b/db/migrate/20181016141739_add_status_to_deployments.rb
@@ -5,5 +5,6 @@ class AddStatusToDeployments < ActiveRecord::Migration
 
   def change
     add_column :deployments, :status, :integer, limit: 2
+    add_column :deployments, :finished_at, :datetime_with_timezone
   end
 end
diff --git a/db/schema.rb b/db/schema.rb
index 1dd2c7636d6..cdfb91204ea 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -773,6 +773,7 @@ ActiveRecord::Schema.define(version: 20181016141739) do
     t.datetime "updated_at"
     t.string "on_stop"
     t.integer "status", limit: 2
+    t.datetime_with_timezone "finished_at"
   end
 
   add_index "deployments", ["created_at"], name: "index_deployments_on_created_at", using: :btree