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

Clean Expired Orders Issue #16779

Closed
magefan opened this issue Jul 13, 2018 · 22 comments
Closed

Clean Expired Orders Issue #16779

magefan opened this issue Jul 13, 2018 · 22 comments
Labels
Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Progress: needs update

Comments

@magefan
Copy link
Contributor

magefan commented Jul 13, 2018

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, 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 ()

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 magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Jul 13, 2018
@magento-engcom-team
Copy link
Contributor

magento-engcom-team commented Jul 13, 2018

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

  • Summary of the issue
  • Information on your environment
  • Steps to reproduce
  • Expected and actual results

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?

  • yes
  • no

@ghost
Copy link

ghost commented Jul 23, 2018

@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
Copy link

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

@ghost
Copy link

ghost commented Aug 6, 2018

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

@ghost ghost closed this as completed Aug 6, 2018
@nikoelgatito
Copy link
Contributor

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
Copy link

adkanojia commented Oct 22, 2018

@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
Copy link

friendscottn commented Nov 13, 2018

@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
Copy link

@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
Copy link
Contributor

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

@pietervanuhm
Copy link

We're having the same issue on Magento 2.2.8

@BradMorrisAlly
Copy link

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
Copy link

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

@gewaechshaus
Copy link

+1 Magento 2.2.9

@quangdog
Copy link

Still a problem. 2.3.3

@michel334
Copy link

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

@quangdog
Copy link

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

@juharintanen
Copy link

juharintanen commented Feb 5, 2020

image
Mage 2.3.4

And @magefan fix still worked

@ktruehl
Copy link

ktruehl commented Apr 21, 2020

This is apparently still an issue even in 2.3.5.

@hterhanian
Copy link

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, 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 () {"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, 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 () 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, CONSTRAINT SALESRULE_CUSTOMER_RULE_ID_SALESRULE_RULE_ID FOREIGN KEY (rule_id) REFERENCES salesrule (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
Copy link

vrielsa commented May 25, 2021

Still an issue 2.4.1

@MyroslavDobra
Copy link

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
Copy link

i can confirm this issue in 2.4.3-p1

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Progress: needs update
Projects
None yet
Development

No branches or pull requests