-
Notifications
You must be signed in to change notification settings - Fork 57
[1501] Availability Overhaul #1538
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
# 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 } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: we should investigate whether using scopes is faster than There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After testing this manually it appears as though using |
||
|
||
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), | ||
|
@@ -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" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
# 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
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)\ | ||
|
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.