From cdd533ccd1a9d5443527f1bf45aa695ffe5fa0f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Fri, 17 Jun 2016 15:08:59 +0000
Subject: [PATCH] Merge branch 'make-sure-that-artifacts-file-is-saved' into
 'master'

Make sure that artifacts_file is nullified after removing artifacts

## What does this MR do?
Fixes a problem that `ExpireBuildArtifactsWorker` is executed for all previously removed artifacts.

## Why was this MR needed?
Currently the `Ci::Build::erase_artifacts!` doesn't ensure that data are saved to database.
The bang at end of this method lets you believe that it should do so.
This adds a missing `save` to this method.

## What are the relevant issue numbers?
None, yet.

## CHANGELOG
Since this is regression in feature introduce in RC4 no CHANGELOG entry is needed.

cc @grzesiek


See merge request !4741
---
 app/models/ci/build.rb                             |  1 +
 spec/workers/expire_build_artifacts_worker_spec.rb | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 764d8e4e13..d618c84e98 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -341,6 +341,7 @@ module Ci
     def erase_artifacts!
       remove_artifacts_file!
       remove_artifacts_metadata!
+      save
     end
 
     def erase(opts = {})
diff --git a/spec/workers/expire_build_artifacts_worker_spec.rb b/spec/workers/expire_build_artifacts_worker_spec.rb
index e3827cae9a..7d6668920c 100644
--- a/spec/workers/expire_build_artifacts_worker_spec.rb
+++ b/spec/workers/expire_build_artifacts_worker_spec.rb
@@ -20,6 +20,10 @@ describe ExpireBuildArtifactsWorker do
       it 'does remove files' do
         expect(build.reload.artifacts_file.exists?).to be_falsey
       end
+
+      it 'does nullify artifacts_file column' do
+        expect(build.reload.artifacts_file_identifier).to be_nil
+      end
     end
 
     context 'with not yet expired artifacts' do
@@ -32,6 +36,10 @@ describe ExpireBuildArtifactsWorker do
       it 'does not remove files' do
         expect(build.reload.artifacts_file.exists?).to be_truthy
       end
+
+      it 'does not nullify artifacts_file column' do
+        expect(build.reload.artifacts_file_identifier).not_to be_nil
+      end
     end
 
     context 'without expire date' do
@@ -44,6 +52,10 @@ describe ExpireBuildArtifactsWorker do
       it 'does not remove files' do
         expect(build.reload.artifacts_file.exists?).to be_truthy
       end
+
+      it 'does not nullify artifacts_file column' do
+        expect(build.reload.artifacts_file_identifier).not_to be_nil
+      end
     end
 
     context 'for expired artifacts' do