magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.38k stars 9.28k forks source link

Clean Expired Orders Issue #16779

Closed magefan closed 5 years ago

magefan commented 5 years ago

Preconditions

When you have pending orders, from time to time magento cancel this orders. It depends on the option Pending Payment Order Lifetime (minutes) (https://prnt.sc/k5zs2m). Magento has a cronjob sales_clean_orders to do this

Magento 2.2.4 PHP 7.0.27

Steps to reproduce

  1. Create a catalog price rule without a coupon
  2. Add product to cart, and make sure that the rule is applied
  3. Plase an order (use payment method without invoice), e.g. credit memo

Expected result

Once Pending Payment Order Lifetime expired, magento run cron job sales_clean_orders without the issue

Actual result

Cron job status = Error Message SQLSTATE[23000]: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails (salesrule_customer, CONSTRAINTSALESRULE_CUSTOMER_RULE_ID_SALESRULE_RULE_IDFOREIGN KEY (rule_id) REFERENCESsalesrule(rule_id) ON DELETE CASCADE), query was: INSERT INTOsalesrule_customer() VALUES ()

More info

We have made an investigation, and the problem is in method

\Magento\SalesRule\Model\Coupon\UpdateCouponUsages::updateCustomerRuleUsages When order has been placed using cart rule without coupon it does not do eny set actions, so the object data before $ruleCustomer->save(); is empty array.

Our temporary sollution was to chage code like this.

Old code

private function updateCustomerRuleUsages(bool $increment, int $ruleId, int $customerId)
    {
        /** @var \Magento\SalesRule\Model\Rule\Customer $ruleCustomer */
        $ruleCustomer = $this->ruleCustomerFactory->create();
        $ruleCustomer->loadByCustomerRule($customerId, $ruleId);
        if ($ruleCustomer->getId()) {
            if ($increment || $ruleCustomer->getTimesUsed() > 0) {
                $ruleCustomer->setTimesUsed($ruleCustomer->getTimesUsed() + ($increment ? 1 : -1));
            }
        } elseif ($increment) {
            $ruleCustomer->setCustomerId($customerId)->setRuleId($ruleId)->setTimesUsed(1);
        }
        $ruleCustomer->save();
    }
private function updateCustomerRuleUsages(bool $increment, int $ruleId, int $customerId)
    {
        /** @var \Magento\SalesRule\Model\Rule\Customer $ruleCustomer */
        $ruleCustomer = $this->ruleCustomerFactory->create();
        $ruleCustomer->loadByCustomerRule($customerId, $ruleId);
        if ($ruleCustomer->getId()) {
            if ($increment || $ruleCustomer->getTimesUsed() > 0) {
                $ruleCustomer->setTimesUsed($ruleCustomer->getTimesUsed() + ($increment ? 1 : -1));
                $ruleCustomer->save();
            }
        } elseif ($increment) {
            $ruleCustomer->setCustomerId($customerId)->setRuleId($ruleId)->setTimesUsed(1);
            $ruleCustomer->save();
        }
    }
magento-engcom-team commented 5 years ago

Hi @magefan. Thank you for your report. To help us process this issue please make sure that you provided the following information:

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento-engcom-team give me {$VERSION} instance

where {$VERSION} is version tags (starting from 2.2.0+) or develop branches (2.2-develop +). For more details, please, review the Magento Contributor Assistant documentation.

@magefan do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?

ghost commented 5 years ago

@magefan, thank you for your report. We were not able to reproduce this issue by following the steps you provided. Please provide more detailed steps to reproduce or try to reproduce this issue on a clean installation or latest release.

RaphaelBronsveld commented 5 years ago

Hi, We have the same problem with the cancel order cron. We did not have the time yet to research the issue though.

ghost commented 5 years ago

@magefan, we are closing this issue due to inactivity. If you'd like to update it, please reopen the issue.

nikoelgatito commented 5 years ago

Hi,

Magento Open Source v2.2.5

We encountered a very similar issue when trying to cancel an order with a coupon code applied that had been created by a Guest that later created an account to which the order had been assigned.

In our case, the issue has been triggered when manually cancelling the order from the backend.


As a customer ID is now assigned to this order, it is passed as a parameter to this function:

Magento\SalesRule\Model\Coupon\UpdateCouponUsages::updateRuleUsages()

The $customerId variable then satisfies the condition below:

...
if ($customerId) {
    $this->updateCustomerRuleUsages($increment, $ruleId, $customerId);
}
...

Then within the below function, as 1. there is no existing rule associated to the current customer ID due to the fact that the order had been placed by a Guest and that 2. the $increment variable is false due to the fact that the current order cancellation action is intended to subtract 1 use, 3. the result is that there is no data set to the object before executing the save() function:

Magento\SalesRule\Model\Coupon\UpdateCouponUsages::updateCustomerRuleUsages()

if ($ruleCustomer->getId()) { //1. CONDITION NOT MET
    if ($increment || $ruleCustomer->getTimesUsed() > 0) {
        $ruleCustomer->setTimesUsed($ruleCustomer->getTimesUsed() + ($increment ? 1 : -1));
    }
} elseif ($increment) { //2. CONDITION NOT MET
    $ruleCustomer->setCustomerId($customerId)->setRuleId($ruleId)->setTimesUsed(1);
}
$ruleCustomer->save(); //3. EMPTY OBJECT CAN'T BE SAVED

To reproduce the issue very simply, one might place an order as a guest and then manually add a customer_id for the order in the sales_order database table. Then when trying to cancel the order from the backend, this error will be triggered.

The solution proposed by @magefan which consist of executing the save() method within the conditions directly is valid as it would make sure that there will be no attempt to save an empty object under any circumstances.

Solution:

private function updateCustomerRuleUsages(bool $increment, int $ruleId, int $customerId)
{
    /** @var \Magento\SalesRule\Model\Rule\Customer $ruleCustomer */
    $ruleCustomer = $this->ruleCustomerFactory->create();
    $ruleCustomer->loadByCustomerRule($customerId, $ruleId);

    if ($ruleCustomer->getId()) {
        if ($increment || $ruleCustomer->getTimesUsed() > 0) {
            $ruleCustomer->setTimesUsed($ruleCustomer->getTimesUsed() + ($increment ? 1 : -1));
            $ruleCustomer->save(); //ADD SAVE METHOD WITHIN THE CONDITION
        }
    } elseif ($increment) {
        $ruleCustomer->setCustomerId($customerId)->setRuleId($ruleId)->setTimesUsed(1);
        $ruleCustomer->save(); //ADD SAVE METHOD WITHIN THE CONDITION
    }
    //$ruleCustomer->save(); //REMOVE SAVE METHOD
}
adkanojia commented 5 years ago

@magefan thanks for the snippet.

I had the same issue (magento 2.2.5 ) when trying to cancel one order which is in 'pending' state. I'm getting the same error. I did override the method in a custom module. But I still can't mark the order as 'canceled'.

I get exactly same error when I try to cancel from order listing page, SQLSTATE[23000]: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails (`DB_NAME`.`salesrule_customer`, CONSTRAINT `SALESRULE_CUSTOMER_RULE_ID_SALESRULE_RULE_ID` FOREIGN KEY (`rule_id`) REFERENCES `salesrule` (`rule_id`) ON DELETE CASCADE), query was: INSERT INTO `salesrule_customer` () VALUES ()

but when I try to mark the order as canceled from order view page, I get You have not canceled the item.

Does this update only prevents the creation of such orders or it was actually to fix the error caused at cancelling order? I apologize if I'm missing something here. I'm completely new to magento.

Thanks,

friendscottn commented 5 years ago

@engcom-backlog-pb think we could get this reopened? I'm seeing this issue as well (2.2.6). It looks like there's a decent solution and set of steps to recreate the issue from @nikoelgatito above

friendscottn commented 5 years ago

@adkanojia @magefan 's fix actually does fix the issue. It's not to prevent the condition from happening. Your description of "You have not canceled the item" is the generic string that hides the underlying foreign key constraint error. In other words it's the same issue.

Your custom method is probably not getting picked up because the updateCustomerRuleUsages method is unfortunately private.

I just made the change in the base code(I know, bad), but you could potentially use a plugin to jump into the $ruleCustomer->save() and prevent the save from happening. This is a bit clunky though because save() is on the AbstractModel class used by all kinds of things.

Tjitse-E commented 5 years ago

We're having the same issue on Magento 2.2.7. @magefan 's fix worked in our case.

pietervanuhm commented 5 years ago

We're having the same issue on Magento 2.2.8

BradMorrisAlly commented 5 years ago

We ran into the same problem, and wasted a day coming up with the same patch. This is a clear bug with a straight forward solution. I don't understand why this was automatically closed. Note also this affects 2.3.X as well, but the code has been moved to SalesOrderAfterPlaceObserver.php.

dufrygrant commented 4 years ago

@engcom-backlog-pb +1 for seeing this on Magento 2.2.9

gewaechshaus commented 4 years ago

+1 Magento 2.2.9

quangdog commented 4 years ago

Still a problem. 2.3.3

michel334 commented 4 years ago

Same problem here. 2018: we are closing this issue due to inactivity. What a nightmare

quangdog commented 4 years ago

How can we get this re-opened? It's obviously still a problem....

juharintanen commented 4 years ago

image Mage 2.3.4

And @magefan fix still worked

ktruehl commented 4 years ago

This is apparently still an issue even in 2.3.5.

hterhanian commented 3 years ago

This is still an issue for me. Ran a sale and cannot cancel some orders now.

Magento 2.3.4, PHP 7.2.26

[2020-09-18 22:05:28] main.CRITICAL: SQLSTATE[23000]: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails (prodmagento.salesrule_customer, CONSTRAINTSALESRULE_CUSTOMER_RULE_ID_SALESRULE_RULE_IDFOREIGN KEY (rule_id) REFERENCESsalesrule(rule_id) ON DELETE CASCADE), query was: INSERT INTOsalesrule_customer() VALUES () {"exception":"[object] (Zend_Db_Statement_Exception(code: 23000): SQLSTATE[23000]: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails (prodmagento.salesrule_customer, CONSTRAINTSALESRULE_CUSTOMER_RULE_ID_SALESRULE_RULE_IDFOREIGN KEY (rule_id) REFERENCESsalesrule(rule_id) ON DELETE CASCADE), query was: INSERT INTOsalesrule_customer() VALUES () at /data/www/www.uncleharrys.com/html/releases/master-20200915121452/vendor/magento/framework/DB/Statement/Pdo/Mysql.php:110, PDOException(code: 23000): SQLSTATE[23000]: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails (prodmagento.salesrule_customer, CONSTRAINTSALESRULE_CUSTOMER_RULE_ID_SALESRULE_RULE_IDFOREIGN KEY (rule_id) REFERENCESsalesrule(rule_id) ON DELETE CASCADE) at /data/www/www.uncleharrys.com/html/releases/master-20200915121452/vendor/magento/framework/DB/Statement/Pdo/Mysql.php:91)"} []

vrielsa commented 3 years ago

Still an issue 2.4.1

MyroslavDobra commented 2 years ago

Still an issue 2.4.3 Code moved to another class https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/SalesRule/Model/Coupon/Usage/Processor.php#L147 but issue still exists

simonmaass commented 2 years ago

i can confirm this issue in 2.4.3-p1