Change how we store bundles in Order.

@skyredwang Thanks for the issue. That's a really good description.

If I understand all that correctly, then all the data we don't want to change needs to be in OrderItems.

In case of a product bundle we have three objects that can potentially change.

1) The bundle itself
2) Each of the bundle items
3) Each of the product variations referenced by the bundle items

We had once decided to make the bundle the purchasable entity but not the bundle items. That has some consequences we have to live with, as long as we stick to this decision.

The order item holds the bundle price only. If you want to know the portion of the price that belongs to a bundle item at the time the order item was created, you are lost. And I don't know if we will find a solution. Price these days isn't a static value. The unit price of the variation oder bundle item will probably not answer your question. And how should we know that at all? The price is resolved at runtime and we simply have no idea which part of the price belongs to a specific variation. Even if we have a snapshot of the bundle item and the product variation, you cannot simply use these data for analytics.
In our current implementation the unit price of a product bundle would override all bundle item/variation prices. So I guess, one will need custom logic for reporting anyway, as far as product bundles are concerned.

I think the idea of referencing something with an defined interface is a good starting point. Though I think the OrderItemInterface is too broad on one site and may miss some bundle item specific stuff on the other side.

So I suggest that we first outline an interface and than see what comes out of it. As a provisional name I suggest BundleItemOrderItemInterface and hope we fill find a better one.

Here is my first idea: https://gist.github.com/olafkarsten/7324794e56b14dd68c94cd4e2bf3405c

We could also make the product bundle items the purchasable entities - but that will probably open up a whole can of worms.
:)

The purchased bundle item's variation is decided when the bundle was added to the cart.

I suggest BundleItemOrderItemInterface and hope we fill find a better one.

.
BundleItemOrderItemInterface give the ability to know the order item's variation.

When buy a product bundle, we just lack purchased variation's title and price information comparing to directly buy a product variation in a order.
So maybe we don't need a Entity Reference field type, it's complex.
We can extend the commerce_product_bundle_item_selection field type's schema:
http://cgit.drupalcode.org/commerce_product_bundle/tree/src/Plugin/Field...

  public static function schema(FieldStorageDefinitionInterface $field_definition) {
    return [
      'columns' => [
        'bundle_item' => [
          'description' => 'The bundle item.',
          'type' => 'numeric',
          'precision' => 19,
          'scale' => 0,
        ],
        'selected_qty' => [
          'description' => 'The selected quantity.',
          'type' => 'numeric',
          'precision' => 17,
          'scale' => 2,
        ],
        'selected_entity' => [
          'description' => 'The selected entity id.',
          'type' => 'numeric',
          'precision' => 19,
          'scale' => 0,
        ],
        'selected_entity_title' => [
          ...
        ],
        'selected_entity_unit_price' => [
          ...
        ],
        'selected_entity_unit_price_currency_code' => [
          ...
        ],
      ],
    ];
  }

then save the purchased variation's title and price.

The issue Refactoring of the bundle item / bundle item interface created by @olafkarsten

2) Remove the bundle item price override:

will have a little effect on the implement of this issue. So set it as related.

Okay, after some days I'm with @skyredwang. I'm pretty sure it will save us some headache in the future, if we do the work now and use a reference field. And, yes we should use the OrderItemInterface. I think this will allow for better/easier integration with other contrib modules in the long run. Updated the issue summary

The proposed change is too complex to be feasible.

Generally, we have a need for the order item to retain the bundle contents.
An alternative approach could be to replace the BundleItem entity type with a field type, then use the same field type on both bundles and order items, cloning the data on purchase. This buys a lot of simplicity, at the expense of extendability. I'd assume the fields would be something like product_id|variation_id|quantity|price|data with data being a serialized escape hatch.

I like to summarize my Slack chat I've just had with @bojanz:

I needed quite urgently a product set/bundle solution, like commerce_product_bundle offers. Especially due to this ongoing data model discussion and time pressure on our project, I saw the need for writing a custom module instead of contributing here.

Of course I want to share the results with others on the one side, but don't want to fragment the Drupal module landscape on the other side. That's why I've uploaded the module to Github and discussed with @bojanz whether or not it would be a good idea to host it on d.o as well. As the functionality is too close to CPB, we decided to just leave it on Github.

