Conversation
e069578 to
fa3bf14
Compare
valentinych
pushed a commit
that referenced
this pull request
Dec 14, 2017
…rce_items_stock_items_synchronization
…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
reviewed
Dec 20, 2017
| @@ -0,0 +1,103 @@ | |||
| <?php | |||
naydav
suggested changes
Dec 20, 2017
| $sourceItemsDelete->execute($sourceItems); | ||
| try { | ||
| $sourceItemsDelete->execute($sourceItems); | ||
| } catch (NoSuchEntityException $e) { |
There was a problem hiding this comment.
Why do we need wrap this code in try catch? Looks like sourceItemsDelete does not throw NoSuchEntityException
| [ | ||
| StockItemInterface::STOCK_ID . ' = ?' => $this->defaultStockProvider->getId(), | ||
| StockItemInterface::PRODUCT_ID . ' = ?' => $productId, | ||
| 'website_id = ?' => 0 |
There was a problem hiding this comment.
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)
| return; | ||
| } | ||
|
|
||
| $productIds = $this->productIdLocator->retrieveProductIdsBySkus([$sourceItem->getSku()]); |
There was a problem hiding this comment.
Pls merge develop branch
We have provided global refactoring of product ids by sku resolving
Pls use \Magento\InventoryCatalog\Model\GetProductIdsBySkusInterface
| * 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 |
There was a problem hiding this comment.
Class name and description are not equal to behavior of this class
| use Magento\CatalogInventory\Api\StockItemCriteriaInterfaceFactory; | ||
| use Magento\CatalogInventory\Api\Data\StockItemCollectionInterface; | ||
|
|
||
| class DeleteLegacyCatalogInventoryPluginTest extends TestCase |
There was a problem hiding this comment.
Change name
We does not test plugin but we test some testcase
| * @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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Need to do this in all cases
It is related to the other notices
| $stockItemsBeforeDelete = $this->oldStockItemRepository->getList($criteria)->getItems(); | ||
| $this->assertCount(1, $stockItemsBeforeDelete); | ||
|
|
||
| $searchCriteria = $this->searchCriteriaBuilder |
There was a problem hiding this comment.
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)
| $this->sourceItemsSave->execute([$sourceItem]); | ||
|
|
||
| /** @var StockItemCollectionInterface $collectionBeforeChange */ | ||
| $stockItemsAfterUpdate = $this->oldStockItemRepository->getList($criteria)->getItems(); |
| */ | ||
| public function testThatReservationPlacedDefersUpdatingLegacyStockWhenCanSubtractOff() | ||
| { | ||
| $this->productRepository = Bootstrap::getObjectManager()->get(ProductRepositoryInterface::class); |
There was a problem hiding this comment.
Extra line
We have productRepository initialization in setUp
…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
…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 -- 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
added 3 commits
December 21, 2017 17:23
This was referenced Dec 21, 2017
…faultSourceId #269 -- build fixes, new task introducing
This was referenced Dec 21, 2017
magento-devops-reposync-svc
pushed a commit
that referenced
this pull request
Oct 20, 2022
…9_2022 Arrows Team - Bugfix delivery
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Synchronization between sourceItem and stockItem deletion for defaultSourceId
Fixed Issues (if relevant)
Contribution checklist