Merge HasStatus into Pipeline and CommitStatus,21948-warning-icon-allowed-fail

also remove unused methods. Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6801#note_18671943

5 files changed, 202 insertions, 252 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 672374d874f..611bd592fcc 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -1,7 +1,6 @@
 module Ci
   class Pipeline < ActiveRecord::Base
     extend Ci::Model
-    include HasStatus
     include Importable
     include AfterCommitQueue
 
@@ -23,7 +22,10 @@ module Ci
 
     delegate :stages, to: :statuses
 
-    # Used in HasStatus
+    def self.status
+      all.pluck(status_sql).first
+    end
+
     def self.status_sql
       total = all.select('count(*)').to_sql
       created = all.created.select('count(*)').to_sql
@@ -47,15 +49,13 @@ module Ci
     end
 
     def self.available_statuses
-      super << 'success_with_warnings'
+      %w[created pending running
+         success success_with_warnings
+         failed canceled skipped]
     end
 
     def self.completed_statuses
-      super << 'success_with_warnings'
-    end
-
-    scope :success_with_warnings, -> do
-      where(status: 'success_with_warnings')
+      %w[success success_with_warnings failed canceled]
     end
 
     validates :status, inclusion: { in: available_statuses }
@@ -134,6 +134,28 @@ module Ci
       end
     end
 
+    scope :created, -> { where(status: 'created') }
+    scope :relevant, -> { where.not(status: 'created') }
+    scope :running, -> { where(status: 'running') }
+    scope :pending, -> { where(status: 'pending') }
+    scope :success, -> { where(status: 'success') }
+    scope :failed, -> { where(status: 'failed')  }
+    scope :canceled, -> { where(status: 'canceled') }
+    scope :skipped, -> { where(status: 'skipped') }
+    scope :running_or_pending, -> { where(status: [:running, :pending]) }
+    scope :finished, -> { where(status: [:success, :failed, :canceled]) }
+    scope :success_with_warnings, -> do
+      where(status: 'success_with_warnings')
+    end
+
+    def active?
+      %w[pending running].include?(status)
+    end
+
+    def success?
+      %w[success success_with_warnings].include?(status)
+    end
+
     # ref can't be HEAD or SHA, can only be branch/tag name
     def self.latest_successful_for(ref)
       where(ref: ref).order(id: :desc).success.first
@@ -201,10 +223,6 @@ module Ci
       !tag?
     end
 
-    def success?
-      status == 'success' || status == 'success_with_warnings'
-    end
-
     def manual_actions
       builds.latest.manual_actions
     end
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 7a58ca61e31..9c7d7849ad0 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -1,5 +1,4 @@
 class CommitStatus < ActiveRecord::Base
-  include HasStatus
   include Importable
   include AfterCommitQueue
 
@@ -44,7 +43,14 @@ class CommitStatus < ActiveRecord::Base
   scope :latest_ci_stages, -> { latest.ordered.includes(project: :namespace) }
   scope :retried_ci_stages, -> { retried.ordered.includes(project: :namespace) }
 
-  # Used in HasStatus
+  def self.all_state_names
+    state_machines.values.flat_map(&:states).flat_map { |s| s.map(&:name) }
+  end
+
+  def self.status
+    all.pluck(status_sql).first
+  end
+
   def self.status_sql
     total = exclude_ignored.select('count(*)').to_sql
     created = exclude_ignored.created.select('count(*)').to_sql
@@ -67,6 +73,14 @@ class CommitStatus < ActiveRecord::Base
     END)"
   end
 
+  def self.available_statuses
+    %w[created pending running success failed canceled skipped]
+  end
+
+  def self.ordered_statuses
+    %w[failed pending running canceled success skipped]
+  end
+
   validates :status, inclusion: { in: available_statuses }
 
   state_machine :status, initial: :created do
@@ -136,6 +150,25 @@ class CommitStatus < ActiveRecord::Base
     end
   end
 
+  scope :created, -> { where(status: 'created') }
+  scope :relevant, -> { where.not(status: 'created') }
+  scope :running, -> { where(status: 'running') }
+  scope :pending, -> { where(status: 'pending') }
+  scope :success, -> { where(status: 'success') }
+  scope :failed, -> { where(status: 'failed')  }
+  scope :canceled, -> { where(status: 'canceled') }
+  scope :skipped, -> { where(status: 'skipped') }
+  scope :running_or_pending, -> { where(status: [:running, :pending]) }
+  scope :finished, -> { where(status: [:success, :failed, :canceled]) }
+
+  def active?
+    %w[pending running].include?(status)
+  end
+
+  def complete?
+    %w[success failed canceled].include?(status)
+  end
+
   delegate :sha, :short_sha, to: :pipeline
 
   def before_sha
@@ -169,7 +202,11 @@ class CommitStatus < ActiveRecord::Base
   end
 
   def duration
-    calculate_duration
+    if started_at && finished_at
+      finished_at - started_at
+    elsif started_at
+      Time.now - started_at
+    end
   end
 
   def stuck?
diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb
deleted file mode 100644
index 59354247d7e..00000000000
--- a/app/models/concerns/has_status.rb
+++ /dev/null
@@ -1,76 +0,0 @@
-module HasStatus
-  extend ActiveSupport::Concern
-
-  class_methods do
-    def available_statuses
-      %w[created pending running success failed canceled skipped]
-    end
-
-    def started_statuses
-      %w[running success failed skipped]
-    end
-
-    def active_statuses
-      %w[pending running]
-    end
-
-    def completed_statuses
-      %w[success failed canceled]
-    end
-
-    def ordered_statuses
-      %w[failed pending running canceled success skipped]
-    end
-
-    def status
-      all.pluck(status_sql).first
-    end
-
-    def started_at
-      all.minimum(:started_at)
-    end
-
-    def finished_at
-      all.maximum(:finished_at)
-    end
-
-    def all_state_names
-      state_machines.values.flat_map(&:states).flat_map { |s| s.map(&:name) }
-    end
-  end
-
-  included do
-    scope :created, -> { where(status: 'created') }
-    scope :relevant, -> { where.not(status: 'created') }
-    scope :running, -> { where(status: 'running') }
-    scope :pending, -> { where(status: 'pending') }
-    scope :success, -> { where(status: 'success') }
-    scope :failed, -> { where(status: 'failed')  }
-    scope :canceled, -> { where(status: 'canceled') }
-    scope :skipped, -> { where(status: 'skipped') }
-    scope :running_or_pending, -> { where(status: [:running, :pending]) }
-    scope :finished, -> { where(status: [:success, :failed, :canceled]) }
-  end
-
-  def started?
-    self.class.started_statuses.include?(status) && started_at
-  end
-
-  def active?
-    self.class.active_statuses.include?(status)
-  end
-
-  def complete?
-    self.class.completed_statuses.include?(status)
-  end
-
-  private
-
-  def calculate_duration
-    if started_at && finished_at
-      finished_at - started_at
-    elsif started_at
-      Time.now - started_at
-    end
-  end
-end
diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb
index 80c2a1bc7a9..2c8e0a83f3f 100644
--- a/spec/models/commit_status_spec.rb
+++ b/spec/models/commit_status_spec.rb
@@ -35,32 +35,6 @@ describe CommitStatus, models: true do
     it { is_expected.to eq(commit_status.user) }
   end
 
-  describe '#started?' do
-    subject { commit_status.started? }
-
-    context 'without started_at' do
-      before { commit_status.started_at = nil }
-
-      it { is_expected.to be_falsey }
-    end
-
-    %w[running success failed].each do |status|
-      context "if commit status is #{status}" do
-        before { commit_status.status = status }
-
-        it { is_expected.to be_truthy }
-      end
-    end
-
-    %w[pending canceled].each do |status|
-      context "if commit status is #{status}" do
-        before { commit_status.status = status }
-
-        it { is_expected.to be_falsey }
-      end
-    end
-  end
-
   describe '#active?' do
     subject { commit_status.active? }
 
@@ -277,4 +251,136 @@ describe CommitStatus, models: true do
       end
     end
   end
+
+  describe '.status' do
+    subject { CommitStatus.status }
+
+    shared_examples 'build status summary' do
+      context 'all successful' do
+        let!(:statuses) { Array.new(2) { create(type, status: :success) } }
+        it { is_expected.to eq 'success' }
+      end
+
+      context 'at least one failed' do
+        let!(:statuses) do
+          [create(type, status: :success), create(type, status: :failed)]
+        end
+
+        it { is_expected.to eq 'failed' }
+      end
+
+      context 'at least one running' do
+        let!(:statuses) do
+          [create(type, status: :success), create(type, status: :running)]
+        end
+
+        it { is_expected.to eq 'running' }
+      end
+
+      context 'at least one pending' do
+        let!(:statuses) do
+          [create(type, status: :success), create(type, status: :pending)]
+        end
+
+        it { is_expected.to eq 'running' }
+      end
+
+      context 'success and failed but allowed to fail' do
+        let!(:statuses) do
+          [create(type, status: :success),
+           create(type, status: :failed, allow_failure: true)]
+        end
+
+        it { is_expected.to eq 'success_with_warnings' }
+      end
+
+      context 'one failed but allowed to fail' do
+        let!(:statuses) do
+          [create(type, status: :failed, allow_failure: true)]
+        end
+
+        it { is_expected.to eq 'success_with_warnings' }
+      end
+
+      context 'one canceled but allowed to fail' do
+        let!(:statuses) do
+          [create(type, status: :success),
+           create(type, status: :canceled, allow_failure: true)]
+        end
+
+        it { is_expected.to eq 'success_with_warnings' }
+      end
+
+      context 'one success with allowed to fail' do
+        let!(:statuses) do
+          [create(type, status: :success),
+           create(type, status: :success, allow_failure: true)]
+        end
+
+        it { is_expected.to eq 'success' }
+      end
+
+      context 'success and canceled' do
+        let!(:statuses) do
+          [create(type, status: :success), create(type, status: :canceled)]
+        end
+
+        it { is_expected.to eq 'canceled' }
+      end
+
+      context 'one failed and one canceled' do
+        let!(:statuses) do
+          [create(type, status: :failed), create(type, status: :canceled)]
+        end
+
+        it { is_expected.to eq 'failed' }
+      end
+
+      context 'one failed but allowed to fail and one canceled' do
+        let!(:statuses) do
+          [create(type, status: :failed, allow_failure: true),
+           create(type, status: :canceled)]
+        end
+
+        it { is_expected.to eq 'canceled' }
+      end
+
+      context 'one running one canceled' do
+        let!(:statuses) do
+          [create(type, status: :running), create(type, status: :canceled)]
+        end
+
+        it { is_expected.to eq 'running' }
+      end
+
+      context 'all canceled' do
+        let!(:statuses) do
+          [create(type, status: :canceled), create(type, status: :canceled)]
+        end
+
+        it { is_expected.to eq 'canceled' }
+      end
+
+      context 'one finished and second running but allowed to fail' do
+        let!(:statuses) do
+          [create(type, status: :success),
+           create(type, status: :running, allow_failure: true)]
+        end
+
+        it { is_expected.to eq 'running' }
+      end
+    end
+
+    context 'ci build statuses' do
+      let(:type) { :ci_build }
+
+      it_behaves_like 'build status summary'
+    end
+
+    context 'generic commit statuses' do
+      let(:type) { :generic_commit_status }
+
+      it_behaves_like 'build status summary'
+    end
+  end
 end
diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb
deleted file mode 100644
index c7ae72b79a8..00000000000
--- a/spec/models/concerns/has_status_spec.rb
+++ /dev/null
@@ -1,135 +0,0 @@
-require 'spec_helper'
-
-describe HasStatus do
-  describe '.status' do
-    subject { CommitStatus.status }
-
-    shared_examples 'build status summary' do
-      context 'all successful' do
-        let!(:statuses) { Array.new(2) { create(type, status: :success) } }
-        it { is_expected.to eq 'success' }
-      end
-
-      context 'at least one failed' do
-        let!(:statuses) do
-          [create(type, status: :success), create(type, status: :failed)]
-        end
-
-        it { is_expected.to eq 'failed' }
-      end
-
-      context 'at least one running' do
-        let!(:statuses) do
-          [create(type, status: :success), create(type, status: :running)]
-        end
-
-        it { is_expected.to eq 'running' }
-      end
-
-      context 'at least one pending' do
-        let!(:statuses) do
-          [create(type, status: :success), create(type, status: :pending)]
-        end
-
-        it { is_expected.to eq 'running' }
-      end
-
-      context 'success and failed but allowed to fail' do
-        let!(:statuses) do
-          [create(type, status: :success),
-           create(type, status: :failed, allow_failure: true)]
-        end
-
-        it { is_expected.to eq 'success_with_warnings' }
-      end
-
-      context 'one failed but allowed to fail' do
-        let!(:statuses) do
-          [create(type, status: :failed, allow_failure: true)]
-        end
-
-        it { is_expected.to eq 'success_with_warnings' }
-      end
-
-      context 'one canceled but allowed to fail' do
-        let!(:statuses) do
-          [create(type, status: :success),
-           create(type, status: :canceled, allow_failure: true)]
-        end
-
-        it { is_expected.to eq 'success_with_warnings' }
-      end
-
-      context 'one success with allowed to fail' do
-        let!(:statuses) do
-          [create(type, status: :success),
-           create(type, status: :success, allow_failure: true)]
-        end
-
-        it { is_expected.to eq 'success' }
-      end
-
-      context 'success and canceled' do
-        let!(:statuses) do
-          [create(type, status: :success), create(type, status: :canceled)]
-        end
-
-        it { is_expected.to eq 'canceled' }
-      end
-
-      context 'one failed and one canceled' do
-        let!(:statuses) do
-          [create(type, status: :failed), create(type, status: :canceled)]
-        end
-
-        it { is_expected.to eq 'failed' }
-      end
-
-      context 'one failed but allowed to fail and one canceled' do
-        let!(:statuses) do
-          [create(type, status: :failed, allow_failure: true),
-           create(type, status: :canceled)]
-        end
-
-        it { is_expected.to eq 'canceled' }
-      end
-
-      context 'one running one canceled' do
-        let!(:statuses) do
-          [create(type, status: :running), create(type, status: :canceled)]
-        end
-
-        it { is_expected.to eq 'running' }
-      end
-
-      context 'all canceled' do
-        let!(:statuses) do
-          [create(type, status: :canceled), create(type, status: :canceled)]
-        end
-
-        it { is_expected.to eq 'canceled' }
-      end
-
-      context 'one finished and second running but allowed to fail' do
-        let!(:statuses) do
-          [create(type, status: :success),
-           create(type, status: :running, allow_failure: true)]
-        end
-
-        it { is_expected.to eq 'running' }
-      end
-    end
-
-    context 'ci build statuses' do
-      let(:type) { :ci_build }
-
-      it_behaves_like 'build status summary'
-    end
-
-    context 'generic commit statuses' do
-      let(:type) { :generic_commit_status }
-
-      it_behaves_like 'build status summary'
-    end
-  end
-end