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

Extract payment plugins #4669

Merged
merged 62 commits into from
Sep 20, 2019
Merged

Extract payment plugins #4669

merged 62 commits into from
Sep 20, 2019

Conversation

salwator
Copy link
Contributor

@salwator salwator commented Aug 21, 2019

This PRs purpose is to use the recently introduced plugin system to saleor payment gateways. All payment implementations will be turned into saleor plugins and configured through API instead of the environment.

Current Payment uses lazy imports and checks for implementation when processing payment. This kind of dispatch and parametrization is no longer useful since we can use PluginManager interface and plugin mechanism for dispatching and establishing contract for payment flow process. This is why I introduce new simpler way of calling payment methods and pre/post transaction processing.

Business logic for payments will be encapsulated in payment.gateway module, which will execute actions on payment gateways using plugins. All payment related plugin hooks will be grouped into PaymentInterface so it easy to test against and divide PluginManager in the future.

We should make payment methods interface more verbose and probably since we're no longer forced to reuse same data structure as an argument for different actions, creating action-specific parameters and structure types makes more sense.

This PR provides both storefronts (old and new) with dynamic gateway list during checkout process, gateways can be enabled/disabled and entirely configured in saleor dashboard or directly through GraphQL plugin mutation.

NOTE: GraphQL schema is static, for now it is necessary to add new payment gateway plugin directly in settings and extend gateway list there to generate proper enum type in schema. Will be changed soon in next PR.


Roadmap:

  • Extend BasePlugin with payment methods
  • Introduce DummyGatewayPlugin
  • Implement BraintreeGatewayPlugin
  • Implement StripeGatewayPlugin
  • Implement RazorpayGatewayPlugin
  • Use properly gateway plugins in plugin manager
  • Introduce new PaymentInterface abstract class so we can isolate payment stuff from the rest of manager methods.
  • Create PaymentGateway and test with mocked PaymentInterface
  • Switch to PaymentGateway in API:
    • process_payment
    • authorize
    • capture
    • confirm
    • refund
    • void
    • list_payment_sources
  • Add create_form to payment plugins for backward compatibility
  • Remove dead code from old payment architecture
  • Serve available gateway plugin list in checkout field
  • Serve available gateway plugin list in old storefront view

Pull Request Checklist

  1. Privileged views and APIs are guarded by proper permission checks.
  2. All visible strings are translated with proper context.
  3. All data-formatting is locale-aware (dates, numbers, and so on).
  4. Database queries are optimized and the number of queries is constant.
  5. Database migration files are up to date.
  6. The changes are tested.
  7. GraphQL schema and type definitions are up to date.
  8. Changes are mentioned in the changelog.

@salwator salwator added in progress payments Issues related to payments implementation plugins labels Aug 21, 2019
@salwator salwator self-assigned this Aug 21, 2019
Copy link

django-queries commented Aug 21, 2019

Here is the report for 24fa2cc (mirumee/saleor @ extract-payment-plugins)
Base comparison is d369a4b.

**Found 2 differences!** (click me)

# api.benchmark checkout
  test name                           	left count 	right count	duplicate count
  ------------------------------------	-----------	-----------	---------------
- add billing address to checkout     	         34	         38	             20
  add shipping to checkout            	          7	          7	              0
  checkout payment charge             	         14	         14	              0
  complete checkout                   	          6	          6	              0
- create checkout                     	         48	         52	             24

# api.benchmark homepage
  test name                           	left count 	right count	duplicate count
  ------------------------------------	-----------	-----------	---------------
  retrieve main menu                  	          5	          5	              0
  retrieve product list               	          4	          4	              0
  retrieve secondary menu             	          5	          5	              0
  retrieve shop                       	          2	          2	              0

# api.benchmark product
  test name                           	left count 	right count	duplicate count
  ------------------------------------	-----------	-----------	---------------
  product details                     	         15	         15	              3

# api.benchmark variant
  test name                           	left count 	right count	duplicate count
  ------------------------------------	-----------	-----------	---------------
  retrieve variant list               	         18	         18	              8

@codecov
Copy link

codecov bot commented Aug 26, 2019

Codecov Report

Merging #4669 into master will decrease coverage by 0.43%.
The diff coverage is 53.48%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4669      +/-   ##
==========================================
- Coverage   91.43%   90.99%   -0.44%     
==========================================
  Files         307      311       +4     
  Lines       18419    18655     +236     
  Branches     1842     1853      +11     
