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

[1229] Seed script generates reservations with start date after due date #1567

Merged
merged 1 commit into from
May 22, 2016
Merged

[1229] Seed script generates reservations with start date after due date #1567

merged 1 commit into from
May 22, 2016

Conversation

esoterik
Copy link
Collaborator

@esoterik esoterik commented May 18, 2016 •
edited
Loading

Resolves #1229, and incidentally #1263 as well.

Note that this is kind of a "band-aid" fix; the script still generates bad dates, but it doesn't save them, instead trying to build the reservation again (which it can do up to 50 times). #474 involves an overhaul of seed script functionality anyway, so I'm okay with this quick fix for now. I'm planning on working on #1327 and then #474 next.

esoterik
esoterik reviewed May 18, 2016
View reviewed changes
db/seeds.rb
generate_objs(:generate_reservation, 'reservation', n)
# TODO: remove
Copy link
Collaborator Author

@esoterik esoterik May 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I left the below code for review; it's how I tested that my fix was working. I ran the seed script 20 times (generating 2,000 reservations altogether) and didn't get any errors. On master modified with the same testing code, I got 66 runs with date errors out of 100.

@esoterik
Copy link
Collaborator Author

esoterik commented May 18, 2016 •
edited
Loading

New intermittent failure: seed 2825, commit b848f63, build 4575 #1514

@esoterik
Copy link
Collaborator Author

Rubocop is going to fail on the last commit, but only because of the test statements, so I'm going to leave it for now

@esoterik esoterik mentioned this pull request May 18, 2016
Merged
@orenyk
Copy link
Contributor

orenyk commented May 19, 2016

Looks great, TimeHelpers for the win. Feel free to remove the testing blocks and we can squash/merge. Nice work!

@esoterik
Copy link
Collaborator Author

Squashed!

esoterik
esoterik reviewed May 19, 2016
View reviewed changes
db/seeds.rb
@@ -279,7 +280,7 @@ def throw_into_past(res)
res.due_date = res.due_date - factor
mark_checked_out res
mark_checked_in(res, res.equipment_model.category.max_checkout_length)
res.save(validate: false)
travel_to(res.start_date) { res.save }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this marks overdue reservations as not overdue and probably messes with the status as well if the reservation is checked in

@esoterik
Copy link
Collaborator Author

Never mind, I noticed a problem that applied here while working on #1327. Ready for re-review.

@orenyk
Copy link
Contributor

orenyk commented May 20, 2016

Great catch, callbacks are nice to keep things sane but they're also easy to forget. Let's re-squash and then we should be good to merge.

Seed script no longer generates reservations with impossible dates
6d30de1 
Resolves #1229 and #1263
 - Seed script uses TimeHelpers to save reservations in the past,
   allowing validations to be run on them
@esoterik
Copy link
Collaborator Author

Ready to merge!

@orenyk
Copy link
Contributor

orenyk commented May 22, 2016

Awesome, merging! Don't forget to update the CHANGELOG in #1548 (I'll stop reminding you after this 😛).

@orenyk orenyk merged commit 1736e62 into master May 22, 2016
This was referenced May 22, 2016
Closed
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Seed script generates reservations with start date after due date
2 participants
@esoterik @orenyk