Move build erasable implementation from concern to model

Discussion:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2560#note_3659905

app/controllers/projects/builds_controller.rb

@@ -57,9 +57,9 @@ class Projects::BuildsController < Projects::ApplicationController
   end
 
   def erase
-    @build.erase!(erased_by: current_user)
+    @build.erase(erased_by: current_user)
     redirect_to namespace_project_build_path(project.namespace, project, @build),
-                notice: "Build ##{@build.id} has been sucessfully erased!"
+                notice: "Build has been sucessfully erased!"
   end
 
   private

app/models/ci/build.rb

@@ -38,12 +38,12 @@
 module Ci
   class Build < CommitStatus
     include Gitlab::Application.routes.url_helpers
-    include Build::Erasable
 
     LAZY_ATTRIBUTES = ['trace']
 
     belongs_to :runner, class_name: 'Ci::Runner'
     belongs_to :trigger_request, class_name: 'Ci::TriggerRequest'
+    belongs_to :erased_by, class_name: 'User'
 
     serialize :options
 
@@ -367,6 +367,33 @@ module Ci
       Gitlab::Ci::Build::Artifacts::Metadata.new(artifacts_metadata.path, path, **options).to_entry
     end
 
+    def erase(opts = {})
+      return false unless erasable?
+
+      remove_artifacts_file!
+      remove_artifacts_metadata!
+      erase_trace!
+      update_erased!(opts[:erased_by])
+    end
+
+    def erasable?
+      complete? && (artifacts? || has_trace?)
+    end
+
+    def erased?
+      !self.erased_at.nil?
+    end
+
+    private
+
+    def erase_trace!
+      self.trace = nil
+    end
+
+    def update_erased!(user = nil)
+      self.update(erased_by: user, erased_at: Time.now)
+    end
+
     private
 
     def yaml_variables

app/models/ci/build/erasable.rb

-module Ci
-  class Build
-    module Erasable
-      extend ActiveSupport::Concern
-
-      included do
-        belongs_to :erased_by, class_name: 'User'
-      end
-
-      def erase!(opts = {})
-        raise StandardError, 'Build not erasable!' unless erasable?
-
-        remove_artifacts_file!
-        remove_artifacts_metadata!
-        erase_trace!
-        update_erased!(opts[:erased_by])
-      end
-
-      def erasable?
-        complete? && (artifacts? || has_trace?)
-      end
-
-      def erased?
-        !self.erased_at.nil?
-      end
-
-      private
-
-      def erase_trace!
-        self.trace = nil
-      end
-
-      def update_erased!(user = nil)
-        self.update(erased_by: user, erased_at: Time.now)
-      end
-    end
-  end
-end

spec/models/build_spec.rb

@@ -498,4 +498,102 @@ describe Ci::Build, models: true do
       end
     end
   end
+
+  describe 'build erasable' do
+    shared_examples 'erasable' do
+      it 'should remove artifact file' do
+        expect(build.artifacts_file.exists?).to be_falsy
+      end
+
+      it 'should remove artifact metadata file' do
+        expect(build.artifacts_metadata.exists?).to be_falsy
+      end
+
+      it 'should erase build trace in trace file' do
+        expect(build.trace).to be_empty
+      end
+
+      it 'should set erased to true' do
+        expect(build.erased?).to be true
+      end
+
+      it 'should set erase date' do
+        expect(build.erased_at).to_not be_falsy
+      end
+    end
+
+    context 'build is not erasable' do
+      let!(:build) { create(:ci_build) }
+
+      describe '#erase' do
+        subject { build.erase }
+
+        it { is_expected.to be false }
+      end
+
+      describe '#erasable?' do
+        subject { build.erasable? }
+        it { is_expected.to eq false }
+      end
+    end
+
+    context 'build is erasable' do
+      let!(:build) { create(:ci_build_with_trace, :success, :artifacts) }
+
+      describe '#erase' do
+        before { build.erase(erased_by: user) }
+
+        context 'erased by user' do
+          let!(:user) { create(:user, username: 'eraser') }
+
+          include_examples 'erasable'
+
+          it 'should record user who erased a build' do
+            expect(build.erased_by).to eq user
+          end
+        end
+
+        context 'erased by system' do
+          let(:user) { nil }
+
+          include_examples 'erasable'
+
+          it 'should not set user who erased a build' do
+            expect(build.erased_by).to be_nil
+          end
+        end
+      end
+
+      describe '#erasable?' do
+        subject { build.erasable? }
+        it { is_expected.to eq true }
+      end
+
+      describe '#erased?' do
+        let!(:build) { create(:ci_build_with_trace, :success, :artifacts) }
+        subject { build.erased? }
+
+        context 'build has not been erased' do
+          it { is_expected.to be false }
+        end
+
+        context 'build has been erased' do
+          before { build.erase }
+
+          it { is_expected.to be true }
+        end
+      end
+
+      context 'metadata and build trace are not available' do
+        let!(:build) { create(:ci_build, :success, :artifacts) }
+        before { build.remove_artifacts_metadata! }
+
+        describe '#erase' do
+          it 'should not raise error' do
+            expect { build.erase }.to_not raise_error
+          end
+        end
+      end
+    end
+  end
 end

spec/models/ci/build/erasable_spec.rb

-require 'spec_helper'
-
-describe Ci::Build::Erasable, models: true do
-  shared_examples 'erasable' do
-    it 'should remove artifact file' do
-      expect(build.artifacts_file.exists?).to be_falsy
-    end
-
-    it 'should remove artifact metadata file' do
-      expect(build.artifacts_metadata.exists?).to be_falsy
-    end
-
-    it 'should erase build trace in trace file' do
-      expect(build.trace).to be_empty
-    end
-
-    it 'should set erased to true' do
-      expect(build.erased?).to be true
-    end
-
-    it 'should set erase date' do
-      expect(build.erased_at).to_not be_falsy
-    end
-  end
-
-  context 'build is not erasable' do
-    let!(:build) { create(:ci_build) }
-
-    describe '#erase!' do
-      it { expect { build.erase! }.to raise_error(StandardError, /Build not erasable!/ )}
-    end
-
-    describe '#erasable?' do
-      subject { build.erasable? }
-      it { is_expected.to eq false }
-    end
-  end
-
-  context 'build is erasable' do
-    let!(:build) { create(:ci_build_with_trace, :success, :artifacts) }
-
-    describe '#erase!' do
-      before { build.erase!(erased_by: user) }
-
-      context 'erased by user' do
-        let!(:user) { create(:user, username: 'eraser') }
-
-        include_examples 'erasable'
-
-        it 'should record user who erased a build' do
-          expect(build.erased_by).to eq user
-        end
-      end
-
-      context 'erased by system' do
-        let(:user) { nil }
-
-        include_examples 'erasable'
-
-        it 'should not set user who erased a build' do
-          expect(build.erased_by).to be_nil
-        end
-      end
-    end
-
-    describe '#erasable?' do
-      subject { build.erasable? }
-      it { is_expected.to eq true }
-    end
-
-    describe '#erased?' do
-      let!(:build) { create(:ci_build_with_trace, :success, :artifacts) }
-      subject { build.erased? }
-
-      context 'build has not been erased' do
-        it { is_expected.to be false }
-      end
-
-      context 'build has been erased' do
-        before { build.erase! }
-
-        it { is_expected.to be true }
-      end
-    end
-
-    context 'metadata and build trace are not available' do
-      let!(:build) { create(:ci_build, :success, :artifacts) }
-      before { build.remove_artifacts_metadata! }
-
-      describe '#erase!' do
-        it 'should not raise error' do
-          expect { build.erase! }.to_not raise_error
-        end
-      end
-    end
-  end
-end