Saving Shipment overwrites its associated Inventory Source with 'default'

francescosalvi commented 3 years ago

Preconditions (*)

Magento 2.4.2 (tested with a vanilla-ish installation, i.e. disabling all non-core modules)

composer info | grep module-inventory

magento/module-inventory                                          1.2.0     N/A
magento/module-inventory-admin-ui                                 1.2.0     N/A
magento/module-inventory-advanced-checkout                        1.2.0     N/A
magento/module-inventory-api                                      1.2.0     N/A
magento/module-inventory-bundle-import-export                     1.1.0     N/A
magento/module-inventory-bundle-product                           1.2.0     N/A
magento/module-inventory-bundle-product-admin-ui                  1.2.0     N/A
magento/module-inventory-bundle-product-indexer                   1.1.0     N/A
magento/module-inventory-cache                                    1.2.0     N/A
magento/module-inventory-catalog                                  1.2.0     N/A
magento/module-inventory-catalog-admin-ui                         1.2.0     N/A
magento/module-inventory-catalog-api                              1.3.0     N/A
magento/module-inventory-catalog-frontend-ui                      1.0.0     N/A
magento/module-inventory-catalog-search                           1.2.0     N/A
magento/module-inventory-configurable-product                     1.2.0     N/A
magento/module-inventory-configurable-product-admin-ui            1.2.0     N/A
magento/module-inventory-configurable-product-frontend-ui         1.0.0     N/A
magento/module-inventory-configurable-product-indexer             1.2.0     N/A
magento/module-inventory-configuration                            1.2.0     N/A
magento/module-inventory-configuration-api                        1.2.0     N/A
magento/module-inventory-distance-based-source-selection          1.2.0     N/A
magento/module-inventory-distance-based-source-selection-admin-ui 1.2.0     N/A
magento/module-inventory-distance-based-source-selection-api      1.2.0     N/A
magento/module-inventory-elasticsearch                            1.2.0     N/A
magento/module-inventory-export-stock                             1.2.0     N/A
magento/module-inventory-export-stock-api                         1.2.0     N/A
magento/module-inventory-graph-ql                                 1.2.0     N/A
magento/module-inventory-grouped-product                          1.2.0     N/A
magento/module-inventory-grouped-product-admin-ui                 1.2.0     N/A
magento/module-inventory-grouped-product-indexer                  1.2.0     N/A
magento/module-inventory-import-export                            1.2.0     N/A
magento/module-inventory-in-store-pickup                          1.1.0     N/A
magento/module-inventory-in-store-pickup-admin-ui                 1.1.0     N/A
magento/module-inventory-in-store-pickup-api                      1.1.0     N/A
magento/module-inventory-in-store-pickup-frontend                 1.1.0     N/A
magento/module-inventory-in-store-pickup-graph-ql                 1.1.0     N/A
magento/module-inventory-in-store-pickup-multishipping            1.1.0     N/A
magento/module-inventory-in-store-pickup-quote                    1.1.0     N/A
magento/module-inventory-in-store-pickup-quote-graph-ql           1.1.0     N/A
magento/module-inventory-in-store-pickup-sales                    1.1.0     N/A
magento/module-inventory-in-store-pickup-sales-admin-ui           1.1.0     N/A
magento/module-inventory-in-store-pickup-sales-api                1.1.0     N/A
magento/module-inventory-in-store-pickup-shipping                 1.1.0     N/A
magento/module-inventory-in-store-pickup-shipping-admin-ui        1.1.0     N/A
magento/module-inventory-in-store-pickup-shipping-api             1.1.0     N/A
magento/module-inventory-in-store-pickup-webapi-extension         1.1.0     N/A
magento/module-inventory-indexer                                  2.1.0     N/A
magento/module-inventory-low-quantity-notification                1.2.0     N/A
magento/module-inventory-low-quantity-notification-admin-ui       1.2.0     N/A
magento/module-inventory-low-quantity-notification-api            1.2.0     N/A
magento/module-inventory-multi-dimensional-indexer-api            1.2.0     N/A
magento/module-inventory-product-alert                            1.2.0     N/A
magento/module-inventory-requisition-list                         1.2.0     N/A
magento/module-inventory-reservation-cli                          1.2.0     N/A
magento/module-inventory-reservations                             1.2.0     N/A
magento/module-inventory-reservations-api                         1.2.0     N/A
magento/module-inventory-sales                                    1.2.0     N/A
magento/module-inventory-sales-admin-ui                           1.2.0     N/A
magento/module-inventory-sales-api                                1.2.0     N/A
magento/module-inventory-sales-frontend-ui                        1.2.0     N/A
magento/module-inventory-setup-fixture-generator                  1.2.0     N/A
magento/module-inventory-shipping                                 1.2.0     N/A
magento/module-inventory-shipping-admin-ui                        1.2.0     N/A
magento/module-inventory-source-deduction-api                     1.2.0     N/A
magento/module-inventory-source-selection                         1.2.0     N/A
magento/module-inventory-source-selection-api                     1.4.0     N/A
magento/module-inventory-swatches-frontend-ui                     1.0.0     N/A
magento/module-inventory-visual-merchandiser                      1.1.0     N/A
magento/module-inventory-wishlist                                 1.0.0     N/A

