Commit Graph

27 Commits

Author SHA1 Message Date
David Rodríguez
27975252f5 Rename pm_card to payment_method_id
And also remove a couple of now unused `let`'s that were already using
this terminology.

Co-authored-by: David Cook <david@openfoodnetwork.org.au>
2025-10-27 10:52:36 +01:00
David Rodríguez
9fc82776ec Move Stripe test payment method handling to a single place
And comment a bit on them.

Co-authored-by: David Cook <david@openfoodnetwork.org.au>
2025-10-27 10:45:45 +01:00
David Rodríguez
2c8bf82426 Migrate some spec to not send raw card numbers to Stripe API
If I regenerate the VCR cassetes for
spec/lib/stripe/payment_intent_validator_spec.rb, I get a lot of errors
like this:

```
Stripe::CardError:

Sending credit card numbers directly to the Stripe API is generally
unsafe. We suggest you use test tokens that map to the test card you are
using, see https://stripe.com/docs/testing. To enable testing raw card
data APIs, see
https://support.stripe.com/questions/enabling-access-to-raw-card-data-apis.
```

It seems the sandbox environment associated to my developer API keys is
not allowed to send raw credit card data.

Instead of requesting Stripe support to enable that, or regenerate
cassettes with the API keys in Bitwarden, I figured we could migrate the
tests to not use raw credit card data.
2025-10-27 09:08:30 +01:00
David Rodríguez
121019411c Fix spec instantiating payment method with wrong source
Previous error is fixed, which allows the spec to proceed further, and
reveals that the current cassettes are missing some requests:

```
  1) Stripe::PaymentIntentValidator#call as a guest when payment intent is valid valid non-3D credit cards are correctly handled behaves like payments intents from Visa returns payment intent id and does not raise
     Failure/Error:
       payment_intent_response = Stripe::PaymentIntent.retrieve(
         payment_intent_id,
         stripe_account: stripe_account_id
       )

     VCR::Errors::UnhandledHTTPRequestError:

       ================================================================================
       An HTTP request has been made that VCR does not know how to handle:
         GET https://api.stripe.com/v1/payment_intents/pi_3P8hNGKuuB1fWySn0dvhu9lG

       VCR is currently using the following cassette:

       (...)
```
2025-10-27 09:08:29 +01:00
David Rodríguez
1a5eb5b39a Remove raise_error negative block to fix RSpec warnings
Currently RSpec warns these specs like this:

```
WARNING: Using `expect { }.not_to raise_error(SpecificErrorClass)` risks false positives, since literally any other error would cause the
expectation to pass, including those raised by Ruby (e.g. `NoMethodError`, `NameError` and `ArgumentError`), meaning the code you are intending
to test may not even get reached. Instead consider using `expect { }.not_to raise_error` or `expect { }.to raise_error(DifferentSpecificErrorClass)`.
This message can be suppressed by setting: `RSpec::Expectations.configuration.on_potential_false_positives = :nothing`.
Called from /path/to/spec/lib/stripe/payment_intent_validator_spec.rb:53:in `block (7 levels) in <main>'.
```

The warnings are super accurate in this particular case: the inner
assertion is not actually getting reached due to a previous unrelated
error.

Since there's an inner assertion already, I think it's best to
completely remove to `raise_error` negative block, since it just hides
errors and buys us nothing.

By removing it, the underlying error surfaces:

```
  1) Stripe::PaymentIntentValidator#call as a guest when payment intent is valid valid non-3D credit cards are correctly handled behaves like payments intents from Visa returns payment intent id and does not raise
     Failure/Error:
       create(:payment, amount: payment_intent.amount, payment_method:,
                        response_code: payment_intent.id, source: pm_card)

     NoMethodError:
       undefined method `has_query_constraints?' for Stripe::PaymentMethod:Class

               elsif (klass || self.klass).has_query_constraints? || options[:query_constraints]
                                          ^^^^^^^^^^^^^^^^^^^^^^^
     Shared Example Group: "payments intents" called from ./spec/lib/stripe/payment_intent_validator_spec.rb:75
     # ./spec/lib/stripe/payment_intent_validator_spec.rb:16:in `block (3 levels) in <main>'
     # ./spec/lib/stripe/payment_intent_validator_spec.rb:19:in `block (3 levels) in <main>'
     # ./spec/lib/stripe/payment_intent_validator_spec.rb:53:in `block (7 levels) in <main>'
     # ./spec/base_spec_helper.rb:208:in `block (2 levels) in <main>'
     # ./spec/base_spec_helper.rb:155:in `block (3 levels) in <main>'
     # ./spec/base_spec_helper.rb:155:in `block (2 levels) in <main>'
     # -e:1:in `<main>'