==========================================
+ Hits        16841    16976     +135     
- Misses       1060     1157      +97     
- Partials      518      522       +4
Impacted Files Coverage Δ
saleor/extensions/manager.py 78.66% <45.94%> (-9.47%) ⬇️
saleor/extensions/base_plugin.py 81.52% <46.66%> (-6.79%) ⬇️
saleor/payment/gateways/stripe/plugin.py 51.11% <51.11%> (ø)
saleor/payment/gateways/braintree/plugin.py 51.11% <51.11%> (ø)
saleor/payment/gateways/dummy/plugin.py 53.65% <53.65%> (ø)
saleor/core/payments.py 71.87% <71.87%> (ø)
saleor/account/emails.py 76.74% <0%> (-9.47%) ⬇️
saleor/graphql/account/utils.py 97.56% <0%> (-2.44%) ⬇️
saleor/account/models.py 93.49% <0%> (-0.21%) ⬇️
saleor/graphql/account/mutations/account.py 100% <0%> (ø) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b75634...4ece55e. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 26, 2019

Codecov Report

Merging #4669 into master will decrease coverage by 0.78%.
The diff coverage is 61.52%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4669      +/-   ##
==========================================
- Coverage   91.96%   91.18%   -0.79%     
==========================================
  Files         324      330       +6     
  Lines       19491    19831     +340     
  Branches     1863     1874      +11     
==========================================
+ Hits        17925    18082     +157     
- Misses       1047     1230     +183     
  Partials      519      519
Impacted Files Coverage Δ
saleor/payment/gateways/stripe/plugin.py 0% <0%> (ø)
saleor/payment/gateways/razorpay/plugin.py 0% <0%> (ø)
saleor/payment/gateways/braintree/plugin.py 0% <0%> (ø)
saleor/graphql/account/resolvers.py 100% <100%> (ø) ⬆️
saleor/payment/models.py 95.87% <100%> (+0.87%) ⬆️
saleor/graphql/checkout/mutations.py 94.14% <100%> (-0.47%) ⬇️
saleor/graphql/checkout/types.py 94.18% <100%> (ø) ⬆️
saleor/graphql/payment/mutations.py 96.42% <100%> (ø) ⬆️
saleor/dashboard/order/forms.py 87.43% <100%> (ø) ⬆️
saleor/graphql/extensions/schema.py 100% <100%> (ø) ⬆️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d369a4b...24fa2cc. Read the comment docs.

@salwator salwator force-pushed the extract-payment-plugins branch 2 times, most recently from a2cb1bb to 7f27bea Compare August 27, 2019 09:00
saleor/core/payments.py Show resolved Hide resolved
saleor/extensions/base_plugin.py Outdated Show resolved Hide resolved
saleor/extensions/manager.py Outdated Show resolved Hide resolved
saleor/payment/gateways/braintree/plugin.py Show resolved Hide resolved
saleor/payment/utils.py Outdated Show resolved Hide resolved
saleor/settings.py Outdated Show resolved Hide resolved
saleor/settings.py Outdated Show resolved Hide resolved
tests/api/test_checkout.py Outdated Show resolved Hide resolved
tests/api/test_checkout.py Outdated Show resolved Hide resolved
@korycins
Copy link
Member

korycins commented Sep 5, 2019

@salwator FYI populate_db is failing.

saleor/core/payments.py Show resolved Hide resolved
saleor/core/payments.py Show resolved Hide resolved
@salwator salwator marked this pull request as ready for review September 6, 2019 13:35
@salwator salwator force-pushed the extract-payment-plugins branch 2 times, most recently from 334be21 to dd40002 Compare September 6, 2019 13:49
Creates new plugin module in Braintree payment gateway that overrides
all necessary methods and uses existing implementation.

  - Use BasePlugin

  - Create Braintree config structure

  - Call existing implementation from all plugin methods
saleor/extensions/base_plugin.py Outdated Show resolved Hide resolved
saleor/extensions/manager.py Outdated Show resolved Hide resolved
saleor/extensions/manager.py Outdated Show resolved Hide resolved
saleor/extensions/manager.py Outdated Show resolved Hide resolved
saleor/payment/__init__.py Show resolved Hide resolved
saleor/payment/gateways/braintree/__init__.py Outdated Show resolved Hide resolved
saleor/settings.py Show resolved Hide resolved
saleor/settings.py Outdated Show resolved Hide resolved
saleor/settings.py Outdated Show resolved Hide resolved
@korycins korycins merged commit 60d0a8d into master Sep 20, 2019
@korycins korycins deleted the extract-payment-plugins branch September 20, 2019 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
payments Issues related to payments implementation plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants