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

bugfix: Ensure all date errors returned at once #2288

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

froddd
Copy link
Contributor

@froddd froddd commented Jan 13, 2025

When submitting a new booking without an arrival or departure date, or with either of those invalid, only one error would be indicated for the first error field, with the other field ignored. This ensures all errors are returned at once.

A small refactor of the booking post handler makes it follow the more usual try/catch pattern.

Screenshot of changes

Screenshot 2025-01-13 at 16 20 06

Copy link
Contributor

@bobmeredith bobmeredith left a comment

Choose a reason for hiding this comment

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

Looks good to me.
A super minor query is all.

@@ -47,7 +48,7 @@ describe('OccupancyViewController', () => {
})

beforeEach(() => {
jest.resetAllMocks()
jest.clearAllMocks()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering why the change to just clear the mocks rather than reset? Doesn't that leave the mock implementation intact?
It's not a biggie as all the mock implementations get set up again anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using resetAllMocks results in the describe block on line 301 to fail if run consecutively -- not entirely sure why, I suspect the way we're mocking requests and such may play a part!

Refactor of booking post handler to follow usual try/catch pattern
@froddd froddd force-pushed the bugfix/show-all-booking-date-errors-at-once branch from d03a928 to fe539b9 Compare January 15, 2025 11:16
@froddd froddd enabled auto-merge January 15, 2025 11:17
@froddd froddd merged commit a78d331 into main Jan 15, 2025
7 checks passed
@froddd froddd deleted the bugfix/show-all-booking-date-errors-at-once branch January 15, 2025 11:29
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.

2 participants