```
2025-10-27 09:08:29 +01:00
David Rodríguez
2bd536298b Showcase the problem with some specs
They're always passing because an error (different from `StripeError`),
is actually making them pass.
2025-10-27 09:08:29 +01:00
Maikel Linke
6317fe1b71 Disable RSpec monkey patching 2024-05-09 12:24:41 +10:00
filipefurtad0
85e3d5d858 Tests payment intents for offline payments
Moves let variables outside shared examples block

Re-records cassettes
2024-04-23 12:24:00 +01:00
Maikel Linke
bd6b0ddbf3 Enforce RSpec expect(..).not_to over to_not 2024-03-07 16:57:54 +11:00
David Cook
68cc9ed2fe Safely autocorrect Layout/EmptyLinesAroundBlockBody
Inspecting 1540 files
.........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................C.............................................................................................................................................................................................................................................................................................................................................................C............................................................................

Offenses:

spec/lib/stripe/payment_intent_validator_spec.rb:7:1: C: [Corrected] Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body beginning.
spec/system/admin/products_v3/products_spec.rb:219:1: C: [Corrected] Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end.

1540 files inspected, 2 offenses detected, 2 offenses corrected
2024-02-23 10:02:23 +11:00
filipefurtad0
edae56345b Removes unnecessary Stripe api key / secret declaration 2024-02-08 14:53:48 +00:00
filipefurtad0
6c9556f2db Removes UnionPay (debit) from non-3DS card list
Rubocop fixes
2024-02-07 16:32:34 +00:00
filipefurtad0
3802447bfe Removes credit card mock, pointing to real Stripe object
Re-records cassettes after source change
2023-12-07 18:30:23 +00:00
filipefurtad0
e50baa9a18 Adds payment capture scenario 2023-12-05 15:27:46 +00:00
filipefurtad0
f3f59ed6ad Adds valid credid card types (non-3D) 2023-12-04 20:48:53 +00:00
filipefurtad0
beb47f03c0 Removes Stripe module
...and calls it within the describe block
2023-11-16 14:46:25 +00:00
filipefurtad0
9d00b7dc43 Re-records relevant cassettes 2023-11-15 13:05:15 +00:00
filipefurtad0
4c3f87f047 Adds payment intent tests with invalid cards 2023-11-15 13:05:15 +00:00
filipefurtad0
9d50b11090 Adds re-recorded Stripe cassettes within Stripe gem folder name 2023-11-08 14:25:19 +11:00
filipefurtad0
a88919299f Replaces mocks with VCR calls 2023-10-31 19:06:40 +00:00
Matt-Yorkley
32a2610e7d Refactor arguments for PaymentIntentValidator 2021-07-09 13:42:41 +01:00
Matt-Yorkley
e5a85caef6 Refactor Stripe::PaymentIntentValidator
This makes the interface a lot simpler and moves the fetching of the information it requires inside the service itself.
2021-07-09 13:39:52 +01:00
Andy Brett
b3e08b47b9 set Stripe api key directly instead of using allow 2021-06-01 22:33:12 +01:00
Matt-Yorkley
471a7903f6 Update PaymentIntentValidator spec 2021-05-16 12:19:04 +01:00
Maikel Linke
fce8d3a2f8 Record invalid payment states for debugging
We observed invalid payment states in Bugsnag but we don't actually know
in which state the payment intent was in. From the context we can guess
that it was "succeeded" but it would be good to validate this. And in
the future it would be good to know if there are other invalid states we
can end up in.

The notification to Bugsnag happens in another part of the code.
2021-03-22 16:35:22 +11:00
Andy Brett
3d47ad7e33 add stubs for stripe requests 2020-12-08 07:52:42 -08:00
Luis Ramos
b54b981740 Improve readability of PaymentIntentValidator and cover with specs 2020-03-02 17:30:12 +00:00