#465: Update specs for Projects::PullRequestsController

app/controllers/projects/pull_requests_controller.rb

@@ -1,11 +1,10 @@
 class Projects::PullRequestsController < Projects::BaseController
   before_action :authenticate_user!
   skip_before_action :authenticate_user!, only: [:index, :show] if APP_CONFIG['anonymous_access']
-  load_and_authorize_resource :project
 
-  load_resource :issue, through: :project, find_by: :serial_id, parent: false, except: [:index, :autocomplete_to_project]
-  load_and_authorize_resource instance_name: :pull, through: :issue, singleton: true, except: [:index, :autocomplete_to_project]
-  before_action :find_collaborators, only: [:new, :create, :show]
+  before_action :load_issue,         except: %i(index autocomplete_to_project new create)
+  before_action :load_pull,          except: %i(index autocomplete_to_project new create)
+  before_action :find_collaborators, only:   %i(new create show)
 
   def new
     to_project = find_destination_project(false)
@@ -15,6 +14,7 @@ class Projects::PullRequestsController < Projects::BaseController
     @pull.issue = to_project.issues.new
     set_attrs
 
+    authorize @pull
     if PullRequest.check_ref(@pull, 'to', @pull.to_ref) && PullRequest.check_ref(@pull, 'from', @pull.from_ref) || @pull.uniq_merge
       flash.now[:warning] = @pull.errors.full_messages.join('. ')
     else
@@ -42,6 +42,7 @@ class Projects::PullRequestsController < Projects::BaseController
     @pull.from_project_name         = @pull.from_project.name
     @pull.issue.new_pull_request    = true
 
+    authorize @pull
     if @pull.valid? # FIXME more clean/clever logics
       @pull.save # set pull id
       @pull.reload
@@ -67,11 +68,13 @@ class Projects::PullRequestsController < Projects::BaseController
   end
 
   def merge
+    authorize @pull
     status = @pull.merge!(current_user) ? 200 : 422
     render nothing: true, status: status
   end
 
   def update
+    authorize @pull
     status = 422
     if (action = params[:pull_request_action]) && %w(close reopen).include?(params[:pull_request_action])
       if @pull.send("can_#{action}?")
@@ -116,6 +119,17 @@ class Projects::PullRequestsController < Projects::BaseController
 
   protected
 
+  # Private: before_action hook which loads Issue.
+  def load_issue
+    @issue = @project.issues.find_by!(serial_id: params[:id])
+  end
+
+  # Private: before_action hook which loads PullRequest.
+  def load_pull
+    @pull = @issue.pull_request
+    authorize @pull, :show? if @pull
+  end
+
   def pull_params
     @pull_params ||= params[:pull_request].presence
   end

app/policies/pull_request_policy.rb

@@ -5,7 +5,7 @@ class PullRequestPolicy < ApplicationPolicy
   end
 
   def show?
-    ProjectPolicy.new(user, record.to_project).show?
+    is_admin? || ProjectPolicy.new(user, record.to_project).show?
   end
   alias_method :read?,      :show?
   alias_method :commits?,   :show?
@@ -16,11 +16,11 @@ class PullRequestPolicy < ApplicationPolicy
   end
 
   def update?
-    record.user_id == record.id || local_writer?(record.to_project)
+    is_admin? || record.user_id == record.id || local_writer?(record.to_project)
   end
 
   def merge?
-    local_writer?(record.to_project)
+    is_admin? || local_writer?(record.to_project)
   end
 
 end