refactor: Cleans tests, improves coverage

Author: cbaudouinjr

app/controllers/manage/application_controller.rb

@@ -9,10 +9,12 @@ def logged_in
   end
 
   def require_director
+    return redirect_to manage_root_path if current_user.staff? unless current_user.try(:director?)
     return redirect_to root_path unless current_user.try(:director?)
   end
 
   def require_director_or_organizer
+    return redirect_to manage_root_path if current_user.staff? unless current_user.try(:director?) || current_user.try(:organizer?)
     return redirect_to root_path unless current_user.try(:director?) || current_user.try(:organizer?)
   end
 

app/controllers/manage/checkins_controller.rb

@@ -1,5 +1,4 @@
 class Manage::CheckinsController < Manage::ApplicationController
-  before_action :require_director_or_organizer
   before_action :set_questionnaire, only: [:show]
 
   respond_to :html, :json

app/controllers/manage/dashboard_controller.rb

@@ -1,4 +1,5 @@
 class Manage::DashboardController < Manage::ApplicationController
+  before_action :require_director_or_organizer
 
   def index
   end

app/controllers/manage/messages_controller.rb

@@ -1,4 +1,5 @@
 class Manage::MessagesController < Manage::ApplicationController
+  before_action :require_director_or_organizer
   before_action :set_message, only: [:show, :edit, :update, :destroy, :deliver, :preview, :duplicate]
   before_action :check_message_access, only: [:edit, :update, :destroy]
   before_action :limit_template_access_to_directors, only: [:template, :template_preview, :template_update, :template_replace_with_default]

app/controllers/manage/stats_controller.rb

@@ -1,4 +1,5 @@
 class Manage::StatsController < Manage::ApplicationController
+  before_action :require_director_or_organizer
 
   respond_to :html, :json
 

app/models/user.rb

@@ -56,6 +56,14 @@ def full_name
     "#{first_name} #{last_name}"
   end
 
+  def staff?
+    self.director? || self.organizer? || self.volunteer?
+  end
+
+  def organizing_staff?
+    self.director? || self.organizer?
+  end
+
   def self.from_omniauth(auth)
     matching_provider = where(provider: auth.provider, uid: auth.uid)
     matching_email = where(email: auth.info.email)

app/views/layouts/_header.html.haml

@@ -10,6 +10,6 @@
         = btn_link_to "Home", homepage_url
     .header-nav
       - if user_signed_in?
-        - if current_user.director? or current_user.organizer? or current_user.volunteer?
+        - if current_user.staff?
           = btn_link_to "Manage", manage_root_path
         = btn_link_to "Sign Out", destroy_user_session_path, method: :delete

app/views/layouts/manage/application.html.haml

@@ -28,20 +28,32 @@
             %h6.sidebar-heading.d-flex.justify-content-between.align-items-center.px-3.mt-4.mb-1.text-muted
               %span
                 = t(:overview, scope: 'layouts.manage.navigation')
-            %ul.nav.flex-column.mb-2
-              %li.nav-item
-                = active_link_to manage_root_path, class: "nav-link", active_children: false do
-                  .fa.fa-area-chart.fa-fw.icon-space-r-half
-                  = t(:title, scope: 'pages.manage.dashboard')
-              %li.nav-item
-                = active_link_to manage_questionnaires_path, class: "nav-link" do
-                  .fa.fa-inbox.fa-fw.icon-space-r-half
-                  = t(:title, scope: 'pages.manage.questionnaires')
-              %li.nav-item
-                = active_link_to manage_messages_path, class: "nav-link" do
-                  .fa.fa-bullhorn.fa-fw.icon-space-r-half
-                  = t(:title, scope: 'pages.manage.messages')
+            - if current_user.director?
+              %ul.nav.flex-column.mb-2
+                %li.nav-item
+                  = active_link_to manage_root_path, class: "nav-link", active_children: false do
+                    .fa.fa-area-chart.fa-fw.icon-space-r-half
+                    = t(:title, scope: 'pages.manage.dashboard')
+                %li.nav-item
+                  = active_link_to manage_questionnaires_path, class: "nav-link" do
+                    .fa.fa-inbox.fa-fw.icon-space-r-half
+                    = t(:title, scope: 'pages.manage.questionnaires')
+                %li.nav-item
+                  = active_link_to manage_messages_path, class: "nav-link" do
+                    .fa.fa-bullhorn.fa-fw.icon-space-r-half
+                    = t(:title, scope: 'pages.manage.messages')
+            - else
+              %ul.nav.flex-column.mb-2
+                %li.nav-item
+                = active_link_to manage_checkins_path, class: "nav-link" do
+                  .fa.fa-drivers-license-o.fa-fw.icon-space-r-half
+                  = t(:title, scope: 'pages.manage.check-in')
+                %li.nav-item
+                  = active_link_to manage_questionnaires_path, class: "nav-link" do
+                    .fa.fa-inbox.fa-fw.icon-space-r-half
+                    = t(:title, scope: 'pages.manage.questionnaires')
 
+            - if current_user.organizing_staff?
               %h6.sidebar-heading.d-flex.justify-content-between.align-items-center.px-3.mt-4.mb-1.text-muted
                 %span
                   = t(:logistics, scope: 'layouts.manage.navigation')
@@ -59,18 +71,18 @@
                     .fa.fa-tag.fa-fw.icon-space-r-half
                     = t(:title, scope: 'pages.manage.trackable-tags')
 
-              %h6.sidebar-heading.d-flex.justify-content-between.align-items-center.px-3.mt-4.mb-1.text-muted
-                %span
-                  = t(:travel, scope: 'layouts.manage.navigation')
-              %ul.nav.flex-column.mb-2
-                %li.nav-item
-                  = active_link_to manage_bus_lists_path, class: "nav-link" do
-                    .fa.fa-bus.fa-fw.icon-space-r-half
-                    = t(:title, scope: 'pages.manage.bus-lists')
-                %li.nav-item
-                  = active_link_to manage_schools_path, class: "nav-link" do
-                    .fa.fa-home.fa-fw.icon-space-r-half
-                    = t(:title, scope: 'pages.manage.schools')
+            %h6.sidebar-heading.d-flex.justify-content-between.align-items-center.px-3.mt-4.mb-1.text-muted
+              %span
+                = t(:travel, scope: 'layouts.manage.navigation')
+            %ul.nav.flex-column.mb-2
+              %li.nav-item
+                = active_link_to manage_bus_lists_path, class: "nav-link" do
+                  .fa.fa-bus.fa-fw.icon-space-r-half
+                  = t(:title, scope: 'pages.manage.bus-lists')
+              %li.nav-item
+                = active_link_to manage_schools_path, class: "nav-link" do
+                  .fa.fa-home.fa-fw.icon-space-r-half
+                  = t(:title, scope: 'pages.manage.schools')
 
             - if current_user.director?
               %h6.sidebar-heading.d-flex.justify-content-between.align-items-center.px-3.mt-4.mb-1.text-muted

config/routes.rb

@@ -38,7 +38,15 @@
   end
 
   namespace :manage do
-    root to: "dashboard#index"
+    authenticate :user, ->(u) { u.director? } do
+      root to: "dashboard#index"
+    end
+    authenticate :user, ->(u) { u.organizer? } do
+      root to: "dashboard#index"
+    end
+    authenticate :user, ->(u) { u.volunteer? } do
+      root to: "checkins#index"
+    end
     resources :dashboard do
       get :map_data, on: :collection
       get :todays_activity_data, on: :collection

test/controllers/manage/bus_lists_controller_test.rb

@@ -137,10 +137,76 @@ class Manage::BusListsControllerTest < ActionController::TestCase
     end
   end
 
+  context "while authenticated as a volunteer" do
+    setup do
+      @user = create(:volunteer)
+      @request.env["devise.mapping"] = Devise.mappings[:user]
+      sign_in @user
+    end
+
+    should "allow access to manage_bus_lists#index" do
+      get :index
+      assert_response :success
+    end
+
+    should "allow access to manage_bus_lists#show" do
+      get :show, params: { id: @bus_list }
+      assert_response :success
+    end
+
+    should "not allow access to manage_bus_lists#new" do
+      get :new
+      assert_response :redirect
+      assert_redirected_to manage_bus_lists_path
+    end
+
+    should "not allow access to manage_bus_lists#edit" do
+      get :edit, params: { id: @bus_list }
+      assert_response :redirect
+      assert_redirected_to manage_bus_lists_path
+    end
+
+    should "not allow access to manage_bus_lists#create" do
+      post :create, params: { bus_list: { email: "[email protected]" } }
+      assert_response :redirect
+      assert_redirected_to manage_bus_lists_path
+    end
+
+    should "not allow access to manage_bus_lists#update" do
+      patch :update, params: { id: @bus_list, bus_list: { email: "[email protected]" } }
+      assert_response :redirect
+      assert_redirected_to manage_bus_lists_path
+    end
+
+    should "not allow access to manage_bus_lists#toggle_bus_captain" do
+      questionnaire = create(:questionnaire)
+      assert_difference "enqueued_jobs.size", 0 do
+        patch :toggle_bus_captain, params: { id: @bus_list, questionnaire_id: questionnaire.id, bus_captain: "1" }
+      end
+      assert_equal false, questionnaire.reload.is_bus_captain
+      assert_response :redirect
+      assert_redirected_to manage_bus_lists_path
+    end
+
+    should "not allow access to manage_bus_lists#send_update_email" do
+      assert_difference "enqueued_jobs.size", 0 do
+        patch :send_update_email, params: { id: @bus_list }
+      end
+      assert_response :redirect
+      assert_redirected_to manage_bus_lists_path
+    end
+
+    should "not allow access to manage_bus_lists#destroy" do
+      patch :destroy, params: { id: @bus_list }
+      assert_response :redirect
+      assert_redirected_to manage_bus_lists_path
+    end
+  end
+
   context "while authenticated as an organizer" do
     setup do
       @user = create(:organizer)
-      @request.env["devise.mapping"] = Devise.mappings[:director]
+      @request.env["devise.mapping"] = Devise.mappings[:user]
       sign_in @user
     end
 
@@ -203,7 +269,7 @@ class Manage::BusListsControllerTest < ActionController::TestCase
     end
   end
 
-  context "while authenticated as an admin" do
+  context "while authenticated as a director" do
     setup do
       @user = create(:director)
       @request.env["devise.mapping"] = Devise.mappings[:user]