diff options
author | Steve Azzopardi <steveazz@outlook.com> | 2018-12-06 11:55:50 +0100 |
---|---|---|
committer | Steve Azzopardi <steveazz@outlook.com> | 2019-01-07 17:29:26 +0100 |
commit | f9c8822afdddcd83434432dc2c36b87cc1886602 (patch) | |
tree | e63fac7d0564ec0d948221188ff64941a67c42bc | |
parent | 7ac32ae282fa2d35a3651de08f35aad1b85ffca0 (diff) | |
download | gitlab-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.rb | 4 | ||||
-rw-r--r-- | app/controllers/projects/build_artifacts_controller.rb | 4 | ||||
-rw-r--r-- | app/models/project.rb | 6 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 23 |
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 |