[#152604] Prevent abandoned reservation carts #2292
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
i'm allowed to buy in the past. why?
jhanggi
commented
May 12, 2020
| @@ -0,0 +1,84 @@ | |||
| class SingleReservationsController < ApplicationController | |||
There was a problem hiding this comment.
I'm not in love with the name--especially in the route/url--but I haven't come up with anything better.
jhanggi
commented
May 12, 2020
| load_resource :facility, find_by: :url_name | ||
| load_resource :instrument, through: :facility, find_by: :url_name | ||
| before_action :build_order | ||
| before_action { @submit_action = facility_instrument_single_reservations_path } |
There was a problem hiding this comment.
This seemed to be the simplest way of still using the same view partial as the old reservations form. I'm open to other ideas.
jhanggi
commented
May 12, 2020
| render "reservations/new" | ||
| end | ||
|
|
||
| def create |
There was a problem hiding this comment.
I copied this from ReservationsController and cleaned out some of the unneeded pieces (like merge orders). I still want to do some further refactors to remove some of the duplication.
jhanggi
commented
May 12, 2020
| @instrument.min_reserve_mins.to_i > 0 ? @instrument.min_reserve_mins : 30 | ||
| end | ||
|
|
||
| def set_windows |
There was a problem hiding this comment.
This and some of the other helper methods are ripe for refactoring.
jhanggi
commented
May 12, 2020
| @@ -206,6 +206,16 @@ def assign_actuals_off_reserve | |||
| self.actual_end_at ||= reserve_end_at | |||
| end | |||
|
|
|||
| def valid_as_user?(user) | |||
There was a problem hiding this comment.
This is essentially a duplicate of save_as_user, but with valid?. I might try again to DRY it up, but my initial attempts didn't work.
cardoce
approved these changes
May 19, 2020
jhanggi
added a commit
that referenced
this pull request
Jun 29, 2020
# Release Notes Fixes the "Cancel" button behavior on new reservation page. # Additional Context This does modify behavior a little bit, but the behavior before felt unexpected. There are a couple ways to get to this page: * Clicking on an instrument from the facility homepage. This is the new order-less reservation flow from #2292. * Before: The cancel button was missing * After: The cancel button will return you the facility homepage. * The reservation was in the cart because it was a bundle. * Before: The cancel button brought you back to the facility homepage, but left everything in your cart * After: The cancel button brings you back to your cart * The reservation was in a cart because it was added with the order form * Before: The order detail was removed from your cart (this seemed wrong) and brought you to the facility homepage * After: You are returned to your cart and everything is left alone. If you want to remove the line from your cart, you can use the "Remove" button * Adding to an existing order * Before: You are brought back to the existing order's page and the line item still requires resolution. * After: Same behavior
jhanggi
added a commit
that referenced
this pull request
Jul 15, 2020
# Release Notes Fix hard error when a next reservation cannot be found. # Additional Context Easy way to reproduce: create a series of schedule rules that does not include the current day (e.g. Wednesday). Try to create a new reservation. Caused by a bad copy/paste in #2292.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Release Notes
Prevent abandoned carts created when clicking an instrument name. This should also help prevent order/detail IDs from growing too fast.
TODO
@add_to_orderin the old controllerAdditional Context
Before, if you click an instrument, a new order and order detail are created. Users are often doing this to check the schedule for the instrument. If the user closes the tab comes back to the instrument, yet another order detail is created. We were deleting the cart if the user clicks "Cancel", but the order and order detail IDs were still eaten up.
Now, for normal single-reservation purchases, we go to a parallel controller where we do not create the order/detail prior to the user filling out the form. I did need to rework some of the code to avoid persisting and then rolling back the order/detail. Without these changes, the abandoned carts would not be created, but the IDs would still be used up.
For the order form (purchasing more than one or with other things) or part of a bundle, as well as edit/update, we are continuing to use the old controller.
Some offline discussion for context:
From Aaron:
Jason: