Merge pull request #629 from ChrisBr/refactoring_new

First stage of refactoring of the ability model and spec

app/models/ability.rb

@@ -1,21 +1,8 @@
 class Ability
   include CanCan::Ability
 
+  # Initializes the ability class
   def initialize(user)
-    # The first argument to `can` is the action you are giving the user permission to do.
-    # If you pass :manage it will apply to every action. Other common actions here are
-    # :read, :create, :update and :destroy.
-    #
-    # The second argument is the resource the user can perform the action on. If you pass
-    # :all it will apply to every resource. Otherwise pass a Ruby class of the resource.
-    #
-    # The third argument is an optional hash of conditions to further filter the objects.
-    # For example, here the user can only update published articles.
-    #
-    #   can :update, Article, :published => true
-    #
-    # See the wiki for details: https://github.com/ryanb/cancan/wiki/Defining-Abilities
-
     # Order Abilities
     # (Check https://github.com/CanCanCommunity/cancancan/wiki/Ability-Precedence)
     # Check roles of user, using rolify. Role name is *case sensitive*
@@ -27,21 +14,75 @@ class Ability
     # The following is wrong because a user will only have 'cfp' role for a specific conference
     # user.is_cfp? # This is always false
 
-    user ||= User.new # guest user (not logged in)
+    user ||= User.new
 
+    # This is what sets up the different abilities
     if user.new_record?
-      guest
+      not_signed_in
     else
+      # This maps the actionables name of the role to its name in the DB.
       roles = Role::ACTIONABLES.map {|i| i.parameterize.underscore}
-      if (user.roles.pluck(:name) & roles).empty? && !user.is_admin # User has no roles
+      # Checks if the user does not have any role and is not an admin
+      if (user.roles.pluck(:name) & roles).empty? && !user.is_admin
         signed_in(user)
       else
-        user_with_roles(user)
+        signed_in_with_roles(user)
       end
     end
   end
 
-  def user_with_roles(user)
+  # Abilities for not signed in users (guests)
+  def not_signed_in
+    can [:index], Conference
+    can [:show], Conference do |conference|
+      conference.splashpage && conference.splashpage.public == true
+    end
+    # Can view the schedule
+    can [:schedule], Conference do |conference|
+      conference.call_for_paper && conference.call_for_paper.schedule_public
+    end
+
+    can :show, Event do |event|
+      event.state == 'confirmed'
+    end
+
+    # can view Commercials of confirmed Events
+    can :show, Commercial, commercialable_type: 'Event', commercialable_id: Event.where(state: 'confirmed').pluck(:id)
+    can :show, User
+    can [:show, :create], Registration do |registration|
+      registration.new_record?
+    end
+  end
+
+  # Abilities for signed in users
+  def signed_in(user)
+    # Abilities from not_signed_in user are also inherited
+    not_signed_in
+
+    can :manage, User, id: user.id
+
+    can :manage, Registration, user_id: user.id
+
+    can :index, Ticket
+    can :manage, TicketPurchase, user_id: user.id
+
+    can [:create, :destroy], Subscription, user_id: user.id
+
+    can :manage, Event do |event|
+      event.users.include?(user)
+    end
+    # can create an event until the last day of a conference
+    can :create, Event, conference_id: Conference.where('end_date >= ?', Date.today).pluck(:id)
+
+    # can manage the commercials of their own events
+    can :manage, Commercial, commercialable_type: 'Event', commercialable_id: user.events.pluck(:id)
+  end
+
+  # Abilities for signed in users with roles
+  def signed_in_with_roles(user)
+    # Abilities from not_signed_in and signed_in are also inherited
+    signed_in(user)
+
     conf_ids_for_organizer = []
     conf_ids_for_cfp = []
     conf_ids_for_info_desk = []
@@ -59,7 +100,6 @@ class Ability
     conf_ids_for_volunteer_coordinator =
         Conference.with_role(:volunteer_coordinator, user).pluck(:id) if user.has_role? :volunteer_coordinator, :any
 
-    signed_in(user) # Inherit abilities from signed user
     # User with role
     can [:new, :create], Conference if user.is_admin || (user.has_role? :organizer, :any)
     can [:index, :show, :gallery_photos], Conference
@@ -101,52 +141,4 @@ class Ability
     can :index, Venue, conference_id: conf_ids_for_organizer + conf_ids_for_cfp
     can :manage, :all if user.is_admin
   end
-
-  # Abilities for everyone, even guests (not logged in users)
-  def guest
-    # can view conferences
-    can [:index], Conference
-    can [:show], Conference do |conference|
-      conference.splashpage && conference.splashpage.public == true
-    end
-    can [:schedule], Conference do |conference|
-      conference.call_for_paper && conference.call_for_paper.schedule_public
-    end
-
-    # can view confirmed Events
-    can :show, Event do |event|
-      event.state == 'confirmed'
-    end
-    # can view Commercials of confirmed Events
-    can :show, Commercial, commercialable_type: 'Event', commercialable_id: Event.where(state: 'confirmed').pluck(:id)
-    # can view others
-    can :show, User
-    # can register
-    can [:read, :create], Registration do |registration|
-        registration.new_record?
-    end
-  end
-
-  def signed_in(user)
-    guest # Inherits abilities of guest
-
-    # subscribe, unsubscribe to a Conference
-    can [:create, :destroy], Subscription, user_id: user.id
-
-    # can manage their own Registration
-    can :manage, Registration, user_id: user.id
-    can :index, Ticket
-    can :manage, TicketPurchase, user_id: user.id
-
-    # can manage their own User
-    can :manage, User, id: user.id
-    cannot :index, User
-
-    # can manage their own Event
-    can :manage, Event, id: user.events.pluck(:id)
-    # can submit Events for conferences that are not over yet
-    can :create, Event, conference_id: Conference.where('end_date >= ?', Date.today).pluck(:id)
-    # can manage their own commercials
-    can :manage, Commercial, commercialable_type: 'Event', commercialable_id: user.events.pluck(:id)
-  end
 end

spec/factories/subscriptions.rb

@@ -0,0 +1,7 @@
+FactoryGirl.define do
+  factory :subscription do
+    user
+    conference
+  end
+
+end

spec/features/ability_spec.rb

@@ -13,11 +13,21 @@ feature 'Has correct abilities' do
   let(:role_info_desk) { create(:role, name: 'info_desk', resource: conference3) }
   let(:role_volunteer_coordinator) { create(:role, name: 'volunteer_coordinator', resource: conference4) }
 
+  let(:user) { create(:user) }
   let(:user_organizer) { create(:user, role_ids: [role_organizer.id]) }
   let(:user_cfp) { create(:user, role_ids: [role_cfp.id]) }
   let(:user_info_desk) { create(:user, role_ids: [role_info_desk.id]) }
   let(:user_volunteer_coordinator) { create(:user, role_ids: [role_volunteer_coordinator.id]) }
 
+  scenario 'when user has no role' do
+    user_organizer.is_admin = false
+    sign_in user
+
+    visit admin_conference_path(conference1.short_title)
+    expect(current_path).to eq root_path
+    expect(flash).to eq 'You are not authorized to access this area!'
+  end
+
   scenario 'when user is organizer' do
     user_organizer.is_admin = false
     sign_in user_organizer

spec/features/base_controller_spec.rb

@@ -0,0 +1,76 @@
+require 'spec_helper'
+
+feature 'BaseController' do
+  let(:conference) { create(:conference) }
+  let(:user) { create(:user) }
+
+  let!(:organizer_role) { create(:role, name: 'organizer', resource: conference) }
+  let!(:volunteers_coordinator_role) { create(:role, name: 'volunteers_coordinator', resource: conference) }
+  let!(:cfp_role) { create(:role, name: 'cfp', resource: conference) }
+  let!(:info_desk_role) { create(:role, name: 'info_desk', resource: conference) }
+  let!(:speaker_role) { create(:role, name: 'speaker', resource: conference) }
+
+  describe 'GET #verify_user_admin' do
+    context 'when user is a guest' do
+      it 'redirects to sign in page' do
+        visit admin_conference_index_path
+        expect(current_path).to eq new_user_session_path
+      end
+    end
+
+    context 'when user is ' do
+      before(:each) do
+        sign_in(user)
+      end
+
+      it 'not an admin it redirects to root_path' do
+        user.is_admin = false
+        visit admin_conference_index_path
+        expect(current_path).to eq root_path
+        expect(flash).to eq 'You are not authorized to access this area!'
+      end
+
+      it 'a speaker it redirects to the root_path' do
+        user.is_admin = false
+        user.role_ids = speaker_role.id
+        visit admin_conference_index_path
+        expect(current_path).to eq root_path
+        expect(flash).to eq 'You are not authorized to access this area!'
+      end
+
+      it 'an admin he can access the admin area' do
+        user.is_admin = true
+        visit admin_conference_index_path
+        expect(current_path).to eq admin_conference_index_path
+      end
+
+      it 'an organizer he can access the admin area' do
+        user.is_admin = false
+        user.role_ids = organizer_role.id
+        visit admin_conference_index_path
+        expect(current_path).to eq admin_conference_index_path
+      end
+
+      it 'a volunteers_coordinator he can access the admin area' do
+        user.is_admin = false
+        user.role_ids = volunteers_coordinator_role.id
+        visit admin_conference_index_path
+        expect(current_path).to eq admin_conference_index_path
+      end
+
+      it 'a cfp he can access the admin area' do
+        user.is_admin = false
+        user.role_ids = cfp_role.id
+        visit admin_conference_index_path
+        expect(current_path).to eq admin_conference_index_path
+      end
+
+      it 'an info_desk he can access the admin area' do
+        user.is_admin = false
+        user.role_ids = info_desk_role.id
+        visit admin_conference_index_path
+        expect(current_path).to eq admin_conference_index_path
+      end
+    end
+  end
+end

spec/models/ability_spec.rb

@@ -3,74 +3,85 @@ require 'cancan/matchers'
 
 describe 'User' do
   describe 'Abilities' do
+    # automatically becomes admin
+    let!(:first_user) { create(:user) }
+
+    # see https://github.com/CanCanCommunity/cancancan/wiki/Testing-Abilities
     subject(:ability){ Ability.new(user) }
-    let!(:first_user) { create(:user) } # automatically becomes admin
     let(:user){ nil }
+
     let(:conference_not_public) { create(:conference, splashpage: create(:splashpage, public: false)) }
     let(:conference_public) { create(:conference, splashpage: create(:splashpage, public: true), call_for_paper: create(:call_for_paper, schedule_public: true)) }
-    let(:event_confirmed) { create(:event, state: 'confirmed') }
-    let(:someevent) { create(:event) }
 
-    context 'when user is a guest' do  # Test abilities for guest users
+    let(:event_confirmed) { create(:event, state: 'confirmed') }
+    let(:event_unconfirmed) { create(:event) }
+
+    let(:commercial_event_confirmed) { create(:commercial, commercialable: event_confirmed) }
+    let(:commercial_event_unconfirmed) { create(:commercial, commercialable: event_unconfirmed) }
+
+    let(:registration) { create(:registration) }
+
+    # Test abilities for not signed in users
+    context 'when user is not signed in' do
+      it{ should be_able_to(:index, Conference)}
 
       it{ should be_able_to(:show, conference_public)}
       it{ should_not be_able_to(:show, conference_not_public)}
 
-      it{ should be_able_to(:show, event_confirmed)}
-      it{ should_not be_able_to(:show, someevent)}
-
       it{ should be_able_to(:schedule, conference_public)}
+      it{ should_not be_able_to(:schedule, conference_not_public)}
+
+      it{ should be_able_to(:show, event_confirmed)}
+      it{ should_not be_able_to(:show, event_unconfirmed)}
+
+      it{ should be_able_to(:show, commercial_event_confirmed)}
+      it{ should_not be_able_to(:show, commercial_event_unconfirmed)}
+
+      it{ should be_able_to(:show, User)}
+
+      it{ should be_able_to(:create, Registration)}
+      it{ should be_able_to(:show, Registration.new)}
+      it{ should_not be_able_to(:manage, registration)}
 
-      it{ should_not be_able_to(:create, Event)}
-      it{ should_not be_able_to(:manage, Event)}
-      it{ should_not be_able_to(:manage, Conference)}
       it{ should_not be_able_to(:manage, :any)}
     end
 
-    context 'when user is a Signed In User' do # Test abilities for signed in users (without any role)
+    # Test abilities for signed in users (without any role)
+    context 'when user is a Signed In User' do
       let(:user) { create(:user) }
