Switching from `#invalidate` to `#update_column` skipped both
validations and callbacks and thus, `#ensure_correct_adjustments` was no
longer called for older payments.
The original payment may not be valid because its credit card may be
expired. Stripe gives this as a valid scenario returning a success and
we should do too.
When creating the credit payment we end up validating all sources in
a chain as follows.
```
Payment being persisted -> source payment -> original credit card.
```
The source payment was valid when created (It would not be persisted
otherwise) but its source card may now be expired, and that's legit.
There was also an issue with the `#invalidate_old_payments` callback. It
was causing the original payment to be validated again and thus the
credit payment failed to be persisted due to the original credit card
being expired. Switching this callback to use `#update_column` skips
validations and so we don't validate the source payment. We only care
about the state there, so it should be fine.
Given the importance of this code, it doesn't bring me much confidence.
Apparently, this specs where using a non-existent state by mistake and
this went unnoticed because the payment creation was failing silently in
payment/processing.rb.
This unearthed the fact that our `#ensure_correct_adjustment` needs the
order to be persisted to succeed.
* master: (91 commits)
Bump ddtrace from 0.37.0 to 0.38.0
Add spec to cover SQL query issue with OCs where the only products from the coordinator inventory are renderer
Remove unnecessary order statement, the relation will only be used for counting products
Move select out of scope visible_for because it is breaking exchange_product queries and it's just not needed there. The only other use of this product's scope visible_for is the enterprise serializer so we add the select to it.
Make OC advanced settings work by permitting the extra parameter
Remove conflicting and duplicate route
Bump bugsnag from 6.13.1 to 6.14.0
Make charges update method update the first pending payment
Move require_login_then_redirect_to to the only place where it is called
Make broken spec fail reliably and set it pending
Updating translations for config/locales/en_GB.yml
Update all locales with the latest Transifex translations
Doc defensive coding needed by pin payments
Make method a little simple by extracting method
Simplify spec, the 2 minutes wait is not necessary anylonger
Make unauthorized in ControllerHelpers::Auth the same as in Spree::Admin::BaseController
Move unauthorized view to HomeController only, all other calls to unauthorized will go through Auth which will redirect to the home controller IF the user is logged in or to login if user is not logged in
Adapt specs to the move of unauthorized route from the spree routes to the main app routes
Delete spree_user_signup which is from spree promotions code that we dont use
Remove try_spree_current_user
...
This Spree route conflicts with the one we define:
```
get "/login", to: redirect("/#/login")
```
for whatever reason there are 7 users that managed to hit the Spree one
instead of ours when confirming their signup email. It's not clear to me
though when this `/login?validation=confirmed` is really hit. The
confirmation email link passes a token in the query params and this is
not the case.
The idea is that `GET /login` makes the login modal to show up instead
of Devise's default behaviour (through inheritance) of showing a login
form page. OFN was never prepared to handle this as this bug proofs.
This spec has been broken for a long time, at least eight months. But it
regularly passed because the search filter is applied with a delay and
in that time the content matches. And once the filter is applied, no
products are shown and the negative matchers pass.
