Merge pull request #487 from metamaps/feature/pundit

policy specs & remove old permission tests

app/policies/mapping_policy.rb

@@ -16,20 +16,28 @@ class MappingPolicy < ApplicationPolicy
   end
 
   def show?
-    map = Pundit.policy(user, record.map)
-    mappable = Pundit.policy(user, record.mappable)
-    map.show? && mappable.show?
+    map_policy.show? && mappable_policy.show?
   end
 
   def create?
-    Pundit.policy(user, record.map).update?
+    map_policy.update?
   end
 
   def update?
-    Pundit.policy(user, record.map).update?
+    record.mappable_type == 'Topic' && map_policy.update?
   end
 
   def destroy?
-    record.user == user || admin_override
+    map_policy.update? || admin_override
+  end
+
+  # Helpers
+
+  def map_policy
+    @map_policy ||= Pundit.policy(user, record.map)
+  end
+
+  def mappable_policy
+    @mappable_policy ||= Pundit.policy(user, record.mappable)
   end
 end

spec/models/map_spec.rb

@@ -5,34 +5,5 @@ RSpec.describe Map, type: :model do
   it { is_expected.to validate_presence_of :name }
   it { is_expected.to validate_presence_of :permission }
   it { is_expected.to validate_inclusion_of(:permission).in_array Perm::ISSIONS.map(&:to_s) }
-
-  context 'permissions' do
-    let(:owner) { create :user }
-    let(:other_user) { create :user }
-    let(:map) { create :map, user: owner, permission: :commons }
-    let(:private_map) { create :map, user: owner, permission: :private }
-    let(:public_map) { create :map, user: owner, permission: :public }
-
-    it 'prevents deletion by non-owner' do
-      expect(map.authorize_to_delete(other_user)).to eq false
-      expect(map.authorize_to_delete(owner)).to eq map
-    end
-
-    it 'prevents visibility if private' do
-      expect(map.authorize_to_show(other_user)).to eq map
-      expect(map.authorize_to_show(owner)).to eq map
-      expect(private_map.authorize_to_show(owner)).to eq private_map
-      expect(private_map.authorize_to_show(other_user)).to eq false
-    end
-
-    it 'only allows editing if commons or owned' do
-      expect(map.authorize_to_edit(other_user)).to eq map
-      expect(map.authorize_to_edit(owner)).to eq map
-      expect(private_map.authorize_to_edit(other_user)).to eq false
-      expect(private_map.authorize_to_edit(owner)).to eq private_map
-      expect(public_map.authorize_to_edit(other_user)).to eq false
-      expect(public_map.authorize_to_edit(owner)).to eq public_map
-    end
-  end
 end
 

spec/models/synapse_spec.rb

@@ -10,33 +10,4 @@ RSpec.describe Synapse, type: :model do
   it { is_expected.to validate_inclusion_of(:permission).in_array Perm::ISSIONS.map(&:to_s) }
   it { is_expected.to validate_inclusion_of(:category).in_array ['from-to', 'both'] }
   it { is_expected.to validate_length_of(:desc).is_at_least(0) } # TODO don't allow nil
-
-  context 'permissions' do
-    let(:owner) { create :user }
-    let(:other_user) { create :user }
-    let(:synapse) { create :synapse, user: owner, permission: :commons }
-    let(:private_synapse) { create :synapse, user: owner, permission: :private }
-    let(:public_synapse) { create :synapse, user: owner, permission: :public }
-
-    it 'prevents deletion by non-owner' do
-      expect(synapse.authorize_to_delete(other_user)).to eq false
-      expect(synapse.authorize_to_delete(owner)).to eq synapse
-    end
-
-    it 'prevents visibility if private' do
-      expect(synapse.authorize_to_show(other_user)).to eq synapse
-      expect(synapse.authorize_to_show(owner)).to eq synapse
-      expect(private_synapse.authorize_to_show(owner)).to eq private_synapse
-      expect(private_synapse.authorize_to_show(other_user)).to eq false
-    end
-
-    it 'only allows editing if commons or owned' do
-      expect(synapse.authorize_to_edit(other_user)).to eq synapse
-      expect(synapse.authorize_to_edit(owner)).to eq synapse
-      expect(private_synapse.authorize_to_edit(other_user)).to eq false
-      expect(private_synapse.authorize_to_edit(owner)).to eq private_synapse
-      expect(public_synapse.authorize_to_edit(other_user)).to eq false
-      expect(public_synapse.authorize_to_edit(owner)).to eq public_synapse
-    end
-  end
 end

spec/models/topic_spec.rb

@@ -49,33 +49,4 @@ RSpec.describe Topic, type: :model do
       end
     end
   end
-
-  context 'permssions' do
-    let(:owner) { create :user }
-    let(:other_user) { create :user }
-    let(:topic) { create :topic, user: owner, permission: :commons }
-    let(:private_topic) { create :topic, user: owner, permission: :private }
-    let(:public_topic) { create :topic, user: owner, permission: :public }
-
-    it 'prevents deletion by non-owner' do
-      expect(topic.authorize_to_delete(other_user)).to eq false
-      expect(topic.authorize_to_delete(owner)).to eq topic
-    end
-
-    it 'prevents visibility if private' do
-      expect(topic.authorize_to_show(other_user)).to eq topic
-      expect(topic.authorize_to_show(owner)).to eq topic
-      expect(private_topic.authorize_to_show(owner)).to eq private_topic
-      expect(private_topic.authorize_to_show(other_user)).to eq false
-    end
-
-    it 'only allows editing if commons or owned' do
-      expect(topic.authorize_to_edit(other_user)).to eq topic
-      expect(topic.authorize_to_edit(owner)).to eq topic
-      expect(private_topic.authorize_to_edit(other_user)).to eq false
-      expect(private_topic.authorize_to_edit(owner)).to eq private_topic
-      expect(public_topic.authorize_to_edit(other_user)).to eq false
-      expect(public_topic.authorize_to_edit(owner)).to eq public_topic
-    end
-  end
 end

spec/policies/map_policy_spec.rb

@@ -7,12 +7,12 @@ RSpec.describe MapPolicy, type: :policy do
     context 'commons' do
       let(:map) { create(:map, permission: :commons) }
       permissions :show? do
-        it 'can view' do
+        it 'permits access' do
           expect(subject).to permit(nil, map)
         end
       end
       permissions :create?, :update?, :destroy? do
-        it 'can not modify' do
+        it 'denies access' do
           expect(subject).to_not permit(nil, map)
         end
       end
@@ -21,7 +21,7 @@ RSpec.describe MapPolicy, type: :policy do
     context 'private' do
       let(:map) { create(:map, permission: :private) }
       permissions :show?, :create?, :update?, :destroy? do
-        it 'can not view or modify' do
+        it 'permits access' do
           expect(subject).to_not permit(nil, map)
         end
       end
@@ -39,15 +39,15 @@ RSpec.describe MapPolicy, type: :policy do
       let(:owner) { create(:user) }
       let(:map) { create(:map, permission: :commons, user: owner) }
       permissions :show?, :create?, :update? do
-        it 'can view and modify' do
+        it 'permits access' do
           expect(subject).to permit(user, map)
         end
       end
       permissions :destroy? do
-        it 'can not destroy' do
+        it 'denies access' do
           expect(subject).to_not permit(user, map)
         end
-        it 'owner can destroy' do
+        it 'permits access to owner' do
           expect(subject).to permit(owner, map)
         end
       end
@@ -56,21 +56,16 @@ RSpec.describe MapPolicy, type: :policy do
     context 'public' do
       let(:owner) { create(:user) }
       let(:map) { create(:map, permission: :public, user: owner) }
-      permissions :show? do
-        it 'can view' do
-          expect(subject).to permit(user, map)
-        end
-      end
-      permissions :create? do
-        it 'can create' do
+      permissions :show?, :create? do
+        it 'permits access' do
           expect(subject).to permit(user, map)
         end
       end
       permissions :update?, :destroy? do
-        it 'can not update/destroy' do
+        it 'denies access' do
           expect(subject).to_not permit(user, map)
         end
-        it 'owner can update/destroy' do
+        it 'permits access to owner' do
           expect(subject).to permit(owner, map)
         end
       end
@@ -80,15 +75,15 @@ RSpec.describe MapPolicy, type: :policy do
       let(:owner) { create(:user) }
       let(:map) { create(:map, permission: :private, user: owner) }
       permissions :create? do
-        it 'can create' do
+        it 'permits access' do
           expect(subject).to permit(user, map)
         end
       end
       permissions :show?, :update?, :destroy? do
-        it 'can not view or modify' do
+        it 'denies access' do
           expect(subject).to_not permit(user, map)
         end
-        it 'owner can view and modify' do
+        it 'permits access to owner' do
           expect(subject).to permit(owner, map)
         end
       end

spec/policies/mapping_policy_spec.rb

@@ -0,0 +1,7 @@
+require 'rails_helper'
+
+RSpec.describe MappingPolicy, type: :policy do
+  subject { described_class }
+
+  pending 'Implement some mapping tests!'
+end

spec/policies/synapse_policy.rb

