Add more integration tests

Resolves #416
- add tests for Reservation actions (creation, equipment handling, renewal)
- add tests for Rails Admin routes
- resolve issues with renewals and add specs (see #1218)
- add numerous helpers for integration tests
- misc cleanup of other tests
- resolve deprecation warning reintroduced by #1081
- clean up tests from #987
- refactored the for_eq_model Reservation scope not to call .finalized

app/controllers/categories_controller.rb

@@ -56,7 +56,7 @@ def deactivate
       redirect_to @category
     elsif params[:deactivation_confirmed]
       @category.equipment_models.each do |em|
-        Reservation.for_eq_model(em).each do |r|
+        Reservation.for_eq_model(em).finalized.each do |r|
           r.archive(current_user, 'The category was deactivated.')
             .save(validate: false)
         end

app/controllers/equipment_models_controller.rb

@@ -37,17 +37,16 @@ def show # rubocop:disable AbcSize, MethodLength
 
     calendar_length = 1.month
 
-    @reservation_data =
-      Reservation.active.for_eq_model(@equipment_model).collect do |r|
-        if r.overdue
-          end_date = Time.zone.today + calendar_length
-        else
-          end_date = r.due_date
-        end
-        { start: r.start_date, end: end_date }
-        # the above code mimics the current available? setup to show overdue
-        # equipment as permanently 'out'.
+    @reservation_data = relevant_reservations.collect do |r|
+      if r.overdue
+        end_date = Time.zone.today + calendar_length
+      else
+        end_date = r.due_date
       end
+      { start: r.start_date, end: end_date }
+      # the above code mimics the current available? setup to show overdue
+      # equipment as permanently 'out'.
+    end
 
     @blackouts = Blackout.active.collect do |b|
       { start: b.start_date, end: b.end_date }
@@ -125,7 +124,7 @@ def deactivate
       flash[:notice] = 'Deactivation cancelled.'
       redirect_to @equipment_model
     elsif params[:deactivation_confirmed]
-      Reservation.for_eq_model(@equipment_model).each do |r|
+      Reservation.for_eq_model(@equipment_model).finalized.each do |r|
         r.archive(current_user, 'The equipment model was deactivated.')
           .save(validate: false)
       end

app/decorators/category_decorator.rb

@@ -15,7 +15,7 @@ def make_deactivate_btn
       # find reservations for models in the category in the next week
       res = 0
       object.equipment_models.each do |em|
-        res += Reservation.for_eq_model(em)
+        res += Reservation.for_eq_model(em).finalized
                .reserved_in_date_range(Time.zone.today - 1.day,
                                        Time.zone.today + 7.days)
                .count

app/decorators/equipment_model_decorator.rb

@@ -13,7 +13,7 @@ class EquipmentModelDecorator < ApplicationDecorator
   def make_deactivate_btn
     unless object.deleted_at
       # find reservations in the next week
-      res = Reservation.for_eq_model(object)
+      res = Reservation.for_eq_model(object).finalized
             .reserved_in_date_range(Time.zone.today - 1.day,
                                     Time.zone.today + 7.days)
             .count

app/models/equipment_model.rb

@@ -191,7 +191,7 @@ def num_available_from_source(start_date, due_date, source_reservations)
   def num_available(start_date, due_date)
     # for if you just want the number available, 1 query to get
     # relevant reservations
-    relevant_reservations = Reservation.for_eq_model(self)
+    relevant_reservations = Reservation.for_eq_model(self).finalized
                             .reserved_in_date_range(start_date, due_date)
                             .all
     num_available_from_source(start_date, due_date, relevant_reservations)

app/models/reservation.rb

@@ -168,25 +168,23 @@ def find_renewal_date
     due_date
   end
 
-  def eligible_for_renew?
+  def eligible_for_renew? # rubocop:disable all
     # determines if a reservation is eligible for renewal, based on how many
-    # days before the due date it is and the max number of times one is
-    # allowed to renew
+    # days before the due date it is, the max number of times one is allowed
+    # to renew, and other factors
     #
-    return false if reserver.role == 'banned'
 
-    self.times_renewed ||= 0
-
-    # you can't renew a checked in reservation, or one without an equipment
-    # model
-    return false if checked_in || equipment_item.nil?
+    # check some basic conditions
+    return false if !checked_out? || overdue? || reserver.role == 'banned'
+    return false unless equipment_model.maximum_renewal_length > 0
+    return false unless equipment_model.available_count(due_date + 1.day) > 0
 
-    max_renewal_times = equipment_model.maximum_renewal_times
+    self.times_renewed ||= 0
 
-    max_renewal_days = equipment_model.maximum_renewal_days_before_due
-    ((due_date - Time.zone.today).to_i < max_renewal_days) &&
-      (self.times_renewed < max_renewal_times) &&
-      equipment_model.maximum_renewal_length > 0
+    return false if self.times_renewed >= equipment_model.maximum_renewal_times
+    return false if (due_date - Time.zone.today).to_i >=
+                    equipment_model.maximum_renewal_days_before_due
+    true
   end
 
   def to_cart

app/models/reservation_scopes.rb

@@ -55,7 +55,7 @@ def self.included(base) # rubocop:disable MethodLength, AbcSize
         overlaps_with_date(date).active
       }
       scope :for_eq_model, lambda { |eq_model|
-        where(equipment_model_id: eq_model.id).finalized
+        where(equipment_model_id: eq_model.id)
       }
       scope :active_or_requested, lambda {
         where(status: Reservation.statuses.values_at(

spec/controllers/equipment_items_controller_spec.rb

@@ -13,7 +13,7 @@
       end
       it { is_expected.to respond_with(:success) }
       it { is_expected.to render_template(:index) }
-      it { is_expected.not_to set_the_flash }
+      it { is_expected.not_to set_flash }
       context 'without show deleted' do
         let!(:item_other_cat_active) { FactoryGirl.create(:equipment_item) }
         let!(:item_other_cat_inactive) do
@@ -111,7 +111,7 @@
       end
       it { is_expected.to respond_with(:success) }
       it { is_expected.to render_template(:show) }
-      it { is_expected.not_to set_the_flash }
+      it { is_expected.not_to set_flash }
       it 'should set to correct equipment item' do
         expect(assigns(:equipment_item)).to eq(item)
       end
@@ -136,7 +136,7 @@
       end
       it { is_expected.to respond_with(:success) }
       it { is_expected.to render_template(:new) }
-      it { is_expected.not_to set_the_flash }
+      it { is_expected.not_to set_flash }
       it 'assigns a new equipment item to @equipment_item' do
         expect(assigns(:equipment_item)).to be_new_record
         expect(assigns(:equipment_item)).to be_kind_of(EquipmentItem)
@@ -181,15 +181,15 @@
           expect(EquipmentItem.last.notes).not_to be_nil
           expect(EquipmentItem.last.notes).not_to be('')
         end
-        it { is_expected.to set_the_flash }
+        it { is_expected.to set_flash }
         it { is_expected.to redirect_to(EquipmentItem.last.equipment_model) }
       end
       context 'without valid attributes' do
         before do
           post :create, equipment_item: FactoryGirl.attributes_for(
             :equipment_item, name: nil)
         end
-        it { is_expected.not_to set_the_flash }
+        it { is_expected.not_to set_flash }
         it { is_expected.to render_template(:new) }
         it 'should not save' do
           expect do
@@ -218,7 +218,7 @@
       end
       it { is_expected.to respond_with(:success) }
       it { is_expected.to render_template(:edit) }
-      it { is_expected.not_to set_the_flash }
+      it { is_expected.not_to set_flash }
       it 'sets @equipment_item to selected item' do
         expect(assigns(:equipment_item)).to eq(item)
       end
@@ -242,7 +242,7 @@
               equipment_item: FactoryGirl.attributes_for(:equipment_item,
                                                          name: 'Obj')
         end
-        it { is_expected.to set_the_flash }
+        it { is_expected.to set_flash }
         it 'sets @equipment_item to selected item' do
           expect(assigns(:equipment_item)).to eq(item)
         end
@@ -262,7 +262,7 @@
               equipment_item: FactoryGirl.attributes_for(:equipment_item,
                                                          name: nil)
         end
-        it { is_expected.not_to set_the_flash }
+        it { is_expected.not_to set_flash }
         it 'should not update attributes' do
           item.reload
           expect(item.name).not_to be_nil

spec/factories/equipment_models.rb

@@ -18,7 +18,7 @@
       category { FactoryGirl.create(:category, max_per_user: 1) }
     end
 
-    factory :equipment_model_with_object do
+    factory :equipment_model_with_item do
       after(:create) do |model|
         FactoryGirl.create(:equipment_item, equipment_model: model)
       end

spec/factories/users.rb

@@ -25,6 +25,11 @@
       password_confirmation 'passw0rd'
     end
 
+    factory :superuser do
+      role 'superuser'
+      view_mode 'superuser'
+    end
+
     factory :admin do
       role 'admin'
       view_mode 'admin'

spec/features/announcements_spec.rb

@@ -2,7 +2,6 @@
 
 describe 'Announcements' do
   before :each do
-    app_setup
     Announcement.create! message: 'Hello World',
                          starts_at: Time.zone.today - 1.day,
                          ends_at: Time.zone.today + 1.day

spec/features/auth_spec.rb

@@ -2,9 +2,6 @@
 
 describe 'Authentication' do
   subject { page }
-  before(:each) do
-    app_setup
-  end
 
   shared_examples_for 'valid registration' do
     it { is_expected.to have_content 'Successfully created user.' }
@@ -58,6 +55,22 @@
       env_wrapper('CAS_AUTH' => nil) { example.run }
     end
 
+    context 'testing login and logout helpers' do
+      before { sign_in_as_user(@checkout_person) }
+      it 'signs in the right user' do
+        visit root_path
+        expect(page).to have_content(@checkout_person.name)
+        expect(page).not_to have_link 'Sign In', href: new_user_session_path
+      end
+
+      it 'can also sign out' do
+        sign_out
+        visit root_path
+        expect(page).to have_link 'Sign In', href: new_user_session_path
+        expect(page).not_to have_content(@checkout_person.name)
+      end
+    end
+
     context 'with new user' do
       context 'can register' do
         before do

spec/features/eq_model_spec.rb

@@ -1,11 +1,6 @@
 require 'spec_helper'
 
 describe 'Equipment Model views', type: :feature do
-  before(:each) do
-    app_setup
-    @eq_model = EquipmentModel.first
-  end
-
   context 'show view' do
     context 'pending reservations' do
       before(:each) do
@@ -61,70 +56,46 @@
 
       context 'as superuser' do
         before do
-          # sign_in_as_user @superuser
-          @superuser = FactoryGirl.create :user, role: 'superuser',
-                                                 view_mode: 'superuser'
-          visit root_path
-          click_link 'Sign In', match: :first
-          fill_in 'Email', with: @superuser.email
-          fill_in 'Password', with: 'passw0rd'
-          click_button 'Sign in'
+          sign_in_as_user @superuser
           visit equipment_model_path(@eq_model)
         end
-        after { click_link 'Log Out' } # sign_out }
+        after { sign_out }
 
         it_behaves_like 'can see pending reservations'
       end
 
       context 'as admin' do
         before do
-          # sign_in_as_user @admin
-          visit root_path
-          click_link 'Sign In', match: :first
-          fill_in 'Email', with: @admin.email
-          fill_in 'Password', with: 'passw0rd'
-          click_button 'Sign in'
+          sign_in_as_user @admin
           visit equipment_model_path(@eq_model)
         end
-        after { click_link 'Log Out' } # sign_out }
+        after { sign_out }
 
         it_behaves_like 'can see pending reservations'
       end
 
       context 'as checkout person' do
         before do
-          # sign_in_as_user @superuser
-          @checkout_person = FactoryGirl.create :checkout_person
-          visit root_path
-          click_link 'Sign In', match: :first
-          fill_in 'Email', with: @checkout_person.email
-          fill_in 'Password', with: 'passw0rd'
-          click_button 'Sign in'
+          sign_in_as_user @checkout_person
           visit equipment_model_path(@eq_model)
         end
-        after { click_link 'Log Out' } # sign_out }
+        after { sign_out }
 
         it_behaves_like 'cannot see pending reservations'
       end
 
       context 'as patron' do
         before do
-          # sign_in_as_user @user
-          visit root_path
-          click_link 'Sign In', match: :first
-          fill_in 'Email', with: @user.email
-          fill_in 'Password', with: 'passw0rd'
-          click_button 'Sign in'
+          sign_in_as_user @user
           visit equipment_model_path(@eq_model)
         end
-        after { click_link 'Log Out' } # sign_out }
+        after { sign_out }
 
         it_behaves_like 'cannot see pending reservations'
       end
 
       context 'as guest' do
         before { visit equipment_model_path(@eq_model) }
-
         it_behaves_like 'cannot see pending reservations'
       end
     end

spec/features/equipment_model_views_spec.rb

@@ -0,0 +1,11 @@
+require 'spec_helper'
+
+describe 'Equipment model views' do
+  subject { page }
+
+  context 'index view' do
+    before { visit equipment_models_path }
+    it { is_expected.to have_content('Equipment Models') }
+    it { is_expected.to have_content(@eq_model.name) }
+  end
+end