🇷🇴Romania @silviuchingaru

@jsacksick as I said above:

If multiple bundles of purchased entity are required for same order, multiple order item types can be created with required bundles selected and same order type selected.

In #4 🐛 Notice: Trying to get property 'timestamp' of non-object în AuthcacheNodeHistorySetting->get() Active I reverted the changes to check if Authcache 7.x-2.x-dev passes the tests against the latest Drupal 7.x-dev version and it is not passing.

SilviuChingaru → created an issue.

I also think that, if you have the NID, you could and should be able to relate to original node, and not to have to add all needed fields to the index. Is contra productive: why use more resursces when you already have the data in Drupal database?!
And from another perspective, you only care about texts in searches, why have to add images, for example, or other elements like buy it now form from commerce to index?
It could be implemented in Search API query plugin, quite like original views, if you have an relation, a secondary query could be made to Drupal's db on nids obtained from search index to relate all needed fields.
This way you could use the full potential of Drupal views on search results.

Can you try also, please, with:

"patches": {
  "drupal/commerce_stock": {
    "#3330022 D10": "https://git.drupalcode.org/project/commerce_stock/-/merge_requests/14.diff"
  }
},

Thank you!

Can you post exact commands and their output of how you try to patch, please?

I queued a test agains Drupal 9.5.2, let's see if it passes.
Meanwhile please post the content of.rej file.

In the last commit, I enhanced testTraits to:

• Test traits overridden base fields (that can be implemented by traits if the trait will be aware of bundle that is currently applying to);
• Test if traits of entity types with entities (that have data), can be uninstalled;
• Test if traits that have data (disabled) can be forcedly uninstalled;
• Test if traits can be installed in duplicate form;
• Test if conflicting traits are detected.

Using this tests, I found a issue of CommerceBundleEntityFormBase::validateTraitForm() that was not detecting conflicts on traits that will be installed on same form submit (same request) and fix it. I modified validateTraitForm() to check conflicts against already installed traits but also other selected traits in the form, traits that will be installed on same request.

Last commit, a9f158b7, make this ready for review.

With MR !136, a module could implement a trait that is aware of $bundle it applies to and even allows overrides of base fields for that specific bundle like this:

namespace Drupal\custom_module\Plugin\Commerce\EntityTrait;

use Drupal\commerce\Plugin\Commerce\EntityTrait\EntityTraitBase;

class CustomTrait extends EntityTraitBase {

  /**
   * {@inheritdoc}
   */
  public function buildFieldDefinitions() {
    $fields = [];

    /** @var \Drupal\Core\Field\FieldDefinitionInterface[] $base_fields */
    $base_fields = \Drupal::service('entity_field.manager')->getBaseFieldDefinitions('commerce_shipment');

    $bundle = $this->configuration['bundle_id'];
    $fields['base_field'] = BaseFieldOverride::createFromBaseFieldDefinition($base_fields['base_field'], $bundle);
    $fields['base_field']->setLabel('New custom lable');

    // (...)
    // Other bundle field definitions or base fields override.
    // (...)

    return $fields;
  }

}

I thinnk this is very usefull for modules that implements Commerce traits and want to change settings to base fields also for that bundle, for example reuse them in a custom way. There should not be required to implement a new bundle field if the base field already exists and can be reused with an override.

I don't quite understand either but, as far as I understand, seems more related to 🐛 Changes are lost when collapsing a paragraphs subform including an inline_entity_form Needs work .
Can you please check if that patch on IEF module → solves the problem?

silviuchingaru → created an issue.

The easiest way to achieve that, is at theme level, in template preprocess or even in twig.

Against D10 I tested locally and all pass, after commit, we'll have a dev release for composer to install and test further, D9.x has some deprecated functions that won't pass against PHP 8.1, 8s a known issue, but all should pass also from 7.3 to 8.0.
I think this is ready for commit now.

Great, all tests passes now. I think is ready for commit. Thank you!

silviuchingaru → created an issue.

Patch to add a test.

All tests were fixed in ✨ Drupal 10 compatibility (Fixed all tests) Fixed .

I'm sorry that I did not continue this threa but i thought that this should be implemented in the Drupal 10 compatibility thread.

Fixed all tests (error and deprecation)!

After this patch, the output of run-tests.sh is:

[obfuscated-user@localhost d10]$ ./run-tests.sh --verbose --directory modules/contrib/commerce_stock

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\commerce_stock\Kernel\StockServiceManagerTest
  - Drupal\Tests\commerce_stock\Functional\ProductAdminTest
  - Drupal\Tests\commerce_stock\Functional\StockServiceTest
  - Drupal\Tests\commerce_stock_local\Kernel\Entity\StockLocationTest
  - Drupal\Tests\commerce_stock_local\Kernel\LocalStockServiceTest
  - Drupal\Tests\commerce_stock_local\Kernel\LocalStockUpdaterTest
  - Drupal\Tests\commerce_stock_local\Kernel\OrderEventTransactionsKernelTest
  - Drupal\Tests\commerce_stock_local\Kernel\OrderEventsTransactionsTest
  - Drupal\Tests\commerce_stock_local\Kernel\StockLocationStorageTest
  - Drupal\Tests\commerce_stock_field\Kernel\StockLevelTest
  - Drupal\Tests\commerce_stock_field\Functional\StockLevelFormatterTest
  - Drupal\Tests\commerce_stock_field\Functional\StockLevelWidgetsTest
  - Drupal\Tests\commerce_stock_enforcement\Functional\EnforcementBrowserTest
  - Drupal\Tests\commerce_stock_enforcement\Functional\OutOfStockTest

Test run started:
  Wednesday, January 25, 2023 - 23:58

Test summary
------------

Drupal\Tests\commerce_stock\Kernel\StockServiceManagerTest     1 passes
Drupal\Tests\commerce_stock\Functional\ProductAdminTest        2 passes
Drupal\Tests\commerce_stock\Functional\StockServiceTest        1 passes
Drupal\Tests\commerce_stock_local\Kernel\Entity\StockLocatio   1 passes
Drupal\Tests\commerce_stock_local\Kernel\LocalStockServiceTe   1 passes
Drupal\Tests\commerce_stock_local\Kernel\LocalStockUpdaterTe   1 passes
Drupal\Tests\commerce_stock_local\Kernel\OrderEventTransacti   3 passes
Drupal\Tests\commerce_stock_local\Kernel\OrderEventsTransact   7 passes
Drupal\Tests\commerce_stock_local\Kernel\StockLocationStorag   1 passes
Drupal\Tests\commerce_stock_field\Kernel\StockLevelTest        6 passes
Drupal\Tests\commerce_stock_field\Functional\StockLevelForma   2 passes
Drupal\Tests\commerce_stock_field\Functional\StockLevelWidge   6 passes
Drupal\Tests\commerce_stock_enforcement\Functional\Enforceme   2 passes
Drupal\Tests\commerce_stock_enforcement\Functional\OutOfStoc   2 passes

Test run duration: 45 min 19 sec

Detailed test results
---------------------


---- Drupal\Tests\commerce_stock\Functional\ProductAdminTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      ProductAdminTest.   27 Drupal\Tests\commerce_stock\Functio

Pass      Other      ProductAdminTest.   73 Drupal\Tests\commerce_stock\Functio



---- Drupal\Tests\commerce_stock\Functional\StockServiceTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockServiceTest.   40 Drupal\Tests\commerce_stock\Functio



---- Drupal\Tests\commerce_stock\Kernel\StockServiceManagerTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockServiceManag   38 Drupal\Tests\commerce_stock\Kernel\



---- Drupal\Tests\commerce_stock_enforcement\Functional\EnforcementBrowserTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      EnforcementBrowse   34 Drupal\Tests\commerce_stock_enforce

Pass      Other      EnforcementBrowse   42 Drupal\Tests\commerce_stock_enforce



---- Drupal\Tests\commerce_stock_enforcement\Functional\OutOfStockTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      OutOfStockTest.ph   34 Drupal\Tests\commerce_stock_enforce

Pass      Other      OutOfStockTest.ph   42 Drupal\Tests\commerce_stock_enforce



---- Drupal\Tests\commerce_stock_field\Functional\StockLevelFormatterTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockLevelFormatt   28 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelFormatt   42 Drupal\Tests\commerce_stock_field\F



---- Drupal\Tests\commerce_stock_field\Functional\StockLevelWidgetsTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockLevelWidgets   24 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelWidgets  155 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelWidgets  242 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelWidgets  357 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelWidgets  452 Drupal\Tests\commerce_stock_field\F

Pass      Other      StockLevelWidgets  470 Drupal\Tests\commerce_stock_field\F

---- Drupal\Tests\commerce_stock_field\Kernel\StockLevelTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockLevelTest.ph  151 Drupal\Tests\commerce_stock_field\K

Pass      Other      StockLevelTest.ph  169 Drupal\Tests\commerce_stock_field\K

Pass      Other      StockLevelTest.ph  181 Drupal\Tests\commerce_stock_field\K

Pass      Other      StockLevelTest.ph  191 Drupal\Tests\commerce_stock_field\K

Pass      Other      StockLevelTest.ph  199 Drupal\Tests\commerce_stock_field\K

Pass      Other      StockLevelTest.ph  230 Drupal\Tests\commerce_stock_field\K



---- Drupal\Tests\commerce_stock_local\Kernel\Entity\StockLocationTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockLocationTest   54 Drupal\Tests\commerce_stock_local\K



---- Drupal\Tests\commerce_stock_local\Kernel\LocalStockServiceTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      LocalStockService   47 Drupal\Tests\commerce_stock_local\K



---- Drupal\Tests\commerce_stock_local\Kernel\LocalStockUpdaterTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      LocalStockUpdater  149 Drupal\Tests\commerce_stock_local\K



---- Drupal\Tests\commerce_stock_local\Kernel\OrderEventTransactionsKernelTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      OrderEventTransac  240 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventTransac  325 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventTransac  447 Drupal\Tests\commerce_stock_local\K



---- Drupal\Tests\commerce_stock_local\Kernel\OrderEventsTransactionsTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      OrderEventsTransa  262 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  299 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  318 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  340 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  376 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  453 Drupal\Tests\commerce_stock_local\K

Pass      Other      OrderEventsTransa  557 Drupal\Tests\commerce_stock_local\K



---- Drupal\Tests\commerce_stock_local\Kernel\StockLocationStorageTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      StockLocationStor   49 Drupal\Tests\commerce_stock_local\K

@geek-merlin looks ok now 😉

@geek-merlin let's try then... 😉

@guy_schneerson you are welcomed but I think that the patch is not ready because it uses event from Symfony 6 and is marked as compatible with Drupal 8 which is not the case because Symfony 6 was implemented in Drupal > 9.0.
I think we should use it like Commerce does to make it also backward compatible.
I'll update merge request soon with this.

@RgnYLDZ you can apply the patch against current dev version and review it. You can even clone the 3330022-drupal-10-compatibility branch.
Thank you for your interest!

@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...

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...

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?

silviuchingaru → made their first commit to this issue’s fork.

silviuchingaru → made their first commit to this issue’s fork.