netresearch / module-shipping-inventory

The Netresearch shipping inventory package adds Magento Inventory capabilities to the Netresearch shipping modules.
Open Software License 3.0
0 stars 2 forks source link

BulkShipment: Shipment creation does not work for bundled products #1

Closed nobodyMO closed 2 years ago

nobodyMO commented 2 years ago

module version: 1.1.0 magento version 2.4.3

We use bundled products in Magento for selling multiple items of the same simple product or one product of a list of similar products. These bundle products are very helpful in combination with ebay (m2epro).

However, once we have a purchase order that contains a bundled product, no purchase order can be created for that order at Bulk Shipment. If only one order is selected in Bulk Shipment, the following error message will be in the log: main.ERROR: Es konnte kein gemeinsames Lager für den Versand aller Produkte ermittelt werden.

{"exception":"[object] (Magento\\Sales\\Exception\\CouldNotShipException(code: 0): Es konnte kein gemeinsames Lager für den Versand aller Produkte ermittelt werden. at /var/www/magento/vendor/netresearch/module-shipping-inventory/Model/BulkShipment/ShipOrder.php:138)"}

In the example, a bundle product with 3 units of a simple product was ordered. There are several 100 units of the simple product in stock and we only use the default warehouse. Of course, no warehouse can be found for the bundle product itself, because bundle products do not physically exist. The shipment can be created normally within the order.

powli commented 2 years ago

Hello @nobodyMO,

thanks for reporting this issue. We will investigate internally and try to fix the problem.

Internal ref: DHLGW-1249

mam08ixo commented 2 years ago

I can confirm the issue.

@nobodyMO We may be able to provide a fix but it requires to configure the bundle items as "ship separately". Would that work for you?

bundle_ship_separately

nobodyMO commented 2 years ago

Thank you for the answers. Marking the product as separately shippable would probably be a workable workaround. But revising all products because of this is not really nice.

mam08ixo commented 2 years ago

Module version 1.1.1 handles bundle products properly during batch processing if items are set to ship separately.

But revising all products because of this is not really nice.

Use mass attribute update on bundle products.

nobodyMO commented 2 years ago

Hello, Please excuse the feedback but unfortunately the fix misses the real problem. The topic of "sending separately" also completely misses the point. When creating a shipment and a label, the aim is to include exactly those items that are actually in the parcel. It is wrong to exclude items from the warehouse consideration here. Regardless of which stock consideration is carried out in this class, all partial items of the bundle product are added to the consignment and thus also to exactly this package, and the weight is also included in the package. Therefore, if someone wants to send partial items from a bundle product individually, they must not use this function, but must create the shipment manually. Moreover, the fix does not solve the actual problem. Bundle items have neither a count in stock nor location. It therefore never makes sense to take them into account when locating them.
Likewise, it makes no sense not to consider the sub-items because these will have to be added to the package by Magento. Otherwise the table inventory_reservation and the inventory will not be correct.

Simply removing the partial products from the bundle product also leads to the next problem that getSourceForOrderItems does not find a warehouse because the item list is empty according to the new rules in getQuantitiesFromOrderItems if the order consists only of the bundle product.

So if you use the bulk shipment function all the products from the order goes into exactly one shipment with one label, why not just do it that way?

private function getQuantitiesFromOrderItems(array $items): array
    {
        $shipmentItems = [];
        foreach ($items as $item) {
            $this->_logger->info('item:' . $item->getSku());            
            if ($item->getIsVirtual()) {
                continue;
            }

            if ($item->getParentItem() && $item->getParentItem()->getProductType() === Configurable::TYPE_CODE) {
                // children of a configurable are not shipped, ignore.
                continue;
            }

            if ($item->getProductType() === Bundle::TYPE_CODE) {
                continue;
            }

            $qty = $shipmentItems[$item->getSku()] ?? 0;
            $shipmentItems[$item->getSku()] = $qty + ($item->getQtyOrdered() - $item->getQtyShipped());
        }

        return array_filter($shipmentItems);
    }
mam08ixo commented 2 years ago

I must admit that I do not understand the issue. As far as I understand, the "ship separately" setting only leads to the individual child items being considered for shipping, not the bundle item (which, as you state, has no stock by itself). All the bundle's child items can still be included in one package (and will be during bulk shipment). I do not see where items are excluded.

The bulk shipment operation always packs all the order's items into one package. That is true for any product type, not only bundles. Partial shipments can only be created manually.

Did you test the proposed solution? Is the stock quantity of the individual items not deducted properly?

If you have a solution that works for you, then I suggest you create a pull request. Thank you!

nobodyMO commented 2 years ago

I did not set the items to be sent separately, because this is already logically wrong.

Let's assume a fictitious bundled product: A gift basket consisting of a bottle of wine, a jar of jam and of course the basket itself. There are no pre-packed gift baskets in the warehouse, otherwise they would be in the catalogue as a separate simple product.

The products assigned to the bundle product are:

What should the worker in the warehouse do when he finds the bundled product on the delivery note? He gets a basket, a bottle of wine and a glas of jam from the warehouse and packs the gift basket. Sending the parts individually from different warehouses is not an option because the customer has not ordered a construction kit.

Now let's look at the fix from version 1.1.1 both for the case where the sub-products are set to send individually and not. Case 1 shipped individually is activated: The bundle product has been removed, the partial products are taken over in shipmentItems. All products remain in the item array and are assigned to the shipment. The result would be correct, but the semantics are wrong. If shipping separately would play a role, then it should not be required for the partial products that all partial products must come from the same warehouse and these should also not automatically be forced into one delivery.

Case 2 shipped individually is deactivated: Then the bundle product would be taken over in shipmentItems and the partial products removed. This always leads to an error.

There are also bundled products for which separate shipment would make sense. This would be, for example, an air-conditioning split system consisting of an indoor unit and an outdoor unit. Of course, these can also be shipped from different warehouses without any problems. However, sending separately is only a permitted option here and is not a requirement. The part items can still be sent together. In this case, however, the option is irrelevant.

However, if the Bulk Shipment function is used for this order, this still does not matter because the execute method always reads all items from the order and passes all items with all partial items to the shipOrder->execute function. This means that all items are always sent together and there is only one label for the hole order.

What does this mean for the consideration of whether everything can be delivered from one warehouse?

mam08ixo commented 2 years ago

Thank you for the explanation. So the issue with "ship separately" during bulk shipment is that the MSI source selection algorithm may pick different sources, although all the bundled items are required to be shipped from the same physical location.

But isn't that exactly what the module tries to achieve? To find one source for all the order's items for shipping?

https://github.com/netresearch/module-shipping-inventory/blob/972be61bf146e9d42612e29c08a8fe0e46920377/Model/BulkShipment/ShipOrder.php#L210-L211

nobodyMO commented 2 years ago

no, searching for a warehouse for all items is already correct. The only difference is that only the sub-items should be considered for all cases. The option "ship bundled items separately" is irrelevant.

mam08ixo commented 2 years ago

Okay I can accept that all the bundle's items are used for source allocation regardless of the Ship Bundle Items setting.

Your PR has a Property '_logger' not found in ShipOrder error. Please remove that and from my perspective it is ready to be merged. Thank you!

mam08ixo commented 2 years ago

Thank you for the contribution. The PR was merged and released with version 1.1.2.