Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FW][IMP] payment_stripe: support webhooks #71558

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Jun 1, 2021

Allow configuring a webhook in Stripe to send s2s notifications to Odoo
when a Checkout payment is completed. Note that SetupIntent and
PaymentIntent events are not listened to, since they are handled 'live'
with the customer actively present; the main use case for Stripe
webhooks is a Checkout session that gets interrupted before the customer
is redirected to Odoo (e.g. network loss, browser crash, closing the
tab, etc.).

The webhook should be configured to send its events to
<base_url>/payment/stripe/webhook and should only subscribe to
checkout.session.completed events to avoid spamming the Odoo server with
useless notifications.

opw-2488452
opw-2451463
opw-2449738

BACKPORT of commit: dc4f6ad

Should not be merged beyond 14.0 (14.0 excluded)

Forward-Port-Of: #71520
Forward-Port-Of: #69809

@robodoo robodoo added conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot labels Jun 1, 2021
@robodoo
Copy link
Contributor

robodoo commented Jun 1, 2021

Pull request status dashboard

@fw-bot
Copy link
Contributor Author

fw-bot commented Jun 1, 2021

Ping @simongoffin
Cherrypicking f806dad of source #69809 failed

stderr:

15:46:08.358470 git.c:344               trace: built-in: git cherry-pick f806dad74daf1a95ce0b02b6088859824d9d626b
error: could not apply f806dad74da... [IMP] payment_stripe: support webhooks
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'
----------
status:

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Jun 1, 2021
@simongoffin simongoffin force-pushed the 13.0-12.0-payment_stripe-checkout-webhook-backport-81o_-fw branch from 0061abc to 63adf98 Compare June 1, 2021 15:30
@xmo-odoo
Copy link
Collaborator

xmo-odoo commented Jun 2, 2021

@robodoo override=ci/security

cf source pr

@Xavier-Do
Copy link
Contributor

@simongoffin there is an upgrade exception active for this pr.
Please forwardport this asap.

@wtaferner
Copy link
Contributor

❗ Do not forward port without merging #71933 before

@AdrienHorgnies AdrienHorgnies force-pushed the 13.0-12.0-payment_stripe-checkout-webhook-backport-81o_-fw branch from 9f0e7e7 to 9fb18d9 Compare June 17, 2021 10:20
@AdrienHorgnies
Copy link

AdrienHorgnies commented Jun 17, 2021

@simongoffin
I've added my changes to fix this FW commit into a separate commit, so you can inspect them separately. We can squash it into the 1st commit when review is done.

I fixed all unit tests, and tested the full flow using stripe test environment with success.
However, I had to make changes that makes me wonder if this module actually works in other versions.
Especially, the code assumes the object charge contains a metadata dictionary that contains a reference key.
I didn't find it during my tests. And as my mocks were copy and paste from stripe API responses, I conclude that Stripe is doing something differently (mocks were written in odoo version 14.0 a few months ago). I checked documentation, and this metadata field should exist... Anyway, I fixed this using a safe statement that won't break anything:

if 'metadata' in data and not data.get('metadata').get('reference'):
                data['metadata']['reference'] = reference

Allow configuring a webhook in Stripe to send s2s notifications to Odoo
when a Checkout payment is completed. Note that SetupIntent and
PaymentIntent events are not listened to, since they are handled 'live'
with the customer actively present; the main use case for Stripe
webhooks is a Checkout session that gets interrupted before the customer
is redirected to Odoo (e.g. network loss, browser crash, closing the
tab, etc.).

The webhook should be configured to send its events to
<base_url>/payment/stripe/webhook and should only subscribe to
checkout.session.completed events to avoid spamming the Odoo server with
useless notifications.

opw-2488452
opw-2451463
opw-2449738

BACKPORT of commit: dc4f6ad

Should not be merged beyond 14.0 (14.0 excluded)

X-original-commit: 443da3a
@simongoffin simongoffin force-pushed the 13.0-12.0-payment_stripe-checkout-webhook-backport-81o_-fw branch from 0e56d1b to 67fbb30 Compare June 21, 2021 08:49
@simongoffin
Copy link
Contributor

robodoo r+

robodoo pushed a commit that referenced this pull request Jun 21, 2021
Allow configuring a webhook in Stripe to send s2s notifications to Odoo
when a Checkout payment is completed. Note that SetupIntent and
PaymentIntent events are not listened to, since they are handled 'live'
with the customer actively present; the main use case for Stripe
webhooks is a Checkout session that gets interrupted before the customer
is redirected to Odoo (e.g. network loss, browser crash, closing the
tab, etc.).

The webhook should be configured to send its events to
<base_url>/payment/stripe/webhook and should only subscribe to
checkout.session.completed events to avoid spamming the Odoo server with
useless notifications.

opw-2488452
opw-2451463
opw-2449738

BACKPORT of commit: dc4f6ad

Should not be merged beyond 14.0 (14.0 excluded)

closes #71558

X-original-commit: 443da3a
Signed-off-by: Simon Goffin (sig) <sig@openerp.com>
@robodoo robodoo closed this Jun 22, 2021
@robodoo robodoo temporarily deployed to merge June 22, 2021 00:02 Inactive
@fw-bot fw-bot deleted the 13.0-12.0-payment_stripe-checkout-webhook-backport-81o_-fw branch July 6, 2021 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants