[Fix Cart Latency] Remove CartReservations!

app/controllers/application_controller.rb

@@ -66,7 +66,7 @@ def first_time_user
   def cart
     session[:cart] ||= Cart.new
     if session[:cart].reserver_id.nil?
-      session[:cart].set_reserver_id(current_user.id) if current_user
+      session[:cart].reserver_id = current_user.id if current_user
     end
     session[:cart]
   end
@@ -110,7 +110,7 @@ def check_view_mode
   end
 
   def fix_cart_date
-    cart.set_start_date(Date.today) if cart.start_date < Date.today
+    cart.start_date = (Date.today) if cart.start_date < Date.today
   end
 
   #-------- end before_filter methods --------#
@@ -120,16 +120,16 @@ def update_cart
     cart = session[:cart]
     flash.clear
     begin
-      cart.set_start_date(Date.strptime(params[:cart][:start_date_cart],'%m/%d/%Y'))
-      cart.set_due_date(Date.strptime(params[:cart][:due_date_cart],'%m/%d/%Y'))
-      cart.set_reserver_id(params[:reserver_id])
-    rescue ArgumentError => e
+      cart.start_date = Date.strptime(params[:cart][:start_date_cart],'%m/%d/%Y')
+      cart.due_date = Date.strptime(params[:cart][:due_date_cart],'%m/%d/%Y')
+      cart.reserver_id = params[:reserver_id]
+    rescue ArgumentError
       cart.set_start_date(Date.today)
       flash[:error] = "Please enter a valid start or due date."
     end
 
     # validate
-    errors = Reservation.validate_set(cart.reserver, cart.cart_reservations)
+    errors = Reservation.validate_set(cart.reserver, cart.prepare_all)
     # don't over-write flash if invalid date was set above
     flash[:error] ||= errors.to_sentence
     flash[:notice] = "Cart updated."
@@ -143,13 +143,8 @@ def update_cart
   end
 
   def empty_cart
-    #destroy old cart reservations
-    current_cart = session[:cart]
-    CartReservation.where(reserver_id: current_cart.reserver.id).destroy_all
-
     session[:cart] = nil
     flash[:notice] = "Cart emptied."
-
     redirect_to root_path
   end
 

app/controllers/catalog_controller.rb

@@ -56,8 +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.cart_reservations)
+      errors = Reservation.validate_set(cart.reserver, cart.prepare_all)
       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.cart_reservations)
+      @errors = Reservation.validate_set(cart.reserver, cart.prepare_all)
       unless @errors.empty?
         if can? :override, :reservation_errors
           flash[:error] = 'Are you sure you want to continue? Please review the errors below.'
@@ -68,7 +68,8 @@ def create
     #using http://stackoverflow.com/questions/7233859/ruby-on-rails-updating-multiple-models-from-the-one-controller as inspiration
     Reservation.transaction do
       begin
-        @errors = Reservation.validate_set(cart.reserver, cart.cart_reservations)
+        cart_reservations = cart.prepare_all
+        @errors = Reservation.validate_set(cart.reserver, cart_reservations)
         if @errors.empty?
           # If the reservation is a finalized reservation, save it as auto-approved ...
           params[:reservation][:approval_status] = "auto"
@@ -83,7 +84,7 @@ def create
           success_message = "This request has been successfully submitted, and is now subject to approval by an administrator."
         end
 
-        cart.cart_reservations.each do |cart_res|
+        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.
@@ -92,7 +93,6 @@ def create
           successful_reservations << @reservation
         end
 
-        cart.items.each { |item| CartReservation.delete(item) }
         session[:cart] = Cart.new
 
         # emails are probably failing---this code was already commented out 2014.06.19, and we don't know why.

app/models/cart.rb

@@ -11,7 +11,7 @@ class Cart
 
   def initialize
     @errors = ActiveModel::Errors.new(self)
-    @items = []
+    @items = Hash.new()
     @start_date = Date.today
     @due_date = Date.tomorrow
     @reserver_id = nil
@@ -37,107 +37,51 @@ def Cart.lookup_ancestors
 
   ## Item methods
 
-  # Adds CartReservation to database; saves ID into items array
+  # Adds equipment model id to items hash
   def add_item(equipment_model)
-    current_item = CartReservation.new(start_date: @start_date,
-      due_date: @due_date, reserver: self.reserver)
-    current_item.equipment_model = equipment_model
-    if current_item.save
-      @items << current_item.id
-    end
+    return if equipment_model.nil?
+    key = equipment_model.id.to_s
+    self.items[key] = self.items[key] ? self.items[key] + 1 : 1
   end
 
-  # Removes CartReservation from database and ID from items array
+  # Remove equipment model id from items hash, or decrement its count
   def remove_item(equipment_model)
-    to_be_deleted = nil
-    @items.each { |item| to_be_deleted = item if CartReservation.find(item).equipment_model == equipment_model }
-    CartReservation.delete(to_be_deleted)
-    @items.delete(to_be_deleted)
-  end
-
-  # Returns CartReservations that correspond to the IDs in the items array
-  def cart_reservations
-    CartReservation.where(id: @items)
-  end
-
-  # Returns a hash of the equipment models in the cart with their quantities
-  def models_with_quantities
-    models = Hash.new
-    cart_reservations.each { |res| models[res.equipment_model.id] = res.same_model_count(cart_reservations) }
-    models
+    return if equipment_model.nil?
+    key = equipment_model.id.to_s
+    self.items[key] = self.items[key] ? self.items[key] - 1 : 0
+    self.items = self.items.except(key) if self.items[key] == 0
   end
 
   def empty?
     @items.empty?
   end
 
-  ## Date methods
-
-  # Sets start date and updates all CartReservations to match
-  def set_start_date(date)
-    date = date.to_time.in_time_zone
-    if date >= Date.today
-      @start_date = date
-      fix_due_date
-      cart_reservations.update_all({start_date: @start_date, due_date: @due_date})
-    end
-  end
-
-  # Sets due date and updates all CartReservations to match
-  def set_due_date(date)
-    date = date.to_time.in_time_zone
-    @due_date = date
-    fix_due_date
-    cart_reservations.update_all({start_date: @start_date, due_date: @due_date})
-  end
-
-  # If the dates were illogical, sets due date to day after start date
-  def fix_due_date
-    if @start_date.to_time.in_time_zone > @due_date
-      #TODO: allow admin to set default reservation length and respect that length here
-      @due_date = @start_date + 1.day
-    end
+  # remove all items from cart
+  def purge_all
+    @items = Hash.new()
   end
 
-  #Create an array of all the reservations that should be renewed instead of having a new reservation
-  def renewable_reservations
-    user_reservations = reserver.reservations
-    renewable_reservations = []
-    @items.each do |item|
-      cart_item_count = item.quantity #renew up to this many of the item
-      matching_reservations = user_reservations.each do |res|
-        # the end date should be the same as the start date
-        # the reservation should be renewable
-        # also the user should only renew as many reservations as they have in their cart
-        if (res.due_date.to_date == @start_date &&
-           res.equipment_model_id == item.equipment_model_id &&
-           cart_item_count > 0 &&
-           res.is_eligible_for_renew?)
-          renewable_reservations << res
-          cart_item_count-= 1
-        end
+  # return array of reservations crafted from the cart contents
+  def prepare_all
+    reservations = []
+    @items.each do |id, quantity|
+      quantity.times do
+        reservations << Reservation.new(reserver: self.reserver,
+                                        start_date: @start_date,
+                                        due_date: @due_date,
+                                        equipment_model_id: id)
       end
     end
-    return renewable_reservations
+    reservations
   end
 
-
   # Returns the cart's duration
   def duration #in days
     @due_date.to_date - @start_date.to_date + 1
   end
 
-  ## Reserver methods
-
-  #TODO: should only have to set reserver OR reserver_id
-  # Sets reserver id and updates the CartReservations to match
-  def set_reserver_id(user_id)
-    @reserver_id = user_id
-    cart_reservations.update_all(reserver_id: @reserver_id)
-  end
-
   # Returns the reserver
   def reserver
-    reserver = User.find(@reserver_id)
+    User.find(@reserver_id)
   end
 end

app/models/reservation.rb

@@ -105,11 +105,11 @@ def self.validate_set(user, res_array = [])
     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? if self.class == CartReservation
+      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? if self.class == CartReservation
+      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?
@@ -162,7 +162,6 @@ def is_eligible_for_renew?
 
     max_renewal_days = self.equipment_model.maximum_renewal_days_before_due
     max_renewal_days = Float::INFINITY if max_renewal_days == 'unrestricted'
-
     return ((self.due_date.to_date - Date.today).to_i < max_renewal_days ) &&
       (self.times_renewed < max_renewal_times)
   end

app/models/reservation_validations.rb

@@ -58,7 +58,6 @@ def matched_object_and_model?
   # Checks that the reservation is not renewable
   #TODO: allow admin override
   def not_renewable?
-    return true unless self.class == CartReservation || self.status == "reserved"
     reserver.reservations.each do |res|
       if res.equipment_model == self.equipment_model && res.due_date.to_date == self.start_date.to_date && res.is_eligible_for_renew?
         errors.add(:base, res.equipment_model.name + " should be renewed instead of re-checked out.\n")
@@ -184,7 +183,7 @@ def get_overlapping_reservations(reservations)
     #include all reservations made by user
     reservations.concat(reserver.reservations)
     reservations.uniq!
-    reservations.select{ |res| res.overlaps_with?(self) && (res.class == CartReservation || res.checked_in == nil) }
+    reservations.select{ |res| res.overlaps_with?(self) && (res.checked_in == nil) }
   end
 
 

app/views/reservations/_list_items_in_cart.html.erb

@@ -1,9 +1,9 @@
 <div id="list_items_in_cart">
-  <% if cart.cart_reservations.size == 0 %>
+  <% if cart.items.empty? %>
     <span id="cart_is_empty">(empty)</span>
   <% else %>
     <table id="table_list_items" class="table table-bordered table-striped table-condensed">
-      <% cart.models_with_quantities.each_pair do |model_id, quantity| %>
+      <% cart.items.each do |model_id, quantity| %>
         <tr>
           <td colspan="2"><%= quantity %> x
             <div id="remove_button"><%= link_to "Remove", remove_from_cart_path(EquipmentModel.find(model_id)),
@@ -14,4 +14,4 @@
       <% end %>
     </table>
   <% end %>
-</div>
+</div>

app/views/reservations/_new_request.html.erb

@@ -30,7 +30,7 @@
   </p>
   <p>
     <ul class="rounded-list">
-      <% cart.models_with_quantities.each_pair do |model_id, quantity| %>
+      <% cart.items.each do |model_id, quantity| %>
         <li data-value="<%= quantity %>"><%= link_to EquipmentModel.find(model_id).name, EquipmentModel.find(model_id) %></li>
       <% end %>
     </ul>

app/views/reservations/_new_reservation.html.erb

@@ -30,7 +30,7 @@
   </p>
   <p>
     <ul class="rounded-list">
-      <% cart.models_with_quantities.each_pair do |model_id, quantity| %>
+      <% cart.items.each do |model_id, quantity| %>
         <li data-value="<%= quantity %>"><%= link_to EquipmentModel.find(model_id).name, EquipmentModel.find(model_id) %></li>
       <% end %>
     </ul>

db/migrate/20140702014415_remove_cart_reservations.rb

@@ -0,0 +1,9 @@
+class RemoveCartReservations < ActiveRecord::Migration
+  def up
+    drop_table :cart_reservations
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

spec/controllers/application_controller_spec.rb

@@ -219,7 +219,7 @@ def method_requiring_user
     it 'changes cart.start_date to today if date is in the past' do
       session[:cart].start_date = Date.yesterday
       get :index
-      session[:cart].start_date.should eq(Date.today.to_time)
+      session[:cart].start_date.should eq(Date.today)
     end
     it 'does not change the start_date if date is in the future' do
       session[:cart].start_date = Date.tomorrow
@@ -250,9 +250,9 @@ def method_requiring_user
     before(:each) do
       session[:cart] = Cart.new
       session[:cart].reserver_id = @first_user.id
-      session[:cart].set_start_date(Date.today + 1.day)
-      session[:cart].set_due_date(Date.today + 2.days)
-      
+      session[:cart].start_date = (Date.today + 1.day)
+      session[:cart].due_date = (Date.today + 2.days)
+
 
       equipment_model = FactoryGirl.create(:equipment_model)
       session[:cart].add_item(equipment_model)
@@ -263,11 +263,11 @@ def method_requiring_user
       it 'should update cart dates' do
         new_start = Date.today + 3.days
         new_end = Date.today + 4.days
-        
+
         put :update_cart, cart: {start_date_cart: new_start.strftime('%m/%d/%Y'), due_date_cart: new_end.strftime('%m/%d/%Y')}, reserver_id: @new_reserver.id
-        
-        session[:cart].start_date.should eq(new_start.to_time)
-        session[:cart].due_date.should eq(new_end.to_time)
+
+        session[:cart].start_date.should eq(new_start)
+        session[:cart].due_date.should eq(new_end)
         session[:cart].reserver_id.should eq(@new_reserver.id.to_s)
       end
 
@@ -280,9 +280,9 @@ def method_requiring_user
       it 'should set the flash' do
         new_start = Date.today - 300.days
         new_end = Date.today + 4000.days
-        
+
         put :update_cart, cart: {start_date_cart: new_start.strftime('%m/%d/%Y'), due_date_cart: new_end.strftime('%m/%d/%Y')}, reserver_id: @new_reserver.id
-        
+
         flash.should_not be_empty
      end
    end
@@ -292,12 +292,8 @@ def method_requiring_user
     before(:each) do
       session[:cart] = Cart.new
       session[:cart].reserver_id = @first_user.id
-      @cart_reservation = FactoryGirl.create(:cart_reservation, reserver: @first_user)
       delete :empty_cart
     end
-    it 'destroys cart reservations for the reserver associated with the current cart' do
-      CartReservation.find_by_reserver_id(@first_user.id).should be_nil
-    end
     it 'sets the session[:cart] variable back to nil' do
       session[:cart].should be_nil
     end