Codexshaper / laravel-woocommerce

WooCommerce Rest API for Laravel
MIT License
189 stars 59 forks source link

X-WP-Total and X-WP-TotalPages are case-sensitive #67

Open morvy opened 3 years ago

morvy commented 3 years ago

Hi,

I moved my store from one hosting to another and suddenly everything broke up. After debugging I found out I get Fatals on WooCommerceTrait / Undefined Index: X-WP-Total.

I've checked the response from last call and it seems like some servers modify headers and set them all lowercase, so I had to replace functions countResults and countPages with this:

    public static function getTotalWooPages()
    {
        $headers = WooCommerce::getResponse()->getHeaders();

        foreach ($headers as $key => $value) {
            if (strtolower($key) == 'x-wp-totalpages') {
                return (int) $value;
            }
        }

        return 1;
    }

    public static function getTotalWooItems()
    {
        $headers = WooCommerce::getResponse()->getHeaders();

        foreach ($headers as $key => $value) {
            if (strtolower($key) == 'x-wp-total') {
                return (int) $value;
            }
        }

        return 0;
    }
maab16 commented 3 years ago

Hello @morvy,

Thanks for creating the issue. It's happened because of UPPER CASE. You can solve this issue easily by adding env

WOOCOMMERCE_WP_HEADER_TOTAL=x-wp-total
WOOCOMMERCE_WP_HEADER_TOTAL_PAGES=x-wp-totalpages

If you still face same issue then let me know.

morvy commented 3 years ago

I tried that, but it's just not 100% dumbserver-proof.. I've seen other Laravel/Woo integration had the same issue. I'll leave my solution in the code as it's just 18 headers to check and even if the header is missing I get a failsafe value back ..

yusufusta commented 3 years ago

@maab16 Thank you! It's worked for me.

azam1 commented 2 years ago

I have few sites using the same backend and they all have a different casing. Is there any chance we could move this with the core so we don't have to rely on env file?

maab16 commented 2 years ago

@azam1 Thanks for the comment. I don't get you. If you mean that you have multiple sites and you want to change the credential dynamically. If yes then follow this issue #12

config([
        'woocommerce.store_url' => $endpoint['store_url'],
        'woocommerce.consumer_key' => $endpoint['consumer_key'],
        'woocommerce.consumer_secret' => $endpoint['consumer_secret'],
        'woocommerce.verify_ssl' => $endpoint['verify_ssl'],
    ]);
    $products = Product::all();
    var_dump($products);

If you mean different things please explain little bit more.

azam1 commented 2 years ago

Sorry what I meant was I have few sites connecting to the same applications. Some sites works fine when I add:

WOOCOMMERCE_WP_HEADER_TOTAL=x-wp-total WOOCOMMERCE_WP_HEADER_TOTAL_PAGES=x-wp-totalpages

Other sites don’t and wise versa.

So is there anyway to check if it’s upper case or lower case and set it accordingly?

morvy commented 2 years ago

@maab16 I believe the solution I proposed above could increase "comfort" for all of us, checking header after converting to lowercase and returning the value if matched shouldn't slow down the request much and it will work on all servers. I had the same experience as @azam1 and setting those 2 constants per server is sometimes about trial and error ..

azam1 commented 2 years ago

@azam1 Thanks for the comment. I don't get you. If you mean that you have multiple sites and you want to change the credential dynamically. If yes then follow this issue #12

config([
        'woocommerce.store_url' => $endpoint['store_url'],
        'woocommerce.consumer_key' => $endpoint['consumer_key'],
        'woocommerce.consumer_secret' => $endpoint['consumer_secret'],
        'woocommerce.verify_ssl' => $endpoint['verify_ssl'],
    ]);
    $products = Product::all();
    var_dump($products);

If you mean different things please explain little bit more.

When I have Cloudflare enabled sites they return the header text in lowercase. It could be due to HTTPS or compression not sure. More details here

I was just wondering what if we did not set the header_total & header_total_pages in the config. We can simply check the response header to see which value is set?

$this->getResponse()->getHeaders()[$this->headers['header_total']]

maab16 commented 2 years ago

@morvy @azam1 Thanks for the comments. I'll check the issue and update there. Thanks