changes from comments, fix commercials failing test

app/assets/stylesheets/osem.css.scss

@@ -168,3 +168,7 @@ body {
   -webkit-flex: 2;
   flex: 2;
 }
+
+.table20 {
+  width: 20%;
+}

app/controllers/admin/campaigns_controller.rb

@@ -4,7 +4,7 @@ module Admin
     load_and_authorize_resource :campaign, through: :conference
 
     def index
-      authorize! :show, Campaign.new(conference_id: @conference.id)
+      authorize! :index, Campaign.new(conference_id: @conference.id)
       @campaigns = @conference.campaigns
     end
 

app/controllers/admin/commercials_controller.rb

@@ -1,7 +1,7 @@
 module Admin
   class CommercialsController < ApplicationController
     load_and_authorize_resource :conference, find_by: :short_title
-    load_and_authorize_resource through: :conference
+    load_and_authorize_resource through: :conference, except: [:new, :create]
 
     def index
       @commercials = @conference.commercials
@@ -9,12 +9,14 @@ module Admin
 
     def new
       @commercial = @conference.commercials.build
+      authorize! :create, @commercial
     end
 
     def edit; end
 
     def create
       @commercial = @conference.commercials.build(commercial_params)
+      authorize! :create, @commercial
 
       if @commercial.save
         redirect_to admin_conference_commercials_path,

app/controllers/admin/conference_controller.rb

@@ -202,17 +202,14 @@ module Admin
       @user = User.new
       @roles = Role::ACTIONABLES + Role::LABELS
 
-      params[:user] ? (@selected = params[:user][:roles]) : (@selected = 'Organizer')
+      params[:user] ? (@selection = params[:user][:roles].parameterize.underscore) : (@selection = 'organizer')
-      @selection = @selected.parameterize.underscore
       @role = Role.where(name: @selection, resource: @conference)
       @role_users = get_users(@selection)
     end
 
     def add_user
       user = User.find_by(email: params[:user][:email])
-      @selected = params[:role]
+      @selection = params[:role].parameterize.underscore
-      @selection = @selected.parameterize.underscore
-
       @role_users = get_users(@selection)
 
       user.add_role @selection.to_sym, @conference
@@ -220,23 +217,22 @@ module Admin
     end
 
     def remove_user
-      @selected = params[:role]
+      @selection = params[:role]
-      @selection = @selected.parameterize.underscore
-      role = Role.where(name: @selection, resource: @conference).first
-
       @role_users = get_users(@selection)
 
-      @user.revoke role.name.to_sym, @conference
+      @user.revoke @selection.to_sym, @conference
       render 'roles', formats: [:js]
     end
 
     protected
 
-    def get_users(role)
+    def get_users(role_name)
       @role_users = {}
-      get_role = Role.where(name: role, resource: @conference)
+      role = Role.where(name: role_name, resource: @conference)
-      get_role.blank? ? @role_users[role] = get_role : @role_users[role] = get_role.first.users
+      role.blank? ? @role_users[role_name] = role : @role_users[role_name] = role.first.users
 
+      # Initialize @role variable, so that view can show the role description
+      @role = Role.where(name: role_name, resource: @conference)
       @role_users
     end
   end

app/controllers/admin/contacts_controller.rb

@@ -12,18 +12,12 @@ module Admin
     # PATCH/PUT /:conference/contact
     def update
       if @contact.update(contact_params)
-        redirect_to admin_conference_contact_path, notice: 'Contact details were successfully updated.'
+        redirect_to edit_admin_conference_contact_path, notice: 'Contact details were successfully updated.'
       else
         render :edit
       end
     end
 
-    # DELETE /:conference/contact
-    def destroy
-      @contact.destroy
-      redirect_to admin_conference_contacts_url, notice: 'Contact details were successfully destroyed.'
-    end
-
     private
     # Only allow a trusted parameter "white list" through.
     def contact_params

app/controllers/admin/questions_controller.rb

@@ -4,7 +4,7 @@ module Admin
     load_and_authorize_resource through: :conference, except: [:new, :create]
 
     def index
