woocommerce / woocommerce-gateway-paypal-express-checkout

59 stars 64 forks source link

Only ensure session on single product pages #793

Closed claudiulodro closed 3 years ago

claudiulodro commented 4 years ago

Issue: #777


Description

This PR improves the session handling to prevent caching issues. It does this by only initializing the session (if needed) on single product pages. Since a session without adding anything to the cart is only required when checking out from single product pages, this should dramatically reduce the number of sessions that are created.

Steps to test:

  1. Set up this gateway with the "Check out on single product" setting enabled.
  2. Before applying this patch, in a new incognito window visit the site with the network inspector open. Observe a session starts on the first page load on the homepage:
Screen Shot 2020-08-18 at 9 34 51 AM
  1. Apply this patch. Start a fresh incognito window. Visit the site with the network inspector open. Observe no session is created on the homepage:
Screen Shot 2020-08-18 at 9 35 35 AM
  1. Visit a single product page, observe the session is created:
Screen Shot 2020-08-18 at 9 36 14 AM

Documentation

Closes #777 .

achyuthajoy commented 4 years ago

Nice one @claudiulodro. I was working on something similar a few weeks back but never got the chance to submit.

$frontend = (
    ( ! is_admin() || defined( 'DOING_AJAX' ) ) &&
    ( is_product() || is_checkout() || ( is_cart() && ! WC()->cart->is_empty() ) ) &&
    ! ( defined( 'DOING_CRON' ) && defined( 'REST_REQUEST' ) )
);

Create WC Session Cookie if:

It seems your solution is a lot cleaner 👍

claudiulodro commented 4 years ago

Thanks!

I think the Cart/Checkout checks aren't necessary:

So the only situation this needs to handle is single product pages.

jorgeatorres commented 3 years ago

Hi @claudiulodro!

This looks good. I've rebased the changes on the latest trunk and also pushed a commit to account for #845, which was something we added after you submitted this PR. Once Travis gives me the green light, I'll merge the PR.

jorgeatorres commented 3 years ago

Tests I ran on this branch: