Add IsProductSalableForRequestedQty service

[aleron75]

This work-in-progress PR is related to issue #520 and is intended to discuss about adding a new service for InventorySalesAPI to be used to check whether a customer can buy or put to Shopping Cart a certain amount of product.

The first two commits do the following:

• Rename ManageConfigConditionTest class to ManageStockConditionTest
• Add IsProductSalableForRequestedQtyConditionChain
• Add definition and implementation of conditions for the IsProductSalableForRequestedQtyConditionChain
• Add conditions tests (except for IsCorrectQtyCondition)

Let's also discuss about naming.

[maghamed]

This condition looks incorrect as here we check
that product is salable and its Qty is more than 0

        $qtyWithReservation = $stockItemData['quantity'] + $this->getReservationsQuantity->execute($sku, $stockId);
        return (bool)$stockItemData['is_salable'] && $qtyWithReservation > 0.0001;

I believe for IsProductSalableForRequestedQtyInterface we would not have this condition at all, as you make this check in IsCorrectQty condition

        $qtyWithReservation = $stockItemData['quantity'] + $this->getReservationsQuantity->execute($sku, $stockId);
        if ($requestedQty > $qtyWithReservation) {
            return false;
        }

[maghamed]

you should have dependency on Interface, \Magento\CatalogInventory\Api\StockConfigurationInterface
not concrete implementation

[maghamed]

this check looks excessive.
As StockItemConfiguration which proxy call to legacy StockItemInterface has a method
public function getQtyIncrements(): float;

