Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

MSI compatibility while being backward compatible #1739

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

Open
thomas-kl1 wants to merge 5 commits into algolia:main
base: main
Choose a base branch
Loading
from thomas-kl1:patch-4

Conversation

Copy link
Contributor

@thomas-kl1 thomas-kl1 commented Apr 17, 2025
edited
Loading

Summary

These changes fix the MSI compatibility. It also make this module unnecessary: https://github.com/algolia/algoliasearch-inventory-magento-2

algolia/algoliasearch-inventory-magento-2 is not really maintained anymore and many flaws exists as it does not really follow the changes introduced in the main module.
I've also noticed dead code.

The $product->getIsSalable() method use the alias added by MSI while it also fallback to simple inventory management when MSI is disabled.

I've also updated how the stock_qty is retrieved so it used the method that MSI have plugged, so if MSI is used, it's automatically compatible.

EDIT:

Change in ProductHelper is BIC

acourtiol reacted with thumbs up emoji simonmaass, Dnd-Mafer, and cammonro reacted with heart emoji
Copy link
Contributor

Thank you for your contribution @thomas-kl1 ! It has been on the roadmap to retire the need for a separate module for MSI since it has long been the default in Magento 2.4. We'll be sure to review and incorporate your proposed changes in an upcoming release!

Copy link
Contributor

This is a quality contribution @thomas-kl1 ! Thank you for taking the time to share this.

We have a different intention regarding the core module but while we address that separately your ideas have been incorporated in algolia/algoliasearch-inventory-magento-2#31.

Copy link
Contributor Author

@cammonro things would be less complicated if the changes I suggest were approved. It's a minimal change for a greater support overall

Copy link
Contributor

cammonro commented Jun 23, 2025
edited
Loading

@thomas-kl1

@cammonro things would be less complicated if the changes I suggest were approved. It's a minimal change for a greater support overall

Thanks for your thoughts on this.

A couple of things:

  1. The patch we are proposing on the inventory module is immediately shippable to our customers whereas the core module has other constraints.
  2. We are still intending to incorporate changes in the core module so these fixes are not mutually exclusive.

I also have a concern with your suggested approach as it relies on \Magento\InventoryCatalog\Plugin\CatalogInventory\Model\ResourceModel\Stock\Status\AdaptAddStockDataToCollectionPlugin::aroundAddStockDataToCollection to enforce MSI and we are still retaining deprecated APIs.

While the change is elegant, I personally prefer that we be more explicit here - as code that appears to use legacy logic like the Magento_CatalogInventory module (but is secretly MSI-aware via plugins) is in IMO ambiguous, misleading even, and could lead to maintenance issues down the road.

thomas-kl1 reacted with thumbs up emoji

Copy link
Contributor Author

Your concern are legitimate and I share them. However we didn't see any major changes in the Magento core and we don't expect any soon. Actually many parts of the Magento code is based on this similar mechanic. I don't see Adobe doing this refactor as it's really big. That's why I ended up doing this, would work for both cases (MSI or not) even if it use a ghost plugin (intended by MSI package to add coverage support on vanilla features)

cammonro reacted with thumbs up emoji

Copy link
Contributor

Fair points!

Meanwhile, accurate stock quantities by warehouse were also a concern which the referenced PR aims to address.

Please note although we don't have a timeline we do intend to revisit this in the main module and your feedback will be incorporated. Marking this on hold for now. Thanks again for your help! We really appreciate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /