spree_auth_devise icon indicating copy to clipboard operation
spree_auth_devise copied to clipboard

Fix for #6578 Breaks Carts for New Customers (Reopen #290)

Open fredericboivin opened this issue 10 years ago • 10 comments

Previous issue: #290

The concerned issue is closed and I'm not sure if anyone is watching so I'm opening a new one to be sure :

Problem :

Cart are not merged if when a user logs in. Previous guest_cart is not merged with the signed_in cart and the the guest_cart is in limbo since it was not merged and the guest_token is destroyed on log out so you can't access it again

Steps :

  1. Log in
  2. Add item #2 to cart
  3. Log out
  4. Add item #1 to cart as a guest
  5. Log in

Expected results : Cart contain Item #1 and Item #2 since the carts should be merged

Results :

Guest cart is discarded so you only have Item #2 in your cart

Gemfile https://gist.github.com/dangerdogz/5563861ca6a1d4d0649f

Gemfile.lock https://gist.github.com/dangerdogz/bf19cf07bf569360afd8

Regression introduced in commit 336b0e4

Suggested solution :

The offending commit intend to fix the following problem :

  1. Checkout as a guest
  2. Checkout as a logged in user

The guest user ends up in the user account because since guest_token is cleared on logout, it's not resetted after the guest checkout and the logged in user reuse the same token thus associating both order to the user

In my opinion, it's somewhat an edge case since you have to do the steps on the same computer in the correct order and the fix ends breaking an essential checkout flow (shop, log in, countinue)

Thus, I suggest that we revert the offending commit and fix the original problem in one of the following ways :

  1. Check the order status before the association so we avoid associating completed guest order from a previous session with other users (not sure about this solution)
Warden::Manager.after_set_user except: :fetch do |user, auth, opts|
  if auth.cookies.signed[:guest_token].present?
    if user.is_a?(Spree::User)
      Spree::Order.where(email: user.email, guest_token: auth.cookies.signed[:guest_token], user_id: nil).each do |order|
        order.associate_user!(user) unless order.completed?
      end
    end
  end
end
  1. We could destroy the guest_token after a successful guest checkout since the "guest session" is complete (no cart, order is done, no reason to maintain a guest session since it's only purpose is to shop and then checkout and it's useless after the process is completed)

I'll make the pull request once a solution is selected, feel free to chime in if you have a better way to fix it, I'm not that much familiar with this part of Spree

fredericboivin avatar Oct 20 '15 17:10 fredericboivin

Same issue. It's really annoying when we login during the checkout process

alexandre025 avatar Nov 03 '15 22:11 alexandre025

Same for me

Wooobee avatar Dec 30 '15 17:12 Wooobee

This is a major regression that's just bitten us :(

JoeStanton avatar Jan 30 '16 20:01 JoeStanton

@JoeStanton I know your pain guys - adding to our priorities list to sort this thing out

damianlegawiec avatar Jan 30 '16 20:01 damianlegawiec

@damianlegawiec see https://github.com/wuboy0307/spree_auth_devise/commit/76e890f87537faf684c28c3649876bf45081bf70 for a possible fix.

mvz avatar Mar 29 '16 15:03 mvz

Even better: use the logic of #find_order_by_token_or_user from Spree::Core::ControllerHelpers::Order (i.e., order must be incomplete, and have same currency, token, store id).

mvz avatar Mar 29 '16 15:03 mvz

@mvz looks good, can you submit a PR?

damianlegawiec avatar Mar 29 '16 16:03 damianlegawiec

@damianlegawiec sure.

mvz avatar Mar 29 '16 16:03 mvz

@damianlegawiec done. Looks like CircleCI is having trouble building anything, but Travis seems happy :smile:.

mvz avatar Apr 01 '16 15:04 mvz

@mvz yeah, Circle recently has some serious problems :/ Thank you!

damianlegawiec avatar Apr 01 '16 15:04 damianlegawiec