-
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
Add button to close booking in backoffice #88
Add button to close booking in backoffice #88
Conversation
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 left some comments on the close booking feature. This wasn't an exhaustive review but I wanted to highlight some suggestions early on. The general approach to modals makes sense to me. I'll review that properly once we get your other modal PR in and this merged/rebased.
* Made some visual changes to the close booking modal (unfortunately this resulted in the reformating of the file, so you need to turn off whitespace changes in the diff to see the much more minimal actual changes) * Only show the close booking button where it makes sense to be able too see it. * Add some extra checks to the close_booking view, and move some of the logic out to helper methods on the booking model. * Switch to a form for the should_lock handling as it was brittle the way it was and it was not working in my tests (probably something slightly different between different browsers or some other such annoyance). * Added some error and success messages to provide feedback to the operator.
Rename column to "actions", align the items to the right and make sure the new header appears for the late bookings list too.
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.
@Scilly-guy this is looking pretty great to me now. I had an issue in testing - the box action wasn't being created when it should be - and a few styling/wording bits that I wasn't 100% happy with. Rather than asking you for a bunch of trivial changes, I've made some commits directly to the PR. Let me know what you think of them, and if you're happy then as far as I'm concerned this is ready to merge :)
#72
Created endpoint to set any booking state to STATE_ENDED, creates a lock BoxAction and clears the boxes current booking.