Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set room booking to return from current academic year #4161

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

wilhelmklopp
Copy link
Contributor

@wilhelmklopp wilhelmklopp commented Nov 21, 2023

Just noticed that the room bookings API is still returning booking from the previous academic year and none for the current year.

So for example:
https://uclapi.com/roombookings/bookings?token=uclapi-abcd&start_datetime=2023-09-01
returns:

{
    "bookings": [],
    "next_page_exists": false,
    "count": 0,
    "ok": true
}

Whereas:
https://uclapi.com/roombookings/bookings?token=uclapi-abcd&start_datetime=2023-04-01
returns many bookings:

{
    "bookings": [
        {
            "roomname": "C3.11",
            "siteid": "162",
            "roomid": "C3.11",
            "description": "Room Closed",
            "start_time": "2023-04-01T07:00:00+01:00",
            "end_time": "2023-04-01T21:00:00+01:00",
            "slotid": 6476822,
            "weeknumber": 31.0,
        },
        ...
    ],
    "next_page_exists": true,
    "page_token": "G30KYnWKki",
    "count": 121617,
    "ok": true
}

This PR should fix that (assuming the setid format is the same as it used to be)

Maybe in the future this code could be adjusted so that from say the 1st of September on any given year, UCL API automatically flips to the new academic year. And then no manual change is required each year.

βœ… Pull Request checklist

  • Is this code complete?
  • Are tests done/modified?
  • Are docs written for this PR? (if applicable)

🚨 Is this a breaking change for API clients?

No

πŸš€ Deploy notes

Worth verifying this works as expected after deploying and after the regular rb dump job runs (if that's still how it works)

cc @zipy124 @shu8


PS: I'm coming back to UCL for a guest lecture on the 29th of November at 2pm in Gustave Tuck LT. Would be cool to see you guys if you're free! Guest lecture will be on using AI to help us build software and my experience with that. Naturally, I'm planning one or more shout outs to UCL API 😎

Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Merging #4161 (bea000c) into master (1a24668) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4161   +/-   ##
=======================================
  Coverage   80.23%   80.23%           
=======================================
  Files         151      151           
  Lines        8490     8490           
=======================================
  Hits         6812     6812           
  Misses       1678     1678           

@zipy124 zipy124 merged commit a2bb760 into uclapi:master Nov 27, 2023
8 checks passed
@wilhelmklopp
Copy link
Contributor Author

@zipy124 thanks for merging ❀️

Does the change still need to be deployed?

I'm still getting the same empty response:

{
    "bookings": [],
    "next_page_exists": false,
    "count": 0,
    "ok": true
}

When querying for this: https://uclapi.com/roombookings/bookings?token=uclapi-abcd&start_datetime=2023-09-01

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants