Skip to content
This repository has been archived by the owner on Jul 24, 2020. It is now read-only.

[1501] Availability Overhaul #1538

Merged
merged 1 commit into from
May 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,7 @@ def prepare_catalog_index_vars(eq_models = nil)
# build the hash using class methods that use 0 queries
@availability_hash[em.id] =
[EquipmentItem.for_eq_model(em.id, eq_items)\
- Reservation.number_overdue_for_eq_model(em.id, source_reservations)\
- em.num_reserved(cart.start_date, cart.due_date, source_reservations)\
- em.num_busy(cart.start_date, cart.due_date, source_reservations)\
- cart.items[em.id].to_i, 0].max

# have requirements as part of equipment model itself
Expand Down
19 changes: 2 additions & 17 deletions app/controllers/equipment_models_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -205,31 +205,16 @@ def calendar_name_method
:reserver
end

def calculate_availability # rubocop:disable all
def calculate_availability
# get start and end dates
@start_date = Time.zone.today.beginning_of_week(:sunday)
@end_date = (Time.zone.today + 1.month).end_of_week(:sunday)

reservations =
(Reservation.for_eq_model(@equipment_model.id).active
.overlaps_with_date_range(@start_date, @end_date) + \
Reservation.for_eq_model(@equipment_model).overdue).uniq
max_avail = @equipment_model.equipment_items.active.count

@avail_data = []
(@start_date..@end_date).map do |date|
count = 0
# also hack-y, we need to fix these methods
reservations.each do |res|
if res.overdue && res.checked_out?
count += 1
elsif res.start_date <= date && res.due_date >= date
count += 1
end
end

availability = max_avail - count

availability = @equipment_model.num_available_on(date)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe replace with Reservation.number_for_date_range

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I think that this makes more sense, because the purpose of this loop is setting up the calendar colors, and those depend on the date. Also, overdue_count has to be included manually when the equipment model methods aren't used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I think I meant Reservation.number_for, but your point still stands.

# set up colors
if date < Time.zone.today
color = '#888'
Expand Down
2 changes: 1 addition & 1 deletion app/decorators/equipment_item_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def make_deactivate_btn
res = object.current_reservation
overbooked_dates = []
(Time.zone.today..Time.zone.today + 7.days).each do |date|
overbooked_dates << date.to_s(:short) if em.available_count(date) <= 0
overbooked_dates << date.to_s(:short) if em.num_available_on(date) <= 0
end
onclick_str = "handleDeactivation(this, #{res ? res.id : 'null'}, "\
"#{overbooked_dates});"
Expand Down
10 changes: 10 additions & 0 deletions app/models/cart.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ def get_items # rubocop:disable AccessorMethodName
full_hash
end

def get_categories # rubocop:disable AccessorMethodName
cat_hash = {}
ems = EquipmentModel.where(id: items.keys).includes(:category)
items.each_with_index do |(_em_id, q), index|
cat_hash[ems[index].category] ||= 0
cat_hash[ems[index].category] += q
end
cat_hash
end

# Adds equipment model id to items hash
def add_item(equipment_model, _quantity = nil)
return if equipment_model.nil?
Expand Down
75 changes: 41 additions & 34 deletions app/models/cart_validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,49 +105,57 @@ def check_cookie_limit
errors
end

def check_max_items(count_hash, relevant, count_method)
# generic method for making sure the count_hash doesn't
# contain more than the item max when taking into account
# the relevant reservations

errors = []
count_hash.each do |item, q|
max = item.maximum_per_user

start_date.upto(due_date) do |d|
unless Reservation.send(count_method, d, item.id, relevant) + q > max
next
end
errors << "Only #{max} #{item.name.pluralize(max)} "\
'can be reserved at a time.'
end
end
errors
end

def check_max_ems
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we're no longer running this validation? Or is that something you're planning on adding back? Ignore me, this was an intermediate method, my bad.

# check to make sure that the cart's EMs + the current resever's
# EMs doesn't exceed any limits
# 1 query

# get reserver's active reservations
source = Reservation.active.for_reserver(reserver_id)

count_hash = get_items
relevant = Reservation.for_reserver(reserver_id).active
.includes(:equipment_model).all
check_max_items(count_hash, relevant, :number_for_model_on_date)
errors = []
get_items.each do |model, number_in_cart|
# get highest number of items reserved in date range of cart
valid = Reservation.number_for_date_range(source, start_date..due_date,
equipment_model_id: model.id,
overdue: false).max
valid ||= 0
# count overdue reservations for this eq model
overdue = source.count { |r| r.equipment_model == model && r.overdue }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we should investigate whether using scopes is faster than count (i.e. source.for_eq_model(model.id).overdue)

Copy link
Contributor

@orenyk orenyk May 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After testing this manually it appears as though using ActiveRecord scopes on an existing ActiveRecord_Relation actually ends up running a separate query, so it is definitely not better than using pure Ruby Enumerable methods.


next unless valid + overdue + number_in_cart > model.maximum_per_user

max = model.maximum_per_user
errors << "Only #{max} #{model.name.pluralize(max)} "\
'can be reserved at a time.'
end
errors
end

def check_max_cat
# same but for categories. we need to build the counts of the
# categories ourselves though
# 2 queries

cat_hash = get_categories

cat_hash = {}
ems = EquipmentModel.where(id: items.keys).includes(:category)
items.each_with_index do |(_em_id, q), index|
cat_hash[ems[index].category] ||= 0
cat_hash[ems[index].category] += q
source = Reservation.for_reserver(reserver_id).with_categories.active

# split overdue and non overdue reservations
overdue, source = source.partition(&:overdue)

errors = []
cat_hash.each do |cat, q|
max = cat.maximum_per_user
s = source.select { |r| r.equipment_model.category == cat }
s = Reservation.number_for_date_range(s, start_date..due_date).max
s ||= 0
o = overdue.count { |r| r.equipment_model.category == cat }
next unless s + o + q > max
errors << "Only #{max} #{cat.name.pluralize(max)} "\
'can be reserved at a time.'
end
relevant = Reservation.for_reserver(reserver_id).active
.with_categories.all
check_max_items(cat_hash, relevant, :number_for_category_on_date)
errors
end

def check_availability(model = EquipmentModel.find(items.keys.first),
Expand All @@ -174,8 +182,7 @@ def check_availability(model = EquipmentModel.find(items.keys.first),
# 2 queries

errors = []
max_available = model.num_available_from_source(start_date, due_date,
source_res)
max_available = model.num_available(start_date, due_date, source_res)
return errors unless max_available < quantity
if max_available == 0
error = "No #{model.name.pluralize} are"
Expand Down
77 changes: 30 additions & 47 deletions app/models/equipment_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,70 +164,53 @@ def document_attributes=(document_attributes)
end
end

def num_reserved(start_date, due_date, source_reservations)
# get the number reserved in the source reservations
# uses 0 queries, you need to specify the max number of
# items yourself
max_reserved = 0
start_date.upto(due_date) do |d|
reserved = Reservation.number_for_model_on_date(d, id,
source_reservations)
max_reserved = reserved if reserved > max_reserved
end
max_reserved
def num_busy(start_date, due_date, source)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the thought behind the term "busy," but I feel like it's basically 99% there. Maybe "allocated" or something like that? That's worse, but I'd like to use something that fits more with what we're doing. It's annoying because num_unavailable doesn't work since (in my mind) that would include deactivated items. We'll figure it out.

# get the number busy (not able to be reserved) in the source reservations
# uses 0 queries
max = Reservation.number_for_date_range(source, start_date..due_date,
equipment_model_id: id,
overdue: false).max
max ||= 0
max + overdue_count
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is overdue_count defined? Nvm I found it later 😛

end

def num_available_from_source(start_date, due_date, source_reservations)
def num_available(start_date, due_date, source = nil)
# get the number available in the given date range
# take an array of reservations instead of using a database call
# for database query optimization purposes
# 2 queries to calculate max_num
max_num = equipment_items.active.count - number_overdue
available = max_num - num_reserved(start_date, due_date,
source_reservations)
available < 0 ? 0 : available
end

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(id).active
.overlaps_with_date_range(start_date, due_date)
.all
num_available_from_source(start_date, due_date, relevant_reservations)
end

# Returns true if the reserver is ineligible to checkout the model.
def model_restricted?(reserver_id)
return false if reserver_id.nil?
reserver = User.find(reserver_id)
!(requirements - reserver.requirements).empty?
end

# Returns the number of overdue objects for a given model,
# as long as they have been checked out.
def number_overdue
Reservation.overdue.for_eq_model(id).size
# 1 queries if source given; 2 otherwise
#
# source is an array of reservations that can replace a database call
# for database query optimization purposes
unless source
source = reservations.active.overlaps_with_date_range(start_date,
due_date)
end
equipment_items.active.count - num_busy(start_date, due_date, source)
end

def available_count(date)
def num_available_on(date)
# get the total number of items of this kind then subtract the total
# quantity currently reserved, checked-out, and overdue
total = equipment_items.active.count
reserved = Reservation.reserved_on_date(date).for_eq_model(id).count
total - reserved - number_overdue
busy = reservations.active.overlaps_with_date_range(date, date).count
equipment_items.active.count - busy - overdue_count
end

# figure out the qualitative status of this model's items
def availability(date)
num = available_count(date)
num = num_available_on(date)
total = equipment_items.active.count
if num == 0 then 'none'
if num <= 0 then 'none'
elsif num == total then 'all'
else 'some'
end
end

# Returns true if the reserver is ineligible to checkout the model.
def model_restricted?(reserver_id)
return false if reserver_id.nil?
reserver = User.find(reserver_id)
!(requirements - reserver.requirements).empty?
end

def available_item_select_options
equipment_items.includes(:reservations).active.select(&:available?)\
.sort_by(&:name)\
Expand Down
Loading