summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSteve Azzopardi <steveazz@outlook.com>2018-12-06 11:55:50 +0100
committerSteve Azzopardi <steveazz@outlook.com>2019-01-07 17:29:26 +0100
commitf9c8822afdddcd83434432dc2c36b87cc1886602 (patch)
treee63fac7d0564ec0d948221188ff64941a67c42bc
parent7ac32ae282fa2d35a3651de08f35aad1b85ffca0 (diff)
downloadgitlab-ce-f9c8822afdddcd83434432dc2c36b87cc1886602.tar.gz
Create `get_build` for project model
Inside of `Projects::ArtifactsController` and `Projects::BuildArtifactsController` we fetching the build by id using active record directly which violates `CodeReuse/ActiveRecord` rubocop rule. Create `get_build` inside of `project` model which does the same thing.
-rw-r--r--app/controllers/projects/artifacts_controller.rb4
-rw-r--r--app/controllers/projects/build_artifacts_controller.rb4
-rw-r--r--app/models/project.rb6
-rw-r--r--spec/models/project_spec.rb23
4 files changed, 30 insertions, 7 deletions
diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb
index f4696bcb989..9a0997e92ee 100644
--- a/app/controllers/projects/artifacts_controller.rb
+++ b/app/controllers/projects/artifacts_controller.rb
@@ -85,11 +85,9 @@ class Projects::ArtifactsController < Projects::ApplicationController
end
end
- # rubocop: disable CodeReuse/ActiveRecord
def build_from_id
- project.builds.find_by(id: params[:job_id]) if params[:job_id]
+ project.get_build(params[:job_id]) if params[:job_id]
end
- # rubocop: enable CodeReuse/ActiveRecord
def build_from_ref
return unless @ref_name
diff --git a/app/controllers/projects/build_artifacts_controller.rb b/app/controllers/projects/build_artifacts_controller.rb
index d74656f0ea5..d3d5ba5c75d 100644
--- a/app/controllers/projects/build_artifacts_controller.rb
+++ b/app/controllers/projects/build_artifacts_controller.rb
@@ -44,11 +44,9 @@ class Projects::BuildArtifactsController < Projects::ApplicationController
@job ||= job_from_id || job_from_ref
end
- # rubocop: disable CodeReuse/ActiveRecord
def job_from_id
- project.builds.find_by(id: params[:build_id]) if params[:build_id]
+ project.get_build(params[:build_id]) if params[:build_id]
end
- # rubocop: enable CodeReuse/ActiveRecord
def job_from_ref
return unless @ref_name
diff --git a/app/models/project.rb b/app/models/project.rb
index 681c7c71934..8c8b4d3748a 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -652,7 +652,11 @@ class Project < ActiveRecord::Base
end
def latest_successful_build_for!(job_name, ref = default_branch)
- latest_successful_build_for(job_name, ref) or raise ActiveRecord::RecordNotFound.new("Couldn't find job #{job_name}", job_name)
+ latest_successful_build_for(job_name, ref) || raise(ActiveRecord::RecordNotFound.new("Couldn't find job #{job_name}"))
+ end
+
+ def get_build(id)
+ builds.find_by(id: id)
end
def merge_base_commit(first_commit_id, second_commit_id)
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index aa4ec6493db..b12b81f8f38 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2000,6 +2000,29 @@ describe Project do
end
end
+ describe '#get_build' do
+ let(:project) { create(:project, :repository) }
+ let(:ci_pipeline) { create(:ci_pipeline, project: project) }
+
+ context 'when build exists' do
+ context 'build is associated with project' do
+ let(:build) { create(:ci_build, :success, pipeline: ci_pipeline) }
+
+ it { expect(project.get_build(build.id)).to eq(build) }
+ end
+
+ context 'build is not associated with project' do
+ let(:build) { create(:ci_build, :success) }
+
+ it { expect(project.get_build(build.id)).to be_nil }
+ end
+ end
+
+ context 'build does not exists' do
+ it { expect(project.get_build(rand 100)).to be_nil }
+ end
+ end
+
describe '#import_status' do
context 'with import_state' do
it 'returns the right status' do