Refactor Pundit Policies

I've refactored the pundit policies to all always
be operating on a person object. Because of this change
I also wrote a signed_in? method to improve readability.

Knowing we are always operating on a Person object in our
policies let's us clean up our code a bit and not have to consistenly
check for nil / safe navigation.

app/policies/application_policy.rb

@@ -2,27 +2,33 @@ class ApplicationPolicy
   attr_reader :person, :record
 
   def initialize(person, record)
-    @person = person
+    @person = person || Person.new
     @record = record
   end
 
   def create?
-    person.present? && person.admin?
+    signed_in?(person) && person.admin?
   end
 
   def new?
     create?
   end
 
   def update?
-    person.present? && person.admin?
+    signed_in?(person) && person.admin?
   end
 
   def edit?
     update?
   end
 
   def destroy?
-    person.present? && person.admin?
+    signed_in?(person) && person.admin?
+  end
+
+  private
+
+  def signed_in?(person)
+    person.persisted?
   end
 end

app/policies/presentation_policy.rb

@@ -1,14 +1,14 @@
 class PresentationPolicy < ApplicationPolicy
   def create?
-    person.present?
+    signed_in?(person)
   end
 
   def new?
     create?
   end
 
   def update?
-    (person&.admin? || person&.presentations&.include?(record)).present?
+    person.admin? || person.presentations.include?(record)
   end
 
   def edit?

spec/controllers/events_controller_spec.rb

@@ -2,7 +2,7 @@
 
 RSpec.describe EventsController, type: :controller do
   context 'the current person is an admin' do
-    let(:admin_person) { FactoryGirl.build(:person, :admin) }
+    let(:admin_person) { FactoryGirl.create(:person, :admin) }
 
     before { mock_pundit_user_as(admin_person) }
 
@@ -96,7 +96,7 @@
   end
 
   context 'the current person is a non-admin' do
-    let(:non_admin) { FactoryGirl.build(:person) }
+    let(:non_admin) { FactoryGirl.create(:person) }
 
     before { mock_pundit_user_as(non_admin) }
 

spec/controllers/presentations_controller_spec.rb

@@ -2,8 +2,8 @@
 
 RSpec.describe PresentationsController, type: :controller do
   let(:nil_person) { nil }
-  let(:non_admin) { FactoryGirl.build(:person) }
-  let(:admin_person) { FactoryGirl.build(:person, :admin) }
+  let(:non_admin) { FactoryGirl.create(:person) }
+  let(:admin_person) { FactoryGirl.create(:person, :admin) }
 
   describe 'GET #index' do
     subject { get :index }

spec/policies/application_policy_spec.rb

@@ -24,7 +24,7 @@
 
   context 'the person is an admin' do
     subject { ApplicationPolicy.new(admin, 'any object') }
-    let(:admin) { FactoryGirl.build(:person, :admin) }
+    let(:admin) { FactoryGirl.create(:person, :admin) }
 
     policy_methods.each do |method|
       describe method.to_s do
@@ -37,7 +37,7 @@
 
   context 'the person is a non admin' do
     subject { ApplicationPolicy.new(non_admin, 'any object') }
-    let(:non_admin) { FactoryGirl.build(:person) }
+    let(:non_admin) { FactoryGirl.create(:person) }
 
     policy_methods.each do |method|
       describe method.to_s do

spec/policies/presentation_policy_spec.rb

@@ -28,7 +28,7 @@
 
     context 'the person is an admin' do
       subject { described_class.new(admin, presentation) }
-      let(:admin) { FactoryGirl.build(:person, :admin) }
+      let(:admin) { FactoryGirl.create(:person, :admin) }
       let(:presentation) { FactoryGirl.build(:presentation) }
 
       describe '.update?' do