DonationItemPaneBase::submitPaneForm needs to update its internal state to reflect up to date donation order item

Actually this may need work. I see that you're setting data on $this->donationItem in the validate function as well. I don't know whether or not this solution conflicts at all with what's going on in there.

Another note, submitForm was also problematic if the orderItem is deleted in the custom validate/submit callback. If instead of the snippet in the issue description you do something like:

function my_donation_pane_validate(array $form, \Drupal\Core\Form\FormStateInterface $form_state) {
  $route = \Drupal::routeMatch();
  /** @var \Drupal\commerce_order\Entity\OrderInterface $order */
  $order = $route->getParameter('commerce_order');
  foreach ($order->getItems() as $order_item) {
    if ($order_item->get('type')->isEmpty()
      || $order_item->get('type')->target_id !== 'donation') {
      continue;
    }
    
    $order_item->delete();
  }
}

There's some error like "cannot update entity ID during save," which I think is from the same bug. The order item entity has been deleted, but the pane is saving the old instance with the ID that not longer exists, causing some sort of conflict in Drupal's internal entity handler. I wonder if we really need to save the item in submitForm at all?

No problem! :D

This validate method in DonationItemPaneBase should be updating the pane's copy of the item with the values on the pane's form.

Okay yeah that's what I thought. I think the problems are that this is ineffective if either the changes happen outside of the context of the $form_state or any custom callbacks fire between the built-in validatePaneForm and submitPaneForm, as both situations will lead to a mismatch in internal vs and external donationItem metadata.

Hooks and callbacks can load it from storage, though, if they are looping over $order->getItems() (whether $order is coming from the pane, the route, or wherever). The source of truth for a third-party is really the item on the order, not necessarily the donationItem in the pane, because the pane's donationItem is just a stored reference from when the pane is initialized.

I think this issue stems from modifying the the order item in validatePaneForm, but saving it in submitPaneForm, because it can lead to this flow:

1. Checkout form is loaded and the order item is set on $this->donationItem
2. User submits checkout form
3. DonationItemPaneBase::validatePaneForm fires, which sets state on the local copy of the donation item
4. Custom validation callback (like the one in the summary, attached to the checkout form) fires, which discovers the donation line item from the order itself, modifies it, and saves it. This updates the actual donation line item entity itself but not $this->donationItem in the pane's state (since it is a copy). Now there is a mismatch where the pane has a stale version of the item.
5. submitPaneForm saves the stale, local version of the order item, which undoes any modifications done by the custom callback. It's like loading the order item entity, setting all values to the old ones, and then saving the entity.

In step 4, even if they were just to modify the $form_state, then step 5 would still be out of sync since it doesn't do any discovery on the of the $form_state. If the order item entity is deleted in step 4 instead of just modified, then you end up with the "cannot update entity ID during save" from step 5.

I will say that moving our logic from a #validate callback to a #submit callback avoids the problem. It may be accurate that a third-party callback should not modify+save the entity inside of #validate. But, if they do, it is awkward that the changes are tossed out without any notice or an error is thrown if the entity was deleted.

As far as loading the $order from the route, you're right that it is accessible when you're in the pane, but if you're in a custom callback attached to the checkout form itself then I'm not sure how to load it other than from the route. I don't think it's available in $form_state or the $form array, is it?

Right, it's not present in commerce_donation_flow code. Everything that I'm referring to is the result of us (as in me, on my own project, not us as in commerce_donation_flow) adding a callback on the checkout form in another module that modifies the donation item.

To clarify, when DonationItemPaneBase::create() does $instance->donationItem = reset($donations);, $instance->donationItem is now a copy of the entity. That's what I'm referring to by "stale". It is not affected by outside modifications to the donation item itself, because it's a copy of the object as it existed at instantiation.

The pane gets it from the flow. Does that work for your context? \Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowInterface::getOrder

I don't think that the checkout flow is available from the callback, unless there's a service I'm missing that can fetch it. It's not on $form or $form_state directly.