Commit 293999ca authored by Grzegorz Bizon's avatar Grzegorz Bizon

Fix name of build erasable, remove superfluous method from it

parent ec5c3c02
...@@ -38,7 +38,7 @@ ...@@ -38,7 +38,7 @@
module Ci module Ci
class Build < CommitStatus class Build < CommitStatus
include Gitlab::Application.routes.url_helpers include Gitlab::Application.routes.url_helpers
include Build::Eraseable include Build::Erasable
LAZY_ATTRIBUTES = ['trace'] LAZY_ATTRIBUTES = ['trace']
......
module Ci module Ci
class Build class Build
module Eraseable module Erasable
extend ActiveSupport::Concern extend ActiveSupport::Concern
included do included do
...@@ -8,7 +8,7 @@ module Ci ...@@ -8,7 +8,7 @@ module Ci
end end
def erase!(opts = {}) def erase!(opts = {})
raise StandardError, 'Build not eraseable!' unless eraseable? raise StandardError, 'Build not erasable!' unless erasable?
remove_artifacts_file! remove_artifacts_file!
remove_artifacts_metadata! remove_artifacts_metadata!
...@@ -16,16 +16,10 @@ module Ci ...@@ -16,16 +16,10 @@ module Ci
update_erased!(opts[:erased_by]) update_erased!(opts[:erased_by])
end end
def eraseable? def erasable?
complete? && (artifacts? || has_trace?) complete? && (artifacts? || has_trace?)
end end
def erase_url
if eraseable?
erase_namespace_project_build_path(project.namespace, project, self)
end
end
def erased? def erased?
!self.erased_at.nil? !self.erased_at.nil?
end end
......
...@@ -76,15 +76,15 @@ ...@@ -76,15 +76,15 @@
= link_to '#down-build-trace', class: 'btn' do = link_to '#down-build-trace', class: 'btn' do
%i.fa.fa-angle-down %i.fa.fa-angle-down
- unless @build.erased? - if @build.erased?
.erased.alert.alert-warning
- erased_by = "by #{@build.erased_by.username}" if @build.erased_by
Build has been erased #{erased_by} #{time_ago_with_tooltip(@build.erased_at)}
- else
%pre.trace#build-trace %pre.trace#build-trace
%code.bash %code.bash
= preserve do = preserve do
= raw @build.trace_html = raw @build.trace_html
- else
.erased.alert.alert-warning
- erased_by = "by #{@build.erased_by.username}" if @build.erased_by
Build has been erased #{erased_by} #{time_ago_with_tooltip(@build.erased_at)}
%div#down-build-trace %div#down-build-trace
...@@ -119,8 +119,9 @@ ...@@ -119,8 +119,9 @@
- elsif @build.retry_url - elsif @build.retry_url
= link_to "Retry", @build.retry_url, class: 'btn btn-sm btn-primary', method: :post = link_to "Retry", @build.retry_url, class: 'btn btn-sm btn-primary', method: :post
- if @build.eraseable? - if @build.erasable?
= link_to @build.erase_url, class: 'btn btn-sm btn-warning', method: :delete, = link_to erase_namespace_project_build_path(@project.namespace, @project, @build),
class: 'btn btn-sm btn-warning', method: :delete,
data: { confirm: 'Are you sure you want to erase this build?' } do data: { confirm: 'Are you sure you want to erase this build?' } do
= icon('eraser') = icon('eraser')
Erase Erase
......
class AddEraseableToCiBuild < ActiveRecord::Migration class AddErasableToCiBuild < ActiveRecord::Migration
def change def change
add_reference :ci_builds, :erased_by, references: :users, index: true add_reference :ci_builds, :erased_by, references: :users, index: true
add_column :ci_builds, :erased_at, :datetime add_column :ci_builds, :erased_at, :datetime
......
...@@ -136,7 +136,7 @@ module API ...@@ -136,7 +136,7 @@ module API
build = get_build(params[:build_id]) build = get_build(params[:build_id])
return not_found!(build) unless build return not_found!(build) unless build
return forbidden!('Build is not eraseable!') unless build.eraseable? return forbidden!('Build is not erasable!') unless build.erasable?
build.erase! build.erase!
present build, with: Entities::Build, present build, with: Entities::Build,
......
require 'spec_helper' require 'spec_helper'
describe Ci::Build::Eraseable, models: true do describe Ci::Build::Erasable, models: true do
shared_examples 'eraseable' do shared_examples 'erasable' do
it 'should remove artifact file' do it 'should remove artifact file' do
expect(build.artifacts_file.exists?).to be_falsy expect(build.artifacts_file.exists?).to be_falsy
end end
...@@ -23,25 +23,20 @@ describe Ci::Build::Eraseable, models: true do ...@@ -23,25 +23,20 @@ describe Ci::Build::Eraseable, models: true do
end end
end end
context 'build is not eraseable' do context 'build is not erasable' do
let!(:build) { create(:ci_build) } let!(:build) { create(:ci_build) }
describe '#erase!' do describe '#erase!' do
it { expect { build.erase! }.to raise_error(StandardError, /Build not eraseable!/ )} it { expect { build.erase! }.to raise_error(StandardError, /Build not erasable!/ )}
end end
describe '#eraseable?' do describe '#erasable?' do
subject { build.eraseable? } subject { build.erasable? }
it { is_expected.to eq false } it { is_expected.to eq false }
end end
describe '#erase_url' do
subject { build.erase_url }
it { is_expected.to be_falsy }
end
end end
context 'build is eraseable' do context 'build is erasable' do
let!(:build) { create(:ci_build_with_trace, :success, :artifacts) } let!(:build) { create(:ci_build_with_trace, :success, :artifacts) }
describe '#erase!' do describe '#erase!' do
...@@ -50,7 +45,7 @@ describe Ci::Build::Eraseable, models: true do ...@@ -50,7 +45,7 @@ describe Ci::Build::Eraseable, models: true do
context 'erased by user' do context 'erased by user' do
let!(:user) { create(:user, username: 'eraser') } let!(:user) { create(:user, username: 'eraser') }
include_examples 'eraseable' include_examples 'erasable'
it 'should record user who erased a build' do it 'should record user who erased a build' do
expect(build.erased_by).to eq user expect(build.erased_by).to eq user
...@@ -60,7 +55,7 @@ describe Ci::Build::Eraseable, models: true do ...@@ -60,7 +55,7 @@ describe Ci::Build::Eraseable, models: true do
context 'erased by system' do context 'erased by system' do
let(:user) { nil } let(:user) { nil }
include_examples 'eraseable' include_examples 'erasable'
it 'should not set user who erased a build' do it 'should not set user who erased a build' do
expect(build.erased_by).to be_nil expect(build.erased_by).to be_nil
...@@ -68,16 +63,11 @@ describe Ci::Build::Eraseable, models: true do ...@@ -68,16 +63,11 @@ describe Ci::Build::Eraseable, models: true do
end end
end end
describe '#eraseable?' do describe '#erasable?' do
subject { build.eraseable? } subject { build.erasable? }
it { is_expected.to eq true } it { is_expected.to eq true }
end end
describe '#erase_url' do
subject { build.erase_url }
it { is_expected.to be_truthy }
end
describe '#erased?' do describe '#erased?' do
let!(:build) { create(:ci_build_with_trace, :success, :artifacts) } let!(:build) { create(:ci_build_with_trace, :success, :artifacts) }
subject { build.erased? } subject { build.erased? }
......
...@@ -175,7 +175,7 @@ describe API::API, api: true do ...@@ -175,7 +175,7 @@ describe API::API, api: true do
delete api("/projects/#{project.id}/builds/#{build.id}/content", user) delete api("/projects/#{project.id}/builds/#{build.id}/content", user)
end end
context 'build is eraseable' do context 'build is erasable' do
let(:build) { create(:ci_build_with_trace, :artifacts, :success, project: project, commit: commit) } let(:build) { create(:ci_build_with_trace, :artifacts, :success, project: project, commit: commit) }
it 'should erase build content' do it 'should erase build content' do
...@@ -186,7 +186,7 @@ describe API::API, api: true do ...@@ -186,7 +186,7 @@ describe API::API, api: true do
end end
end end
context 'build is not eraseable' do context 'build is not erasable' do
let(:build) { create(:ci_build_with_trace, project: project, commit: commit) } let(:build) { create(:ci_build_with_trace, project: project, commit: commit) }
it 'should respond with forbidden' do it 'should respond with forbidden' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment