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

Advance the order state to processing when a capture notification is received #25876

Merged
merged 1 commit into from
Dec 30, 2019

Conversation

azambon
Copy link
Contributor

@azambon azambon commented Dec 2, 2019

Description (*)

When the registerCaptureNotificationCommand is executed, if the order is in state "new" or "pending payment", then its state is advanced to "processing", as it was up to Magento 2.2.5.
This should fix issues with orders remaining in "pending payment" state even though they were correctly paid. See issue #25659
Previously this command would set the state to "processing" only if the order did not already have a state set. This PR changes that check to do the same for "new" and "pending payment" orders too.
The unit test covering the command has been updated to include the new test cases.

Fixed Issues (if relevant)

#25659: Paypal Payments Pro IPN keeping payments marked as Pending Payment

Manual testing scenarios (*)

Prerequisite: a payment method must be installed and enabled that initially creates orders in state "new" or "pending payment" and then uses the "registerCaptureNotification" command to indicate when the payment is captured.

  1. Place an order with the aforementioned payment method
  2. When the payment is complete and the payment method has issued the registerCaptureNotification command, check the state of the generated order. Without this PR the state will still be "new" or "pending payment". With this PR the state becomes "processing"

@azambon azambon requested a review from YevSent as a code owner December 2, 2019 12:01
@m2-assistant
Copy link

m2-assistant bot commented Dec 2, 2019

Hi @azambon. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

@magento-engcom-team
Copy link
Contributor

Hi @dmytro-ch, thank you for the review.
ENGCOM-6383 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@azambon thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@sidolov sidolov changed the base branch from 2.3-develop to 2.4-develop December 5, 2019 17:05
@angelo983 angelo983 self-requested a review December 11, 2019 16:44
Copy link
Member

@angelo983 angelo983 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I endorse this PR and I think it should also be backported in version 2.3

@angelo983 angelo983 added the Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release label Dec 11, 2019
@engcom-Delta engcom-Delta self-assigned this Dec 18, 2019
@engcom-Delta
Copy link
Contributor

Hi @azambon I checked PR changes and payment status is remaining "Pending Payment" after order was placed.
Manual testing scenario:

  • Configure Website Payments Pro Hosted Solution:
    • Set Stores->Configuration->Sales->Payment Methods->Merchant Country=United Kingdom
    • Fill all required fields for Website Payments Pro Hosted Solution
    • Set Sandbox Mode=Yes
    • Set Basic Settings - PayPal Website Payments Pro Hosted Solution->Payment Action=Authorization
    • Set Enable this Solution=Yes
      #25876PRConfig
  • Place order using Website Payments Pro Hosted Solution payment method
  • Go to admin panel Sales->Orders

Expected result:
✔️ Order has a status of Processing

Actual result:
❌ Order status is "Pending Payment", but not "Processing"

Could you take a look?

@azambon
Copy link
Contributor Author

azambon commented Dec 19, 2019

Hello @engcom-Delta ,

I think that there are two issues with your test:

  1. I suspect that you finished the test before Paypal had time to send its IPN request back to Magento, or that the test was run in an environment that doesn't allow Magento to recieve these IPN requests. This PR fixes an issue that happens while Magento processes the Paypal IPN (or an equivalent server-to-server request from some other online payment method). Therefore it's imperative that the test includes the processing of such IPN request.
  2. You set the payment action to "Authorize", but that setting uses the "AuthorizeCommand", not the class that I changed. Please switch Paypal Hosted Pro's payment action to "Sale".

For reference, here are a few screenshots of some tests I did.

Here's an order with Paypal Hosted Pro set to "Authorize". This is without this PR, just what happens in a clean Magento installation.
The first screenshot is before the IPN request, the second is after it is received and processed.
authorization only - before IPN
authorization only - after IPN

Notice how the state changes from "Pending payment" to "Processing", even without this PR. Also notice how the transaction data appear in the "Payment information" section.

Now another order, this time with Paypal set to "Sale". Again this is without this PR and the two pictures show the order before and after the IPN request.
authorize and capture - before IPN
authorize and capture - after IPN

The payment information appear just like before, but this time the state remains set to "Pending payment". This is the bug that this PR fixes.

One last picture, this time with the PR. Paypal is still set to "Sale". There's only the "after" picture because I forgot to take the "before" one, but it would be identical to the one of the previous case.
authorize and capture with fix - after IPN

And this time the state gets updated to "Processing" as expected.

@engcom-Delta
Copy link
Contributor

@magento give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @engcom-Delta. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Contributor

Hi @engcom-Delta, here is your new Magento instance.
Admin access: https://pr-25876.instances.magento-community.engineering/admin
Login: admin Password: 123123q

@ghost ghost removed the Progress: needs update label Dec 20, 2019
@engcom-Delta
Copy link
Contributor

✔️ QA passed

@m2-assistant
Copy link

m2-assistant bot commented Dec 30, 2019

Hi @azambon, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Award: bug fix Award: test coverage Component: Sales Progress: accept Release Line: 2.4 Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants