672 roles are no longer in a hierarchy

app/models/ability.rb

@@ -4,7 +4,7 @@ class Ability
   def initialize(user)
     # Define abilities for the passed in user here. For example:
     #
-    #   user ||= User.new # guest user (not logged in)
+    #   user ||= User.new # guest user [not logged in]
     #   if user.admin?
     #     can :manage, :all
     #   else
@@ -29,57 +29,56 @@ def initialize(user)
     # See the wiki for details:
     # https://github.com/CanCanCommunity/cancancan/wiki/Defining-Abilities
 
-    user ||= User.new # guest user (not logged in)
+    user ||= User.new # guest user [not logged in]
 
     # Even guests can see the apply button
     # This is revoked for coaches and organizers below.
-    can :view_apply_button, Event
-    can %i(show index archive), Event
+    can %i[view_apply_button], Event
+    can %i[show index archive], Event
 
     if user.role? :pupil
       # Pupils can only edit their own profiles
-      can %i(new create), Profile
-      can %i(index show edit update destroy), Profile, user: { id: user.id }
+      can %i[new create], Profile
+      can %i[index show edit update destroy], Profile, user: { id: user.id }
       # Pupils can only edit their own applications
-      can %i(new create), ApplicationLetter if user.profile.present?
-      can %i(index show edit update check destroy), ApplicationLetter, user: { id: user.id }
+      can %i[new create], ApplicationLetter if user.profile.present?
+      can %i[index show edit update check destroy view_personal_details], ApplicationLetter, user: { id: user.id }
       # Pupils can upload their letters of agreement
-      can [:create], AgreementLetter
-      can %i(new create), Request
-      cannot :view_personal_details, ApplicationLetter, user: { id: !user.id }
-    end
-    if user.role? :coach
+      can :create, AgreementLetter
+      can %i[new create], Request
+
+    elsif user.role? :coach
+      # Coaches can only edit their own profiles
+      can %i[new create], Profile
+      can %i[index show edit update destroy], Profile, user: { id: user.id }
+
       # Coaches can view Applications and participants for and view, upload and download materials for Event
-      can %i(view_applicants view_participants view_material upload_material print_applications download_material), Event
-      can %i(view_and_add_notes show), ApplicationLetter
-      can %i(show index), Request
-      cannot :view_apply_button, Event
-      cannot :check, ApplicationLetter
-    end
-    if user.role? :organizer
-      can %i(index show), Profile
-      can %i(index show view_and_add_notes update_status), ApplicationLetter
-      cannot :update, ApplicationLetter
-      can %i(view_applicants edit_applicants view_participants print_applications
-             manage view_material upload_material print_agreement_letters download_material
-             view_unpublished show_eating_habits print_applications_eating_habits view_hidden), Event
+      can %i[view_applicants view_participants view_material upload_material print_applications download_material], Event
+      can %i[view_and_add_notes show], ApplicationLetter
+      can %i[show index], Request
+      cannot %i[apply view_apply_button], Event
+
+    elsif user.role? :organizer
+      # Organizers can only edit their own profiles
+      can %i[new create index show], Profile
+      can %i[edit update destroy], Profile, user: { id: user.id }
+      can %i[manage set_contact_person set_notes show index], Request
+      can %i[index show view_and_add_notes update_status], ApplicationLetter
+      can %i[manage view_applicants edit_applicants view_participants print_applications view_material
+             upload_material print_agreement_letters download_material view_unpublished show_eating_habits
+             print_applications_eating_habits view_hidden edit update destroy], Event
+      cannot %i[apply view_apply_button], Event
       can :send_email, Email
-      can %i(manage set_contact_person set_notes), Request
-      cannot :apply, Event
-      cannot :view_apply_button, Event
-      can %i(edit update destroy), Event
-      can [:update], ParticipantGroup
+      can :update, ParticipantGroup
 
       # Organizers can update user roles of pupil, coach and organizer, but cannot manage admins and cannot update a role to admin
-      can :manage, User, role: %w(pupil coach organizer)
+      can :manage, User, role: %w[pupil coach organizer]
       cannot :update_role, User, role: 'admin'
       cannot :update_role_to_admin, User
-    end
-    if user.role? :admin
-      can :manage, :all
 
-      can :view_delete_button, ApplicationLetter
-      cannot %i(edit update), ApplicationLetter
+    elsif user.role? :admin
+      can :manage, :all
+      cannot %i[edit update], ApplicationLetter
     end
   end
 end

app/models/user.rb

@@ -39,12 +39,13 @@ class User < ActiveRecord::Base
 
   before_create :set_default_role
 
-  ROLES = %i(pupil coach organizer admin).freeze
+  ROLES = %i[pupil coach organizer admin].freeze
 
   def role?(base_role)
     return false unless role
+    raise "invalid role: " + base_role unless ROLES.include?(base_role)
 
-    ROLES.index(base_role) <= ROLES.index(role.to_sym)
+    base_role.to_sym == role.to_sym
   end
 
   def set_default_role

spec/models/ability_spec.rb

@@ -4,6 +4,28 @@
 require "cancan/matchers"
 
 describe User do
+
+  it "pupil can create its application" do
+    user = FactoryGirl.create(:user, role: :pupil)
+    FactoryGirl.create(:profile, user: user)
+    ability = Ability.new(user)
+
+    expect(ability).to be_able_to(:new, ApplicationLetter)
+    expect(ability).to be_able_to(:create, ApplicationLetter)
+  end
+
+  it "pupil can access its application" do
+    user = FactoryGirl.create(:user, role: :pupil)
+    application = FactoryGirl.create(:application_letter, user: user)
+    ability = Ability.new(user)
+
+    expect(ability).to be_able_to(:edit, application)
+    expect(ability).to be_able_to(:show, application)
+    expect(ability).to be_able_to(:index, application)
+    expect(ability).to be_able_to(:update, application)
+    expect(ability).to be_able_to(:destroy, application)
+  end
+
   %i[pupil coach].each do |role|
     it "can create its profile" do
       user = FactoryGirl.create(:user, role: role)
@@ -38,27 +60,6 @@
       expect(ability).to_not be_able_to(:destroy, another_profile)
     end
 
-    it "can create its application" do
-      user = FactoryGirl.create(:user, role: role)
-      FactoryGirl.create(:profile, user: user)
-      ability = Ability.new(user)
-
-      expect(ability).to be_able_to(:new, ApplicationLetter)
-      expect(ability).to be_able_to(:create, ApplicationLetter)
-    end
-
-    it "can access its application" do
-      user = FactoryGirl.create(:user, role: role)
-      application = FactoryGirl.create(:application_letter, user: user)
-      ability = Ability.new(user)
-
-      expect(ability).to be_able_to(:edit, application)
-      expect(ability).to be_able_to(:show, application)
-      expect(ability).to be_able_to(:index, application)
-      expect(ability).to be_able_to(:update, application)
-      expect(ability).to be_able_to(:destroy, application)
-    end
-
     it "cannot access another user's application" do
       user = FactoryGirl.create(:user, role: role)
       another_user = FactoryGirl.create(:user)

spec/models/user_spec.rb

@@ -126,6 +126,11 @@
     expect(current_application_letter.user.rejected_applications_count(current_event)).to eq(1)
   end
 
+  it "raises an error when given an invalid role" do
+    user = FactoryGirl.build(:user)
+    expect{user.role? :forty_two }.to raise_error
+  end
+
   it "filters for users with Max in their name" do
     max = FactoryGirl.create(:user)
     max.profile = FactoryGirl.create(:profile, first_name: "Max")