-
Notifications
You must be signed in to change notification settings - Fork 253
Introduce Stock quantity calculation #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
46c9efa
f37b38e
5c20080
a82fe04
c6a642d
2db99e4
8f2b014
2ecd5bc
eeb4545
7ae88aa
89209cc
284738f
555110a
13a74ee
328d920
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\Inventory\Model; | ||
|
||
use Magento\Inventory\Model\ResourceModel\Reservation\ReservationQuantity; | ||
use Magento\Inventory\Model\ResourceModel\Stock\StockItemQuantity; | ||
use Magento\InventoryApi\Api\GetProductQuantityInStockInterface; | ||
|
||
/** | ||
* Return Quantity of product available to be sold by Product SKU and Stock Id | ||
* | ||
* @see \Magento\InventoryApi\Api\GetProductQuantityInStockInterface | ||
* @api | ||
*/ | ||
class GetProductQuantityInStock implements GetProductQuantityInStockInterface | ||
{ | ||
/** | ||
* @var StockItemQuantity | ||
*/ | ||
private $stockItemQty; | ||
|
||
/** | ||
* @var ReservationQuantity | ||
*/ | ||
private $reservationQty; | ||
|
||
/** | ||
* @param StockItemQuantity $stockItemQty | ||
* @param ReservationQuantity $reservationQty | ||
*/ | ||
public function __construct( | ||
StockItemQuantity $stockItemQty, | ||
ReservationQuantity $reservationQty | ||
) { | ||
$this->stockItemQty = $stockItemQty; | ||
$this->reservationQty = $reservationQty; | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
public function execute(string $sku, int $stockId): float | ||
{ | ||
$productQtyInStock = $this->stockItemQty->execute($sku, $stockId) + | ||
$this->reservationQty->execute($sku, $stockId); | ||
return $productQtyInStock; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\Inventory\Model; | ||
|
||
use Magento\Inventory\Model\ResourceModel\Reservation\ReservationQuantity; | ||
use Magento\Inventory\Model\ResourceModel\Stock\StockItemQuantity; | ||
use Magento\InventoryApi\Api\IsProductInStockInterface; | ||
|
||
/** | ||
* Return product availability by Product SKU and Stock Id. | ||
* | ||
* @see \Magento\InventoryApi\Api\GetProductQuantityInStockInterface | ||
* @see \Magento\Inventory\Model\Stock\Command\IsInStockInterface | ||
* @api | ||
*/ | ||
class IsProductInStock implements IsProductInStockInterface | ||
{ | ||
/** | ||
* @var StockItemQuantity | ||
*/ | ||
private $stockItemQty; | ||
|
||
/** | ||
* @var ReservationQuantity | ||
*/ | ||
private $reservationQty; | ||
|
||
/** | ||
* @param StockItemQuantity $stockItemQty | ||
* @param ReservationQuantity $reservationQty | ||
*/ | ||
public function __construct( | ||
StockItemQuantity $stockItemQty, | ||
ReservationQuantity $reservationQty | ||
) { | ||
$this->stockItemQty = $stockItemQty; | ||
$this->reservationQty = $reservationQty; | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
public function execute(string $sku, int $stockId): bool | ||
{ | ||
$productQtyInStock = $this->stockItemQty->execute($sku, $stockId) + | ||
$this->reservationQty->execute($sku, $stockId); | ||
return $productQtyInStock > 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we could leave as-is current implementation for now.
where we would need to add a support of Backorders. Also Dropshipping functionality could also affect this service |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\Inventory\Model; | ||
|
||
/** | ||
* Extension point for reservation cleanup (Service Provider Interface - SPI). | ||
* Provide own implementation of this interface if you would like to replace cleanup strategy. | ||
* | ||
* @api | ||
*/ | ||
interface ReservationCleanupInterface | ||
{ | ||
/** | ||
* Clean reservation table to prevent overloading. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would not expose that reservation table is used on the level of this service, because potentially it could be Redis queue Don't introduce a leaky abstraction , as we need to be agnostic to the DAL at this level of abstraction |
||
* | ||
* @return void | ||
*/ | ||
public function execute(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\Inventory\Model\ResourceModel\Reservation; | ||
|
||
use Magento\Framework\App\ResourceConnection; | ||
use Magento\Inventory\Model\ReservationCleanupInterface; | ||
use Magento\Inventory\Setup\Operation\CreateReservationTable; | ||
use Magento\InventoryApi\Api\Data\ReservationInterface; | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
class ReservationCleanup implements ReservationCleanupInterface | ||
{ | ||
/** | ||
* @var ResourceConnection | ||
*/ | ||
private $resource; | ||
|
||
/** | ||
* @param ResourceConnection $resource | ||
*/ | ||
public function __construct( | ||
ResourceConnection $resource | ||
) { | ||
$this->resource = $resource; | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
public function execute() | ||
{ | ||
$connection = $this->resource->getConnection(); | ||
$reservationTable = $this->resource->getTableName(CreateReservationTable::TABLE_NAME_RESERVATION); | ||
|
||
$select = $connection->select() | ||
->from( | ||
$reservationTable, | ||
['GROUP_CONCAT(' . ReservationInterface::RESERVATION_ID . ')'] | ||
) | ||
->group([ReservationInterface::STOCK_ID, ReservationInterface::SKU]) | ||
->having('SUM(' . ReservationInterface::QUANTITY . ') = 0'); | ||
$groupedReservationIds = implode(',', $connection->fetchCol($select)); | ||
|
||
$condition = [ReservationInterface::RESERVATION_ID . ' IN (?)' => explode(',', $groupedReservationIds)]; | ||
$connection->delete($reservationTable, $condition); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\Inventory\Model\ResourceModel\Reservation; | ||
|
||
use Magento\Framework\App\ResourceConnection; | ||
use Magento\Inventory\Setup\Operation\CreateReservationTable; | ||
use Magento\InventoryApi\Api\Data\ReservationInterface; | ||
|
||
/** | ||
* The resource model responsible for retrieving Reservation Quantity. | ||
* Used by Service Contracts that are agnostic to the Data Access Layer. | ||
*/ | ||
class ReservationQuantity | ||
{ | ||
/** | ||
* @var ResourceConnection | ||
*/ | ||
private $resource; | ||
|
||
/** | ||
* @param ResourceConnection $resource | ||
*/ | ||
public function __construct( | ||
ResourceConnection $resource | ||
) { | ||
$this->resource = $resource; | ||
} | ||
|
||
/** | ||
* Given a product sku and a stock id, return reservation quantity. | ||
* | ||
* @param string $sku | ||
* @param int $stockId | ||
* @return float | ||
*/ | ||
public function execute(string $sku, int $stockId): float | ||
{ | ||
$connection = $this->resource->getConnection(); | ||
$reservationTable = $this->resource->getTableName(CreateReservationTable::TABLE_NAME_RESERVATION); | ||
|
||
$select = $connection->select() | ||
->from($reservationTable, [ReservationInterface::QUANTITY => 'SUM(' . ReservationInterface::QUANTITY . ')']) | ||
->where(ReservationInterface::SKU . ' = ?', $sku) | ||
->where(ReservationInterface::STOCK_ID . ' = ?', $stockId) | ||
->limit(1); | ||
|
||
$reservationQty = $connection->fetchOne($select); | ||
if (false === $reservationQty) { | ||
$reservationQty = 0; | ||
} | ||
return (float)$reservationQty; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This service looks like a perfect candidate to be covered with both:
and
testing
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like to have two separate interfaces for API and SPI is redundant
Implementation of API is only proxy to SPI
We have the same situation with SourceItemsSave operation
And we decided to have only one interface (which presents API and SPI in one interface)
Interface
https://github.com/magento-engcom/msi/blob/develop/app/code/Magento/InventoryApi/Api/SourceItemsSaveInterface.php
Implementation
https://github.com/magento-engcom/msi/blob/develop/app/code/Magento/Inventory/Model/SourceItem/Command/SourceItemsSave.php
For Repository we need to extract SPI parts in separate set of interfaces due Repository represents Facade pattern (accumulate some functionality together)
But for single command, it looks like overhead (after creating we need to maintain this code)
Thus now looks like we have some mess in our interfaces:
GetProductQuantityInStockInterface
GetProductQuantityInterface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As proposed by @naydav and discussed with @maghamed we will simplify the implementation getting rid of the command layer;
thus we will delegate effective computation to resource model, injected into the services in place of the command.