The (potential) unhappy code path here was raising an error which would not be explicitly handled, and would in theory not return a useful message / response.
The checkout was holding a lot of responsibility for knowing which kinds of payment gateways are available and how to initiate the process of redirecting to the external payment page (if needed). This was being hidden somewhat by the way the logic was tucked away in services.
PaymentMethod objects now know whether or not they require an external payment process, and know how that process should be started and how to build the required URL.
So we can now *ask* any payment method if it requires external payment processing or not, and *tell* it to start the process and return the relevant URL (if needed).
This construct was previously used in Spree to switch out the user class with a dummy class during certain tests. We don't use this any more, so it's just mess.
🔥
The code here runs from a callback which was originally designed to make sure the checkout page was set up correctly in the "normal" checkout workflow. It wasn't really designed to be run when the page is being loaded a second time due to the user being redirected back from Stripe (with SCA). The things it's doing here are necessary in the former case, but a really bad idea in the latter (potentially messing up the order's ship and bill addresses in certain cases like guest checkout).
A little micro-optimisation: `@order.checkout_allowed?` requires a database query, whereas `@order.completed?` does not. So in cases where the order is completed we can return early here before hitting the database.
This `session[:guest_token]` doesn't seem to ever be assigned anywhere in the codebase, and it doesn't seem to be read at any point either..? There are some various places where `current_order.token` is used and `session[:access_token]` is used, but not this.
As far as I can tell: it was part of an old version of Spree and related to the spree_auth_devise gem (which we no longer use).
Calling `when variant.id in enterprise_rules` raised an error when
enterprise_rules was null.
Fixing this then revealed a missing require statement in a controller.