Order of specific type allows adding order items of type that are not intended for that order type

This is probably a risky change for 2.x and could potentially be a breaking change.

I don't see it risky because it does not change anything in storage...
For existing sites, that already have Order items types configured, the only thing that could happen is, on order edit form, only configured order item types will be displayed and available to be added. The existing ones will be also displayed, see below.

@see \Drupal\inline_entity_form\Plugin\Field\FieldWidget\InlineEntityFormComplex::formElement()
@see \Drupal\inline_entity_form\Plugin\Field\FieldWidget\InlineEntityFormSimple::formElement()

For forms, order item field uses inline entity form to display / add / edit its contents but it does not restrict in anyway already created order item entities on target bundles, only the new created ones. So users will be able to edit an already existing order item.

@see \Drupal\inline_entity_form\Element\InlineEntityForm

For display InlineEntityForm element does not restrict anything based on target bundles.

So, in my opinion, if nothing is changed in storage, works as before for existing data but only implements features for future data, only a filter actualy, what could go wrong?!
Also the patch passes all the tests...

Actually I see it more like a fix for Commerce to work as designed because when you create order item types that applies to a specific order type, you don't espect when creating an order of that type to be able to add order items of types not intended for that order type, right?!

What other risks do you see?

The main usecase I have in mind would be fees for example.

Let's say I create a "fee" order item type, I probably want to be able to add it to all order types. With this patch, there would be no way for me to do that for example?

In any case, there's no need to "push" for the change like this. We have time to gather feedback from the community on this. There's no "rush". You can live with a patch for a while.

Indeed, if you need "fee" for every order type, you need to create order item type "fee" for each order item types you have configured.
This could be solved with a multi select field for Order type but I imagine that this use case is rarely used like this. Even so, with this patch you still have the ability to implement it.
Let's imagine a scenario where you have 2 types of products, a physical ones like electronics and a virtual product like software license or maybe an insurance policy.
For physical products you define an order type like Physical product that has states like:

• Placed - user placed the order through your website or via phone call
• Confirmed - it will get on this status when the operator confirms the address, maybe the order is paid but could also be with COD, etc.
• Ordered - ordered at supplier maybe
• Processing - received from supplier and preparring for shipping
• Shipped
• Delivered
• COD received

...
For software licenses or insurance policies you have states like:

• Placed - same like physical product
• Confirmed - when the users paid for license if placed by phone and will be paid by bank transfer, payment link, etc.
• Generated - after the order is confirmed, a module could auto generate the license and...
• Sent - ...send it to the user and here the order is completed.

Because of auto nature of software order type, you don't want the callcenter operator add a physical product to a software order by mistake, right?!
With current implementation of Commerce, you have no way to prevent this but with this patch you have!
I think is better to have more control than less.
Not to say if you have orders types that should be handled by a group of users (a department maybe) and order types that should be handled by other groups of users (another department). You will not want both to be able to add all order item types to them, right?!
Maybe you have also an order type like Service contract that could have states like:

• Placed
• Confirmed
• Paid
• On going
• Ended

and so on.
How you limit users not to add by mistake a license order item to a contract order type?!?!
Also if user selects to create a license order type, why should be asked to select between physical products and service contracts on a license order?! Same for every other order type, why the need to select from products types and license type in a contract order?!

I'm nut pusing and I'm opened to discuss the implementation but, from my point of view, things are quite clear, this seems more a bug, and correct me if I'm wrong:
If you are able to add any order item type to any order type, then why asking for order type in order item type config form anyway?! Ok, I can understand that users can have different order item types for different fields, like in example with licenses above, but why ask in the UI for order type if actually the system don't take that field in consideration anywhere?!
I cannot see the logic here... If there is one, then let's make a patch that deletes the Order type field, that is even required, from order item types form because it misleads the user. What is the use of that field?!

I hope I made myself clear why I think this is a bug. Yes, I could patch my version of commerce or even implement the functionality in an alter hook of a custom module and never bother to make a public patch, but I think the actual implementation is wrong for reasons explained above and this patch is very useful for commerce sites, especially for large and complex ones where you could have dozens of order item types and will be quite hard to select the right one in order.

Of course, if you have 3 order item types and 2 order types, this could not be very painful but with large enviroments...

If you are able to add any order item type to any order type, then why asking for order type in order item type config form anyway?! Ok, I can understand that users can have different order item types for different fields, like in example with licenses above, but why ask in the UI for order type if actually the system don't take that field in consideration anywhere?!

It is used, in the default order type resolver:

/**
   * {@inheritdoc}
   */
  public function resolve(OrderItemInterface $order_item) {
    /** @var \Drupal\commerce_order\Entity\OrderItemTypeInterface $order_item_type */
    $order_item_type = $this->orderItemTypeStorage->load($order_item->bundle());

    return $order_item_type->getOrderTypeId();
  }

you can still reproduce the curent behavior to allow user to select between multiple order item types, if desired, by creating multiple order item types that all apply to same order type.

So if this gets released, and I want my N order types to still allow my M order item types, I could needs as many as N * M order item types? And how would that work if a particular product variation type is configured for a specific order item type? It can't be configured for N different order item types, can it?

As for custom order workflows, I find it works well to add a State field at the order item level, to match its specific fulfillment steps. Then the workflow at the order level is more uniform for CSRs. I use the custom Guard functionality to prevent order level transitions until all order items have reached a certain stage.

For me, flexibility is preferable because only about 10% of our orders are placed through the website by customers. The rest are placed by CSRs administratively. As a result, we have a good deal of custom code to make much the process of placing an order as easy as possible for CSRs. The last thing we need is additional limitations on the order/order item architecture.