-      authorize! :update, Question.new(conference_id: @conference.id)
+      authorize! :index, Question.new(conference_id: @conference.id)
       @questions = Question.where(global: true).all | Question.where(conference_id: @conference.id)
       @questions_conference = @conference.questions
       @new_question = @conference.questions.new
@@ -32,7 +32,7 @@ module Admin
 
     # GET questions/1/edit
     def edit
-      if @question.global == true && !(current_user.has_role? :organizer, @conference)
+      if @question.global
         redirect_to(admin_conference_questions_path(conference_id: @conference.short_title), alert: "Sorry, you cannot edit global questions. Create a new one.")
       end
     end
@@ -48,6 +48,7 @@ module Admin
 
     # Update questions used for the conference
     def update_conference
+      authorize! :update, Question.new(conference_id: @conference.id)
       if @conference.update_attributes(params[:conference])
         redirect_to(admin_conference_questions_path(conference_id: @conference.short_title), notice: "Questions for #{@conference.short_title} successfully updated.")
       else
@@ -59,7 +60,7 @@ module Admin
     def destroy
       if can? :destroy, @question
         # Do not delete global questions
-        if @question.global == false
+        if !@question.global
 
           # Delete question and its answers
           begin
@@ -67,7 +68,7 @@ module Admin
 
               @question.destroy
               @question.answers.each do |a|
-                a.delete
+                a.destroy
               end
               flash[:notice] = "Deleted question: #{@question.title} and its answers: #{@question.answers.map {|a| a.title}.join ','}"
             end

app/controllers/admin/users_controller.rb

@@ -20,8 +20,11 @@ module Admin
     end
 
     def update
-      @user.update_attributes!(params[:user])
+      if @user.update_attributes(params[:user])
         redirect_to admin_users_path, notice: "Updated #{@user.email}"
+      else
+        redirect_to admin_users_path, alert: "Could not update #{@user.name}. #{@user.errors.full_messages.join '. '}."
+      end
     end
 
     def edit; end

app/controllers/admin/volunteers_controller.rb

@@ -3,7 +3,7 @@ module Admin
     load_and_authorize_resource :conference, find_by: :short_title
 
     def index
-      if (current_user.has_role? :organizer, @conference) || (current_user.has_role? :volunteer_coordinator, @conference)
+      if can_manage_volunteers(@conference)
         render :index
       else
         authorize! :index, :volunteer
@@ -11,7 +11,7 @@ module Admin
     end
 
     def show
-      if (current_user.has_role? :organizer, @conference) || (current_user.has_role? :volunteer_coordinator, @conference)
+      if can_manage_volunteers(@conference)
         if @conference.use_vpositions
           @volunteers = @conference.registrations.joins(:vchoices).uniq
         else
@@ -23,12 +23,11 @@ module Admin
     end
 
     def update
-      if (current_user.has_role? :organizer, @conference) || (current_user.has_role? :volunteer_coordinator, @conference)
+      if can_manage_volunteers(@conference)
-        begin
+        if @conference.update_attributes(params[:conference])
-          @conference.update_attributes!(params[:conference])
           redirect_to(admin_conference_volunteers_info_path(conference_id: params[:conference_id]), notice: "Volunteering options were successfully updated.")
-        rescue => e
+        else
-          redirect_to(admin_conference_volunteers_info_path(conference_id: params[:conference_id]), alert: "Volunteering options update failed: #{e.message}")
+          redirect_to(admin_conference_volunteers_info_path(conference_id: params[:conference_id]), alert: "Volunteering options update failed: #{@conference.errors.full_messages.join '. '}")
         end
       else
         authorize! :index, :volunteer

app/helpers/application_helper.rb

@@ -203,4 +203,12 @@ module ApplicationHelper
   def show_roles(roles)
     roles.map { |x| x[0].titleize + ' ' + x[1] }.join ', '
   end
+
+  def can_manage_volunteers(conference)
+    if (current_user.has_role? :organizer, conference) || (current_user.has_role? :volunteer_coordinator, conference)
+      true
+    else
+      false
+    end
+  end
 end

app/views/admin/conference/_roles.html.haml

@@ -6,13 +6,13 @@
   %hr
   .row
     .col-md-6
-      = semantic_form_for(:user, url: add_user_admin_conference_path(@conference.short_title, role: @selected), remote: true) do |f|
+      = semantic_form_for(:user, url: add_user_admin_conference_path(@conference.short_title, role: @selection), remote: true) do |f|
         %h4
-          = f.input :email, label: "Add role '#{@selected}' to user: ", placeholder: "User's email"
+          = f.input :email, label: "Add role '#{@selection.humanize.titleize}' to user: ", placeholder: "User's email", input_html: { required: 'required' }
-        = f.action :submit, as: :button, label: "Add User", button_html: {value: 'Add', class: 'btn btn-primary'}
+        = f.action :submit, as: :button, label: 'Add User', button_html: {value: 'Add', class: 'btn btn-primary'}
   .row
     .col-md-12
-      %h3 Users with role #{@selected}
+      %h3 Users with role #{@selection.humanize.titleize}
       %table.table.table-striped.table-bordered.table-hover
         %thead
           %th ID
@@ -22,7 +22,7 @@
           - @role_users[@selection].each do |user|
             %tr
               %td
-                = link_to remove_user_admin_conference_path(@conference.short_title, user_id: user.id, role: @selected), method: :delete, remote: true, title: 'Remove user' do
+                = link_to remove_user_admin_conference_path(@conference.short_title, user_id: user.id, role: @selection), method: :delete, remote: true, title: 'Remove user' do
                   %i{class: 'fa fa-times'}
                 = user.id
                 %td= user.name

app/views/admin/questions/index.html.haml

@@ -1,6 +1,7 @@
 .row
   .col-md-8
     .pull-right
+      - if can? :create, Question.new(conference_id: @conference.id)
         %b= link_to 'Create New Question','#', 'data-toggle' => 'modal',
                     'data-target' => '#new-question', class: 'btn btn-success'
       %br
@@ -9,6 +10,7 @@
       = semantic_form_for(@conference, url: admin_conference_questions_update_conference_path(@conference.short_title)) do |f|
         .questions{id: 'myquestions'}
           = render partial: 'questions'
+        - if can? :update, @conference
           = f.submit "Update Questions for #{@conference.short_title}", class: 'btn btn-primary',
                       confirm: 'Are you sure you want to make these changes?'
 

app/views/admin/users/show.html.haml

@@ -4,7 +4,7 @@
 %table.table
   - @show_attributes.each do |attr|
     %tr
-      %td{style: 'width:20%'}
+      %td{class: 'table20'}
         %b
           = attr.capitalize.gsub('_', ' ')
       - if attr == 'roles'

config/routes.rb

@@ -98,10 +98,11 @@ Osem::Application.routes.draw do
     resource :schedule, only: [] do
       get "/" => "schedule#index"
     end
-    member do
+
     get "/register" => "conference_registration#register"
     patch "/register" => "conference_registration#update"
     delete "/register" => "conference_registration#unregister"
+    member do
       get "gallery_photos"
       patch "subscription" => "conference#subscribe"
       delete "subscription" => "conference#unsubscribe"

db/migrate/20140730104658_migrate_roles_for_can_can.rb

@@ -1,28 +0,0 @@
-class MigrateRolesForCanCan < ActiveRecord::Migration
-
-  def up
-    # Store the number of existing roles
-    old_roles = Role.count
-
-    Role.all.each do |role|
-      role.users.each do |user|
-        Conference.all.each do |conference|
-          if role.name == 'Admin' || role.name == 'Organizer'
-            user.add_role :organizer, conference
-          else
-            user.add_role role.name.parameterize.underscore.to_sym, conference
-          end
-        end
-      end
-    end
-
-    # Delete old roles
-    Role.first(old_roles).each do |role|
-      role.destroy
-    end
-  end
-
-  def down
-    raise ActiveRecord::IrreversibleMigration.new('Cannot reverse migration. Deleted events cannot be re-created')
-  end
-end

spec/features/ability_spec.rb

@@ -205,7 +205,6 @@ feature 'Has correct abilities' do
   scenario 'when user is volunteer coordinator' do
     sign_in user
     visit admin_conference_path(conference4.short_title)
-    save_and_open_page
 
     expect(page.has_content?('Basics')).to be true
     expect(page.has_content?('Dashboard')).to be true