Synchronization between sourceItem and stockItem deletion for defaultSourceId #269

sinisa86 · 2017-12-08T14:42:48Z

Synchronization between sourceItem and stockItem deletion for defaultSourceId

Fixed Issues (if relevant)

Provide possibility to enable/disable synchronization logic of stock deduction in legacy tables at checkout time #216: Provide possibility to enable/disable synchronization logic of stock deduction in legacy tables at checkout time
Add Legacy Stock Synchronization at the time of Deleting StockItem (Legacy) #218: Add Legacy Stock Synchronization at the time of Deleting StockItem (Legacy)
Add Update to the Legacy Inventory (StockItem) when Default Source item deleted #219: Add Update to the Legacy Inventory (StockItem) when Default Source item deleted

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

…ceItems update/delete

…rce_items_stock_items_synchronization

…Status instead of deleting them

…ps://github.com/magento-engcom/msi into delete_source_items_stock_items_synchronization

…rce_items_stock_items_synchronization # Conflicts: # app/code/Magento/InventoryApi/Test/Api/StockRepository/DeleteTest.php # app/code/Magento/InventoryApi/Test/_files/source_items.php

naydav · 2017-12-20T18:10:29Z

app/code/Magento/InventoryApi/Test/Api/StockRepository/DeleteTest.php

@@ -0,0 +1,103 @@
+<?php


Pls remove this file

naydav · 2017-12-20T18:12:29Z

app/code/Magento/InventoryApi/Test/_files/source_items_rollback.php

-    $sourceItemsDelete->execute($sourceItems);
+    try {
+        $sourceItemsDelete->execute($sourceItems);
+    } catch (NoSuchEntityException $e) {


Why do we need wrap this code in try catch? Looks like sourceItemsDelete does not throw NoSuchEntityException

naydav · 2017-12-20T18:14:44Z

app/code/Magento/InventoryCatalog/Model/UpdateLegacyStockItemByDefaultSourceItem.php

+                [
+                    StockItemInterface::STOCK_ID . ' = ?' => $this->defaultStockProvider->getId(),
+                    StockItemInterface::PRODUCT_ID . ' = ?' => $productId,
+                    'website_id = ?' => 0


Need to check how do we write website value in this table
Looks like we need to use some service for resolving website id (instead of hardcoded value)

naydav · 2017-12-20T18:16:25Z

app/code/Magento/InventoryCatalog/Model/UpdateLegacyStockStatusByDefaultSourceItem.php

+            return;
+        }
+
+        $productIds = $this->productIdLocator->retrieveProductIdsBySkus([$sourceItem->getSku()]);


Pls merge develop branch
We have provided global refactoring of product ids by sku resolving

Pls use \Magento\InventoryCatalog\Model\GetProductIdsBySkusInterface

naydav · 2017-12-20T18:18:26Z

app/code/Magento/InventoryCatalog/Plugin/InventoryApi/DeleteLegacyCatalogInventoryPlugin.php

+ * Plugin help to delete related entries from the legacy catalog inventory tables cataloginventory_stock_status and
+ * cataloginventory_stock_item if deleted source item is default source item.
+ */
+class DeleteLegacyCatalogInventoryPlugin


Class name and description are not equal to behavior of this class

naydav · 2017-12-20T18:21:07Z

app/code/Magento/InventoryCatalog/Test/Integration/DeleteLegacyCatalogInventoryPluginTest.php

+use Magento\CatalogInventory\Api\StockItemCriteriaInterfaceFactory;
+use Magento\CatalogInventory\Api\Data\StockItemCollectionInterface;
+
+class DeleteLegacyCatalogInventoryPluginTest extends TestCase


Change name
We does not test plugin but we test some testcase

naydav · 2017-12-20T18:22:15Z

app/code/Magento/InventoryCatalog/Test/Integration/DeleteLegacyCatalogInventoryPluginTest.php

+     * @magentoDataFixture ../../../../app/code/Magento/InventoryApi/Test/_files/stocks.php
+     * @magentoDataFixture ../../../../app/code/Magento/InventoryApi/Test/_files/source_items.php
+     * @magentoDataFixture ../../../../app/code/Magento/InventoryApi/Test/_files/stock_source_link.php
+     * @magentoDbIsolation enabled


magentoDbIsolation is enabled by default

We have problem with this option only if we do not apply any fixtures
But in our case it is extra configuration

Need to do this in all cases
It is related to the other notices

naydav · 2017-12-20T18:23:40Z

app/code/Magento/InventoryCatalog/Test/Integration/DeleteLegacyCatalogInventoryPluginTest.php

+        $stockItemsBeforeDelete = $this->oldStockItemRepository->getList($criteria)->getItems();
+        $this->assertCount(1, $stockItemsBeforeDelete);
+
+        $searchCriteria = $this->searchCriteriaBuilder


Looks like we have a few times with code
Maybe it will be better to create sugar service (not interface but implementation) and put under hood this logic (I mean creating and configuring of Search Criteria)

naydav · 2017-12-20T18:24:23Z

...de/Magento/InventoryCatalog/Test/Integration/UpdateCatalogInventoryOnSourceItemsSaveTest.php

+        $this->sourceItemsSave->execute([$sourceItem]);
+
+        /** @var StockItemCollectionInterface $collectionBeforeChange */
+        $stockItemsAfterUpdate = $this->oldStockItemRepository->getList($criteria)->getItems();


Pls use legacy instead of old

naydav · 2017-12-20T18:24:57Z

...ventoryCatalog/Test/Integration/UpdateLegacyCatalogInventoryDuringReservationPlacingTest.php

+     */
+    public function testThatReservationPlacedDefersUpdatingLegacyStockWhenCanSubtractOff()
+    {
+        $this->productRepository = Bootstrap::getObjectManager()->get(ProductRepositoryInterface::class);


Extra line
We have productRepository initialization in setUp

…SourceId

…faultSourceId #269 -- Set to zero legacy catalocinventory data if deleted source item which is related to default source

…faultSourceId #269 -- set data to legacy catalog inventory at source items related to default source save

…s_stock_items_synchronization

…faultSourceId #269 -- Apply inventory data changes (qty, stock status) to legacy CatalogInventory (cataloginventory_stock_status and cataloginventory_stock_item tables) at the time when Reservation(-s) have been appended using MSI APIs, and these reservation(-s) correspond to Default Stock

…faultSourceId #269 -- Remove legacy code

…faultSourceId #269 -- slight refactoring

…faultSourceId #269 -- build fixes, new task introducing

…9_2022 Arrows Team - Bugfix delivery

sinisa86 added 2 commits December 12, 2017 15:04

Implemented plugins for updating legacy CatalogInventory data on sour…

91ac8ac

…ceItems update/delete

Modified synchronization plugins

fa3bf14

sinisa86 force-pushed the delete_source_items_stock_items_synchronization branch from e069578 to fa3bf14 Compare December 12, 2017 14:19

Various fixes and API renames

a558f3d

valentinych pushed a commit that referenced this pull request Dec 14, 2017

re-commit PR from pull-request #269

d28054c

bartekszymanski and others added 8 commits December 15, 2017 10:06

Merge remote-tracking branch 'remotes/origin/develop' into delete_sou…

11db553

…rce_items_stock_items_synchronization

Fix table names

2e3f0a5

Fix table names

8e8535a

Merge remote-tracking branch 'remotes/origin/develop' into delete_sou…

b21c33e

…rce_items_stock_items_synchronization

Modified DeleteLegacyCatalogInventoryPlugin to update stockItem/stock…

794c7d0

…Status instead of deleting them

fix products fixture

453e724

Merge branch 'delete_source_items_stock_items_synchronization' of htt…

e0d8a5e

…ps://github.com/magento-engcom/msi into delete_source_items_stock_items_synchronization

Fixed Static test issue in the DeleteTest

71301d6

maghamed added Award: complex Award: special achievement labels Dec 18, 2017

bartekszymanski mentioned this pull request Dec 19, 2017

Introduce Plugin for SourceItem update to sync up with Legacy Stock storage #217

Closed

Merge remote-tracking branch 'remotes/origin/develop' into delete_sou…

3e93be6

…rce_items_stock_items_synchronization # Conflicts: # app/code/Magento/InventoryApi/Test/Api/StockRepository/DeleteTest.php # app/code/Magento/InventoryApi/Test/_files/source_items.php

naydav added the WIP label Dec 20, 2017

naydav reviewed Dec 20, 2017

View reviewed changes

app/code/Magento/InventoryApi/Test/Api/StockRepository/DeleteTest.php Outdated

@@ -0,0 +1,103 @@

<?php

Copy link

naydav Dec 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls remove this file

naydav suggested changes Dec 20, 2017

View reviewed changes

Skip functional test

6c9a6a1

bartekszymanski mentioned this pull request Dec 20, 2017

Investigate and fix problem with OnePageCheckoutOfflinePaymentMethodsTest #333

Closed

bartekszymanski and others added 8 commits December 20, 2017 22:39

Synchronization between sourceItem and stockItem deletion for default…

49ffa67

…SourceId

Synchronization between sourceItem and stockItem deletion for default…

be62721

…SourceId

MSI: Synchronization between sourceItem and stockItem deletion for de…

a9eb122

…faultSourceId #269 -- Set to zero legacy catalocinventory data if deleted source item which is related to default source

MSI: Synchronization between sourceItem and stockItem deletion for de…

4a2f9c8

…faultSourceId #269 -- set data to legacy catalog inventory at source items related to default source save

MSI: Synchronization between sourceItem and stockItem deletion for de…

71d771a

…faultSourceId #269 -- set data to legacy catalog inventory at source items related to default source save

Merge remote-tracking branch 'origin/develop' into delete_source_item…

a8d7081

…s_stock_items_synchronization

Valeriy Nayda added 3 commits December 21, 2017 17:23

MSI: Synchronization between sourceItem and stockItem deletion for de…

a0a3380

…faultSourceId #269 -- Remove legacy code

MSI: Synchronization between sourceItem and stockItem deletion for de…

71989ad

…faultSourceId #269 -- slight refactoring

MSI: Synchronization between sourceItem and stockItem deletion for de…

066b080

…faultSourceId #269 -- build fixes, new task introducing

This was referenced Dec 21, 2017

re-commit PR from pull-request #269 #308

Closed

Product's Stock quantity not changed after Stock Source Import #267

Closed

MSI: Synchronization between sourceItem and stockItem deletion for de…

ca6c78a

…faultSourceId #269 -- build fixes, new task introducing

naydav merged commit dce1e25 into develop Dec 21, 2017

naydav mentioned this pull request Feb 26, 2018

Fix Magento\InventoryCatalog\Plugin\InventoryApi\ApplyDataToLegacyCatalogInventoryAtReservationPlacingPlugin #368

Closed

maghamed deleted the delete_source_items_stock_items_synchronization branch December 11, 2018 18:16

magento-devops-reposync-svc pushed a commit that referenced this pull request Oct 20, 2022

Merge pull request #269 from magento-gl/gl_pr_arrows_cloudnative_sep2…

3f86e11

…9_2022 Arrows Team - Bugfix delivery

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Synchronization between sourceItem and stockItem deletion for defaultSourceId #269

Synchronization between sourceItem and stockItem deletion for defaultSourceId #269

sinisa86 commented Dec 8, 2017

naydav Dec 20, 2017

naydav Dec 20, 2017

naydav Dec 20, 2017

naydav Dec 20, 2017

naydav Dec 20, 2017

naydav Dec 20, 2017

naydav Dec 20, 2017

naydav Dec 20, 2017

naydav Dec 20, 2017

naydav Dec 20, 2017

naydav Dec 20, 2017

Synchronization between sourceItem and stockItem deletion for defaultSourceId #269

Synchronization between sourceItem and stockItem deletion for defaultSourceId #269

Conversation

sinisa86 commented Dec 8, 2017

Fixed Issues (if relevant)

Contribution checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment