[1501] Availability Overhaul #1538
Conversation
| @@ -14,6 +14,7 @@ task run_daily_tasks: :environment do | |||
| Rake::Task['email_checkout_reminder'].invoke | |||
| Rake::Task['delete_old_blackouts'].invoke | |||
| Rake::Task['delete_missed_reservations'].invoke | |||
| Reservation.counter_culture_fix_counts | |||
There was a problem hiding this comment.
this is a leftover from a gem that I'm not using anymore and should be deleted
|
@esoterik reviewed a bunch of it, the counter cache stuff looks really good to me, nice work! A few comments on the specs but otherwise that portion of the code was really solid. Did you want me to review the rest of the specs or are they still in progress? Done for now, let me know! |
|
All of the new specs should be complete, it'd be great if you could take a look! |
|
Actually nevermind, I've changed somethings and will push the update soon |
|
Okay, the new equipment model specs are tentatively complete; no need for any rereview at the moment |
|
@esoterik to confirm, you want to wait until this is finished for a complete re-review? Let me know, thanks! |
esoterik
force-pushed
the
1501_eq_availability_improvement
branch
from
a8ed003
to
a7e5e89
Compare
|
The tests that failed are due to the intermittently failing tests, this is ready for rereview |
| @@ -19,6 +19,14 @@ class Reservation < ActiveRecord::Base | |||
| validate :status_final_state | |||
| validate :not_in_past, :available, :check_banned, on: :create | |||
|
|
|||
| # conditional counter cache for overdue reservations | |||
| after_update :increment_cache, if: :checked_out? | |||
| after_update :decrement_cache, if: :overdue | |||
There was a problem hiding this comment.
need to make sure that this works for edited overdue reservations (as in #1479)
|
Okay, I cleaned up the tests and will look into the intermittent failures later |
|
I suspect that the issues might have to do with the use of |
|
Also, rspec bisect (recommended by the testing rails book) is pretty great, though it's pretty slow. |
|
Okay, the intermittent failure should be fixed (seed 1699 and 52941)! |
|
Awesome! Did |
|
Not for those seeds |
|
Just read the blog post you linked to, we definitely need to get rid of all the |
| 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' |
|
Alright I think we can |
, let's merge and rebase onto the latest
esoterik
force-pushed
the
1501_eq_availability_improvement
branch
from
3db8f18
to
e251561
Compare
|
Finally got around to squashing this! |
| count += 1 if r.overdue && r.equipment_model_id == model_id | ||
| end | ||
| count | ||
| def active? |
There was a problem hiding this comment.
Is this used anywhere? Searching across the diff seems to indicate that it wasn't...
|
Looks good, just one final inline comment that came up when I took a last look-through, let me know about it and we can merge! |
Resolves #1501 - Availability methods all work as expected - Adds counter cache on equipment models for current overdue reservations - Reservation factory trait :past allows for creation of past reservations without skipping validations - New reservation model methods for counting reservations, attribute checking, and checking for overlapping
esoterik
force-pushed
the
1501_eq_availability_improvement
branch
from
e251561
to
f7440c0
Compare
|
got rid of that method; should be actually ready now |
|
Great! Just goes to show, you can't review code too many times 😛. Merging, please update the CHANGELOG in #1548, thanks! |
Resolves #1501
This is not finished yet, but it would be great if someone could look over the counter cache implementation and the changes to the reservation and equipment model models!