-      let(:registration1) { create(:registration, conference: conference_public, user: user) }
-      let(:registration2) { create(:registration, conference: conference_not_public, user: user) }
+      let(:user2) { create(:user) }
+      let(:subscription) { create(:subscription, user: user) }
+      let(:registration_public) { create(:registration, conference: conference_public, user: user) }
+      let(:registration_not_public) { create(:registration, conference: conference_not_public, user: user) }
+
+      let(:my_event) { create(:event, users: [user]) }
+
+      let(:commercial) { create(:commercial, commercialable: event_unconfirmed) }
+      let(:my_commercial) { create(:commercial, commercialable: my_event) }
+
+      it{ should be_able_to(:manage, user) }
+
+      it{ should be_able_to(:manage, registration_public) }
+      it{ should be_able_to(:manage, registration_not_public) }
+
+      it{ should be_able_to(:index, Ticket) }
+      it{ should be_able_to(:manage, TicketPurchase.new(user_id: user.id)) }
+
+      it{ should be_able_to(:create, Subscription.new(user_id: user.id)) }
+      it{ should be_able_to(:destroy, subscription) }
 
       it{ should be_able_to(:create, Event) }
-      it{ should be_able_to(:index, Event) }
-      it{ should_not be_able_to(:manage, Event.new) }
-      it{ should be_able_to(:show, event_confirmed) }
+      it{ should be_able_to(:manage, my_event) }
+      it{ should_not be_able_to(:manage, event_unconfirmed) }
 
-      it{ should be_able_to(:manage, registration1) }
-      it{ should be_able_to(:manage, registration2) }
-
-      it{ should be_able_to(:show, conference_public)}
-      it{ should_not be_able_to(:show, conference_not_public)}
-      it{ should_not be_able_to(:manage, Conference) }
+      it{ should be_able_to(:create, my_event.commercials.new) }
+      it{ should be_able_to(:manage, my_commercial) }
+      it{ should_not be_able_to(:manage, commercial) }
     end
 
     context 'user #is_admin?' do
       let(:user) { create(:admin) }
-      it{ should be_able_to(:manage, User) }
-      it{ should be_able_to(:create, Conference) }
-    end
-
-    context 'signed in users can manage their events' do
-      let(:user) { create(:user) }
-      let(:user2) { create(:user) }
-      let(:myevent) { create(:event, users: [user]) }
-      let(:someevent) { create(:event, users: [user2]) }
-      let(:commercial_myevent) { create(:commercial, commercialable: myevent) }
-      let(:commercial_someevent) { create(:commercial, commercialable: someevent) }
-
-      # Users are able to update and destroy their own events
-      it{ should be_able_to(:update, myevent) }
-      it{ should be_able_to(:destroy, myevent) }
-      it{ should be_able_to(:manage, myevent) }
-      it{ should be_able_to(:create, myevent.commercials.new) }
-      it{ should be_able_to(:manage, commercial_myevent) }
-
-      # Users are not able to update and destroy other users events
-      it{ should_not be_able_to(:update, someevent) }
-      it{ should_not be_able_to(:destroy, someevent) }
-      it{ should_not be_able_to(:manage, someevent) }
-      it{ should_not be_able_to(:manage, commercial_someevent) }
+      it{ should be_able_to(:manage, :all) }
     end
 
     context 'when user is an organizer' do
@@ -79,11 +90,11 @@ describe 'User' do
       let(:role) { create(:organizer_role, resource: conference1) }
       let(:user) { create(:user, role_ids: [role.id]) }
       let(:someuser) { create(:user) }
-      let(:registration1) { create(:registration, user: someuser, conference_id: conference1.id) }
+      let(:registration_public) { create(:registration, user: someuser, conference_id: conference1.id) }
 
       it{ should be_able_to(:manage, conference1) }
       it{ should_not be_able_to(:manage, conference2) }
-      it{ should be_able_to(:manage, registration1) }
+      it{ should be_able_to(:manage, registration_public) }
       it{ should be_able_to(:create, Registration) }
     end
 
@@ -93,7 +104,7 @@ describe 'User' do
       let(:role) { create(:role, name: 'cfp', resource: conference1) }
       let(:user) { create(:user, role_ids: role.id) }
       let(:event) { create(:event, conference_id: conference1.id) }
-      let(:someevent) { create(:event, conference_id: conference2.id) }
+      let(:event_unconfirmed) { create(:event, conference_id: conference2.id) }
       let(:cfp) { create(:call_for_paper, conference: conference1) }
 
       it{ should_not be_able_to(:manage, conference1) }
@@ -102,7 +113,7 @@ describe 'User' do
       it{ should be_able_to(:show, conference1) }
 
       it{ should be_able_to(:manage, event) }
-      it{ should_not be_able_to(:manage, someevent) }
+      it{ should_not be_able_to(:manage, event_unconfirmed) }
 
       it{ should be_able_to(:manage, cfp) }
       it{ should be_able_to(:manage, create(:event_type, conference: conference1)) }