unc-csxl / csxl.unc.edu

CS Experience Labs' web application.
https://csxl.unc.edu
MIT License
9 stars 7 forks source link

Fix bug with end_idx logic for future reservations #384

Closed yuvrajjain2003 closed 4 months ago

yuvrajjain2003 commented 4 months ago

The current problem with the code is that at 11 pm, we were not able to see future reservations in red for tomorrow. This was actually a red herring that led me to look at parts of the code that were not responsible for this.

Then suddenly at 12 am, everything started working as intended. This was the biggest hint for me in debugging this.

Basically on line 345, I just had to add in a condition to check that the date I am getting as a param is the same date as today. Only in that case, should I execute the logic of end_idx < current_time_idx. However, earlier this was being executed for all reservations which is why whenever we had any reservations which had an end time of less than 11 pm (which most reservations do), our function would just skip over them. This also aligns with what we saw at 12 am when everything seemed to work as intended. This is again because most reservations don't have an ending time before 12 am.

Anyway, took me over 60 minutes to add in these 30 chars to fix the bug. So I typed an average of 0.5 char per minute... :,)

closes #383

KrisJordan commented 4 months ago

Great work getting to the bottom of this! We can merge in this evening, but it'd be great to have test coverage of the fix here.

yuvrajjain2003 commented 4 months ago

Could you take a stab at adding a test to the test suite that reproduces the bug by failing without the fix and then passes with the fix?

Yeah I can try! However testing this is kind of weird since it relies on "current time". Is there a reliable way of manipulating the system time i.e the value returned by datetime.now()? Because otherwise, the test I write will also be dependent on when it is ran...

yuvrajjain2003 commented 4 months ago
from freezegun import freeze_time

@freeze_time("2012-01-14")
def test():
    assert datetime.datetime.now() == datetime.datetime(2012, 1, 14)

This might be something useful to look into.