that's why initial check currently looks like

    {
        $result = new \Magento\Framework\DataObject();
        if ($stockItem->getSuppressCheckQtyIncrements()) {
            return $result;
        }

        $qtyIncrements = $stockItem->getQtyIncrements() * 1;

        if ($qtyIncrements && $this->mathDivision->getExactDivision($qty, $qtyIncrements) != 0) {
            $result->setHasError(true)
                ->setQuoteMessage(__('Please correct the quantity for some products.'))
                ->setErrorCode('qty_increments')
                ->setQuoteMessageIndex('qty');
            if ($stockItem->getIsChildItem()) {
                $result->setMessage(
                    __(
                        'You can buy %1 only in quantities of %2 at a time.',
                        $stockItem->getProductName(),
                        $qtyIncrements
                    )
                );
            } else {
                $result->setMessage(__('You can buy this product only in quantities of %1 at a time.', $qtyIncrements));
            }
        }
        return $result;

So, taking $stockItemConfiguration->getQtyIncrements() should be enough

This function under the hood looks like

    public function getQtyIncrements()
    {
        if ($this->qtyIncrements === null) {
            if ($this->getEnableQtyIncrements()) {
                if ($this->getUseConfigQtyIncrements()) {
                    $this->qtyIncrements = $this->stockConfiguration->getQtyIncrements($this->getStoreId());
                } else {
                    $this->qtyIncrements = (int) $this->getData(static::QTY_INCREMENTS);
                }
            }
            if ($this->qtyIncrements <= 0) {
                $this->qtyIncrements = false;
            }
        }
        return $this->qtyIncrements;
    }

[aleron75]

I would change it to the following (note: I added the explicit strict comparison with 0)

if ($this->mathDivision->getExactDivision($requestedQty, $this->configuration->getQtyIncrements()) !== 0
            || $this->mathDivision->getExactDivision(
                $requestedQty,
                $stockItemConfiguration->getQtyIncrements()
            ) !== 0) {
            return false;
        }

[maghamed]

why do we always make a check for Qty = 1 ?

[aleron75]

the qty is never used because the tested methods are only proxy methods;
for that reason, let's discuss removing those tests at all

[aleron75]

as discussed in our call, we decided to leave the tests

[maghamed]

You did not cover with integration test the most complex logic - IsCorrectQtyCondition

[maghamed]

For Quote, we need to preserve behavior from this function
\Magento\CatalogInventory\Model\StockStateProvider::checkQuoteItemQty

which works with Min Sale Qty

        if ($stockItem->getMinSaleQty() && $qty < $stockItem->getMinSaleQty()) {
            $result->setHasError(true)
                ->setMessage(__('The fewest you may purchase is %1.', $stockItem->getMinSaleQty() * 1))
                ->setErrorCode('qty_min')
                ->setQuoteMessage(__('Please correct the quantity for some products.'))
                ->setQuoteMessageIndex('qty');
            return $result;
        }

        if ($stockItem->getMaxSaleQty() && $qty > $stockItem->getMaxSaleQty()) {
            $result->setHasError(true)
                ->setMessage(__('The most you may purchase is %1.', $stockItem->getMaxSaleQty() * 1))
                ->setErrorCode('qty_max')
                ->setQuoteMessage(__('Please correct the quantity for some products.'))
                ->setQuoteMessageIndex('qty');
            return $result;
        }

That actually means that we need to have 2 chains:

• one for Quote
• another for Checkout

this could be done with VirtualType DI configuration

[maghamed]

Looks like we need to return result object, not just typical bool
The same as for Validators.

Because we somehow need to return validation message, what exactly went wrong and what should be fixed, like in existing code:

        if ($stockItem->getMaxSaleQty() && $qty > $stockItem->getMaxSaleQty()) {
            $result->setHasError(true)
                ->setMessage(__('The most you may purchase is %1.', $stockItem->getMaxSaleQty() * 1))
                ->setErrorCode('qty_max')
                ->setQuoteMessage(__('Please correct the quantity for some products.'))
                ->setQuoteMessageIndex('qty');
            return $result;
        }

I propose to return next structure

interface IsProductSalableResultInterface
{

    /**
     * @return bool
     */
    public function isSalable(): bool;

    /**
     * @return ProductSalabilityError[]
     */
    public function getErrors(): array;
}

interface ProductSalabilityError
{

    /**
     * @return string
     */
    public function getCode(): string;

    /**
     * @return string
     */
    public function getMessage(): string;
}

ProductSalabilityError entities are created with ProductSalabilityErrorFactory

[maghamed]

Our Get Service for Stock Item configuration looks like

interface GetStockItemConfigurationInterface
{
    /**
     * Return null if configuration for sku per stock is not exist
     *
     * @param string $sku
     * @param int $stockId
     * @return StockItemConfigurationInterface|null
     * @throws \Magento\Framework\Exception\LocalizedException
     */
    public function execute(string $sku, int $stockId);
}

So, we define the scope with $sku and $stockId

existing Save service doesn't have $stockId, so all the config data would be saved for Default stock.

           ->addColumn(
                'stock_id',
                \Magento\Framework\DB\Ddl\Table::TYPE_SMALLINT,
                null,
                ['unsigned' => true, 'nullable' => false, 'default' => '0'],
                'Stock Id'
            )

in cataloginventory_stock_item

Also, we need to remove unneeded constants

interface StockItemConfigurationInterface
{
    const SKU = 'sku';
    const STOCK_ID = 'stock_id';

[maghamed]

Hey @aleron75 ,

I was looking at your PR and looks like you have an error in this check

        // Out-of-Stock Threshold
        // TODO verify whether we should use < or <=
        $globalMinQty = $this->configuration->getMinQty();
        if (($stockItemConfiguration->isUseConfigMinQty() == 1 && $requestedQty < $globalMinQty)
            || ($stockItemConfiguration->isUseConfigMinQty() == 0
                && $requestedQty < $stockItemConfiguration->getMinQty()
            )) {
            $errors = [
                $this->productSalabilityErrorFactory->create([
                    'code' => 'is_correct_qty-out_of_stock_threshold',
                    'message' => __('The requested qty is not available')
                ])
            ];
            return $this->isProductSalableResultFactory->create(['errors' => $errors]);
        }

you make it the same as getMinSaleQty, but these configuration options are different

getMinSaleQty - represents minimal amount in the shopping cart, while MinQty is responsible for minimal Qty available in Stock

for example, in Legacy Magento we do this check :
if ($stockItem->getQty() - $stockItem->getMinQty() - $qty < 0) { .. }
something similar should be here as well

[aleron75]

yes, you are right, I misunderstood the logic, will fix it