search

hyyan / woo-poly-integration

Looking for maintainers! - Wordpress WooCommerce Polylang Integration
https://wordpress.org/plugins-wp/woo-poly-integration/
MIT License
183 stars 66 forks source link

Stock Syncing issue with IPN #299

Closed Tii closed 5 years ago

Tii commented 6 years ago

Can you reproduce this issue on default Wordpress theme (eg Storefront)? YES

Can you reproduce this issue when all other plugins are disabled except WooCommerce, Polylang and Hyyan WooCommerce Polylang Integration? YES

What product versions and settings are you using when this issue occurs? PHP: PHP 7.0 WordPress: 4.9.2 WooCommerce: 3.2.6 Polylang: 2.2.8 Hyyan WooCommerce Polylang Integration: 1.0.4 Browser: Chrome latest

Steps to Reproduce Setup at least 2 languages, let's say EN = default, FR Set a product, translate it into the 2 languages and set the stock to 3 Add 1 of that product into the basket while in FR proceed to checkout an pay with a payment platform like Paypal or Payzen. Once the paiment done, close the page before the redirect to your website. Make sure an IPN was send to your website. Your order is received, the stock of the product will be 1 in EN and 3 in FR !

What I Expected to have 2 as stock in every language as it was 3 and you bought 1.

What Happened Instead When called via IPN, the stock update update 2 times the default language instead of one time each, I digged and found the issue that can be fixed like this:

by removing these lines in Stock.php

$productID = Utilities::get_order_item_productid($item); $productObject = wc_get_product($productID);

Somehow, when called thru IPN, the get_order_item_productid function returns the product id of the default language product instead of the selected product.

I checked the wc-stock-function.php and changed thos two lines by this:

$productObject = $item->get_product(); $productID = $productObject->get_id();

Then, I removed the

unset($translations[$orderLang]);

and simply skipped the current product in the translations loop:

if ($ID == $productID) continue;

Jon007 commented 6 years ago

when called thru IPN, the get_order_item_productid function returns the product id of the default language product instead of the selected product.

