-
Notifications
You must be signed in to change notification settings - Fork 1
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
APS-1775: Update booking confirmation screen #2277
Conversation
b6bf14f
to
1947cf0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one!
It's much better using the post body to pass those parameters and keeping the filter date persist in the query string.
Just a few minor comments and queries.
server/utils/match/index.ts
Outdated
@@ -195,6 +190,37 @@ export const occupancyViewSummaryListForMatchingDetails = ( | |||
] | |||
} | |||
|
|||
export const textRow = (label: string, value: string): SummaryListItem => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These utils look like they could be used widely. there is a similar fn in utils/placements/index.ts
for instance.
Perhaps it would be good to put these somewhere global - utils/formUtils
say.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've refactored the above a bit but not gone on to replace all instances -- this would seem like a sizeable change to add to this. Thoughts?
server/controllers/match/placementRequests/spaceBookingsController.test.ts
Outdated
Show resolved
Hide resolved
84af10a
to
f08a234
Compare
This does a few things: - Ensures the booking confirmation flow uses the arrival date and departure date submitted, rather than relying on filter start date and duration - Removes the `apType` parameter as it is not used - Updates the booking confirmation template to show relevent information - Refactors a lot of the flow to present occupancy view with retained filters where relevant
f08a234
to
e3008ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Context
https://dsdmoj.atlassian.net/browse/APS-1775
Changes in this PR
Implements design and content updates to the 'Confirm booking' page.
Under the hood, this ensures the booking flow relies on the arrival and departure dates entered on the Occupancy view page, rather than recalculating those based on a start date and duration. The path for the confirmation screen has been rewritten to include the premises ID, which ensures a 404 is returned if this ID is invalid/doesn't exist.
The occupancy filter query parameters are carried over to the confirmation page to ensure the back link functions correctly.
The
apType
parameter is now removed, as it is no longer used beyond the initial suitability search.Screenshots of UI changes
Before
After