Fundamental Rewrite of Validation System

app/controllers/application_controller.rb

@@ -130,7 +130,7 @@ def update_cart
     end
 
     # validate
-    errors = Reservation.validate_set(cart.reserver, cart.prepare_all)
+    errors = cart.validate_dates.concat(cart.validate_dates_and_items)
     # don't over-write flash if invalid date was set above
     flash[:error] ||= errors.to_sentence
     flash[:notice] = "Cart updated."

app/controllers/catalog_controller.rb

@@ -56,7 +56,7 @@ def search
     # (or displays the appropriate errors)
     def change_cart(action, item)
       cart.send(action, item)
-      errors = Reservation.validate_set(cart.reserver, cart.prepare_all)
+      errors = cart.validate_items.concat(cart.validate_dates_and_items)
       flash[:error] = errors.to_sentence
       flash[:notice] = "Cart updated."
 

app/controllers/reservations_controller.rb

@@ -49,7 +49,7 @@ def new
       redirect_to catalog_path
     else
       # error handling
-      @errors = Reservation.validate_set(cart.reserver, cart.prepare_all)
+      @errors = cart.validate_all
       unless @errors.empty?
         if can? :override, :reservation_errors
           flash[:error] = 'Are you sure you want to continue? Please review the errors below.'
@@ -69,7 +69,7 @@ def create
     Reservation.transaction do
       begin
         cart_reservations = cart.prepare_all
-        @errors = Reservation.validate_set(cart.reserver, cart_reservations)
+        @errors = cart.validate_all
         if @errors.empty?
           # If the reservation is a finalized reservation, save it as auto-approved ...
           params[:reservation][:approval_status] = "auto"
@@ -87,8 +87,6 @@ def create
         cart_reservations.each do |cart_res|
           @reservation = Reservation.new(params[:reservation])
           @reservation.equipment_model =  cart_res.equipment_model
-          # TODO: is this line needed? it's ugly. we should refactor if it's necessary.
-          @reservation.bypass_validations = true
           @reservation.save!
           successful_reservations << @reservation
         end
@@ -381,10 +379,8 @@ def renew
 
   def review
     set_reservation
-    array_for_validation = []
-    array_for_validation << @reservation
     @all_current_requests_by_user = @reservation.reserver.reservations.requested.delete_if{|res| res.id == @reservation.id}
-    @errors = Reservation.validate_set(@reservation.reserver, array_for_validation)
+    @errors = @reservation.validate
   end
 
   def approve_request

app/models/blackout.rb

@@ -14,8 +14,8 @@ class Blackout < ActiveRecord::Base
   validate :validate_end_date_before_start_date
   # this only matters if a user tries to inject into params because the datepicker
   # doesn't allow form submission of invalid dates
-  
-  scope :active, where("end_date >= ?", Date.today) 
+
+  scope :active, where("end_date >= ?", Date.today)
   def self.blackouts_on_date(date) # Returns the blackout object that blacks out the day if the day is blacked out. Otherwise, returns nil.
     blackouts = []
     Blackout.all.each do |blackout|

app/models/cart.rb

@@ -1,5 +1,6 @@
 class Cart
   include ActiveModel::Validations
+  include CartValidations
   validates :reserver_id, :start_date, :due_date, presence: true
 
   attr_accessor :items, :start_date, :due_date, :reserver_id

app/models/cart_validations.rb

@@ -0,0 +1,79 @@
+module CartValidations
+  def validate_dates
+    # run on date change
+    errors = []
+    # blackouts not on date
+    errors << "blackout exists on start date" if Blackout.hard_blackout_exists_on_date(self.start_date)
+    errors << "blackout exists on end date" if Blackout.hard_blackout_exists_on_date(self.due_date)
+    errors << "overdue reservations" if Reservation.for_reserver(self.reserver_id).overdue.count > 0
+    # for some reason reserver is submitted at the same time as dates
+    return errors
+  end
+
+  def validate_items
+    errors = []
+    relevant = Reservation.for_reserver(self.reserver_id).not_returned
+
+    category = Hash.new
+    # check if under max model count while simultaneously building a category hash
+    self.items.each do |em_id, quantity|
+      model = EquipmentModel.find(em_id)
+      max_models = model.maximum_per_user
+      self.start_date.to_date.upto(self.due_date.to_date) do |d|
+        errors << "over max model count" if relevant.overlaps_with_date(d).for_eq_model(model).count + quantity > max_models
+        break
+      end
+
+      if category.include?(model.category)
+        category[model.category] += quantity
+      else
+        category[model.category] = quantity
+      end
+
+    end
+
+    # check if under max category count
+    category.each do |cat, q|
+      max_cat = cat.maximum_per_user
+      self.start_date.to_date.upto(due_date.to_date) do |d|
+        count = 0
+        relevant.overlaps_with_date(d).each do |r|
+          count += 1 if r.equipment_model.category == cat
+        end
+        errors << "over max category count" if count + q > max_cat
+        break
+      end
+    end
+    return errors
+
+  end
+
+  def validate_dates_and_items
+    # validations that run on both item and date changes
+
+    user_reservations = Reservation.for_reserver(self.reserver_id).checked_out
+    errors = []
+    self.items.each do |item, quantity|
+      model = EquipmentModel.find(item)
+
+      # check availability
+      errors << "not available" if model.num_available(self.start_date, self.due_date) < quantity
+
+      # check maximum checkout length
+      max_length = model.category.max_checkout_length
+      max_length = Float::INFINTIY if max_length == 'unrestricted'
+      errors << "too long checkout length" if self.duration > max_length
+
+      user_reservations.for_eq_model(model).each do |r|
+        errors << "renew, man"  if r.due_date == self.start_date && r.is_eligible_for_renew?
+      end
+    end
+    return errors
+  end
+
+  def validate_all
+    errors = validate_dates
+    errors.concat(validate_items.to_a).concat(validate_dates_and_items.to_a)
+    return errors
+  end
+end

app/models/reservation.rb

@@ -8,24 +8,11 @@ class Reservation < ActiveRecord::Base
   belongs_to :checkout_handler, class_name: 'User'
   belongs_to :checkin_handler, class_name: 'User'
 
-  validates :equipment_model, presence: true
+  validates :equipment_model, :start_date, :due_date, presence: true
+  validate :start_date_before_due_date
+  validate :matched_object_and_model
+  validate :not_in_past, :available, on: :create
 
-  # If there is no equipment model, don't run the validations that would break
-  with_options if: :not_empty? do |r|
-    r.validate  :start_date_before_due_date?, :matched_object_and_model?,
-                :available?
-  end
-
-  # These can't be nested with the above block because with_options clobbers
-  # nested options that are the same (i.e., :if and :if)
-  with_options if: Proc.new {|r| r.not_empty? && !r.bypass_validations} do |r|
-    r.with_options on: :create do |r|
-      r.validate  :not_in_past?, :not_renewable?, :no_overdue_reservations?,
-                  :duration_allowed?, :start_date_is_not_blackout?,
-                  :due_date_is_not_blackout?, :quantity_eq_model_allowed?,
-                  :quantity_cat_allowed?
-    end
-  end
 
   nilify_blanks only: [:notes]
 
@@ -57,10 +44,11 @@ class Reservation < ActiveRecord::Base
   scope :for_reserver, lambda { |reserver| where(reserver_id: reserver) }
   scope :reserved_in_date_range, lambda { |start_date, end_date|
     where("start_date < ? and due_date > ? and (approval_status = ? or approval_status = ?)", end_date, start_date, 'auto', 'approved') }
+  scope :overlaps_with_date, lambda{ |date| where("start_date <= ? and due_date >= ?",date,date) }
 
   #TODO: Why the duplication in checkout_handler and checkout_handler_id (etc)?
-  attr_accessible :checkout_handler, :checkout_handler_id,
-                  :checkin_handler, :checkin_handler_id, :approval_status,
+  attr_accessible :checkout_handler_id,
+                  :checkin_handler_id, :approval_status,
                   :checked_out, :checked_in, :equipment_object,
                   :equipment_object_id, :notes, :notes_unsent, :times_renewed
 
@@ -79,6 +67,18 @@ def reserver
               affiliation: "Deleted")
   end
 
+  def validate
+    # Convert reservation to a cart object and run validations on it
+    # For hard validations, use reservation.valid
+    temp_cart = Cart.new
+    temp_cart.start_date = self.start_date
+    temp_cart.due_date = self.due_date
+    temp_cart.reserver_id = self.reserver_id
+    temp_cart.items = { self.equipment_model.id => 1 }
+    temp_cart.validate_all
+  end
+
+
   def status
     if checked_out.nil?
       if approval_status == 'auto' or approval_status == 'approved'
@@ -95,32 +95,6 @@ def status
     end
   end
 
-  ## Set validation
-  # Checks all validations for
-  # the array of reservations passed in (use with cart.cart_reservations)
-  # Returns an array of error messages or [] if reservations are all valid
-  def self.validate_set(user, res_array = [])
-    errors = []
-    #User reservation validations
-    errors << user.name + " has overdue reservations that prevent new ones from being created. " unless user.reservations.overdue.empty?
-
-    res_array.each do |res|
-      errors << "Reservation cannot be made in the past. " unless res.not_in_past?
-      errors << "Reservation start date must be before due date. " unless res.start_date_before_due_date?
-      errors << "Reservation must be for a piece of equipment. " unless res.not_empty?
-      errors << "#{res.equipment_object.name} must be of type #{res.equipment_model.name}. " unless res.matched_object_and_model?
-      errors << "#{res.equipment_model.name} should be renewed instead of re-checked out. " unless res.not_renewable?
-      errors << "#{res.equipment_model.name} cannot be reserved for more than #{res.equipment_model.category.maximum_checkout_length.to_s} days at a time. " unless res.duration_allowed?
-      errors << "#{res.equipment_model.name} is not available for the full time period requested. " unless res.available?(res_array)
-      errors << "A reservation cannot start on #{res.start_date.strftime('%m/%d')} because equipment cannot be picked up on that date. " unless res.start_date_is_not_blackout?
-      errors << "A reservation cannot end on #{res.due_date.strftime('%m/%d')} because equipment cannot be returned on that date. " unless res.due_date_is_not_blackout?
-      errors << "Cannot reserve more than #{res.equipment_model.maximum_per_user.to_s} #{res.equipment_model.name.pluralize}. " unless res.quantity_eq_model_allowed? res_array
-      errors << "Cannot reserve more than #{res.equipment_model.category.maximum_per_user.to_s} #{res.equipment_model.category.name.pluralize}. " unless res.quantity_cat_allowed? res_array
-    end
-    errors.uniq
-  end
-
-
   def checkout_object_uniqueness(reservations)
     object_ids_taken = []
     reservations.each do |r|