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
Lock checkoutComplete, create order from adyen notification #6048
Conversation
korycins
commented
Aug 24, 2020
•
edited
edited
- refactor checkoutComplete mutation
- extract whole checkoutComplete logic to separate file
- use checkout_complete logic to create order from Adyen notification
- apply lock for handling the adyen notifciation and mutation checkoutComplete
This pull request fixes 2 alerts when merging 5bbe330 into 628e980 - view on LGTM.com fixed alerts:
|
This pull request fixes 2 alerts when merging 7ca37db into 527c92b - view on LGTM.com fixed alerts:
|
Codecov Report
@@ Coverage Diff @@
## master #6048 +/- ##
==========================================
+ Coverage 91.73% 91.82% +0.08%
==========================================
Files 387 389 +2
Lines 25338 25699 +361
Branches 2382 2451 +69
==========================================
+ Hits 23243 23597 +354
- Misses 1509 1514 +5
- Partials 586 588 +2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now it great.
…complete. Fix incorrect mock patchess
This pull request fixes 2 alerts when merging 58c872f into 527c92b - view on LGTM.com fixed alerts:
|
This pull request fixes 2 alerts when merging a8d4ba0 into 527c92b - view on LGTM.com fixed alerts:
|
Are the schema changes intentional? |
This pull request fixes 2 alerts when merging 225a61f into bf87e63 - view on LGTM.com fixed alerts:
|
@patrys , I didn't add any changes for schema. Not sure why it raises the error. I assume that I need to update my branch with master |
@korycins Was this branch rebased after the test change that increased the number of queries on master? Trying to make sure we're not comparing apples to oranges here. |
): | ||
if checkout.is_shipping_required(): | ||
if not checkout.shipping_method: | ||
raise ValidationError( | ||
{ | ||
"shipping_method": ValidationError( | ||
"Shipping method is not set", | ||
code=error_code.SHIPPING_METHOD_NOT_SET, | ||
code=error_code.SHIPPING_METHOD_NOT_SET.value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are those changes needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am getting:
Unexpected type(s): (str, Union[CheckoutErrorCode, PaymentErrorCode]) Possible types: (Any, Optional[str]) (Any, Optional[str])
And then it raises the exception on mypy:
saleor/checkout/checkout_cleaner.py:43: error: Argument "code" to "ValidationError" has incompatible type "Union[CheckoutErrorCode, PaymentErrorCode]"; expected "Optional[str]" [arg-type]
Found 1 error in 1 file (checked 1 source file)
This pull request fixes 2 alerts when merging ca38344 into c1d39ce - view on LGTM.com fixed alerts:
|
This pull request fixes 2 alerts when merging 9e5c8a7 into 344dffc - view on LGTM.com fixed alerts:
|
This pull request fixes 2 alerts when merging 76935d2 into 344dffc - view on LGTM.com fixed alerts:
|
This pull request fixes 2 alerts when merging 4aace2a into 344dffc - view on LGTM.com fixed alerts:
|