Conversation
- Add Documentation - phpcs fixes - Impelement phpunit test for ValidatorChain
- Add SPI for Delete Multiple - Improve Delte Comand - Implement SPI
- Bugfix for Intigrationstest to new source_ids - Impelment new test case for delete behavior
- Refactoring: Add Helperfunction for convert - Impelment Replace Comand - Impelment Replace Comand Test TODO: - Implement clenup for Replace Comand
- phpcs fixes - phpmd fixes
| * Implementation of SourceItem delete multiple operation for specific db layer | ||
| * Delete Multiple used here for performance efficient purposes over single delete operation | ||
| */ | ||
| class DeleteMultiple |
There was a problem hiding this comment.
Looks like we need to remove
SourceItemRepositoryInterface::delete(SourceItemInterface $sourceItem);
method, as we have
SourceItemsDeleteInterface::execute(array $sourceItems);
which do the same
| * @param SourceItemInterface[] $sourceItems | ||
| * @return int[] | ||
| */ | ||
| private function getSourceIds($sourceItems): array |
There was a problem hiding this comment.
name of the method is not correct, as it returns source_id and sku list assigned to it
| { | ||
| if (empty($sourceItems)) { | ||
| throw new InputException(__('Input data is empty')); | ||
| } |
There was a problem hiding this comment.
not sure that we need to throw an exception when someone tries to delete empty array of source Items.
For example, you have a replace strategy and you want to delete existing SourceItems and substitute with new ones, but on clean Magento installation with empty DB - delete would have an empty set
| * See COPYING.txt for license details. | ||
| */ | ||
|
|
||
| namespace Magento\Inventory\Model\ResourceModel\SourceItem; |
There was a problem hiding this comment.
all the tests should be stored under the namespace
Magento\Inventory\Test\Unit|Integration\Model
not mixed with code. Such tests would not be even run by Travis
| * See COPYING.txt for license details. | ||
| */ | ||
|
|
||
| namespace Magento\InventoryApi\Api; |
There was a problem hiding this comment.
don't forget to use strict typing for newly created code
| * | ||
| * @api | ||
| */ | ||
| interface LegacyJsonHelperInterface |
There was a problem hiding this comment.
why do we need to introduce this interface? especially if it's already legacy?
If you want to make Backward Compatible change, legacy class
Magento\Framework\Json\Helper\Data should be adapted as well.
Because currently, you are breaking the contract, as AbstractEntity accept
\Magento\Framework\Json\Helper\Data $jsonHelper,
But you pass
SerializerInterface $jsonHelper, instead
| */ | ||
| public function jsonEncode($valueToEncode) | ||
| { | ||
| return $this->serializer->serialize($valueToEncode); |
There was a problem hiding this comment.
this method should be implemented proxying call to
$this->serialize($valueToEncode)
| */ | ||
| public function jsonDecode($encodedValue) | ||
| { | ||
| return $this->serializer->unserialize($encodedValue); |
There was a problem hiding this comment.
this method should be implemented proxying call to
$this->unserialize($valueToEncode)
to make all Pluginization of SerializerInterface work correctly
| /** | ||
| * Extension point for row validation | ||
| * | ||
| * @api |
There was a problem hiding this comment.
why do we have @api on class implementation?
| /** | ||
| * Extension point for source validation | ||
| * | ||
| * @api |
…2 into msi-source-export
…o msi-source-export
Msi source export
MC-41874: Out of Stock Product is back in stock after shipping an order
Description
Import module for Multi Source Inventory
Manual testing scenarios
System > Importpage in Magento backendStock Sourcesand import the CSV with append behaviorTODOs
Contribution checklist