Merge pull request #356 from hennevogel/proposals

Refactored proposal. Enabled cancancan

Gemfile

@@ -20,8 +20,8 @@ gem 'omniauth-facebook'
 gem 'omniauth-openid'
 gem 'omniauth-google-oauth2'
 
-# Use cancan as authorization framework
-gem 'cancan'
+# Use cancancan as authorization framework
+gem 'cancancan'
 
 # Use transitions as state machine
 gem 'transitions', :require => %w( transitions active_record/transitions )

Gemfile.lock

@@ -64,7 +64,7 @@ GEM
     byebug (3.1.2)
       columnize (~> 0.8)
       debugger-linecache (~> 1.2)
-    cancan (1.6.10)
+    cancancan (1.8.4)
     capybara (2.2.1)
       mime-types (>= 1.16)
       nokogiri (>= 1.3.3)
@@ -399,7 +399,7 @@ DEPENDENCIES
   axlsx_rails
   bootstrap-sass
   byebug
-  cancan
+  cancancan
   capybara
   chart-js-rails
   cocoon

app/controllers/application_controller.rb

@@ -64,10 +64,6 @@ class ApplicationController < ActionController::Base
     redirect_to root_path unless has_role?(current_user, 'admin')
   end
 
-  def current_ability
-    @current_ability ||= AdminAbility.new(current_user)
-  end
-
   rescue_from CanCan::AccessDenied do |exception|
     Rails.logger.debug("Access denied!")
     redirect_to root_path, alert: exception.message

app/controllers/proposal_controller.rb

@@ -1,203 +1,162 @@
 class ProposalController < ApplicationController
   before_filter :verify_user, except: [:show]
-  before_filter :setup
-  before_filter :verify_access, only: [:edit, :update, :destroy, :confirm, :restart]
-
-  def setup
-    @user = current_user if current_user
-    # FIXME: @conference also comes from verify_user, but we need setup also in show
-    # which can be accessed anonymusly
-    @conference = Conference.find_by(short_title: params[:conference_id])
-    @url = conference_proposal_index_path(@conference.short_title)
-    @event_types = @conference.event_types
-  end
-
-  def verify_access
-    if params.has_key? :proposal_id
-      params[:id] = params[:proposal_id]
-    end
-
-    begin
-      if !organizer_or_admin?
-        @event = @user.events.find(params[:id])
-      else
-        @event = Event.find(params[:id])
-      end
-    rescue => e
-      Rails.logger.debug("Proposal failure in verify_access: #{e.message}")
-      redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
-                  alert: 'Invalid or uneditable proposal.')
-    end
-  end
+  before_action :set_conference, only: [:show]
+  before_action :set_event, only: [:show, :edit, :update, :destroy, :confirm, :restart]
 
   def index
-    @events = @user.proposals @conference
+    @events = current_user.proposals(@conference)
   end
 
-  def destroy
-    proposal = @user.events.find_by_id(params[:id])
-    if proposal
-      proposal.withdraw
-      proposal.save
-      redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
-                  alert: 'Proposal withdrawn.')
-    else
-      redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
-                  alert: 'Error! Could not find proposal!')
-    end
+  def show
+    authorize! :show, @event
+    # FIXME: We should show more than the first speaker
+    @speaker = @event.speakers.first || @event.submitter
   end
 
   def new
+    authorize! :new, Event
+    @url = conference_proposal_index_path(@conference.short_title)
     @event = Event.new
   end
 
   def edit
+    authorize! :edit, @event
     @url = conference_proposal_path(@conference.short_title, params[:id])
-    @event_types = @conference.event_types
     @attachments = @event.event_attachments
-
-    if @event.nil?
-      redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
-                  alert: 'Invalid or uneditable proposal.')
-    end
-  end
-
-  def update
-    session[:return_to] ||= request.referer
-    submitter = params[:user]
-
-    params[:event].delete :users_attributes
-    params[:event].delete :user
-
-    if submitter[:name].blank?
-      redirect_to edit_conference_proposal_path(@conference.short_title, @event),
-                  alert: 'Your name cannot be blank'
-      return
-    end
-
-    if submitter[:biography].blank?
-      redirect_to edit_conference_proposal_path(@conference.short_title, @event),
-                  alert: 'Your biography cannot be blank'
-      return
-    end
-
-    if submitter[:name] != @user.name || submitter[:biography] != @user.biography
-      @user.update_attributes(submitter)
-    end
-
-    event = Event.find_by_id(params[:id])
-
-    begin
-      event.update_attributes!(params[:event])
-      redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
-                  notice: "'#{event.title}' was successfully updated.")
-    rescue => e
-      redirect_to edit_conference_proposal_path(@conference.short_title, @event), alert: e.message
-    end
   end
 
   def create
-    user = current_user
-    session[:return_to] ||= request.referer
+    authorize! :create, Event
+    @url = conference_proposal_index_path(@conference.short_title)
 
-    event_params = params[:event]
-    submitter = params[:user]
     params[:event].delete :user
-
-    @event = Event.new(event_params)
+    @event = Event.new(params[:event])
     @event.conference = @conference
 
-    if submitter[:name].blank?
-      flash[:error] = 'Your public name cannot be blank.'
-      render action: 'new'
-      return
-    end
-
-    if submitter[:biography].blank?
-      flash[:error] = 'Your biography cannot be blank.'
-      render action: 'new'
-      return
-    end
-
     # First, update the submitter's info, if they've changed anything
-    if submitter[:name] != user.name || submitter[:biography] != user.biography
-      user.update_attributes(submitter)
+    current_user.assign_attributes(params[:user])
+    if current_user.changed?
+      current_user.save
     end
 
-    @event.event_users.new(user: user,
+    @event.event_users.new(user: current_user,
                            event_role: 'submitter')
-    @event.event_users.new(user: user,
+    @event.event_users.new(user: current_user,
                            event_role: 'speaker')
 
-    begin
-      @event.save!
-    rescue => e
-      @url = conference_proposal_index_path(@conference.short_title)
-      @event_types = @conference.event_types
-      @user = current_user
-
-      flash[:error] = "Could not submit proposal: #{e.message}"
+    if !@event.save
+      flash[:error] = "Could not submit proposal: #{@event.errors.full_messages.join(', ')}"
       render action: 'new'
       return
     end
 
-    registration = user.registrations.where(conference_id: @conference.id).first
+    registration = current_user.registrations.where(conference_id: @conference.id).first
     ahoy.track 'Event submission', title: 'New submission'
     if registration.nil?
       redirect_to(register_conference_path(@conference.short_title),
-                  notice: 'Event was successfully submitted.\
-                 You probably want to register for the conference now!')
+                  alert: 'Event was successfully submitted.
+                 You should register for the conference now.')
     else
       redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
                   notice: 'Event was successfully submitted.')
     end
   end
 
-  def show
-    @event = Event.find(params[:id])
-    @speaker = @event.speakers.first || @event.submitter
+  def update
+    authorize! :update, @event
+    @url = conference_proposal_path(@conference.short_title, params[:id])
+
+    # First, update the submitter's info, if they've changed anything
+    current_user.assign_attributes(params[:user])
+    if current_user.changed?
+      current_user.save
+    end
+
+    # FIXME: Hmmmmm
+    params[:event].delete :users_attributes
+    params[:event].delete :user
+
+    if !@event.update(params[:event])
+      flash[:error] = "Could not update proposal: #{@event.errors.full_messages.join(', ')}"
+      render action: 'new'
+      return
+    end
+
+    redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
+                notice: "Proposal was successfully updated.")
+  end
+
+  def destroy
+    authorize! :destroy, @event
+    @url = conference_proposal_path(@conference.short_title, params[:id])
+
+    begin
+      @event.withdraw
+    rescue Transitions::InvalidTransition
+      redirect_to(:back, error: "Event can't be withdrawn")
+      return
+    end
+
+    @event.save(validate: false)
+    redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
+                notice: "Proposal was successfully withdrawn.")
   end
 
   def confirm
-    if @event.transition_possible? :confirm
-      begin
-        @event.confirm!
-      rescue Transitions::InvalidTransition => e
-        redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
-                    alert: "Event was NOT confirmed: #{e.message}")
-        return
-      end
+    authorize! :update, @event
+    @url = conference_proposal_path(@conference.short_title, params[:id])
 
-      if !@conference.user_registered?(current_user)
-        redirect_to(register_conference_path(@conference.short_title),
-                    notice: 'Event was confirmed. Please register to attend the conference.')
-        return
-      end
-      redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
-                  notice: 'Event was confirmed.')
-    else
-      redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
-                  alert: 'Event was NOT confirmed!')
+    begin
+      @event.confirm!
+    rescue Transitions::InvalidTransition
+      redirect_to(:back, error: "Event can't be confirmed")
+      return
     end
+
+    if !@event.save
+      flash[:error] = "Could not confirm proposal: #{@event.errors.full_messages.join(', ')}"
+      render action: 'new'
+      return
+    end
+
+    if !@conference.user_registered?(current_user)
+      redirect_to(register_conference_path(@conference.short_title),
+                  alert: 'The proposal was confirmed. Please register to attend the conference.')
+      return
+    end
+    redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
+                notice: 'The proposal was confirmed.')
   end
 
   def restart
-    if @event.transition_possible? :restart
-      begin
-        @event.restart
-        @event.save
-      rescue Transitions::InvalidTransition => e
-        redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
-                    alert: "Event was NOT restarted: #{e.message}")
-        return
-      end
-      # Success
+    authorize! :update, @event
+    @url = conference_proposal_path(@conference.short_title, params[:id])
+    
+    begin
+      @event.restart
+    rescue Transitions::InvalidTransition
       redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
-                  notice: 'Event was restarted. Review pending!')
-    else
-      # Error
-      redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
-                  alert: 'Event was NOT restarted!')
+                  error: "The proposal can't be re-submitted.")
+      return
     end
+
+    if !@event.save
+      flash[:error] = "Could not re-submit proposal: #{@event.errors.full_messages.join(', ')}"
+      render action: 'new'
+      return
+    end
+
+    redirect_to(conference_proposal_index_path(conference_id: @conference.short_title),
+                notice: "The proposal was re-submitted. The #{@conference.short_title} organizers will review it again.")
+  end
+
+  private
+
+  def set_conference
+    @conference = Conference.find_by(short_title: params[:conference_id])
+  end
+
+  def set_event
+    @event = Event.find(params[:id])
   end
 end

app/models/ability.rb

@@ -1,28 +1,17 @@
 class Ability
   include CanCan::Ability
 
-  def initialize(user) # rubocop:disable Lint/UnusedMethodArgument
-    # Define abilities for the passed in user here. For example:
-    #
-    #   user ||= User.new # guest user (not logged in)
-    #   if user.admin?
-    #     can :manage, :all
-    #   else
-    #     can :read, :all
-    #   end
-    #
-    # 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
+  def initialize(user)
+    # guest user (not logged in)
+    user ||= User.new
+    if user.admin? || user.organizer?
+      # An admin can manage everything
+      can :manage, :all
+    else
+      can [:update, :destroy], Event do |event|
+        event.users.include?(user)
+      end
+      can [:create, :read], Event
+    end
   end
 end

app/models/admin_ability.rb

@@ -1,17 +0,0 @@
-class AdminAbility
-  include CanCan::Ability
-
-  def initialize(user)
-    @user = user || User.new # for guest
-    @user.get_roles.each { |role| send(role.name.downcase) }
-  end
-
-  def organizer
-    can :manage, Event
-  end
-
-  def admin
-    organizer
-    can :manage, :all
-  end
-end

app/models/event.rb

@@ -31,10 +31,11 @@ class Event < ActiveRecord::Base
 
   validate :abstract_limit
   validate :before_end_of_conference
-  validate :biography_exists
+  validate :name_and_biography_exists
   validates :title, presence: true
   validates :abstract, presence: true
   validates :event_type, presence: true
+  validates :conference, presence: true
   validates :media_type, inclusion: { in: Conference.media_types.values }, allow_blank: true
 
   scope :confirmed, -> { where(state: 'confirmed') }
@@ -218,8 +219,9 @@ class Event < ActiveRecord::Base
     errors.add(:abstract, "cannot have more than #{max_words} words") if len > max_words
   end
 
-  def biography_exists
-    errors.add(:user_biography, 'must be filled out') if submitter.biography_word_count == 0
+  def name_and_biography_exists
+    errors.add(:biography, "cant' be blank") if submitter.biography.blank?
+    errors.add(:username, " can't be blank") if submitter.name.blank?
   end
 
   # TODO: create a module to be mixed into model to perform same operation

app/models/user.rb

@@ -52,6 +52,14 @@ class User < ActiveRecord::Base
     !!roles.find_by_name(role.to_s.downcase.camelize)
   end
 
+  def admin?
+    role?('Admin')
+  end
+
+  def organizer?
+    role?('Organizer')
+  end
+
   def get_roles
     roles
   end

app/views/proposal/_proposal_form.html.haml

@@ -14,7 +14,7 @@
       %br
       %br
     - else
-      = f.input :event_type_id,:as => :select, :collection => @event_types.map {|x| ["#{x.title} - #{show_time(x.length)}", x.id]}, :include_blank => false, :label => "Session Type"
+      = f.input :event_type_id,:as => :select, :collection => @conference.event_types.map {|x| ["#{x.title} - #{show_time(x.length)}", x.id]}, :include_blank => false, :label => "Session Type"
     = f.input :difficulty_level, :as => :select, :collection => @conference.difficulty_levels, :include_blank => "(Please select)" if @conference.use_difficulty_levels
     = f.input :require_registration
     = f.input :abstract, :input_html => {:rows => 5, :class => "span11"},
@@ -41,22 +41,18 @@
     = f.input :media_id, :label => "Media ID", :as => :string
 
   %section#information
-  = f.inputs :name => "Your Information" do
-    = semantic_fields_for @user do |u|
-      = u.input :name, :as => :string, :required => true
-      = u.input :affiliation, :as => :string, :hint => "This could be a company, a user group, or nothing at all."
-      = u.input :biography, :required => true, :input_html => {:rows => 5, :class => 'span11', "onkeyup" => "word_count(this, 'biography-count', 150)"}
-      You have used
-      %span#biography-count #{@user.biography_word_count}
-      words.  Biographies are limited to 150 words.
-      %br
-      %br
-  %section#speakers
-  = f.inputs :name => "Additional Speakers" do
-    Will there be any additional speakers?  If so, please enter their full names, email address, and a short
-    biography for each.
-    = f.input :proposal_additional_speakers, :input_html => {:rows => 5, :class => "span11"}, :label => false
-  = f.action :submit, :as => :button, :label => "Submit Session",  :button_html => {:class => "btn btn-primary"}
+  - if current_user.name.blank? || current_user.biography.blank?
+    = f.inputs :name => "Your Information" do
+      = semantic_fields_for current_user do |u|
+        - if current_user.name.blank?
+          = u.input :name, :as => :string, :required => true
+        - if current_user.biography.blank?
+          = u.input :biography, :required => true, :input_html => {:rows => 5, :class => 'span11', "onkeyup" => "word_count(this, 'biography-count', 150)"}
+          You have used
+          %span#biography-count #{current_user.biography_word_count}
+          words.  Biographies are limited to 150 words.
+  %p.text-right
+    = f.action :submit, :as => :button, :button_html => {:class => "btn btn-success"}
 
 :javascript
   var maxcount = 0;

app/views/proposal/index.html.haml

@@ -3,8 +3,8 @@
     %h1
       = "My Proposals for #{@conference.title}"
     - if @conference.cfp_open? || organizer_or_admin?
-      = link_to "New Proposal", new_conference_proposal_path(@conference.short_title), :class => "btn btn-primary pull-right"
-- if @user.proposal_count(@conference) > 0
+      = link_to "New Proposal", new_conference_proposal_path(@conference.short_title), :class => "btn btn-success pull-right"
+- if current_user.proposal_count(@conference) > 0
   .row
     .col-md-12
       %table.table.table-bordered.table-striped
@@ -32,15 +32,15 @@
               .pull-right
                 - if event.transition_possible? :confirm
                   = link_to 'Confirm',
-                  conference_proposal_confirm_path(@conference.short_title, event),
+                  confirm_conference_proposal_path(@conference.short_title, event),
                   method: :patch, class: 'btn btn-mini btn-success', id: "confirm_proposal_#{event.id}"
                 = link_to 'Edit', edit_conference_proposal_path(@conference.short_title, event.id),
-                class: 'btn btn-mini  btn-primary', id: "edit_proposal_#{event.id}"
+                class: 'btn btn-mini btn-primary', id: "edit_proposal_#{event.id}"
                 - if event.transition_possible? :withdraw
                   = link_to 'Withdraw', conference_proposal_path(@conference.short_title, event.id), method: :delete,
-                              confirm: 'Are you sure you want to withdraw this proposal?', class: 'btn btn-mini btn-danger',
+                              confirm: 'Are you sure you want to withdraw this proposal?', class: 'btn btn-mini btn-warning',
                               id: "delete_proposal_#{event.id}"
                 - if event.state == 'withdrawn' || event.state == 'rejected'
                   = link_to 'Re-Submit',
-                  conference_proposal_restart_path(@conference.short_title, event),
+                  restart_conference_proposal_path(@conference.short_title, event.id),
                   method: :patch, class: 'btn btn-mini btn-success', id: "review_event_#{event.id}"

config/routes.rb

@@ -80,8 +80,10 @@ Osem::Application.routes.draw do
   resources :conference, only: [:show] do
     resources :proposal do
       resources :event_attachment, controller: "event_attachments"
-      patch '/confirm' => 'proposal#confirm'
-      patch '/restart' => 'proposal#restart'
+      member do
+        patch '/confirm' => 'proposal#confirm'
+        patch '/restart' => 'proposal#restart'
+      end
     end
     resource :schedule, only: [] do
       get "/" => "schedule#index"

spec/features/proposal_spec.rb

@@ -11,7 +11,7 @@ feature Event do
              feature: true, js: true do
 
       admin = create(:admin, email: 'admin@example.com')
-      participant = create(:participant, email: 'participant@example.com')
+      participant = create(:participant, email: 'participant@example.com', biography: "")
 
       expected_count = Event.count + 1
       conference = create(:conference)
@@ -37,9 +37,8 @@ feature Event do
       fill_in 'event_media_id', with: '123456'
 
       fill_in 'user_biography', with: 'Lorem ipsum biography'
-      fill_in 'user_name', with: 'Example User'
 
-      click_button 'Submit Session'
+      click_button 'Create Event'
       expect(current_path).to eq(register_conference_path(conference.short_title))
 
       expect(Event.count).to eq(expected_count)
@@ -77,7 +76,7 @@ feature Event do
       expect(page.has_content?('Unconfirmed')).to be true
       click_link "confirm_proposal_#{event.id}"
       expect(flash).
-          to eq('Event was confirmed. Please register to attend the conference.')
+          to eq('The proposal was confirmed. Please register to attend the conference.')
 
       # Register for conference
       find('#register').click
@@ -87,7 +86,7 @@ feature Event do
       visit conference_proposal_index_path(conference.short_title)
       expect(page.has_content?('Confirmed')).to be true
       click_link "delete_proposal_#{event.id}"
-      expect(flash).to eq('Proposal withdrawn.')
+      expect(flash).to eq('Proposal was successfully withdrawn.')
     end
   end
 

spec/models/ability_spec.rb

@@ -0,0 +1,39 @@
+require 'spec_helper'
+require "cancan/matchers"
+
+describe "User" do
+  describe "abilities" do
+    subject(:ability){ Ability.new(user) }
+    let(:user){ nil }
+
+    context "when is an admin" do
+      let!(:user) { create(:admin) }
+
+      it{ should be_able_to(:manage, Event.new) }
+    end
+
+    context "when is an participant" do
+      let(:user) { build(:participant) }
+
+      it{ should_not be_able_to(:manage, Event.new) }
+      it{ should be_able_to(:create, Event.new) }
+      it{ should be_able_to(:read, Event.new) }
+    end
+
+    context "when is an event owner" do
+      let(:user) { create(:participant) }
+      let(:user2) { create(:participant) }
+      let(:myevent) { create(:event, users: [user]) }
+      let(:someevent) { create(:event, users: [user2]) }
+
+      # Users are able to update and destroy their own events
+      it{ should be_able_to(:update, myevent) }
+      it{ should be_able_to(:destroy, 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) }
+    end
+
+  end
+end