MR review feedback6717_extend_reports_for_security_products

4 files changed, 47 insertions, 4 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 97960dec08c..3dadb95443a 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -484,7 +484,8 @@ module Ci
     def erase(opts = {})
       return false unless erasable?
 
-      erase_erasable_artifacts!
+      job_artifacts.destroy_all # rubocop: disable DestroyAll
+      erase_old_artifacts!
       erase_trace!
       update_erased!(opts[:erased_by])
     end
diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb
index 4965eaa2fbc..7c958598553 100644
--- a/app/models/ci/job_artifact.rb
+++ b/app/models/ci/job_artifact.rb
@@ -58,7 +58,7 @@ module Ci
     end
 
     scope :erasable, -> do
-      types = self.file_types.reject { |file_type| file_types.include?(NON_ERASABLE_FILE_TYPES) }.values
+      types = self.file_types.reject { |file_type| NON_ERASABLE_FILE_TYPES.include?(file_type) }.values
 
       where(file_type: types)
     end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 8bd92cdc777..e82d93d5935 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -998,6 +998,32 @@ describe Ci::Build do
     end
   end
 
+  describe '#erase_erasable_artifacts!' do
+    let!(:build) { create(:ci_build, :success) }
+
+    subject { build.erase_erasable_artifacts! }
+
+    before do
+      Ci::JobArtifact.file_types.keys.each do |file_type|
+        create(:ci_job_artifact, job: build, file_type: file_type, file_format: Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS[file_type.to_sym])
+      end
+    end
+
+    it "erases erasable artifacts" do
+      subject
+
+      expect(build.job_artifacts.erasable).to be_empty
+    end
+
+    it "keeps non erasable artifacts" do
+      subject
+
+      Ci::JobArtifact::NON_ERASABLE_FILE_TYPES.each do |file_type|
+        expect(build.send("job_artifacts_#{file_type}")).not_to be_nil
+      end
+    end
+  end
+
   describe '#first_pending' do
     let!(:first) { create(:ci_build, pipeline: pipeline, status: 'pending', created_at: Date.yesterday) }
     let!(:second) { create(:ci_build, pipeline: pipeline, status: 'pending') }
diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb
index a8cc73766a6..e6bb100db98 100644
--- a/spec/models/ci/job_artifact_spec.rb
+++ b/spec/models/ci/job_artifact_spec.rb
@@ -31,6 +31,22 @@ describe Ci::JobArtifact do
     end
   end
 
+  describe '.erasable' do
+    subject { described_class.erasable }
+
+    context 'when there is am erasable artifact' do
+      let!(:artifact) { create(:ci_job_artifact, :junit) }
+
+      it { is_expected.to eq([artifact]) }
+    end
+
+    context 'when there are no erasable artifacts' do
+      let!(:artifact) { create(:ci_job_artifact, :trace) }
+
+      it { is_expected.to be_empty }
+    end
+  end
+
   describe 'callbacks' do
     subject { create(:ci_job_artifact, :archive) }
 
@@ -103,7 +119,7 @@ describe Ci::JobArtifact do
     end
   end
 
-  describe 'validates file format (for all types but "trace")' do
+  describe 'validates file format' do
     subject { artifact }
 
     described_class::TYPE_AND_FORMAT_PAIRS.each do |file_type, file_format|
@@ -120,7 +136,7 @@ describe Ci::JobArtifact do
       end
 
       context "when #{file_type} type with other formats" do
-        (described_class::TYPE_AND_FORMAT_PAIRS.values - [file_format]).each do |other_format|
+        described_class.file_formats.except(file_format).values.each do |other_format|
           let(:artifact) { build(:ci_job_artifact, file_type: file_type, file_format: other_format) }
 
           it { is_expected.not_to be_valid }