Reservations being marked incorrectly as Returned Overdue

[orenyk]

It appears as though we're running into time zone issues again, as a number of reservations that were checked in after midnight UTC (but before midnight EST) on their due date are being flagged as overdue. For example, see Reservation number 43479 on BMEC (or 40511 if working with the earlier database dump).

One possibility is that the issue is due to the fact that the reservations database entries were created in v3.4, before #939, and as such they're being incorrectly flagged in v5.5. If that's the case, we might want to add a database migration that correctly tweaks dates so that we don't run into this issue. This is high priority.

[orenyk]

Ok, this basically seems to be an issue with our migration from when we were dealing with dates. We basically need to convert all of the relevant reservation fields to the local time zone before we do the type conversion from datetime to date, otherwise we convert datetimes that were after midnight UTC but before midnight locally to the following date.

I'm going to end up editing the relevant migration (20150213001312_change_fields_to_dates.rb) and we'll run an SQL query beforehand to do the conversion. We'll then ask ITS to re-clone all of the PROD databases to DEV and re-run the update to make sure everything is happy.

[orenyk]

I'm a bit confused - we only changed the start_date and due_date columns, so I'm not sure what's going on here. I'm going to dig into the DEV database and see what's going on.

[orenyk]

Ohhh, wait a sec. This could be due to #1220 where we have a migration (20150405012126_remove_approval_status_from_reservation.rb) that sets the overdue flag. That's probably the migration that needs to be updated, looking into it now.

[orenyk]

Ok, definitely due to that migration, tweaking it now.

[orenyk]

While I'm in there I'm going to update the migration to be model-independent as well.

[orenyk]

All done! This was fun - it really highlights the fact that we need to be careful with any migrations that manipulate data in the database, and that time zones are nothing to be trifled with.

I had to add some methods to the migration class to replicate the Reservation#flag method as well as the status enum we have set up. It's a little brittle in the sense that if we make any future updates to either how our flags or statuses are defined it might break something, but hopefully any future changes will be implemented through additional migrations that will expect this one to have run under these assumptions (fingers crossed).

I'm just testing the new migration with the old BMEC dump and, assuming everything works, I'll open a PR 😄.

[orenyk]

Ok, it worked! Unfortunately, my first attempt at the new migration increased the time it took by over 500%. Trying some brief refactoring now...

[orenyk]

Didn't work 😞, opening PR's now. Since it's just a one-time thing I'll leave it for now. If I've got time to try and profile the migration to figure out what's slowing it down, I will, but I'm not sure how to do it quickly (given how long all of these migrations take).