From 2c7f36f43072dcffca13c7e248f8df1bf226ce1a Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Tue, 2 Feb 2016 12:12:03 +0100
Subject: [PATCH] Update relevant build fields when build is erased

---
 app/controllers/projects/builds_controller.rb |  2 +-
 app/models/ci/build.rb                        |  6 +--
 app/models/ci/build/eraseable.rb              | 19 ++++++--
 spec/models/ci/build/eraseable_spec.rb        | 46 +++++++++++++++----
 4 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb
index c7c03208c9e..9c7fad3e6f0 100644
--- a/app/controllers/projects/builds_controller.rb
+++ b/app/controllers/projects/builds_controller.rb
@@ -57,7 +57,7 @@ class Projects::BuildsController < Projects::ApplicationController
   end
 
   def erase
-    @build.erase!
+    @build.erase!(erased_by: current_user)
     redirect_to namespace_project_build_path(project.namespace, project, @build),
                 notice: "Build ##{@build.id} has been sucessfully erased!"
   end
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 1d65aeb6acf..f114e57d35f 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -39,7 +39,7 @@
 module Ci
   class Build < CommitStatus
     include Gitlab::Application.routes.url_helpers
-    include Eraseable
+    include Build::Eraseable
 
     LAZY_ATTRIBUTES = ['trace']
 
@@ -208,8 +208,8 @@ module Ci
       end
     end
 
-    def trace_empty?
-      raw_trace.blank?
+    def has_trace?
+      raw_trace.present?
     end
 
     def raw_trace
diff --git a/app/models/ci/build/eraseable.rb b/app/models/ci/build/eraseable.rb
index 96cbbbe5fda..a345a046238 100644
--- a/app/models/ci/build/eraseable.rb
+++ b/app/models/ci/build/eraseable.rb
@@ -1,17 +1,23 @@
 module Ci
   class Build
     module Eraseable
-      include ActiveSupport::Concern
+      extend ActiveSupport::Concern
 
-      def erase!
+      included do
+        belongs_to :erased_by, class_name: 'User'
+      end
+
+      def erase!(opts = {})
         raise StandardError, 'Build not eraseable!' unless eraseable?
+
         remove_artifacts_file!
         remove_artifacts_metadata!
         erase_trace!
+        update_erased!(opts[:erased_by])
       end
 
       def eraseable?
-        complete? && (artifacts_file.exists? || !trace_empty?)
+        complete? && (artifacts? || has_trace?)
       end
 
       def erase_url
@@ -25,6 +31,13 @@ module Ci
       def erase_trace!
         File.truncate(path_to_trace, 0) if File.file?(path_to_trace)
       end
+
+      def update_erased!(user = nil)
+        self.erased_by = user if user
+        self.erased_at = Time.now
+        self.erased = true
+        self.save!
+      end
     end
   end
 end
diff --git a/spec/models/ci/build/eraseable_spec.rb b/spec/models/ci/build/eraseable_spec.rb
index 6d2341cdbc0..fcb82de254f 100644
--- a/spec/models/ci/build/eraseable_spec.rb
+++ b/spec/models/ci/build/eraseable_spec.rb
@@ -1,6 +1,28 @@
 require 'spec_helper'
 
 describe Ci::Build::Eraseable, models: true do
+  shared_examples 'eraseable' 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(File.zero?(build.path_to_trace)).to eq true
+    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 eraseable' do
     let!(:build) { create(:ci_build) }
 
@@ -23,18 +45,26 @@ describe Ci::Build::Eraseable, models: true do
     let!(:build) { create(:ci_build_with_trace, :success, :artifacts) }
 
     describe '#erase!' do
-      before { build.erase! }
+      before { build.erase!(erased_by: user) }
 
-      it 'should remove artifact file' do
-        expect(build.artifacts_file.exists?).to be_falsy
+      context 'erased by user' do
+        let!(:user) { create(:user, username: 'eraser') }
+
+        include_examples 'eraseable'
+
+        it 'should record user who erased a build' do
+          expect(build.erased_by).to eq user
+        end
       end
 
-      it 'should remove artifact metadata file' do
-        expect(build.artifacts_metadata.exists?).to be_falsy
-      end
+      context 'erased by system' do
+        let(:user) { nil }
 
-      it 'should erase build trace in trace file' do
-        expect(File.zero?(build.path_to_trace)).to eq true
+        include_examples 'eraseable'
+
+        it 'should not set user who erased a build' do
+          expect(build.erased_by).to be_nil
+        end
       end
     end