Change timeout_source to enum

6 files changed, 35 insertions, 16 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 2f01a098b0e..cbd2cc6c58f 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -93,6 +93,12 @@ module Ci
 
     chronic_duration_attr_reader :used_timeout_human_readable, :used_timeout
 
+    enum timeout_source: {
+        unknown_timeout_source: nil,
+        project_timeout_source: 1,
+        runner_timeout_source: 2
+    }
+
     class << self
       # This is needed for url_for to work,
       # as the controller is JobsController
@@ -123,10 +129,6 @@ module Ci
       end
 
       after_transition pending: :running do |build|
-        build.used_timeout = build.timeout
-        build.timeout_source = build.timeout < build.project.build_timeout ? 'runner' : 'project'
-        build.save!
-
         build.run_after_commit do
           BuildHooksWorker.perform_async(id)
         end
@@ -160,6 +162,11 @@ module Ci
       before_transition any => [:running] do |build|
         build.validates_dependencies! unless Feature.enabled?('ci_disable_validates_dependencies')
       end
+
+      before_transition pending: :running do |build|
+        build.used_timeout = build.timeout
+        build.timeout_source = build.timeout < build.project.build_timeout ? :runner_timeout_source : :project_timeout_source
+      end
     end
 
     def detailed_status(current_user)
diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index 255475e1fe6..be6cc2e70b1 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -1,5 +1,12 @@
 module Ci
   class BuildPresenter < Gitlab::View::Presenter::Delegated
+
+    TIMEOUT_SOURCES = {
+        unknown_timeout_source: nil,
+        project_timeout_source: 'project',
+        runner_timeout_source: 'runner'
+    }.freeze
+
     presents :build
 
     def erased_by_user?
@@ -18,6 +25,13 @@ module Ci
       end
     end
 
+    def timeout_source
+      return unless build.timeout_source?
+
+      TIMEOUT_SOURCES[build.timeout_source.to_sym] ||
+          build.timeout_source
+    end
+
     def trigger_variables
       return [] unless trigger_request
 
diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb
index 17769790371..0ffc537dfd8 100644
--- a/app/serializers/build_details_entity.rb
+++ b/app/serializers/build_details_entity.rb
@@ -7,7 +7,7 @@ class BuildDetailsEntity < JobEntity
 
   expose :timeout, if: -> (*) { !build.used_timeout.nil? } do |build|
     { value: build.used_timeout_human_readable,
-      source: build.timeout_source }
+      source: build.present.timeout_source }
   end
 
   expose :erased_by, if: -> (*) { build.erased? }, using: UserEntity
diff --git a/db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb b/db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb
index 435fba712aa..18c4fd5bae4 100644
--- a/db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb
+++ b/db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb
@@ -5,6 +5,6 @@ class AddUsedTimeoutAndTimeoutSourceColumnsToCiBuilds < ActiveRecord::Migration
 
   def change
     add_column :ci_builds, :used_timeout, :integer
-    add_column :ci_builds, :timeout_source, :string
+    add_column :ci_builds, :timeout_source, :integer
   end
 end
diff --git a/db/schema.rb b/db/schema.rb
index 61f15944f5a..9b126385045 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -312,7 +312,7 @@ ActiveRecord::Schema.define(version: 20180327101207) do
     t.boolean "protected"
     t.integer "failure_reason"
     t.integer "used_timeout"
-    t.string "timeout_source"
+    t.integer "timeout_source"
   end
 
   add_index "ci_builds", ["artifacts_expire_at"], name: "index_ci_builds_on_artifacts_expire_at", where: "(artifacts_file <> ''::text)", using: :btree
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 78842bd8a92..c3624158b76 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -2046,20 +2046,18 @@ describe Ci::Build do
     end
 
     shared_examples 'saves data on transition' do
-      it 'saves used_timeout and timeout_source on transition' do
-        expect(job.used_timeout).to be_nil
-        expect(job.timeout_source).to be_nil
-
-        job.run!
+      it 'saves used_timeout' do
+        expect { job.run! }.to change { job.reload.used_timeout }.from(nil).to(expected_timeout)
+      end
 
-        expect(job.used_timeout).to eq(expected_timeout)
-        expect(job.timeout_source).to eq(expected_timeout_source)
+      it 'saves timeout_source' do
+        expect { job.run! }.to change { job.reload.timeout_source }.from('unknown_timeout_source').to(expected_timeout_source)
       end
     end
 
     context 'when runner timeout overrides project timeout' do
       let(:expected_timeout) { 900 }
-      let(:expected_timeout_source) { 'Runner' }
+      let(:expected_timeout_source) { 'runner_timeout_source' }
 
       before do
         runner.maximum_job_timeout = 900
@@ -2071,7 +2069,7 @@ describe Ci::Build do
 
     context "when runner timeout doesn't override project timeout" do
       let(:expected_timeout) { 1800 }
-      let(:expected_timeout_source) { 'Project' }
+      let(:expected_timeout_source) { 'project_timeout_source' }
 
       before do
         runner.maximum_job_timeout = 3600