StripeConnect has been replaced by StripeSCA but some people still use
the old StripeConnect. Let's prevent people from creating deprecated
payment methods before migrating existing data.
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).
But let people include out of stock variants by checking a checkbox if they want.
Note, we only apply the variants in stock scope if a distributor is
present. I think this is because this search method is also used when
setting up subscriptions so I don't think we want to change the
behaviour there.
Co-authored-by: Maikel Linke <maikel@email.org.au>
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.