Here's the module: https://github.com/agoradesign/commerce_product_set

It might be a help in solving issues like this in CPB hopefully. I partially implemented, what @bojanz proposes in #7. I've only implemented a field like this on the order item side, while I am referencing product variations incl quantity on the product set/bundle entity. Both can synchronize to a value object, with makes copying/syncing values from product set to order item a one-liner.
here's the field type I'm using for the order items: https://github.com/agoradesign/commerce_product_set/blob/master/src/Plug...

The proposed change is too complex to be feasible.

Yepp, after playing around a bit with the idea, I think @bojanz is right. If we someday get dynamic entity reference field in core, that may be a lot easier. Postpone that for another major release and simple extend our selection field like @caseylau suggested. It's essentially the same like commerce_product_set solution. I will look into the details and see if we can borrow some other ideas.

Thanks @agoradesign for sharing.

This is one of the rare cases, I'm not convinced, that bojanz suggestion is the better solution. The order item hierarchy is a complex beast, but that doesn't necessarly mean we have to dismiss the bundle item. Actually the current working state of the module is a solution with a field on the order item. We only miss some extra data. We can simply extend it with the data we currently missing. That would be the fastest and easiest way, I guess. Also upgrading is a matter of a plain hook_update_N thingy.

We discussed the entity_reference field because it has a big PRO too, We would have the same API like normal OrderItems. I would expect integrations for other contrib modules is way easier than. But TBH it seems currently out of scope, simply because we have no ressources to do it in a reasonable time frame. We don't loose much with extending the current field with some extra information. If we encouter problems later, we will come back here and start over ;)

I'll give that a try in the evening and see, how it works.

One downside of having the custom field approach on the product bundle as well: if that isn't somehow a real entity reference - which is an advantage for persisting data of order items - you'll have to keep the items up to date with every update or delete operation of product variations.

Here is the first try. Seems to work. I'm not happy with that. It somehow smells - hackish. No API, consuming code needs to know the data structure. Tests passes so far. Didn't have the time to add functional tests, too.

https://github.com/olafkarsten/commerce_product_bundle/tree/2937550-exte...

Next thing to do is a database update for existing installs and some docs/comments on how to consume the data.

The concept of an own entity holding our data, works well too. Actually it wasn't that complicated, but somewhat more work than the "stuff it all in a field" approach ;). But TBH, if we consider the field route, we have some more work todo, especially regarding validation and test coverage and data handling (providing full entities from the ID values e.g.) - All that nice little things, that the entity approach gives us out of the box with thoroughly tested code. I think the entity approach is more drupalish and more like commerce core is doing stuff. So I expect it to have better DX, too. Plus, we have a well defined API controlled by us, so consumers don't need to now the implementation details anymore. Plus, some sort of views integration :D

Anyway, we have the choice.

@skyredwang maybe you or someone of your team can test this? Of course everyone is welcome to check it out.
https://github.com/olafkarsten/commerce_product_bundle/tree/proc-own-ord...

It breaks BC and needs a fresh install of the module, I guess.

Cons for this approach: breaks BC. And I have no idea how to provide an upgrade path from the field to the entity concept.
We are prealpha and could simply issue an change record and warn of the BC break, but I don't like that. Thats essentially the only point, which makes me uncomfortable with this idea.

Short summary how it works:
We configure our commerce_product_bundle_default order item type to hold an standard entity reference field (instead of our own field type). The referenced entity is named BundleItemOrderItem [I had no better idea]. It has no bundles. I don't think it needs that flexability. BundleItemOrderItem implements a new Interface BundleItemOrderItemInterface. We are adding one of such entity per purchased bundle item/product variation.

So getting the data is simple done by $order_item->get('bundle_item_order_items')->referencedEntities() , loop over the entities and ask for what you need.

The ProductBundleItemsWidget manages the entities and add them to the order items. That is essentially the same thing as we do with the field.

The backreference to the order item is added by an event listener and acts on ORDER_ITEM_INSERT.

No further feedback. We go with the entity approach.