add field to GSR for the number of days a GSR is bookable in advance.#402
Conversation
4 for AGH, 7 for everything else
|
Some test cases don't look very complete (missing some business logic). But I think Darren might have some testing stuff that hasn't been merged. If not we can update the tests later. |
There was a problem hiding this comment.
Pull request overview
This PR adds a bookable_days field to the GSR (Group Study Room) model to track how many days in advance each room can be booked. According to the PR description, AGH (Amy Gutmann Hall) should have 4 days while all other locations default to 7 days. This field is informational and will be exposed via the API for frontend use to restrict date selection.
Changes:
- Added
bookable_daysIntegerField to GSR model with default value of 7 - Updated CSV data file with bookable_days column (4 for AGH, 7 for others)
- Updated data loading command to read and set bookable_days
- Added bookable_days to admin list display
- Updated tests to include bookable_days in test data and assertions
- Removed debug print statement from share_codes test
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| backend/gsr_booking/models.py | Added bookable_days IntegerField with default value of 7 to GSR model |
| backend/gsr_booking/migrations/0015_gsr_bookable_days.py | Django migration to add bookable_days field to database |
| backend/gsr_booking/data/gsr_data.csv | Added BOOKABLE_DAYS column with values (4 for AGH, 7 for all others) |
| backend/gsr_booking/management/commands/load_gsrs.py | Updated to read bookable_days from CSV and set it during GSR creation/update |
| backend/gsr_booking/admin.py | Added bookable_days to admin list_display for easier viewing |
| backend/tests/gsr_booking/test_gsr_views.py | Updated test data to include bookable_days, added assertions, and updated field count checks |
| backend/tests/gsr_booking/test_share_codes.py | Removed debug print statement |
Comments suppressed due to low confidence (1)
backend/tests/gsr_booking/test_share_codes.py:133
- The test checks for various fields in the GSR object returned by the share code endpoint, but does not assert that 'bookable_days' is present, even though it's now a field on the GSR model and should be included in the serialized response. Consider adding an assertion to check for the presence of this field for completeness, similar to the other GSR field assertions.
self.assertIn("booking_id", payload)
self.assertIn("gsr", payload)
self.assertIn("lid", payload["gsr"])
self.assertIn("gid", payload["gsr"])
self.assertIn("name", payload["gsr"])
self.assertIn("kind", payload["gsr"])
self.assertIn("image_url", payload["gsr"])
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if entry["id"] == 2: | ||
| self.assertEquals(entry["name"], "Amy Gutmann Hall") | ||
| self.assertEquals(entry["kind"], GSR.KIND_PENNGROUPS) | ||
| self.assertEquals(entry["lid"], "20157") | ||
| self.assertEquals(entry["gid"], 42437) | ||
| self.assertEquals(entry["bookable_days"], 4) |
There was a problem hiding this comment.
The test is checking assertions for GSR with id=2 (Amy Gutmann Hall), but the locations endpoint filters to only include KIND_WHARTON and KIND_LIBCAL, explicitly excluding KIND_PENNGROUPS. Amy Gutmann Hall uses KIND_PENNGROUPS, so it will never be returned by this endpoint and these assertions will never be evaluated. Either the test should be updated to only check for GSRs that will actually be returned, or the TODO comment indicates the endpoint filter needs to be updated to include PENNGROUPS.
4 for AGH, 7 for everything else