From 41ae0279b83a80a4bd5ce21315a6ba211b9579ea Mon Sep 17 00:00:00 2001 From: Chaitanya Date: Wed, 19 Apr 2017 21:17:58 +0530 Subject: [PATCH 1/6] Refactor authorization for schedule action of Conferences Controller Remove can ability for schedule and events action in ability.rb as both the actions are no longer present in conferences_controller. Replace load_and_authorize_resource with load_resource for program in the schedules_controller. Closes #1457 --- app/controllers/admin/schedules_controller.rb | 2 +- app/models/ability.rb | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/app/controllers/admin/schedules_controller.rb b/app/controllers/admin/schedules_controller.rb index d3c7cea3..4ed535c2 100644 --- a/app/controllers/admin/schedules_controller.rb +++ b/app/controllers/admin/schedules_controller.rb @@ -3,7 +3,7 @@ module Admin # By authorizing 'conference' resource, we can ensure there will be no unauthorized access to # the schedule of a conference, which should not be accessed in the first place load_and_authorize_resource :conference, find_by: :short_title - load_and_authorize_resource :program, through: :conference, singleton: true + load_resource :program, through: :conference, singleton: true load_and_authorize_resource :schedule, through: :program load_resource :event_schedules, through: :schedule load_resource :selected_schedule, through: :program, singleton: true diff --git a/app/models/ability.rb b/app/models/ability.rb index cc23a631..fcd615cc 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -33,10 +33,6 @@ class Ability can [:show], Conference do |conference| conference.splashpage && conference.splashpage.public == true end - # Can view the schedule - can [:schedule, :events], Conference do |conference| - conference.program.cfp && conference.program.schedule_public - end can :show, Event do |event| event.state == 'confirmed' From 7ea91d7d61a658467d304ebdb0ebe9550bf4553f Mon Sep 17 00:00:00 2001 From: Chaitanya Date: Fri, 21 Apr 2017 10:56:00 +0530 Subject: [PATCH 2/6] Fix models/ability test for schedules --- spec/models/ability_spec.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 6ddc44e6..92218b84 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -24,6 +24,9 @@ describe 'User' do let(:conference_public) { create(:full_conference, splashpage: create(:splashpage, public: true)) } let!(:conference_public_cfp) { create(:cfp, program: conference_public.program) } + let(:schedule_public) { create(:schedule, program: create(:program, schedule_public: true))} + let(:schedule_not_public) { create(:schedule, program: create(:program, schedule_public: false))} + let(:event_confirmed) { create(:event, state: 'confirmed') } let(:event_unconfirmed) { create(:event) } @@ -51,12 +54,8 @@ describe 'User' do it{ should be_able_to(:show, conference_public)} it{ should_not be_able_to(:show, conference_not_public)} - it do - conference_public.program.schedule_public = true - conference_public.program.save - should be_able_to(:schedule, conference_public) - end - it{ should_not be_able_to(:schedule, conference_not_public)} + it{ should be_able_to(:show, schedule_public) } + it{ should_not be_able_to(:show, schedule_not_public) } it{ should be_able_to(:show, event_confirmed)} it{ should_not be_able_to(:show, event_unconfirmed)} From 6de3fdc249f17648df1fd03717dbe826a4684b3a Mon Sep 17 00:00:00 2001 From: Chaitanya Date: Fri, 21 Apr 2017 11:08:12 +0530 Subject: [PATCH 3/6] Fix features/ability test --- app/controllers/admin/schedules_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/admin/schedules_controller.rb b/app/controllers/admin/schedules_controller.rb index 4ed535c2..d3c7cea3 100644 --- a/app/controllers/admin/schedules_controller.rb +++ b/app/controllers/admin/schedules_controller.rb @@ -3,7 +3,7 @@ module Admin # By authorizing 'conference' resource, we can ensure there will be no unauthorized access to # the schedule of a conference, which should not be accessed in the first place load_and_authorize_resource :conference, find_by: :short_title - load_resource :program, through: :conference, singleton: true + load_and_authorize_resource :program, through: :conference, singleton: true load_and_authorize_resource :schedule, through: :program load_resource :event_schedules, through: :schedule load_resource :selected_schedule, through: :program, singleton: true From f38b755e62cbcd2d5bdc455bd9375a951299c134 Mon Sep 17 00:00:00 2001 From: Chaitanya Date: Sat, 22 Apr 2017 16:12:08 +0530 Subject: [PATCH 4/6] Don't allow not_signed_in user to view schedules if not marked as public in program --- app/controllers/schedules_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/schedules_controller.rb b/app/controllers/schedules_controller.rb index 5fdbeecd..96437520 100644 --- a/app/controllers/schedules_controller.rb +++ b/app/controllers/schedules_controller.rb @@ -1,9 +1,9 @@ class SchedulesController < ApplicationController - load_and_authorize_resource protect_from_forgery with: :null_session before_action :respond_to_options load_resource :conference, find_by: :short_title load_resource :program, through: :conference, singleton: true, except: :index + load_and_authorize_resource :selected_schedule, through: :program, singleton: true def show @rooms = @conference.venue.rooms if @conference.venue From f2af6188a109285cd7184e88b89c1958463d87b8 Mon Sep 17 00:00:00 2001 From: Chaitanya Date: Tue, 25 Apr 2017 21:58:13 +0530 Subject: [PATCH 5/6] Fix uninitialized constant selected_event error --- app/controllers/schedules_controller.rb | 10 ++++++---- app/views/conferences/_conference_details.html.haml | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/controllers/schedules_controller.rb b/app/controllers/schedules_controller.rb index 96437520..d46cb219 100644 --- a/app/controllers/schedules_controller.rb +++ b/app/controllers/schedules_controller.rb @@ -3,15 +3,12 @@ class SchedulesController < ApplicationController before_action :respond_to_options load_resource :conference, find_by: :short_title load_resource :program, through: :conference, singleton: true, except: :index + before_action :presence_of_selected_schedule load_and_authorize_resource :selected_schedule, through: :program, singleton: true def show @rooms = @conference.venue.rooms if @conference.venue schedules = @program.selected_event_schedules - unless schedules - redirect_to events_conference_schedule_path(@conference.short_title) - end - @events_xml = schedules.map(&:event).group_by{ |event| event.time.to_date } if schedules @dates = @conference.start_date..@conference.end_date @step_minutes = @program.schedule_interval.minutes @@ -49,4 +46,9 @@ class SchedulesController < ApplicationController format.html { head :ok } end if request.options? end + + def presence_of_selected_schedule + return if @program.selected_event_schedules + redirect_to root_path, notice: 'Program is yet to be scheduled.' + end end diff --git a/app/views/conferences/_conference_details.html.haml b/app/views/conferences/_conference_details.html.haml index bdbd5fc6..8e25850d 100644 --- a/app/views/conferences/_conference_details.html.haml +++ b/app/views/conferences/_conference_details.html.haml @@ -21,7 +21,7 @@ - if !@conference || @conference != conference - if conference.splashpage && conference.splashpage.public = link_to "View Conference", conference_path(conference.short_title), class: 'btn btn-default' - - if conference.program and conference.program.schedule_public + - if conference.program and conference.program.selected_event_schedules and conference.program.schedule_public = link_to "Schedule", conference_schedule_path(conference.short_title), class: 'btn btn-default' - if conference.registration_open? - if conference.user_registered?(current_user) From bf4189132a2ec0a3aeca8bc0ae74b9c1b6ed5183 Mon Sep 17 00:00:00 2001 From: Chaitanya Date: Wed, 26 Apr 2017 12:30:28 +0530 Subject: [PATCH 6/6] Add model-test for access of schedule_public and not_public_schedule to different user roles --- spec/models/ability_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 92218b84..417ed890 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -24,6 +24,7 @@ describe 'User' do let(:conference_public) { create(:full_conference, splashpage: create(:splashpage, public: true)) } let!(:conference_public_cfp) { create(:cfp, program: conference_public.program) } + # Other schedule which is either public or not_public. let(:schedule_public) { create(:schedule, program: create(:program, schedule_public: true))} let(:schedule_not_public) { create(:schedule, program: create(:program, schedule_public: false))} @@ -101,6 +102,9 @@ describe 'User' do it{ should_not be_able_to(:new, Registration.new(conference_id: conference_with_closed_registration.id))} it{ should_not be_able_to(:create, Registration.new(conference_id: conference_with_closed_registration.id))} + it{ should be_able_to(:show, schedule_public) } + it{ should_not be_able_to(:show, schedule_not_public) } + it{ should be_able_to(:index, Ticket) } it{ should be_able_to(:manage, TicketPurchase.new(user_id: user.id)) } @@ -135,6 +139,9 @@ describe 'User' do it{ should be_able_to(:manage, :all) } it{ should_not be_able_to(:destroy, my_conference.program) } it{ should_not be_able_to(:destroy, my_venue) } + + it{ should be_able_to(:show, schedule_public) } + it{ should be_able_to(:show, schedule_not_public) } end shared_examples 'user with any role' do @@ -218,6 +225,9 @@ describe 'User' do it{ should be_able_to(:manage, my_conference.tickets.first) } it{ should_not be_able_to(:manage, conference_public.tickets.first) } + it{ should be_able_to(:show, schedule_public) } + it{ should_not be_able_to(:show, schedule_not_public) } + it{ should be_able_to(:manage, my_registration) } it{ should_not be_able_to(:manage, other_registration) } @@ -289,6 +299,9 @@ describe 'User' do it{ should_not be_able_to(:manage, my_conference.tickets.first) } it{ should_not be_able_to(:manage, conference_public.tickets.first) } + it{ should be_able_to(:show, schedule_public) } + it{ should_not be_able_to(:show, schedule_not_public) } + it{ should_not be_able_to(:manage, my_registration) } it{ should_not be_able_to(:manage, other_registration) } @@ -356,6 +369,9 @@ describe 'User' do it{ should_not be_able_to(:manage, my_conference.tickets.first) } it{ should_not be_able_to(:manage, conference_public.tickets.first) } + it{ should be_able_to(:show, schedule_public) } + it{ should_not be_able_to(:show, schedule_not_public) } + it{ should be_able_to(:manage, my_registration) } it{ should_not be_able_to(:manage, other_registration) } @@ -423,6 +439,9 @@ describe 'User' do it{ should_not be_able_to(:manage, my_conference.tickets.first) } it{ should_not be_able_to(:manage, conference_public.tickets.first) } + it{ should be_able_to(:show, schedule_public) } + it{ should_not be_able_to(:show, schedule_not_public) } + it{ should_not be_able_to(:manage, registration) } it{ should_not be_able_to(:manage, other_registration) }