Skip to content
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

do the stock check on default level because the stock on website leve… #11485

Merged
merged 2 commits into from
Dec 1, 2017

Conversation

joost-florijn-kega
Copy link
Contributor

…l isn't updated and should be ignored

Product's are linked to categories without notice of any website. The visual merchandiser shows to lowest price of in stock and active simples that are associated with the configurable. The stock check ignores the website scope so if a simple is in stock on the website level but out of stock on default level it should been ignored to determine the lowest price. This commit fixes that issue.

Fixed Issues (if relevant)

  1. Visual Merchandiser show prices of out of stock simple products for the associated configurable product. #11484: Visual Merchandiser show prices of out of stock simple products for the associated configurable product

Manual testing scenarios

  1. Have a configurable product that is associated with a simple product which is out of stock on default website level (id: 0) and is in stock on website level.
  2. Go to the visual merchandiser of the category where the configurable product is linked.
  3. Look up the configurable product in the visual merchandiser Tile overview.

Before the fix:

  • The price of the out of stock simple product was shown.

After the fix:

  • The price '0,00' is shown.

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)

…l isn't updated and should be ignored

Product's are linked to categories without notice of any website. The visual merchandiser shows to lowest price of in stock and active simples that are associated with the configurable. The stock check ignores the website scope so if a simple is in stock on the website level but out of stock on default level it should been ignored to determine the lowest price. This commit fixes that issue.
@orlangur
Copy link
Contributor

@naydav could you please take a look on this.

)->where('stock.stock_status = ?', Stock::STOCK_IN_STOCK);
)
->where('stock.stock_status = ?', Stock::STOCK_IN_STOCK)
->where('stock.website_id = ?', 0);
Copy link
Contributor

@maghamed maghamed Nov 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a lot of potential problems if we would accept proposed changes, but currently, Magento 2 is a Single Stock system and if your installation is not customized one, there should not be another data in cataloginventory_stock_item and cataloginventory_stock_status tables except data for Default Website (0). Thus there are no possibilities to have in_stock data on one website for particular SKU, and out of stock data for the same SKU on another website. That's functionality would be provided by Multi-Source Inventory (MSI) project.

All we do now is storing and retrieving data for Default Website only.
That's why most of the code looks like

    /**
     * Subtract product qtys from stock.
     * Return array of items that require full save
     *
     * @param string[] $items
     * @param int $websiteId
     * @return StockItemInterface[]
     * @throws \Magento\Framework\Exception\LocalizedException
     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
     */
    public function registerProductsSale($items, $websiteId = null)
    {
        //if (!$websiteId) {
            $websiteId = $this->stockConfiguration->getDefaultScopeId();
        //}

So, I suppose that there is some customization on Magento Installation where current fix need to be applied.

But that fix could be also achieved pretty easily using Magento extensibility, as the class mentioned in this PR

class StockStatusBaseSelectProcessor implements BaseSelectProcessorInterface

being the implementation of Extension point declared in Magento (BaseSelectProcessorInterface).

Thus even in current code base, there is a possibility to achieve proposed behavior.

But additional filter for DEFAULT WEBSITE (0) we could consider as Defensive Programming practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Progress: accept Release Line: 2.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants