Previous carts persist after guest checkout [#2816059]

Problem/Motivation

After checking out as a Guest, the next Guest cart is messed up:

Product(s) from previous order remains in cart
Yet Cart block shows "0 items"
Hitting Checkout button on second cart brings user to "[step 4]. Complete" page with order id / confirmation of message of previous order.

Proposed resolution

Set caching data/contexts on Cart page
Track completed orders
1. Similar to what was required in 1.x, 2.x CartProvider needs to track not only session orders but completed carts.
2. Basically we just have "commerce_cart_orders" but we also need now need "commerce_cart_completed_orders"; CartProvider needs to be patched to respect these two. and finalizeCart needs to move it from commerce_cart_orders to commerce_cart_completed_orders.

Comments

Comment #1

10 October 2016 at 19:52

steveoliver created an issue. See original summary.

Comment #2

steveoliver CreditAttribution: steveoliver commented 11 October 2016 at 01:49

https://github.com/drupalcommerce/commerce/pull/527/files shows failing test that should pass when this issue is resolved.

Comment #3

bojanz CreditAttribution: bojanz at Centarro commented 11 October 2016 at 09:38

Big thanks for the failing test!

Comment #4

steveoliver CreditAttribution: steveoliver commented 13 October 2016 at 17:24

Issue summary:

View changes

Updating issue summary to include the child issue that was closed as a duplicate of this.

Comment #5

Lukas von Blarer

he/him

CreditAttribution: Lukas von Blarer commented 14 October 2016 at 10:12

Status:

Active

» Needs review

Ok, going to review this on my installation in the coming days...

I guess this needs review?

Comment #6

Lukas von Blarer

he/him

CreditAttribution: Lukas von Blarer commented 14 October 2016 at 13:29

Works for me so far! Thank you! Can't RTBC this tough. I am not familiar with 2.x yet...

Comment #7

mglaman

English

WI, USA

CreditAttribution: mglaman at Centarro commented 14 October 2016 at 18:44

The tests and everything look good. I had one comment I proposed in the PR.

I think the conversion of a cart ID from the commerce_cart_orders session item to commerce_cart_completed_orders should be kept in finalizeCart, keeping deleteCartId as an actual deletion of the cart from the session. We can let commerce_cart_completed_orders persist and be something populated in finalizeCart

Comment #8

steveoliver CreditAttribution: steveoliver commented 17 October 2016 at 02:09

Makes sense. I refactored CartSession methods and usages to be explicit about the handling of active vs. completed carts: https://github.com/drupalcommerce/commerce/pull/527/commits/84714163.

Comment #9

bojanz CreditAttribution: bojanz at Centarro commented 22 October 2016 at 18:34

Title:

Cart issues after Guest checkout

» Previous carts persist after guest checkout

Retitling.

Comment #10

22 October 2016 at 19:57

bojanz committed d7317f7 on 8.x-2.x authored by steveoliver

Issue #2816059 by steveoliver, bojanz: Previous carts persist after...

Comment #11

bojanz CreditAttribution: bojanz at Centarro commented 22 October 2016 at 20:05

Status:

Needs review

» Fixed

Thought quite a bit about which API we wanted to have here.

Commerce 1.x had a $completed = FALSE argument for the regular set of functions, which controlled whether active or completed cart ids are manipulated. Boolean arguments are generally bad for DX, you need to know what the TRUE in hasCartId($cart_id, TRUE) stands for. So it made sense for steveoliver to introduce parallel methods, giving us an active and completed version of each method.
However, reviewing this with Matt it struck us that we now have code duplication, since methods only differ in the session key that they use.

In the end I managed to find a better alternative, a $type argument that uses a class constant. Hence we get this usage:

  // The cart is anonymous, move it to the 'completed' session.
    if (!$cart->getCustomerId()) {
      $this->cartSession->deleteCartId($cart->id(), CartSession::ACTIVE);
      $this->cartSession->addCartId($cart->id(), CartSession::COMPLETED);
    }

Which is clear, but still allows the implementation to stay lean.
$type defaults to self::ACTIVE by default, which also minimizes the BC break, making it almost non-existent.