Yes, the IPN url itself would be treated by Polylang as in default language. [although #218 suggests there could be language specific IPN and proposes change to ensure this doesn't happen]

How are your changes limited to only apply to IPN calls? Can you submit your changes as a Pull request so it is clear to review the diff of the changes?

One naive question, isn't the problem also fixed by not using IPN? What is the benefit of using IPN in this case? (other payment providers don't use IPN and it isn't necessary for PayPal either, so just to clarify...)

Tii commented 6 years ago

Changes don't seem to affect non-IPN calls, I'm using my patch in production and I have no issues at all. I submitted a change request. I really don't see how you can avoid using IPN. Can you explain to me how you are not using it? Not using IPN would make you entirely rely on PDT, which is usually the page they are redirected to after payment. In our case, more than 50% of our customers do not wait until they are redirected (set to 1 second) to our website after payment, which means that, without IPN, half of our order would not switch to payment completed. IPN allows us to be sure our order payment was updated and I cannot understand how you would not be using this.

Jon007 commented 6 years ago

well the standard Stripe checkout for example has either a pop-up dialog or card fields on the page (or Apple Pay button on the page), it seems to control everything via a set of javascripts and ajax calls, I'm not aware that it has an equivalent to Paypal IPN.

Are you using the WooCommerce-provided Paypal integration or one of the other plugins?

Tii commented 6 years ago

As javascript is client side, it is not reliable. JS could fail or be blocked, Pop-up are also usually blocked. Also, many of our customers run IE with Win7, which is known for its bad implementation of Javascript. We are not using Paypal, we are using PayZen and Ingenico Plugins, they both work with IPN. You don't have to use IPN with any plugin, but it's always available, even in Stripe. https://github.com/jatskie/stripe/blob/master/ipn.php We did implement it as it gives you certainty, also, in our line of work, people order their lunch before 10:15 and get delivered before 12, so, if the payment was not notified, the customer will not get his lunch. In this matter of time importance, we cannot rely on chance and must implement IPN to be sure that an order paid will be processed within our timeframe.

carlituxman commented 6 years ago

If we pay with transfer bank is not working.

Jon007 commented 6 years ago

@carlituxman that's not enough detail for anyone to be able to comment, but it's quite likely the situation has changed with woocommerce 3.3.5 due to the woocommerce ajax changing again. you would need to test with both the open pull requests, one of which is suggested to fix this issue and the other fixes the ajax issue.

carlituxman commented 6 years ago

sorry. what is the ajax fix issue? I think you say #329 ?

I applied #329 and #300 fixes and no working. I have the same #332 problem

Jon007 commented 6 years ago

Well what version of woocommerce are you using?

carlituxman commented 6 years ago

woocommerce 3.3.5

Stibo commented 6 years ago

Is there a solution for this problem? Its REALLY bad in one of our shops. We have a lot of unique products (with only 1 item on stock) and if its ordered in one language, the stock in the other language remains at 1 (and the stock for the main language is set to -1). Now we have a few double order of a product thats still on stock in one language, even if its sold out in the other...

Tii commented 6 years ago

you can check my Fix and make changes to the Stock file in the plugin: https://github.com/hyyan/woo-poly-integration/pull/300/files

it has not been accepted yet but I'm using it for month without any issue.

I hope they will accept my pull request one day ...

Stibo commented 6 years ago

@Tii Thanks, I'll try your fix. Hope they will accept you pull request. Its not the first bugfix a have to add to the core files. My current plugin version isn't updatable. :(

raix022 commented 6 years ago

This fix didn't help me.. Stock only changes in that language, what I "buy". I have set up stocks with varibles. Anyone have solution?

Jon007 commented 6 years ago

@Stibo why is your current plugin version not updatable? If you have had to make other changes, why not create your own branch in github, check in your changes there and raise a pull request.

bmpf commented 5 years ago

This option/fix did not work for me. :(

Tii commented 5 years ago

I actually re-wrote the whole code

`

protected function change(\WC_Order_Item_Product $item, $action = self::STOCK_REDUCE_ACTION) {

    $productID = Utilities::get_order_item_productid($item);
    $productObject = wc_get_product($productID);
    $orderLang = pll_get_post_language($item->get_order_id());

    if ($productObject && $orderLang) {
        $translations = Utilities::getProductTranslationsArrayByObject($productObject);
        $isManageStock = $productObject->managing_stock();

        $method = ($action === self::STOCK_REDUCE_ACTION) ?
            'decrease' :
            'increase';
        $change = ($action === self::STOCK_REDUCE_ACTION) ?
            Utilities::get_order_item_quantity($item) :
            Utilities::get_order_item_change($item);

        $mainStock = null;
        $mainStockId = null;
        $products = array();
        foreach ($translations as $ID) {
            $product = wc_get_product($ID);
            $stock = $product->get_stock_quantity();

            if ($mainStock === null) {
                $mainStock = $stock;
                $mainStockId = $ID;
            } else {
                if ($method === 'decrease') {
                    if ($mainStock > $stock) {
                        $mainStock = $stock;
                        $mainStockId = $ID;
                    }
                } else {
                    if ($mainStock < $stock) {
                        $mainStock = $stock;
                        $mainStockId = $ID;
                    }
                }
            }
        }
        unset($products[$mainStock]);

        foreach ($translations as $ID) {

            if ($ID === $mainStockId) continue;

            if ($isManageStock) {
                \wc_update_product_stock($ID, $change, $method);
            }
            $general = Settings::getOption(
                'general', MetasList::getID(), array('total_sales')
            );
            if (in_array('total_sales', $general)) {
                update_post_meta($ID, 'total_sales', get_post_meta($mainStockId, 'total_sales', true)
                );
            }
        }

    }
}

`

So my approach was to check the stock that is different that the others and synchronise all stocks on it.

It's not very efficient but it's working fine until we would be able to get the language thru a IPN.

bmpf commented 5 years ago

Hi Tii,

I ll try it out today :)

bmpf commented 5 years ago

Can you paste all the code for the protected function change ?

bmpf commented 5 years ago

I get the same problem :/ testing

Tii commented 5 years ago

Hi Tii,

I ll try it out today :)

notice that I did not do the work for the variations

bmpf commented 5 years ago

Hi Tii, I ll try it out today :)

notice that I did not do the work for the variations

Can you paste all the code for the protected function change ?

Tii commented 5 years ago

Hi Tii, I ll try it out today :)

notice that I did not do the work for the variations

Can you paste all the code for the protected function change ?

I'm not sure I understand, I did copy the whole function...

bmpf commented 5 years ago

Hi Tii, I ll try it out today :)

notice that I did not do the work for the variations

Can you paste all the code for the protected function change ?

I'm not sure I understand, I did copy the whole function...

Did not work. I got the same issue :/ . https://i.ibb.co/kHNW6fc/stockissue.jpg

Tii commented 5 years ago

Hi Tii, I ll try it out today :)

notice that I did not do the work for the variations

Can you paste all the code for the protected function change ?

I'm not sure I understand, I did copy the whole function...

Did not work. I got the same issue :/ . https://i.ibb.co/kHNW6fc/stockissue.jpg

Did you replace the function change from the wp-content/plugins/woo-poly-integration/src/Hyyan/WPI/Product/Stock.php with mine ?

I am using it for month now and I did not have one single issue.

bmpf commented 5 years ago

Hi Tii, I ll try it out today :)

notice that I did not do the work for the variations

Can you paste all the code for the protected function change ?

I'm not sure I understand, I did copy the whole function...

Did not work. I got the same issue :/ . https://i.ibb.co/kHNW6fc/stockissue.jpg

Did you replace the function change from the wp-content/plugins/woo-poly-integration/src/Hyyan/WPI/Product/Stock.php with mine ?

I am using it for month now and I did not have one single issue.

Yes.

fkoomek commented 5 years ago

The fix doesn't work for me either...

Jon007 commented 5 years ago

@fkoomek @bmpf @Stibo @Tii please try this replacement Stock.php file for a more complete fix