Introduce API interfaces for Assigning Stocks to Sales channels #151 #150
Conversation
| /** | ||
| * Copyright © Magento, Inc. All rights reserved. | ||
| * See COPYING.txt for license details. | ||
| */ |
There was a problem hiding this comment.
Please use declare(strict_types=1); in all Files
| */ | ||
| use PredefinedId; | ||
|
|
||
| /**#@+ |
There was a problem hiding this comment.
PHP documentation toll drop support for /**#@+ pls don't use as anymore
| /** | ||
| * Resource Collection of Source Items entity | ||
| * | ||
| * @api |
There was a problem hiding this comment.
don't use @api in a implemented class @api only for interfaces
| } | ||
|
|
||
| /** | ||
| * {@inheritdoc} |
There was a problem hiding this comment.
use pls @inheritdoc without {} - {@inheritdoc} is only for DocBlocks if you must write a more specific description
| public function install(SchemaSetupInterface $setup, ModuleContextInterface $context) | ||
| { | ||
| $setup->startSetup(); | ||
|
|
…es channels to stocks
belinda-tschampel
changed the title
| * User: tschampelb | ||
| * Date: 26.10.17 | ||
| * Time: 11:53 | ||
| */ |
|
|
||
| use Magento\InventorySales\Model\ResourceModel\SalesChannelsResolver; | ||
|
|
||
| class GetSalesChannelByStock implements GetSalesChannelsByStockInterface |
There was a problem hiding this comment.
Name should be plural GetSalesChannelsByStock the same as interface
| class GetSalesChannelByStock implements GetSalesChannelsByStockInterface | ||
| { | ||
| /** @var SalesChannelsResolver */ | ||
| protected $salesChannelsResolver; |
There was a problem hiding this comment.
all properties in the new code should be private
| protected $salesChannelsResolver; | ||
|
|
||
|
|
||
| public function __construct( |
There was a problem hiding this comment.
Annotation block should be added
| $this->salesChannelsResolver = $salesChannelsResolver; | ||
| } | ||
|
|
||
| public function get(int $stockId) : array |
There was a problem hiding this comment.
PHP DocBlock annotation should be added
| use Magento\InventorySalesApi\Api\Data\StockChannelSearchResultsInterface; | ||
|
|
||
| /** | ||
| * In Magento 2 Repository considered as an implementation of Facade pattern which provides a simplified interface |
There was a problem hiding this comment.
This description is copy pasted from repository , please provide correct one
| <plugin name="stockChannelData" type="Magento\InventorySales\Plugin\Inventory\StockRepository\SalesChannelDataAfterGetPlugin" /> | ||
| </type> | ||
|
|
||
| </config> No newline at end of file |
There was a problem hiding this comment.
add an extra line at the end
| * User: tschampelb | ||
| * Date: 26.10.17 | ||
| * Time: 12:08 | ||
| */ |
There was a problem hiding this comment.
Doc Block should be fixed
| * The resource model responsible for retrieving StockItem Quantity. | ||
| * Used by Service Contracts that are agnostic to the Data Access Layer. | ||
| */ | ||
| class SalesChannelsResolver |
There was a problem hiding this comment.
Better change the name to SalesChannelProvide because this class provides SalesChannels by given stock id
| $salesChannel->setType($channelItem['type']); | ||
| $salesChannel->setCode($channelItem['code']); | ||
| $retArray[] = $salesChannel; | ||
| } |
There was a problem hiding this comment.
this foreach logic could be moved to the model upper level
| * @param string $code | ||
| * @return void | ||
| */ | ||
| public function setCode(string $code); |
There was a problem hiding this comment.
get/setExtension attributes methods should be introduced to this interface
| interface GetSalesChannelsByStockInterface | ||
| { | ||
| /** | ||
| * Get Sales Channel data by given stockId. |
There was a problem hiding this comment.
Provide more clear description
We return not some "data" but SalesChannels object
@return SalesChannels[]
|
|
||
| namespace Magento\InventorySales\Model; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Create proper dockblock, this is copy paste from repository
| /** | ||
| * Provides possibility of saving entity with predefined/pre-generated id | ||
| */ | ||
| use PredefinedId; |
There was a problem hiding this comment.
Looks like for this object we do not need predefined id functionality
And generally looks like we do not need ResourceModel for this entity
| } | ||
|
|
||
|
|
||
| public function addFilterByStockId(int $stockId) |
There was a problem hiding this comment.
Why do we need these "filtering" methods in this collection?
Generally, looks like we do need this class because we already have logic of resolving in StockResolver
| /** @var StockExtension $extensionAttributes */ | ||
| $extensionAttributes = $result->getExtensionAttributes(); | ||
| $salesChannelSearchResults = $this->getSalesChannelByStock->get((int)$result->getStockId()); | ||
| $extensionAttributes->setData('sales_channels', $salesChannelSearchResults); |
There was a problem hiding this comment.
Do not use setData method, you could use method which was described extension_attributes.xml
| <preference for="Magento\InventorySales\Model\GetSalesChannelsByStockInterface" type="Magento\InventorySales\Model\GetSalesChannelByStock"/> | ||
|
|
||
| <type name="Magento\InventoryApi\Api\StockRepositoryInterface"> | ||
| <plugin name="stockChannelData" type="Magento\InventorySales\Plugin\Inventory\StockRepository\SalesChannelDataAfterGetPlugin" /> |
There was a problem hiding this comment.
Pls use underscore for plugin name instead of camelcase
… into msi-inventory-mapping
… into msi-inventory-mapping
… into msi-inventory-mapping
… Fixed wrong foreign key in sales channels link table.
… Fixed bug in plugin.
… Created first test case for sales channels links.
… Completed web api tests.
… Fixed issues found by travis ci.
… into msi-inventory-mapping
- declare Magento_InventorySales sequences
… into msi-inventory-mapping
… Removed wrong fix for issues found by travis ci.
-- slight refactoring
L3_PR_21-10-10
Description
Created additional table to store stock channel items for msi inventory mapping.
Related Issues
#151
Manual testing scenarios
Contribution checklist