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

[Forwardport] Fixed bug, when exception occurred on order with coupons cancel, made by guest after creating of customer account. #20290

Closed
wants to merge 8 commits into from

Conversation

shikhamis11
Copy link
Member

Original Pull Request

#19423

Description (*)

When customer do checkout as a guest using a valid coupon, he makes a new customer account.
During trying to cancel this order, admin does receive SQL exception:
SQLSTATE[23000]: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails (mage.salesrule_customer, ...

Issue related to changes made: #19230

Fixed Issues (if relevant)

  1. Can't Cancel Order #19230 : I Can't Cancel Order

Scenario

Actual problem exist in scenarious when sales rule, used by guest user (using coupon code), it doesn't trigger its coupon usage per customer update, because there is no customer.
After creating of new customer (using of AccountDelegation action), Magento AssignOrderToCustomerObserver using customer_save_after_data_object event tries to assign to this customer order, that has been made previously.
As a result, this Magento\SalesRule\Model\Rule\Customer wasn't created for newly created customer, but normally it should. Furthermore, it should increase time_used attribute for new customer.
As a result, during trying to cancel this type of orders, sales rule plugin is trying to decrease amount of usages for given customer. Actually, method which is responsible for it is not good enough, so it allows to save empty data (empty instert), which provokes cascade adding by foreign keys. As result, admin cannot cancel this order and get SQL error.

Fixing

So, obviously we have to create this entity on customer registration, when Magento assigns order to related new customer.

As you can see, all data we need is set inside of AssignOrderToCustomerObserver and we cannot subscribe on same event, because only this observer (Sales) knows about order and customer, that its going to be assigned.
So, not to mixing contexts (Sales and SalesRule, because assigning of coupon and sales usages happens using Magento\SalesRule\Model\Coupon\UpdateCouponUsages), I decided to make few extending/improvements:

  1. Remove business logic from observer to related service, which will be responsible for delegating orders to customers.
  2. Fire up event event sales_order_customer_assign_after to let another module knows about certain customer has been attached to order.
  3. Catch in SalesRule module to create related salerule_customer model and increase usage time_used value to 1.
  4. Write functional testing, that represent all the flow.

So, this PR not only fixes exception on canceling orders, that has been made by guest before registering new account. But also fixes bug, when sales rule usage has not been increased on the same scenario.

Manual testing scenarios (*)

Checkout as a guest using a valid coupon
After placing the order, register an account using the create account button on the order confirmation page.
Login to the admin
Make sure to get the order to a processing state.
Try to cancel the order
Order is canceled correctly
Sales rule usage for newly created customer is made and set as 1

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-engcom-team
Copy link
Contributor

Hi @shikhamis11. 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-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@Winfle
Copy link
Contributor

Winfle commented Jan 14, 2019

Hello, @shikhamis11.
This issue is not reproducible on Magento 2.3

@ghost
Copy link

ghost commented Jan 15, 2019

Hi @shikhamis11, 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.

@shikhamis11
Copy link
Member Author

Ok m closing this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants