Corrections in SchedulesController

- Controler file name pluralized
- Load and authorization fixed and improved
- Consider that save can fail in create action
- Eliminate unnecessary if in update action
- Ability and tests related to the schedules link in the admin sidebar fixed

app/controllers/admin/schedule_controller.rb

@@ -1,54 +0,0 @@
-module Admin
-  class ScheduleController < Admin::BaseController
-    # 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 :schedule
-    load_and_authorize_resource :conference, find_by: :short_title
-    load_and_authorize_resource :program, through: :conference, singleton: true
-    load_resource :venue, through: :conference, singleton: true
-
-    skip_before_action :verify_authenticity_token, only: [:update]
-
-    def index
-      @schedules = @conference.program.schedules
-      @selected_schedule = @conference.program.selected_schedule
-    end
-
-    def create
-      new_schedule = @program.schedules.create
-      redirect_to action: 'show', id: new_schedule.id
-    end
-
-    def show
-      @schedule_id = params[:id].to_i
-      schedule = Schedule.find(@schedule_id)
-      @event_schedules = schedule.event_schedules
-      @unscheduled_events = @program.events.confirmed - schedule.events
-      @selected_schedule_id = @conference.program.selected_schedule.try(:id)
-      @dates = @conference.start_date..@conference.end_date
-      @rooms = (@venue && @venue.rooms.any?) ? @venue.rooms : [Room.new(name: 'No Rooms!', size: 0)]
-    end
-
-    def update
-      if params[:selected_schedule].present?
-        if params[:selected_schedule] == 'true'
-          @program.selected_schedule_id = params[:id].to_i
-        elsif params[:selected_schedule] == 'false' && (@program.selected_schedule_id == params[:id].to_i)
-          @program.selected_schedule_id = nil
-        end
-        @program.save!
-      end
-      render json: { 'status' => 'ok' }
-    end
-
-    def destroy
-      if @schedule.destroy
-        redirect_to admin_conference_schedule_index_path(conference_id: @conference.short_title),
-                    notice: 'Schedule successfully deleted.'
-      else
-        redirect_to admin_conference_schedule_index_path(conference_id: @conference.short_title),
-                    error: "Schedule couldn't be deleted. #{@schedule.errors.full_messages.join('. ')}."
-      end
-    end
-  end
-end

app/controllers/admin/schedules_controller.rb

@@ -0,0 +1,52 @@
+module Admin
+  class SchedulesController < Admin::BaseController
+    # 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_and_authorize_resource :schedule, through: :program
+    load_resource :event_schedules, through: :schedule
+    load_resource :selected_schedule, through: :program, singleton: true
+    load_resource :venue, through: :conference, singleton: true
+
+    skip_before_action :verify_authenticity_token, only: [:update]
+
+    def index; end
+
+    def create
+      if @schedule.save
+        redirect_to action: 'show', id: @schedule.id
+      else
+        redirect_to admin_conference_schedules_path(conference_id: @conference.short_title),
+                    error: 'Could not create schedule'
+      end
+    end
+
+    def show
+      @event_schedules = @schedule.event_schedules
+      @unscheduled_events = @program.events.confirmed - @schedule.events
+      @dates = @conference.start_date..@conference.end_date
+      @rooms = (@venue && @venue.rooms.any?) ? @venue.rooms : [Room.new(name: 'No Rooms!', size: 0)]
+    end
+
+    def update
+      if params[:selected_schedule] == 'true'
+        @program.selected_schedule_id = params[:id].to_i
+      elsif params[:selected_schedule] == 'false' && (@selected_schedule.id == params[:id].to_i)
+        @program.selected_schedule_id = nil
+      end
+      @program.save
+      render json: { 'status' => 'ok' }
+    end
+
+    def destroy
+      if @schedule.destroy
+        redirect_to admin_conference_schedules_path(conference_id: @conference.short_title),
+                    notice: 'Schedule successfully deleted.'
+      else
+        redirect_to admin_conference_schedules_path(conference_id: @conference.short_title),
+                    error: "Schedule couldn't be deleted. #{@schedule.errors.full_messages.join('. ')}."
+      end
+    end
+  end
+end

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

@@ -28,5 +28,5 @@
 
 .row
   .col-md-12.text-right
-    = link_to 'Add Schedule', admin_conference_schedule_index_path(@conference.short_title),
+    = link_to 'Add Schedule', admin_conference_schedules_path(@conference.short_title),
     method: :post, class: 'btn btn-primary'

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

@@ -8,8 +8,8 @@
 .row
   .col-md-2
     Selected schedule
-    = check_box_tag @conference.short_title, @schedule_id, (@schedule_id == @selected_schedule_id),
+    = check_box_tag @conference.short_title, @schedule.id, (@schedule.id == @selected_schedule.try(:id)),
-      method: :put, url: (admin_conference_schedule_path(@conference.short_title, @schedule_id) + '?selected_schedule='),
+      method: :put, url: (admin_conference_schedule_path(@conference.short_title, @schedule) + '?selected_schedule='),
       class: 'switch-checkbox', data: { size: 'small',
                                         off_color: 'warning',
                                         on_text: 'Yes',
@@ -32,5 +32,5 @@
 
 :javascript
   $(document).ready( function() {
-    Schedule.initialize("#{admin_conference_event_schedule_index_path(@conference)}", "#{@schedule_id}");
+    Schedule.initialize("#{admin_conference_event_schedule_index_path(@conference)}", "#{@schedule.id}");
   });

app/views/layouts/_admin_sidebar.html.haml

@@ -81,9 +81,9 @@
           - if can? :update, @conference.program.difficulty_levels.build, conference_id: @conference.id
             %li{:class=> active_nav_li(admin_conference_program_difficulty_levels_path(@conference.short_title))}
               = link_to 'Difficulty Levels', admin_conference_program_difficulty_levels_path(@conference.short_title)
-          - if can? :update, @conference.program.events.build
+          - if can? :update, @conference.program.schedules.build
-            %li{class: active_nav_li(admin_conference_schedule_index_path(@conference.short_title))}
+            %li{class: active_nav_li(admin_conference_schedules_path(@conference.short_title))}
-              = link_to 'Schedules', admin_conference_schedule_index_path(@conference.short_title)
+              = link_to 'Schedules', admin_conference_schedules_path(@conference.short_title)
 
   - if can? :update, Registration.new(conference_id: @conference.id)
     %li{:class=> active_nav_li(admin_conference_registrations_path(@conference.short_title))}

config/routes.rb

@@ -22,7 +22,7 @@ Osem::Application.routes.draw do
     resources :comments, only: [:index]
     resources :conference do
       resource :contact, except: [:index, :new, :create, :show, :destroy]
-      resources :schedule, only: [:index, :create, :show, :update, :destroy]
+      resources :schedules, only: [:index, :create, :show, :update, :destroy]
       resources :event_schedule, only: [:create, :update, :destroy]
       get 'commercials/render_commercial' => 'commercials#render_commercial'
       resources :commercials, only: [:index, :create, :update, :destroy]

spec/features/ability_spec.rb

@@ -39,7 +39,7 @@ feature 'Has correct abilities' do
     expect(page).to have_link('Commercials', href: "/admin/conference/#{conference1.short_title}/commercials")
     expect(page).to have_link('Events', href: "/admin/conference/#{conference1.short_title}/program/events")
     expect(page).to have_link('Registrations', href: "/admin/conference/#{conference1.short_title}/registrations")
-    expect(page).to have_link('Schedule', href: "/admin/conference/#{conference1.short_title}/schedule")
+    expect(page).to have_link('Schedules', href: "/admin/conference/#{conference1.short_title}/schedules")
     expect(page).to have_link('Campaigns', href: "/admin/conference/#{conference1.short_title}/campaigns")
     expect(page).to have_link('Goals', href: "/admin/conference/#{conference1.short_title}/targets")
     expect(page).to have_link('Venue', href: "/admin/conference/#{conference1.short_title}/venue")
@@ -69,8 +69,8 @@ feature 'Has correct abilities' do
     visit admin_conference_program_events_path(conference1.short_title)
     expect(current_path).to eq(admin_conference_program_events_path(conference1.short_title))
 
-    visit admin_conference_schedule_index_path(conference1.short_title)
+    visit admin_conference_schedules_path(conference1.short_title)
-    expect(current_path).to eq(admin_conference_schedule_index_path(conference1.short_title))
+    expect(current_path).to eq(admin_conference_schedules_path(conference1.short_title))
 
     visit admin_conference_campaigns_path(conference1.short_title)
     expect(current_path).to eq(admin_conference_campaigns_path(conference1.short_title))
@@ -117,7 +117,7 @@ feature 'Has correct abilities' do
     expect(page).to have_link('Commercials', href: "/admin/conference/#{conference2.short_title}/commercials")
     expect(page).to have_link('Events', href: "/admin/conference/#{conference2.short_title}/program/events")
     expect(page).to_not have_link('Registrations', href: "/admin/conference/#{conference2.short_title}/registrations")
-    expect(page).to have_link('Schedule', href: "/admin/conference/#{conference2.short_title}/schedule")
+    expect(page).to_not have_link('Schedules', href: "/admin/conference/#{conference2.short_title}/schedules")
     expect(page).to_not have_link('Campaigns', href: "/admin/conference/#{conference2.short_title}/campaigns")
     expect(page).to_not have_link('Goals', href: "/admin/conference/#{conference2.short_title}/targets")
     expect(page).to have_link('Venue', href: "/admin/conference/#{conference2.short_title}/venue")
@@ -147,7 +147,7 @@ feature 'Has correct abilities' do
     visit admin_conference_program_events_path(conference2.short_title)
     expect(current_path).to eq(admin_conference_program_events_path(conference2.short_title))
 
-    visit admin_conference_schedule_index_path(conference2.short_title)
+    visit admin_conference_schedules_path(conference2.short_title)
     expect(current_path).to eq(root_path)
 
     visit admin_conference_campaigns_path(conference2.short_title)
@@ -191,7 +191,7 @@ feature 'Has correct abilities' do
     expect(page).to have_link('Commercials', href: "/admin/conference/#{conference3.short_title}/commercials")
     expect(page).to_not have_link('Events', href: "/admin/conference/#{conference3.short_title}/program/events")
     expect(page).to have_link('Registrations', href: "/admin/conference/#{conference3.short_title}/registrations")
-    expect(page).to_not have_link('Schedule', href: "/admin/conference/#{conference3.short_title}/schedule")
+    expect(page).to_not have_link('Schedules', href: "/admin/conference/#{conference3.short_title}/schedules")
     expect(page).to_not have_link('Campaigns', href: "/admin/conference/#{conference3.short_title}/campaigns")
     expect(page).to_not have_link('Targets', href: "/admin/conference/#{conference3.short_title}/targets")
     expect(page).to_not have_link('Venue', href: "/admin/conference/#{conference3.short_title}/venue")
@@ -221,7 +221,7 @@ feature 'Has correct abilities' do
     visit admin_conference_program_events_path(conference3.short_title)
     expect(current_path).to eq(root_path)
 
-    visit admin_conference_schedule_index_path(conference3.short_title)
+    visit admin_conference_schedules_path(conference3.short_title)
     expect(current_path).to eq(root_path)
 
     visit admin_conference_campaigns_path(conference3.short_title)