Steps to reproduce (*)

// $om = instance of object manager
// wrap in transaction in order to make the test repeatable

/** @var \Magento\Sales\Model\ResourceModel\Order\Shipment\Collection $shipmentCollection */
$shipmentCollection = $om->get(\Magento\Sales\Model\ResourceModel\Order\Shipment\CollectionFactory::class)->create();
$shipmentCollection->setPageSize(2);
$shipmentCollection->setOrder('created_at', \Magento\Framework\Data\Collection::SORT_ORDER_ASC);

$connection = $resourceConnection->getConnection();

/** @var \Magento\Sales\Model\Order\Shipment $shipment */
foreach ($shipmentCollection as $shipment) {
    $shipmentSourceSelect = $connection->select();
    $shipmentSourceSelect->from($connection->getTableName('inventory_shipment_source'));
    $shipmentSourceSelect->where('shipment_id = ?', $shipment->getId());
    $shipmentSourceSelect->columns('source_code');

    $shipmentSourceData = $connection->fetchRow($shipmentSourceSelect);

    $before = $shipmentSourceData['source_code'];

    $shipment->setHasDataChanges(true);
    $shipment->getResource()->save($shipment);

    $shipmentSourceData = $connection->fetchRow(clone $shipmentSourceSelect);

    $after = $shipmentSourceData['source_code'];

    printf(
        "Shipment %d\nBefore: %s\nAfter: %s\n\n",
        $shipment->getId(),
        $before,
        $after,
    );
}

Expected result (*)

Shipment 42
Before: some_source_code_1
After: some_source_code_1

Shipment 43
Before: some_source_code_2
After: some_source_code_2

Actual result (*)

Shipment 42
Before: some_source_code_1
After: default

Shipment 43
Before: some_source_code_2
After: default

Root cause, additional information

\Magento\InventoryShipping handles saving of the sourceCode Shipment's extension attribute in \Magento\InventoryShipping\Plugin\Sales\ResourceModel\Order\Shipment\SaveSourceForShipmentPlugin::afterSave, and defaults to default in case it was not previously hydrated. The issue is that the only hydration points I found so far for such extension attribute

\Magento\InventoryShipping\Plugin\Sales\Model\Order\GetListShipmentRepositoryPlugin::afterGetList
\Magento\InventoryShipping\Plugin\Sales\ResourceModel\Order\Shipment\LoadSourceForShipmentPlugin::afterLoad
\Magento\InventoryShipping\Plugin\Sales\Shipment\AssignSourceCodeToShipmentPlugin::afterCreate

, do not cover the Shipment being loaded via collection such as in the repro snippet above, which emulates the real word scenario of async shipment email sending (which is what let me discover the bug in the first place):

// \Magento\Sales\Model\EmailSenderHandler::sendEmails:132

/** @var \Magento\Sales\Model\AbstractModel $item */
foreach ($entityCollection->getItems() as $item) {
    if ($this->emailSender->send($item, true)) {
        $this->entityResource->save(
            $item->setEmailSent(true)
        );
    }
}

m2-assistant[bot] commented 3 years ago

Hi @francescosalvi. Thank you for your report. To help us process this issue please make sure that you provided sufficient information.

Please, add a comment to assign the issue: @magento I am working on this

Join Magento Community Engineering Slack and ask your questions in #github channel.

ilnytskyi commented 3 years ago

I confirm I faced similar issue when using

Magento\Sales\Api\ShipOrderInterface

The plugin takes the sourceCode from the request instance. It would be nice to check is sourceCode already assigned as ExtensionAttribute and if not then resolve it form request or other places.

\Magento\InventoryShipping\Plugin\Sales\Shipment\AssignSourceCodeToShipmentPlugin::afterCreate

magento / inventory