Introduce API interfaces for Assigning Stocks to Sales channels #151 by belinda-tschampel · Pull Request #150 · magento/inventory

belinda-tschampel · 2017-10-25T13:47:56Z

Description

Created additional table to store stock channel items for msi inventory mapping.

Related Issues

#151

Manual testing scenarios

Install InventorySales and InventorySalesApi Module and check if 'inventory_stock_sales_channel' table was created.

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)

larsroettig · 2017-10-25T19:18:55Z

+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */


Please use declare(strict_types=1); in all Files

larsroettig · 2017-10-25T19:21:49Z

+     */
+    use PredefinedId;
+
+    /**#@+


PHP documentation toll drop support for /**#@+ pls don't use as anymore

larsroettig · 2017-10-25T19:23:05Z

+/**
+ * Resource Collection of Source Items entity
+ *
+ * @api


don't use @api in a implemented class @api only for interfaces

larsroettig · 2017-10-25T19:25:34Z

+    }
+
+    /**
+     * {@inheritdoc}


use pls @inheritdoc without {} - {@inheritdoc} is only for DocBlocks if you must write a more specific description

larsroettig · 2017-10-25T19:25:58Z

+    public function install(SchemaSetupInterface $setup, ModuleContextInterface $context)
+    {
+        $setup->startSetup();
+


remove new line pls

…es channels to stocks

… into msi-inventory-mapping

maghamed

Two more plugins:
afterSave and afterGetList
should be provided

and code covered with Integration test

maghamed · 2017-10-27T07:26:15Z

+ * User: tschampelb
+ * Date: 26.10.17
+ * Time: 11:53
+ */


Header block is wrong

maghamed · 2017-10-27T07:26:49Z

+
+use Magento\InventorySales\Model\ResourceModel\SalesChannelsResolver;
+
+class GetSalesChannelByStock implements GetSalesChannelsByStockInterface


Name should be plural GetSalesChannelsByStock the same as interface

maghamed · 2017-10-27T07:27:12Z

+class GetSalesChannelByStock implements GetSalesChannelsByStockInterface
+{
+    /** @var SalesChannelsResolver  */
+    protected $salesChannelsResolver;


all properties in the new code should be private

maghamed · 2017-10-27T07:27:29Z

+    protected $salesChannelsResolver;
+
+
+    public function __construct(


Annotation block should be added

maghamed · 2017-10-27T07:27:57Z

+        $this->salesChannelsResolver = $salesChannelsResolver;
+    }
+
+    public function get(int $stockId) : array


PHP DocBlock annotation should be added

maghamed · 2017-10-27T08:01:14Z

+use Magento\InventorySalesApi\Api\Data\StockChannelSearchResultsInterface;
+
+/**
+ * In Magento 2 Repository considered as an implementation of Facade pattern which provides a simplified interface


This description is copy pasted from repository , please provide correct one

maghamed · 2017-10-27T08:01:58Z

+        <plugin name="stockChannelData" type="Magento\InventorySales\Plugin\Inventory\StockRepository\SalesChannelDataAfterGetPlugin" />
+    </type>
+
+</config>


add an extra line at the end

maghamed · 2017-10-27T08:02:56Z

+ * User: tschampelb
+ * Date: 26.10.17
+ * Time: 12:08
+ */


Doc Block should be fixed

maghamed · 2017-10-27T08:06:30Z

+ * The resource model responsible for retrieving StockItem Quantity.
+ * Used by Service Contracts that are agnostic to the Data Access Layer.
+ */
+class SalesChannelsResolver


Better change the name to SalesChannelProvide because this class provides SalesChannels by given stock id

maghamed · 2017-10-27T08:08:02Z

+            $salesChannel->setType($channelItem['type']);
+            $salesChannel->setCode($channelItem['code']);
+            $retArray[] = $salesChannel;
+        }


this foreach logic could be moved to the model upper level

maghamed · 2017-10-27T08:34:30Z

+     * @param string $code
+     * @return void
+     */
+    public function setCode(string $code);


get/setExtension attributes methods should be introduced to this interface

naydav · 2017-10-27T08:19:21Z

+interface GetSalesChannelsByStockInterface
+{
+    /**
+     * Get Sales Channel data by given stockId.


Provide more clear description
We return not some "data" but SalesChannels object

@return SalesChannels[]

naydav · 2017-10-27T08:20:00Z

+
+namespace Magento\InventorySales\Model;
+
+/**


Create proper dockblock, this is copy paste from repository

naydav · 2017-10-27T08:48:54Z

+    /**
+     * Provides possibility of saving entity with predefined/pre-generated id
+     */
+    use PredefinedId;


Looks like for this object we do not need predefined id functionality
And generally looks like we do not need ResourceModel for this entity

naydav · 2017-10-27T08:52:55Z

+    }
+
+
+    public function addFilterByStockId(int $stockId)


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

naydav · 2017-10-27T09:24:54Z

+        /** @var StockExtension $extensionAttributes */
+        $extensionAttributes = $result->getExtensionAttributes();
+        $salesChannelSearchResults = $this->getSalesChannelByStock->get((int)$result->getStockId());
+        $extensionAttributes->setData('sales_channels', $salesChannelSearchResults);


