From f9c8822afdddcd83434432dc2c36b87cc1886602 Mon Sep 17 00:00:00 2001
From: Steve Azzopardi <steveazz@outlook.com>
Date: Thu, 6 Dec 2018 11:55:50 +0100
Subject: [PATCH] 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.
---
 .../projects/artifacts_controller.rb          |  4 +---
 .../projects/build_artifacts_controller.rb    |  4 +---
 app/models/project.rb                         |  6 ++++-
 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