One thing we do is allow a single product variation to be added to orders as different "order item types," (bypassing the product variation's order item type setting.) A CSR can add a product variation to an order as a free "product sample" or as a "credit item" (when goods are returned.) Each of those is a separate order item type with its own fields. (Credit items need to reference back to original order items and can have a credit amount not equal to qty*price. Sample items also store a "reserve" amount of inventory to be set aside for the customer for future orders.) For orders entered administratively, we don't use the standard Order edit form and inline-entity-form widget for items; instead we have a custom add-item widget on an order items form similar to a customer's "cart" page.

Through the website, when a customer adds to cart, the variation is just added as a standard/default order item. So the existence of the "order item type" setting is needed for our 10%, customer-facing, use case, but it would be very bad if all of a sudden it was impossible to create other order item types for our product variations. I realize that your patch does not directly relate to this at all. But... when I first saw this issue I thought about how for us, it's not a "bug" that we can add a product variation to orders as different types of order items. And for other websites, it's not a "bug" that they can add a single order item type to different types of orders (perhaps only administratively, like we do.)

Personally, I don't think this change would actually affect my site as it is now. I'm just concerned generally about an architectural change that could potentially break existing sites. And so, I agree with Jonathan that the best thing to do is to keep this issue open for a while. Perhaps create a patch. See whether there is widespread interest. If the majority agrees this is a bug that needs fixing, then the minority that requires the current behavior will just need to patch the code if it's released to keep their sites operational.

@jsacksick:

It is used, in the default order type resolver:

Yes, I know that but it resolve an order type but in fact all orders are available for that item type, right?
Why not change the resolver to something like:

  /**
   * {@inheritdoc}
   */
  public function resolve(OrderItemInterface $order_item) {
    /** @var \Drupal\commerce_order\Entity\OrderItemTypeInterface $order_item_type */
    $order_item_types = $this->orderItemTypeStorage->loadmultiple();
    $order_item_type = reset($order_item_types);

    return $order_item_type->getOrderTypeId();
  }

All order types are available to add any order item type to them so why bother user to select an order type. Actually, why have an order type at all?! Why not use just one order type?!?! If you need any custom fields, you add them at order item level or at purchasable entity level.
By the way, with your previous example with "fee", why are you using multiple order types and not just the default with all the item types pointing to it?
I agree with @lisastreeter that

Typically a good reason for having multiple bundle types for an entity is because you want to have different fields for each.

but I cannot imagine a real use case example where you would need more order types and even more order item types that share all order types. Maybe you can help me with this.

@lisastreeter

So if this gets released, and I want my N order types to still allow my M order item types, I could needs as many as N * M order item types? And how would that work if a particular product variation type is configured for a specific order item type? It can't be configured for N different order item types, can it?

Yes, you could create as many order item typse you would like with same variation or any other entity that is implementing purchaseble entity interface (I use it like this myself).
But I personally think that your implementation is less common than the one found in most online stores. Out of the box, Commerce should work for the most common use cases but, of course, for special cases like yours, it is simple to implement in a custom module, maybe the one with your custom workflow, something like this:

function hook_entity_bundle_field_info_alter(&$fields, \Drupal\Core\Entity\EntityTypeInterface $entity_type, $bundle) {
  if ($entity_type->id() == 'commerce_order' && $bundle == 'custom_order_type_with_all_order_items_available') {
    // Allow all order item types to this special order type.
    $fields['order_items']->setSetting('handler_settings', [
      'target_bundles' => [],
    ]);
  }
}

You could even remove $bundle from checking and all order types of your store will have all order item types available to them but, as I said, this I don't think is the common use case for most online stores.

As for custom order workflows, I find it works well to add a State field at the order item level, to match its specific fulfillment steps. Then the workflow at the order level is more uniform for CSRs. I use the custom Guard functionality to prevent order level transitions until all order items have reached a certain stage.

I find this implementation more hacky than just having an order type that has certain states and other order type that has other states.

One thing we do is allow a single product variation to be added to orders as different "order item types," (bypassing the product variation's order item type setting.) A CSR can add a product variation to an order as a free "product sample" or as a "credit item" (when goods are returned.) Each of those is a separate order item type with its own fields. (Credit items need to reference back to original order items and can have a credit amount not equal to qty*price. Sample items also store a "reserve" amount of inventory to be set aside for the customer for future orders.) For orders entered administratively, we don't use the standard Order edit form and inline-entity-form widget for items; instead we have a custom add-item widget on an order items form similar to a customer's "cart" page.

Like I said, you have a special need but most of commerce sites use form orders and should be able to use form orders by default, right?

Through the website, when a customer adds to cart, the variation is just added as a standard/default order item. So the existence of the "order item type" setting is needed for our 10%, customer-facing, use case, but it would be very bad if all of a sudden it was impossible to create other order item types for our product variations. I realize that your patch does not directly relate to this at all. But... when I first saw this issue I thought about how for us, it's not a "bug" that we can add a product variation to orders as different types of order items. And for other websites, it's not a "bug" that they can add a single order item type to different types of orders (perhaps only administratively, like we do).

I still think that is interpreted like a bug because I'm a programmer and i thought like this first time when I saw that even if I created an order type that should have only one item type, in my case purchasable entity was actually the shipment, I could also add to that order type a regular product variation. I did not expect that, if I want to "grant" access to all, I expect to be warned about or to do it deliberately, by implementing a hook, like above.
Also I think you are talking from the perspective of small setup with very good trained admin users but even for a very good trained user is harder to chose from a list of item types (imagine you have 20 item types) every time he create an order, especially an order that is expecting to have a single item type, like software license in example above. In a large setup, at least with a lot of admin users that have access to certain sections of the site, like the one I mostly program for, you will not have so good trained user for phone to order "mapping", callcenter, and is even harder to explain to them why should chose from a long list of order item types when that order type have, or should have, just one item type, like software license. User is not a programmer and want simplicity. I work with this type of users every day ;-) and is way better to not show an option than showing it and expecting user to not choose it by mistake.

Personally, I don't think this change would actually affect my site as it is now. I'm just concerned generally about an architectural change that could potentially break existing sites. And so, I agree with Jonathan that the best thing to do is to keep this issue open for a while. Perhaps create a patch. See whether there is widespread interest. If the majority agrees this is a bug that needs fixing, then the minority that requires the current behavior will just need to patch the code if it's released to keep their sites operational.

I don't think a patch will be a good solution, a custom module that implements the hook explained above is way better and it won't need any maintainence when Commerce code updates like a patch will do.
I explained in a previous comment that the patch won't break things for existing sites because it does not modify the storage or existing data, not in db, not in displays or forms.
I agree that community should decide but I don't see this as a milestone like you do. Maybe I'm missing something but right now I cannot imagine something that could go wrong with this patch, even in special implementations like yours...

The same type of issue is present on product variation types - you can only choose one order item type for each in their config, but they can be added to any order item type during the order creation process.

However, I agree with the concerns that limiting the default behaviour to a 1:1 relationship would only make sense for specific use-cases. A multiple select field should be the approach on this.