OCA / connector-magento

Connect Odoo with Magento
http://odoo-magento-connector.com/
GNU Affero General Public License v3.0
105 stars 206 forks source link

[10.0] Odoo is pushing product out of stock that are backorder able. #268

Open jaredkipe opened 6 years ago

jaredkipe commented 6 years ago

In https://github.com/OCA/connector-magento/blob/10.0/connector_magento/models/product/importer.py

    def _get_data(self, binding, fields):
        result = {}
        if 'magento_qty' in fields:
            result.update({
                'qty': binding.magento_qty,
                # put the stock availability to "out of stock"
                'is_in_stock': int(binding.magento_qty > 0)
            })

This is dangerous. Magento has its own calculations and configuration for when a product should be in stock or not.

I'd rather this be something like this.

    def _get_data(self, binding, fields):
        result = {}
        if 'magento_qty' in fields:
            result.update({
                'qty': binding.magento_qty,
            })
            if binding.magento_qty > 0:
                result.update({
                    'is_in_stock': 1,
                })

Or...

    def _get_data(self, binding, fields):
        result = {}
        if 'magento_qty' in fields:
            result.update({
                'qty': binding.magento_qty,
            })
            if binding.magento_qty > 0 or self._map_backorders[binding.backorders] >= 1:
                result.update({
                    'is_in_stock': 1,
                })

Or just don't touch 'is_in_stock' at all and let Magento manage it. I'm open to debate for sure.

guewen commented 6 years ago

I don't know enough the magento internals so I trust you :) If Magento always computes it then my feeling is that we should never change it, would it work?

jaredkipe commented 6 years ago

All of this is related to Magento EE 1.14.3.6 (CE 1.9.3.6) Ever sending {'is_in_stock': 0} is asking for problems as a lot of things happen when saving a stock item (which the API does do), especially with or without that flag set. Mage_CatalogInventory_Model_Stock_Item::_beforeSave() will call verifyStock() which has logic related to if the saved qty is 0 and it is not setup to backorder, then _beforeSave() will flip the is_in_stock to out of stock.

I know less about what code exists to set the product back in stock. The only place in code that it happens literally (setIsInStock(true)) is in Mage_CatalogInventory_Model_Stock::backItemQty() which has the dubious comment of "Get back to stock (when order is canceled or whatever else)". This only gets called in observers for canceling or refunding order items.

So it would appear that my first suggestion should be sufficient. Tell Magento when it is in stock, let Magento decide when it isn't.

This change shouldn't cause too many surprises.

jaredkipe commented 6 years ago

@guewen I'll open up a pull request. Does the above meet your style guidelines?

guewen commented 6 years ago

@jaredkipe sorry for not answering you before, totally fine with me! :)