From 08624c5b532b2e8c5241b8f24c8c13ee7208ef3c Mon Sep 17 00:00:00 2001 From: Christian Bruckmayer Date: Fri, 1 Apr 2016 15:37:28 -0600 Subject: [PATCH] Adapt ability to allow create a proposal if cfp is open This is necessary because the manage block does not cover it. event.users.include?(user) is always false for :new and :create because the event_users are not saved yet. Introduced in 3e32c58 --- app/controllers/proposal_controller.rb | 19 ++++--------------- app/models/ability.rb | 17 +++++++++++------ app/views/proposal/new.html.haml | 4 ++++ spec/models/ability_spec.rb | 25 +++++++++++++++++-------- 4 files changed, 36 insertions(+), 29 deletions(-) diff --git a/app/controllers/proposal_controller.rb b/app/controllers/proposal_controller.rb index 4efe8555..7eab5cae 100644 --- a/app/controllers/proposal_controller.rb +++ b/app/controllers/proposal_controller.rb @@ -2,7 +2,7 @@ class ProposalController < ApplicationController before_filter :authenticate_user!, except: [:show, :new, :create] load_resource :conference, find_by: :short_title load_resource :program, through: :conference, singleton: true - load_and_authorize_resource :event, parent: false, through: :program, except: [:new, :create] + load_and_authorize_resource :event, parent: false, through: :program def index @event = @program.events.new @@ -16,8 +16,8 @@ class ProposalController < ApplicationController end def new - @event = @program.events.new @event.event_users.new(user: current_user, event_role: 'submitter') if current_user + @event.event_users.new(user: current_user, event_role: 'speaker') if current_user authorize! :new, @event @user = User.new @url = conference_program_proposal_index_path(@conference.short_title) @@ -45,18 +45,7 @@ class ProposalController < ApplicationController end params[:event].delete :user - - @event = Event.new(event_params) - @event.program = @program - - # User which creates the proposal is both `submitter` and `speaker` of proposal - # by default. - # TODO: Allow submitter to add speakers to proposals - @event.event_users.new(user: current_user, - event_role: 'submitter') - @event.event_users.new(user: current_user, - event_role: 'speaker') - authorize! :new, @event + authorize! :create, @event if @event.save ahoy.track 'Event submission', title: 'New submission' @@ -152,7 +141,7 @@ class ProposalController < ApplicationController def event_params params.require(:event).permit(:event_type_id, :track_id, :difficulty_level_id, :title, :subtitle, :abstract, :description, - :require_registration, :max_attendees) + :require_registration, :max_attendees, event_users_attributes: [:user_id, :event_role]) end def user_params diff --git a/app/models/ability.rb b/app/models/ability.rb index 3570c90b..59a37c00 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -58,7 +58,7 @@ class Ability event.new_record? end - can [:new, :create], Event do |event| + can :new, Event do |event| event.program.cfp_open? && event.new_record? end end @@ -82,14 +82,19 @@ class Ability can [:create, :destroy], Subscription, user_id: user.id - can :manage, Event do |event| + can [:read, :update, :destroy], Event do |event| event.users.include?(user) end - # cannot create an event if program does not have open cfp - cannot [:new, :create], Event do |event| - user_inclusion = event.event_users.map { |event_user| event_user.user.id }.compact.include? user.id - !event.program.cfp_open? || !event.new_record? || !user_inclusion + can [:new], Event do |event| + event.program.cfp_open? && event.new_record? + end + + can [:create], Event do |event| + # event.users.include?(user) doesn't work here because the event_user object isn't saved when we call authorize! + # Checks that only the current_user is in event_users + user_inclusion = event.event_users.map { |event_user| event_user.user_id }.uniq - [ user.id ] + user_inclusion.empty? && event.program.cfp_open? && event.new_record? end # can manage the commercials of their own events diff --git a/app/views/proposal/new.html.haml b/app/views/proposal/new.html.haml index 37e415f7..a23b949c 100644 --- a/app/views/proposal/new.html.haml +++ b/app/views/proposal/new.html.haml @@ -61,6 +61,10 @@ = f.input :description, input_html: { rows: 5 }, label: 'Requirements', placeholder: 'Eg. Whiteboard, printer, or something like that.' %p.text-right + = f.semantic_fields_for :event_users do |event_user| + = event_user.inputs :user, :style => 'display: none' + = event_user.inputs :event_role, :style => 'display: none' + = f.submit 'Create Proposal', class: 'btn btn-success' .tab-pane{role: 'tabpanel', id: 'signin'} diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 71884e48..afd75b26 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -34,6 +34,7 @@ describe 'User' do let(:program_with_cfp) { create(:program, cfp: create(:cfp)) } let(:program_without_cfp) { create(:program) } + let(:program_with_cfp_in_the_past) { create(:program, cfp: create(:cfp, start_date: 2.day.ago, end_date: 1.day.ago)) } let(:conference_with_open_registration) { create(:conference) } let!(:open_registration_period) { create(:registration_period, conference: conference_with_open_registration, start_date: Date.current - 6.days) } let(:conference_with_closed_registration) { create(:conference) } @@ -70,8 +71,8 @@ describe 'User' do it{ should_not be_able_to(:manage, registration)} it{ should be_able_to(:new, Event.new(program: program_with_cfp)) } - it{ should_not be_able_to(:new, Event.new(program: program_without_cfp)) } - it{ should_not be_able_to(:create, Event.new(program: program_without_cfp))} + it{ should_not be_able_to(:create, Event.new(program: program_without_cfp)) } + it{ should_not be_able_to(:create, Event.new(program: program_with_cfp_in_the_past)) } it{ should be_able_to(:show, Event.new)} it{ should_not be_able_to(:manage, :any)} @@ -81,6 +82,7 @@ describe 'User' do context 'when user is signed in' do let(:user) { create(:user) } let(:user2) { create(:user) } + let(:event_user) { create(:submitter, user: user) } let(:event_user2) { create(:submitter, user: user2) } let(:subscription) { create(:subscription, user: user) } @@ -103,13 +105,20 @@ describe 'User' do it{ should be_able_to(:create, Subscription.new(user_id: user.id)) } it{ should be_able_to(:destroy, subscription) } - it{ should be_able_to(:manage, user_event_with_cfp) } - it{ should_not be_able_to(:new, Event.new(program: program_without_cfp)) } - it{ should_not be_able_to(:create, Event.new(program: program_without_cfp)) } - it{ should_not be_able_to(:new, Event.new(program: program_with_cfp, event_users: [event_user2])) } - it{ should_not be_able_to(:create, Event.new(program: program_with_cfp, event_users: [event_user2])) } + it{ should be_able_to(:read, user_event_with_cfp) } + it{ should be_able_to(:update, user_event_with_cfp) } + it{ should be_able_to(:destroy, user_event_with_cfp) } + it{ should be_able_to(:create, Event.new(program: program_with_cfp, event_users: [event_user])) } + it{ should be_able_to(:new, Event.new(program: program_with_cfp, event_users: [event_user])) } - it{ should_not be_able_to(:manage, event_unconfirmed) } + it{ should_not be_able_to(:read, create(:event, event_users: [event_user2])) } + it{ should_not be_able_to(:update, create(:event, event_users: [event_user2])) } + it{ should_not be_able_to(:delete, create(:event, event_users: [event_user2])) } + it{ should_not be_able_to(:create, Event.new(program: program_without_cfp, event_users: [event_user])) } + it{ should_not be_able_to(:create, Event.new(program: program_with_cfp_in_the_past, event_users: [event_user])) } + it{ should_not be_able_to(:create, Event.new(program: program_with_cfp, event_users: [event_user2])) } + it{ should_not be_able_to(:new, Event.new(program: program_without_cfp)) } + it{ should_not be_able_to(:new, Event.new(program: program_with_cfp_in_the_past)) } it{ should be_able_to(:create, user_event_with_cfp.commercials.new) } it{ should be_able_to(:manage, user_commercial) }