@@ -0,0 +1,92 @@
+require 'rails_helper'
+
+RSpec.describe SynapsePolicy, type: :policy do
+  subject { described_class }
+
+  context 'unauthenticated' do
+    context 'commons' do
+      let(:synapse) { create(:synapse, permission: :commons) }
+      permissions :show? do
+        it 'permits access' do
+          expect(subject).to permit(nil, synapse)
+        end
+      end
+      permissions :create?, :update?, :destroy? do
+        it 'denies access' do
+          expect(subject).to_not permit(nil, synapse)
+        end
+      end
+    end
+
+    context 'private' do
+      let(:synapse) { create(:synapse, permission: :private) }
+      permissions :show?, :create?, :update?, :destroy? do
+        it 'denies access' do
+          expect(subject).to_not permit(nil, synapse)
+        end
+      end
+    end
+  end
+
+  #
+  # Now begin the logged-in tests
+  #
+
+  context 'logged in' do
+    let(:user) { create(:user) }
+
+    context 'commons' do
+      let(:owner) { create(:user) }
+      let(:synapse) { create(:synapse, permission: :commons, user: owner) }
+      permissions :show?, :create?, :update? do
+        it 'permits access' do
+          expect(subject).to permit(user, synapse)
+        end
+      end
+      permissions :destroy? do
+        it 'denies access' do
+          expect(subject).to_not permit(user, synapse)
+        end
+        it 'permits access to owner' do
+          expect(subject).to permit(owner, synapse)
+        end
+      end
+    end
+
+    context 'public' do
+      let(:owner) { create(:user) }
+      let(:synapse) { create(:synapse, permission: :public, user: owner) }
+      permissions :show?, :create? do
+        it 'permits access' do
+          expect(subject).to permit(user, synapse)
+        end
+      end
+      permissions :update?, :destroy? do
+        it 'denies access' do
+          expect(subject).to_not permit(user, synapse)
+        end
+        it 'permits access to owner' do
+          expect(subject).to permit(owner, synapse)
+        end
+      end
+    end
+
+    context 'private' do
+      let(:owner) { create(:user) }
+      let(:synapse) { create(:synapse, permission: :private, user: owner) }
+      permissions :create? do
+        it 'permits access' do
+          expect(subject).to permit(user, synapse)
+        end
+      end
+      permissions :show?, :update?, :destroy? do
+        it 'denies access' do
+          expect(subject).to_not permit(user, synapse)
+        end
+        it 'permits access to owner' do
+          expect(subject).to permit(owner, synapse)
+        end
+      end
+    end
+  end
+end

spec/policies/topic_policy_spec.rb

@@ -0,0 +1,92 @@
+require 'rails_helper'
+
+RSpec.describe TopicPolicy, type: :policy do
+  subject { described_class }
+
+  context 'unauthenticated' do
+    context 'commons' do
+      let(:topic) { create(:topic, permission: :commons) }
+      permissions :show? do
+        it 'permits access' do
+          expect(subject).to permit(nil, topic)
+        end
+      end
+      permissions :create?, :update?, :destroy? do
+        it 'denies access' do
+          expect(subject).to_not permit(nil, topic)
+        end
+      end
+    end
+
+    context 'private' do
+      let(:topic) { create(:topic, permission: :private) }
+      permissions :show?, :create?, :update?, :destroy? do
+        it 'denies access' do
+          expect(subject).to_not permit(nil, topic)
+        end
+      end
+    end
+  end
+
+  #
+  # Now begin the logged-in tests
+  #
+
+  context 'logged in' do
+    let(:user) { create(:user) }
+
+    context 'commons' do
+      let(:owner) { create(:user) }
+      let(:topic) { create(:topic, permission: :commons, user: owner) }
+      permissions :show?, :create?, :update? do
+        it 'permits access' do
+          expect(subject).to permit(user, topic)
+        end
+      end
+      permissions :destroy? do
+        it 'denies access' do
+          expect(subject).to_not permit(user, topic)
+        end
+        it 'permits access to owner' do
+          expect(subject).to permit(owner, topic)
+        end
+      end
+    end
+
+    context 'public' do
+      let(:owner) { create(:user) }
+      let(:topic) { create(:topic, permission: :public, user: owner) }
+      permissions :show?, :create? do
+        it 'permits access' do
+          expect(subject).to permit(user, topic)
+        end
+      end
+      permissions :update?, :destroy? do
+        it 'denies access' do
+          expect(subject).to_not permit(user, topic)
+        end
+        it 'permits access to owner' do
+          expect(subject).to permit(owner, topic)
+        end
+      end
+    end
+
+    context 'private' do
+      let(:owner) { create(:user) }
+      let(:topic) { create(:topic, permission: :private, user: owner) }
+      permissions :create? do
+        it 'permits access' do
+          expect(subject).to permit(user, topic)
+        end
+      end
+      permissions :show?, :update?, :destroy? do
+        it 'denies access' do
+          expect(subject).to_not permit(user, topic)
+        end
+        it 'permits access to owner' do
+          expect(subject).to permit(owner, topic)
+        end
+      end
+    end
+  end
+end