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

nextOpeningDates start at and include today #126

Merged
merged 1 commit into from
May 8, 2024

Conversation

agnesgaroux
Copy link
Contributor

@agnesgaroux agnesgaroux commented May 7, 2024

What does this change?

Following the discovery of a bug in the itemsAPI, we realised there was a mismatch between the content-api's venues response, and the way the itemsAPI was using it, ie. content-api starting the list of openingTimes at today+1 while itemsAPI assuming it started at "today"
https://wellcome.slack.com/archives/C02ANCYL90E/p1714748440989459
It was decided that the content-api should respond with a list of openingTimes that start with "today" if today is an open day

How to test

Running the api locally, hit http://localhost:3000/venues?title=library and check the nextOpeningDates start at today if today is an open day

How can we measure success?

Have we considered potential risks?

ItemsAPI is the only client of this endpoint. There will be checks there and in weco.org before this change is used by live systems

@agnesgaroux agnesgaroux requested a review from a team as a code owner May 7, 2024 08:06
Copy link
Contributor

@kenoir kenoir left a comment

Choose a reason for hiding this comment

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

👍 Bit more context in the PR for future GitHub divers would be useful.

@agnesgaroux agnesgaroux merged commit 66155ad into main May 8, 2024
3 checks passed
@agnesgaroux agnesgaroux deleted the nextOpeningtimes-list-starts-today branch May 8, 2024 12:38
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