[Issue] Fix stock qty threshold trigger
This issue is automatically created based on existing pull request: magento/inventory#3425: Fix stock qty threshold trigger
Description (*)
This change fix the priority in condition for the minQty. Before this change, wether or not the backorder were enabled, it had the same result (basically it was if has no backorder, or has backorder).
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 are green)
Hello @thomas-kl1,
Thanks for the report and contribution!
We have tried to reproduce the mentioned scenario with the following settings:
Create a product with:
- Stock Quantity: 10 units
- Minimum Qty (Out-of-Stock Threshold): 5 units
- Manage Stock: Enabled
but it seems this is the expected behaviour, it is written here as well, please go through with the below documentation:
https://experienceleague.adobe.com/en/docs/commerce-admin/config/catalog/inventory#:~:text=Determines%20the%20stock,to%20this%20amount.
Please let us know if we missed anything in getting the issue.
Thanks
The issue occurs when we want to display the remaining stock on PDP:
Only X left Threshold | Website | Establishes the threshold for the Only x left message. For example, if set to 3, the message appears when there are three or fewer of an item in stock. The message does not appear if the value is set to 0.
This does not handle the out-of-stock threshold setting. Meaning that even if the product is going out of stock, the remaining quantity is displayed.
Hello @thomas-kl1,
Thanks for the reply!
This time I have set the Out-of-stock threshold as -5 and the original quantity of the product is set to 10, that means if the backorders are enabled then it can be order till 15 quantity.
So I have tried to add the product in to the cart till 15 quantity and after that I am getting the error as Not enough items for sale. Please refer to the below screenshot:
The issue is about the Only X left Threshold configuration, which display on PDP the remaining quantities once the below limit is reached.
It's not about being able to add the product to the cart or not.
Thanks @thomas-kl1 for the input!
As per the further steps to reproduce the issue, we have followed the below steps:
Steps to Reproduce
- Step 1: Configure Stock Settings
- Go to Stores > Configuration > Catalog > Inventory
- Navigate to Stock Options
- Create a test product with inventory management enabled
- Step 2: Set Up Backorder Configuration
- Go to Catalog > Products
- Edit your test product
- Navigate to the Advanced Inventory section
Configure the following settings:
- Manage Stock: Yes
- Qty: Set to a low number (e.g., 5)
- Out-of-Stock Threshold: Set to 2
- Minimum Qty: Set to -1 (negative value)
Scenario 1
3. Step 3: Backorders: Disable backorders
4. Step 4: Run the indexer and clean the cache: bin/magento indexer:reindex && bin/magento c:c
5. Step 5: Check the PDP page, it is showing Only 3 left (which is Expected :heavy_check_mark:):
Scenario 2
6. Step 6: Backorders: enable backorders
7. Step 7: Check the PDP page, it is not visible Only 3 left (which is Expected :heavy_check_mark:):
Still the original issue is not reproducible, please let us know if we missed anything.
Thanks
My bad @engcom-Hotel, I do remember now why I did this fix!
Actually this class \Magento\InventoryCatalogFrontendUi\Model\IsSalableQtyAvailableForDisplaying injects \Magento\InventoryConfigurationApi\Api\Data\StockItemConfigurationInterface as a property.
It is used to retrieve the stock threshold quantity (so the limit at which the remaining quantities are displayed on frontend).
However, StockItemConfigurationInterface, represents a stock item's configuration. So basically StockItemConfigurationInterface should be passed as an argument to the method and not as an empty model in the class's property.
Having StockItemConfigurationInterface as an argument will also mean we don't rely on the strict implementation of \Magento\InventoryConfiguration\Model\StockItemConfiguration::getStockThresholdQty which fetch the value from the store configuration.
Indeed, as a developer I could easily add a stock threshold quantity by stock item instead of global value per store.
So yes, the PR does not aims to change the result, the output expectation remains the same, but it would allow developer to use their own implementation of the interface and having their behavior reflected as expected in this feature.
Hello @thomas-kl1,
Thanks for the reply!
If I understand correctly you are trying the fix the design Issue with Dependency Injection. The IsSalableQtyAvailableForDisplaying class injects StockItemConfigurationInterface as a property, the problematic part because:
- It's using a generic stock item configuration instead of the specific configuration for each product being evaluated.
- It prevents developers from easily extending or customising the behavior per product.
- It doesn't follow proper dependency injection patterns where the specific configuration should be passed as a method parameter.
Am I able to understand the issue correctly? Let me know if I misunderstood anything or missed anything.
Thanks
Yes exactly! Thank you and sorry for my really bad explanation!
Hello @thomas-kl1,
Can you please let us know why do you think that the specific configuration should be passed as a method parameter? Also replacing the use of interface: GetProductSalableQtyInterface with a concrete class is not a best practice as you did in the PR.
Thanks
I don't get this part:
Also replacing the use of interface: GetProductSalableQtyInterface with a concrete class is not a best practice as you did in the PR.
Where did I?
For your question, as I said earlier, the StockItemConfigurationInterface is the definition of the configuration of a stock item, meaning that the state of the this configuration is owned by a particular stock item. You wouldn't want to inject a model in the construct of a service, it doesn't make sens because it doesn't reflect the state of the object you are looking at
Hello @thomas-kl1,
Thank you for the clarification. Upon reviewing the PR more thoroughly, I see that there was a misunderstanding on my part regarding the interfaces.
We are confirming this issue for further processing.
Thanks