FIxed race condition on deleting old adjustment records

[lukeholder]

Description

No need to delete the Record if we already drop the rows directly in the database.

Related issues

#3283

[linear]

PT-2407 [4.x]: Internal adjustsments cache not updated after saving new adjustments?

[lukeholder]

To get this potential fix early and test out the PR, change your craftcms/commerce requirement in composer.json to:

"require": {
  "craftcms/commerce": "dev-feature/pt-2407-fix-adjustment-id-race-condition as 4.8.1.2",
  "...": "..."
}

Then run composer update.

We will update this ticket once the release is out.

[boboldehampsink]

@lukeholder any news on this?

[boboldehampsink]

@lukeholder I want to test this on Craft 5, can you make an updated branch for that?

[boboldehampsink]

Now seeing a lot of these errors so I can test it properly if you want

[lukeholder]

@boboldehampsink Yes please, testing this would be useful. Updated to 5.x base branch.

[boboldehampsink]

@lukeholder have been running this for a while but the errors are still showing up

[lukeholder]

@boboldehampsink we have decided that throwing an exception when trying to save a line item or adjustment whose ID is already deleted from the database shouldn't throw an exception, and instead will log it and return false from the \craft\commerce\services\OrderAdjustments::saveOrderAdjustment and \craft\commerce\services\LineItems::saveLineItem methods.

We think rapid re-saves of the order is causing this but since nowhere in our code do we look at the return value of those saves (and don't run validation) its safe to return false. We could look to add a validation error to the id attribute if that is something people would like, but I doubt developers using these methods are saving line items that don't exist often.

Can you please retest on this branch and let me know if the errors have stopped. (You will see messages in the logs but only as information, the same as saving an invalid line item or adjustment).

As for the race condition on order save, we are looking to other solutions but will do so in a larger release of commerce.

[lukeholder]

Replaced by #4030 which will be merged into 5.4 soon.