From 680bc629dc473d877218b1ed9351fac4020d4657 Mon Sep 17 00:00:00 2001
From: Lucas Di Pentima <lucas@curoverse.com>
Date: Mon, 19 Dec 2016 17:26:38 -0300
Subject: [PATCH] 10671: Set up values for pipeline instance started_at and
 finished_at attributes on state changes. Updated tests accordingly.

---
 services/api/app/models/pipeline_instance.rb     | 12 ++++++++++++
 services/api/test/unit/pipeline_instance_test.rb |  8 ++++++++
 2 files changed, 20 insertions(+)

diff --git a/services/api/app/models/pipeline_instance.rb b/services/api/app/models/pipeline_instance.rb
index 77a0736b00..dada5d6836 100644
--- a/services/api/app/models/pipeline_instance.rb
+++ b/services/api/app/models/pipeline_instance.rb
@@ -12,6 +12,7 @@ class PipelineInstance < ArvadosModel
   before_validation :verify_status
   before_create :set_state_before_save
   before_save :set_state_before_save
+  before_save :update_timestamps_when_state_changes
 
   api_accessible :user, extend: :common do |t|
     t.add :pipeline_template_uuid
@@ -136,4 +137,15 @@ class PipelineInstance < ArvadosModel
     end
   end
 
+  def update_timestamps_when_state_changes
+    return if not (state_changed? or new_record?)
+
+    case state
+    when RunningOnServer, RunningOnClient
+      self.started_at ||= db_current_time
+    when Failed, Complete
+      self.finished_at ||= db_current_time
+    end
+  end
+
 end
diff --git a/services/api/test/unit/pipeline_instance_test.rb b/services/api/test/unit/pipeline_instance_test.rb
index fc40d06b2c..05ba135700 100644
--- a/services/api/test/unit/pipeline_instance_test.rb
+++ b/services/api/test/unit/pipeline_instance_test.rb
@@ -38,6 +38,8 @@ class PipelineInstanceTest < ActiveSupport::TestCase
     pi = PipelineInstance.find_by_uuid 'zzzzz-d1hrv-f4gneyn6br1xize'
     assert_equal PipelineInstance::New, pi.state, 'expected state to be New after adding component with input'
     assert_equal pi.components.size, 1, 'expected one component'
+    assert_nil pi.started_at, 'expected started_at to be nil on new pipeline instance'
+    assert_nil pi.finished_at, 'expected finished_at to be nil on new pipeline instance'
 
     # add a component with no input not required
     component = {'script_parameters' => {"input_not_provided" => {"required" => false}}}
@@ -61,6 +63,8 @@ class PipelineInstanceTest < ActiveSupport::TestCase
     pi.save
     pi = PipelineInstance.find_by_uuid 'zzzzz-d1hrv-f4gneyn6br1xize'
     assert_equal PipelineInstance::RunningOnServer, pi.state, 'expected state to be RunningOnServer after updating state to RunningOnServer'
+    assert_not_nil pi.started_at, 'expected started_at to have a value on a running pipeline instance'
+    assert_nil pi.finished_at, 'expected finished_at to be nil on a running pipeline instance'
 
     pi.state = PipelineInstance::Paused
     pi.save
@@ -71,6 +75,8 @@ class PipelineInstanceTest < ActiveSupport::TestCase
     pi.save
     pi = PipelineInstance.find_by_uuid 'zzzzz-d1hrv-f4gneyn6br1xize'
     assert_equal PipelineInstance::Complete, pi.state, 'expected state to be Complete after updating state to Complete'
+    assert_not_nil pi.started_at, 'expected started_at to have a value on a completed pipeline instance'
+    assert_not_nil pi.finished_at, 'expected finished_at to have a value on a completed pipeline instance'
 
     pi.state = 'bogus'
     pi.save
@@ -81,6 +87,8 @@ class PipelineInstanceTest < ActiveSupport::TestCase
     pi.save
     pi = PipelineInstance.find_by_uuid 'zzzzz-d1hrv-f4gneyn6br1xize'
     assert_equal PipelineInstance::Failed, pi.state, 'expected state to be Failed after updating state to Failed'
+    assert_not_nil pi.started_at, 'expected started_at to have a value on a failed pipeline instance'
+    assert_not_nil pi.finished_at, 'expected finished_at to have a value on a failed pipeline instance'
   end
 
   test "update attributes for pipeline with two components" do
-- 
2.30.2