Do not use setData method, you could use method which was described extension_attributes.xml

naydav · 2017-10-27T09:25:31Z

+    <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" />


Pls use underscore for plugin name instead of camelcase

… 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

belinda-tschampel added 2 commits October 25, 2017 14:51

MSI: created additional table to store stock channel items

f7ddb48

MSI: added foreign key to inventory sales stock channel table

a259fa7

larsroettig suggested changes Oct 25, 2017

View reviewed changes

WIP: implemented interfaces and attribute extension logic to link sal…

ed01e65

…es channels to stocks

belinda-tschampel changed the title ~~Store stock channel table for msi inventory mapping~~ Introduce API interfaces for Assigning Stocks to Sales channels #151 Oct 26, 2017

hamplr and others added 3 commits October 27, 2017 09:25

WIP: improve namespace of factory class

8952b43

WIP: improved code following reviewer requests

fd196d5

Merge branch 'msi-inventory-mapping' of github.com:magento-engcom/msi…

c5a3f3b

… into msi-inventory-mapping

maghamed suggested changes Oct 27, 2017

View reviewed changes

WIP: added missing typecast to int for stock id

3ca6de9

maghamed suggested changes Oct 27, 2017

View reviewed changes

maghamed mentioned this pull request Oct 27, 2017

Stock Removal. Throw an exception and allow to delete just unassigned stocks #157

Closed

naydav reviewed Oct 27, 2017

View reviewed changes

maghamed added Techdivision Contribution Week Award: advanced Award: special achievement labels Oct 27, 2017

belinda-tschampel and others added 9 commits October 27, 2017 13:14

WIP: followed review comments

92e5937

WIP: improve code like review comments and add plugin for getList method

9aa2d5e

Merge branch 'msi-inventory-mapping' of github.com:magento-engcom/msi…

03de324

… into msi-inventory-mapping

WIP: another implementation of review comments

932699a

remove obsolete collection and resource model

14cf3b4

Merge branch 'msi-inventory-mapping' of github.com:magento-engcom/msi…

db4403a

… into msi-inventory-mapping

Merge branch 'msi-inventory-mapping' of github.com:magento-engcom/msi…

b7d0b11

… into msi-inventory-mapping

bugfix

4c6f6f5

bugfix

57873d6

maghamed mentioned this pull request Oct 28, 2017

Assign Website at the time of Creation to the Default Stock #163

Closed

Valeriy Nayda and others added 4 commits October 28, 2017 11:45

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

1985acc

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

ede56c5

removed old and unused id definition from interface

3e030af

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

deb9889

belinda-tschampel and others added 21 commits October 28, 2017 17:40

MSI: Introduce API interfaces for Assigning Stocks to Sales channels.…

33df249

… Fixed wrong foreign key in sales channels link table.

MSI: Introduce API interfaces for Assigning Stocks to Sales channels.…

08e9c97

… Fixed bug in plugin.

MSI: Introduce API interfaces for Assigning Stocks to Sales channels.…

3c7e465

… Created first test case for sales channels links.

MSI: Introduce API interfaces for Assigning Stocks to Sales channels.…

68bbd24

… Completed web api tests.

Merge branch 'develop' into msi-inventory-mapping

a1e6f1c

MSI: Introduce API interfaces for Assigning Stocks to Sales channels.…

0cfb6a2

… Fixed issues found by travis ci.

Merge branch 'msi-inventory-mapping' of github.com:magento-engcom/msi…

64844dd

… into msi-inventory-mapping

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

643a0c0

- declare Magento_InventorySales sequences

Merge branch 'msi-inventory-mapping' of github.com:magento-engcom/msi…

9717141

… into msi-inventory-mapping

MSI: Introduce API interfaces for Assigning Stocks to Sales channels.…

2387b38

… Removed wrong fix for issues found by travis ci.

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

eca0775

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

dd34411

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

e7d7f56

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

94fc80a

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

2b2d1aa

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

5ab2060

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

d414201

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

58ad448

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

c17ecbb

MSI: Introduce API interfaces for Assigning Stocks to Sales channels

2ec3519

MSI: Assign Website at the time of Creation to the Default Stock

77d66a0

-- slight refactoring

naydav merged commit fb81eee into develop Nov 2, 2017

maghamed deleted the msi-inventory-mapping branch December 11, 2018 18:13

magento-devops-reposync-svc pushed a commit that referenced this pull request Oct 19, 2021

Merge pull request #150 from magento-l3/L3_PR_21-10-10

d8be487

L3_PR_21-10-10


		use Magento\InventorySales\Model\ResourceModel\SalesChannelsResolver;

		class GetSalesChannelByStock implements GetSalesChannelsByStockInterface

		protected $salesChannelsResolver;


		public function __construct(

Conversation

belinda-tschampel commented Oct 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Manual testing scenarios

Contribution checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maghamed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

belinda-tschampel commented Oct 25, 2017 •

edited

Loading