feat(stripe): Update payment method in a job #3381
Merged
Conversation
This file contains hidden or 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
ivannovosad
previously requested changes
Mar 26, 2025
With a job you can still have the payment not updated, it may be rare but it can still happen no? |
|
@jdenquin At this point it shouldn't happen because the code right before enqueuing this job will already fail if the payment is not updated. And if that happens, we'll see it in the dead queue. |
ivannovosad
approved these changes
Mar 26, 2025
jdenquin
requested changes
Mar 26, 2025
app/services/payment_providers/stripe/webhooks/payment_intent_succeeded_service.rb
Show resolved
Hide resolved
jdenquin
approved these changes
Mar 26, 2025
julienbourdeau
added a commit
that referenced
this pull request
Mar 27, 2025
## Description See #3381 If the initial service updating the payment method (which might also create payment if not found doesn't return a payment), ignore the next job. What I got wrong initially is that I assumed if no payment was found in the end, the service was raising an error. It doesn't. https://github.com/getlago/lago-api/blob/953b92a96de74ab613479dd4db1d5e85977d9d47/app/services/invoices/payments/stripe_service.rb#L16-L28 https://github.com/getlago/lago-api/blob/953b92a96de74ab613479dd4db1d5e85977d9d47/app/services/payment_requests/payments/stripe_service.rb#L32-L42
jdenquin
pushed a commit
that referenced
this pull request
Mar 27, 2025
## Description I was hesitant to make it async but it's better for retries. Note that `update_payment_status` might raise a not found error if the webhook arrived before the `payment._provider_payment_id` is updated in the DB, which will retry the HandleEventJob. So when `UpdatePaymentMethodDataJob` is dispatched, we're guaranteed that payment is updated... until the implementation of `update_payment_status` changes 😄
jdenquin
pushed a commit
that referenced
this pull request
Mar 27, 2025
## Description See #3381 If the initial service updating the payment method (which might also create payment if not found doesn't return a payment), ignore the next job. What I got wrong initially is that I assumed if no payment was found in the end, the service was raising an error. It doesn't. https://github.com/getlago/lago-api/blob/953b92a96de74ab613479dd4db1d5e85977d9d47/app/services/invoices/payments/stripe_service.rb#L16-L28 https://github.com/getlago/lago-api/blob/953b92a96de74ab613479dd4db1d5e85977d9d47/app/services/payment_requests/payments/stripe_service.rb#L32-L42
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.
Description
I was hesitant to make it async but it's better for retries.
Note that
update_payment_statusmight raise a not found error if the webhook arrived before thepayment._provider_payment_idis updated in the DB, which will retry the HandleEventJob.So when
UpdatePaymentMethodDataJobis dispatched, we're guaranteed that payment is updated... until the implementation ofupdate_payment_statuschanges 😄