Order items added by Buy X Get Y does not dispatch CART_EMPTY event when removed

Here is a patch that uses the cart manager in the PromotionOrderProcessor to remove the order item.
I had to add the commerce_cart module dependency in some tests to not them to fail.

Unfortunately, I think I'm going to mark this as "won't fix" because we can't have the promotion module depend on commerce_cart...

Thank you for your reply :)

I still think we can do something because it really breaks some commerce_cart features.
Isn't it possible to check if the 'commerce_cart' module exists (with the module handler) in the PromotionOrderProcessor to use removeOrderItem() ? And if we don't use dependency injection to inject the cart manager, it won't break any tests.
If you say it's a good idea, I'll make another patch.

I'm not sure this is the right approach as to me this event means "The customer removed an item from its cart" and this is an automatic programmatic removal which has a different meaning.

When you say "this event", do you talk about the CART_EMPTY event or the CART_ORDER_ITEM_REMOVE event also dispatch in removeOrderItem() ? If you talk about CART_ORDER_ITEM_REMOVE event, then I agree it should not be triggered for an order item removed automatically. And in this case, we could call only emptyCart() method instead of all the removeOrderItem() method. But if you talk about the CART_EMPTY event, I don't understand the problem because the cart is litteraly emptied by the user removing the "X" item.

Moreover, I've just seen that CART_EMPTY is used in commerce_promotion to remove the coupons when the cart is empty as mentionned here : https://www.drupal.org/project/commerce/issues/3065951#comment-13178606 →
I have tested it and indeed it doesn't work when the last item removed comes form a Buy X get Y promotion.

As I didn't get any answer, I made a new patch removing dependency to commerce_cart and calling only emptyCart() method when it's needed. I did not edit any test then.
I put the issue in "Needs review" again but if @jsacksick you think it is still not a right approach, you can close it.
I still post the patch for those who might need it.

I don't think the promotion module should worry about cart events... The "CART_EMPTY" event to me indicates a deliberate action from the customer emptying its cart... You could react to an order item being deleted in your custom code.

Thank's for your answer @jsacksick, I understand the problem in my patch.
But something is bothering me, as you said :

The "CART_EMPTY" event to me indicates a deliberate action from the customer emptying its cart

This is exactly the issue here. When a customer removes the X item from his cart (the deliberate action), the Y item is removed too so the cart is empty but the CART_EMPTY event was not triggered at all. In this case, however, the empty cart comes from a deliberate action from the user. In my mind, it is a breaking issue in commerce_cart.

If the problem won't be fixed, maybe we should write it somewhere in a documentation ?

I will keep my patch for now but if anyone got a better idea, I'll be happy to hear it !

We could maybe then optionally inject the cart manager or... maybe dispatch another event from the promotion module that the cart module reacts to.. none of the solutions are really "elegant". Optionally injecting the cart manager is better than skipping dependency injection though.

I think dispatch another event from the promotion module is a better option to avoid requiring commerce_cart.
Here is a new patch that implements this solution.

I did not realise that the new patch could failed some tests, I set the issue in Needs work while I fix this.

Here is a new patch that should fix tests.