Merge pull request zendesk#600 from zendesk/sbrnunes/refactoring-find…

…-project-filters

Refactoring filters responsible for loading a project before actions get executed.

app/controllers/builds_controller.rb

@@ -1,17 +1,12 @@
 class BuildsController < ApplicationController
-  include CurrentProject
   include ProjectLevelAuthorization
 
-  before_action do
-    find_project(params[:project_id])
-  end
-
   before_action :authorize_project_deployer!
 
   before_action :find_build, only: [:show, :build_docker_image, :edit, :update]
 
   def index
-    @builds = @project.builds.order('id desc').page(params[:page])
+    @builds = current_project.builds.order('id desc').page(params[:page])
 
     respond_to do |format|
       format.html
@@ -20,7 +15,7 @@ def index
   end
 
   def new
-    @build = @project.builds.build
+    @build = current_project.builds.build
   end
 
   def create
@@ -33,7 +28,7 @@ def create
     respond_to do |format|
       format.html do
         if @build.persisted?
-          redirect_to [@project, @build]
+          redirect_to [current_project, @build]
         else
           render :new, status: 422
         end
@@ -57,7 +52,7 @@ def update
     respond_to do |format|
       format.html do
         if success
-          redirect_to [@project, @build]
+          redirect_to [current_project, @build]
         else
           render :edit, status: 422
         end
@@ -74,7 +69,7 @@ def build_docker_image
 
     respond_to do |format|
       format.html do
-        redirect_to [@project, @build]
+        redirect_to [current_project, @build]
       end
 
       format.json do
@@ -102,15 +97,15 @@ def start_docker_build
   end
 
   def create_build
-    if old_build = @project.builds.where(git_sha: git_sha).last
+    if old_build = current_project.builds.where(git_sha: git_sha).last
       old_build.update_attributes(new_build_params)
       old_build
     else
-      @project.builds.build(new_build_params)
+      current_project.builds.build(new_build_params)
     end
   end
 
   def git_sha
-    @project.repository.commit_from_ref(new_build_params[:git_ref], length: nil)
+    current_project.repository.commit_from_ref(new_build_params[:git_ref], length: nil)
   end
 end

app/controllers/changelogs_controller.rb

@@ -2,15 +2,12 @@ class ChangelogsController < ApplicationController
   include CurrentProject
 
   before_action :check_params
-  before_action do
-    find_project(params[:project_id])
-  end
 
   def show
     @start_date = Date.strptime(params[:start_date], '%Y-%m-%d')
     @end_date = Date.strptime(params[:end_date], '%Y-%m-%d')
 
-    @changeset = Changeset.new(@project.github_repo, "master@{#{@start_date}}", "master@{#{@end_date}}")
+    @changeset = Changeset.new(current_project.github_repo, "master@{#{@start_date}}", "master@{#{@end_date}}")
   end
 
   private

app/controllers/commit_statuses_controller.rb

@@ -1,11 +1,6 @@
 class CommitStatusesController < ApplicationController
-  include CurrentProject
   include ProjectLevelAuthorization
 
-  before_action do
-    find_project(params[:project_id])
-  end
-
   before_action :authorize_project_deployer!
 
   def show
@@ -15,6 +10,6 @@ def show
   private
 
   def commit_status
-    @commit_status ||= CommitStatus.new(@project.github_repo, params[:ref])
+    @commit_status ||= CommitStatus.new(current_project.github_repo, params[:ref])
   end
 end

app/controllers/concerns/current_project.rb

@@ -2,17 +2,19 @@ module CurrentProject
   extend ActiveSupport::Concern
 
   included do
+    before_action :require_project
+
     helper_method :current_project
   end
 
   def current_project
-    @project
+    @project ||= require_project
   end
 
   protected
 
-  def find_project(param)
-    # Will this return not found if project does not exist ?
-    @project = Project.find_by_param!(param)
+  def require_project
+    @project = (Project.find_by_param!(params[:project_id]) if params[:project_id])
   end
+
 end

app/controllers/deploys_controller.rb

@@ -1,19 +1,14 @@
 class DeploysController < ApplicationController
-  include CurrentProject
   include ProjectLevelAuthorization
 
-
-  before_action except: [:active, :active_count, :recent, :changeset] do
-    find_project(params[:project_id])
-  end
+  skip_before_action :require_project, only: [:active, :active_count, :recent, :changeset]
 
   before_action :authorize_project_deployer!, only: [:new, :create, :confirm, :buddy_check, :pending_start, :destroy]
-
   before_action :find_deploy, except: [:index, :recent, :active, :active_count, :new, :create, :confirm]
   before_action :stage, only: :new
 
   def index
-    @deploys = @project.deploys.page(params[:page])
+    @deploys = current_project.deploys.page(params[:page])
 
     respond_to do |format|
       format.html
@@ -44,7 +39,7 @@ def recent
   end
 
   def new
-    @deploy = @project.deploys.build(params.except(:project_id).permit(:stage_id, :reference))
+    @deploy = current_project.deploys.build(params.except(:project_id).permit(:stage_id, :reference))
   end
 
   def create
@@ -54,7 +49,7 @@ def create
     respond_to do |format|
       format.html do
         if @deploy.persisted?
-          redirect_to [@project, @deploy]
+          redirect_to [current_project, @deploy]
         else
           render :new
         end
@@ -76,15 +71,15 @@ def buddy_check
       @deploy.confirm_buddy!(current_user)
     end
 
-    redirect_to [@project, @deploy]
+    redirect_to [current_project, @deploy]
   end
 
   def pending_start
     if @deploy.pending_non_production?
       @deploy.pending_start!
     end
 
-    redirect_to [@project, @deploy]
+    redirect_to [current_project, @deploy]
   end
 
   def show
@@ -93,7 +88,7 @@ def show
       format.text do
         datetime = @deploy.updated_at.strftime "%Y%m%d_%H%M%Z"
         send_data @deploy.output,
-          filename: "#{@project.repo_name}-#{@deploy.stage.name.parameterize}-#{@deploy.id}-#{datetime}.log",
+          filename: "#{current_project.repo_name}-#{@deploy.stage.name.parameterize}-#{@deploy.id}-#{datetime}.log",
           type: 'text/plain'
       end
     end
@@ -113,7 +108,7 @@ def destroy
       flash[:error] = "You do not have privileges to stop this deploy."
     end
 
-    redirect_to [@project, @deploy]
+    redirect_to [current_project, @deploy]
   end
 
   protected
@@ -127,7 +122,7 @@ def reference
   end
 
   def stage
-    @stage ||= @project.stages.find_by_param!(params[:stage_id])
+    @stage ||= current_project.stages.find_by_param!(params[:stage_id])
   end
 
   def deploy_params
@@ -139,6 +134,6 @@ def find_deploy
   end
 
   def active_deploy_scope
-    @project ? @project.deploys.active : Deploy.active
+    current_project ? current_project.deploys.active : Deploy.active
   end
 end

app/controllers/jobs_controller.rb

@@ -1,10 +1,8 @@
 class JobsController < ApplicationController
-  include CurrentProject
   include ProjectLevelAuthorization
 
-  before_action except: [:enabled] do
-    find_project(params[:project_id])
-  end
+  skip_before_action :require_project, only: [:enabled]
+
   before_action :authorize_project_admin!, only: [:new, :create, :destroy]
   before_action :find_job, only: [:show, :destroy]
 
@@ -74,6 +72,6 @@ def command_params
   end
 
   def find_job
-    @job = @project.jobs.find(params[:id])
+    @job = current_project.jobs.find(params[:id])
   end
 end

app/controllers/locks_controller.rb

@@ -1,13 +1,11 @@
 class LocksController < ApplicationController
   include ProjectLevelAuthorization
 
-  before_action unless: :for_global_lock? do
-    find_project
-    authorize_project_deployer!
-  end
-
   before_action :authorize_admin!, if: :for_global_lock?
 
+  before_action :require_project, unless: :for_global_lock?
+  before_action :authorize_project_deployer!, unless: :for_global_lock?
+
   def create
     attributes = params.require(:lock).
       permit(:description, :stage_id, :warning).
@@ -38,7 +36,8 @@ def lock
     @lock ||= Lock.find(params[:id])
   end
 
-  def find_project
+  #Overrides CurrentProject#require_project
+  def require_project
     case action_name
     when 'create' then
       @project = Stage.find(params[:lock][:stage_id]).project

app/controllers/macros_controller.rb

@@ -1,10 +1,6 @@
 class MacrosController < ApplicationController
-  include CurrentProject
   include ProjectLevelAuthorization
 
-  before_action do
-    find_project(params[:project_id])
-  end
   before_action :authorize_project_deployer!
   before_action :authorize_project_admin!, only: [:new, :create, :edit, :update, :destroy]
   before_action :find_macro, only: [:edit, :update, :execute, :destroy]

app/controllers/new_relic_controller.rb

@@ -1,11 +1,6 @@
 class NewRelicController < ApplicationController
-  include CurrentProject
   include ProjectLevelAuthorization
 
-  before_action do
-    find_project(params[:project_id])
-  end
-
   before_action :authorize_project_deployer!
   before_action :ensure_new_reclic_api_key
 

app/controllers/project_roles_controller.rb

@@ -1,10 +1,7 @@
 class ProjectRolesController < ApplicationController
-  include CurrentProject
   include ProjectLevelAuthorization
 
-  before_action only: [:create, :update] do
-    find_project(params[:project_id])
-  end
+  skip_before_action :require_project, only: [:index]
 
   before_action :authorize_project_admin!, only: [:create, :update]
 

app/controllers/projects_controller.rb

@@ -1,19 +1,17 @@
 class ProjectsController < ApplicationController
-  include CurrentProject
   include ProjectLevelAuthorization
   include StagePermittedParams
 
-  attr_reader :project
-
-  before_action except: [:index, :new, :create] do
-    find_project(params[:id])
-  end
+  skip_before_action :require_project, only: [:index, :new, :create]
+
   before_action :authorize_admin!, only: [:new, :create, :destroy]
   before_action :authorize_project_admin!, except: [:show, :index, :deploy_group_versions]
   before_action :get_environments, only: [:new, :create]
 
   helper_method :project
 
+  alias_method :project, :current_project
+
   def index
     respond_to do |format|
       format.html do
@@ -109,4 +107,9 @@ def projects_for_user
   def get_environments
     @environments = Environment.all
   end
+
+  # Overriding require_project from CurrentProject
+  def require_project
+    @project = (Project.find_by_param!(params[:id]) if params[:id])
+  end
 end

app/controllers/references_controller.rb

@@ -1,11 +1,6 @@
 class ReferencesController < ApplicationController
-  include CurrentProject
   include ProjectLevelAuthorization
 
-  before_action do
-    find_project(params[:project_id])
-  end
-
   before_action :authorize_project_deployer!
 
   def index

app/controllers/releases_controller.rb

@@ -1,11 +1,6 @@
 class ReleasesController < ApplicationController
-  include CurrentProject
   include ProjectLevelAuthorization
 
-  before_action do
-    find_project(params[:project_id])
-  end
-
   before_action :authorize_project_deployer!, except: [:show, :index]
 
   def show

app/controllers/stages_controller.rb

@@ -1,16 +1,11 @@
 require 'open-uri' # needed to fetch from img.shields.io using open()
 
 class StagesController < ApplicationController
-  include CurrentProject
   include ProjectLevelAuthorization
   include StagePermittedParams
 
   skip_before_action :login_users, if: :badge?
 
-  before_action do
-    find_project(params[:project_id])
-  end
-
   before_action :authorize_project_deployer!, unless: :badge?
   before_action :authorize_project_admin!, except: [:index, :show]
   before_action :check_token, if: :badge?
@@ -125,7 +120,7 @@ def stage_params
   end
 
   def find_stage
-    @stage = @project.stages.find_by_param!(params[:id])
+    @stage = current_project.stages.find_by_param!(params[:id])
   end
 
   def get_environments