Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue #2816059 - Cart issues after Guest checkout #527

Closed
wants to merge 3 commits into from
Closed

Issue #2816059 - Cart issues after Guest checkout #527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2baf008
Issue #2816059 by steveoliver: Cart issues after Guest checkout
steveoliver Oct 11, 2016
8471416
Distinguish between active and completed carts in CartSession.
steveoliver Oct 16, 2016
246eae1
Fix spelling of 'cacheability'.
steveoliver Oct 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions modules/cart/src/CartProvider.php
View file
Open in desktop
Expand Up @@ -91,7 +91,7 @@ public function createCart($order_type, StoreInterface $store, AccountInterface
// Store the new cart order id in the anonymous user's session so that it
// can be retrieved on the next page load.
if ($account->isAnonymous()) {
$this->cartSession->addCartId($cart->id());
$this->cartSession->addActiveCartId($cart->id());
}
// Cart data has already been loaded, add the new cart order to the list.
if (isset($this->cartData[$uid])) {
Expand All @@ -112,6 +112,10 @@ public function finalizeCart(OrderInterface $cart, $save_cart = TRUE) {
if ($save_cart) {
$cart->save();
}
if ($cart->getCustomer() && $cart->getCustomer()->isAnonymous()) {
$this->cartSession->deleteActiveCartId($cart->id());
$this->cartSession->addCompletedCartId($cart->id());
}
// Remove the cart order from the internal cache, if present.
unset($this->cartData[$cart->getCustomerId()][$cart->id()]);
}
Expand Down Expand Up @@ -191,7 +195,7 @@ protected function loadCartData(AccountInterface $account = NULL) {
$cart_ids = $query->execute();
}
else {
$cart_ids = $this->cartSession->getCartIds();
$cart_ids = $this->cartSession->getActiveCartIds();
}

$this->cartData[$uid] = [];
Expand Down
26 changes: 22 additions & 4 deletions modules/cart/src/CartSession.php
View file
Open in desktop
Expand Up @@ -29,14 +29,14 @@ public function __construct(SessionInterface $session) {
/**
* {@inheritdoc}
*/
public function getCartIds() {
public function getActiveCartIds() {
return $this->session->get('commerce_cart_orders', []);
}

/**
* {@inheritdoc}
*/
public function addCartId($cart_id) {
public function addActiveCartId($cart_id) {
$ids = $this->session->get('commerce_cart_orders', []);
$ids[] = $cart_id;
$this->session->set('commerce_cart_orders', array_unique($ids));
Expand All @@ -45,15 +45,23 @@ public function addCartId($cart_id) {
/**
* {@inheritdoc}
*/
public function hasCartId($cart_id) {
public function hasActiveCartId($cart_id) {
$ids = $this->session->get('commerce_cart_orders', []);
return in_array($cart_id, $ids);
}

/**
* {@inheritdoc}
*/
public function deleteCartId($cart_id) {
public function hasCompletedCartId($cart_id) {
$ids = $this->session->get('commerce_cart_completed_orders', []);
return in_array($cart_id, $ids);
}

/**
* {@inheritdoc}
*/
public function deleteActiveCartId($cart_id) {
$ids = $this->session->get('commerce_cart_orders', []);
$ids = array_diff($ids, [$cart_id]);
if (!empty($ids)) {
Expand All @@ -65,4 +73,14 @@ public function deleteCartId($cart_id) {
}
}

/**
* {@inheritdoc}
*/
public function addCompletedCartId($cart_id) {
// Keep a list of completed order for access checks to completed orders.
$completed_ids = $this->session->get('commerce_cart_completed_orders', []);
$completed_ids = array_merge($completed_ids, [$cart_id]);
$this->session->set('commerce_cart_completed_orders', $completed_ids);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So should this just go in ::finalizeCart and make sure ::finalizeCart doesn't invoke delete? Seems weird we have deleteCartId but its actually doing a conversions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up moving the handling of completed_orders to a method in CartSession (called addCompletedCartId()).

}

}
27 changes: 23 additions & 4 deletions modules/cart/src/CartSessionInterface.php
View file
Open in desktop
Expand Up @@ -19,15 +19,15 @@ interface CartSessionInterface {
* @return int[]
* A list of cart orders ids.
*/
public function getCartIds();
public function getActiveCartIds();

/**
* Adds the given cart order id to the session.
*
* @param int $cart_id
* The cart order ID.
*/
public function addCartId($cart_id);
public function addActiveCartId($cart_id);

/**
* Checks whether the given cart order id exists in the session.
Expand All @@ -38,14 +38,33 @@ public function addCartId($cart_id);
* @return bool
* TRUE if the given cart order id exists in the session, FALSE otherwise.
*/
public function hasCartId($cart_id);
public function hasActiveCartId($cart_id);

/**
* Checks whether the given cart order id exists in the session's completed carts.
*
* @param int $cart_id
* The cart order ID.
*
* @return bool
* TRUE if the given cart order id exists in the session, FALSE otherwise.
*/
public function hasCompletedCartId($cart_id);

/**
* Deletes the given cart order id from the session.
*
* @param int $cart_id
* The cart order ID.
*/
public function deleteCartId($cart_id);
public function deleteActiveCartId($cart_id);

/**
* Tracks the given cart order id as a completed order in the session.
*
* @param int $cart_id
* The cart order ID.
*/
public function addCompletedCartId($cart_id);

}
10 changes: 10 additions & 0 deletions modules/cart/src/Controller/CartController.php
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@
namespace Drupal\commerce_cart\Controller;

use Drupal\commerce_cart\CartProviderInterface;
use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Controller\ControllerBase;
use Symfony\Component\DependencyInjection\ContainerInterface;

Expand Down Expand Up @@ -45,6 +46,9 @@ public static function create(ContainerInterface $container) {
*/
public function cartPage() {
$build = [];
$cacheable_metadata = new CacheableMetadata();
$cacheable_metadata->addCacheContexts(['user', 'session']);

$carts = $this->cartProvider->getCarts();
$carts = array_filter($carts, function ($cart) {
/** @var \Drupal\commerce_order\Entity\OrderInterface $cart */
Expand All @@ -61,6 +65,7 @@ public function cartPage() {
'#arguments' => [$cart_id],
'#embed' => TRUE,
];
$cacheable_metadata->addCacheableDependency($cart);
}
}
else {
Expand All @@ -70,6 +75,11 @@ public function cartPage() {
'#suffix' => '</div>',
];
}
$build['#cache'] = [
'contexts' => $cacheable_metadata->getCacheContexts(),
'tags' => $cacheable_metadata->getCacheTags(),
'max-age' => $cacheable_metadata->getCacheMaxAge(),
];

return $build;
}
Expand Down
2 changes: 1 addition & 1 deletion modules/checkout/src/Controller/CheckoutController.php
View file
Open in desktop
Expand Up @@ -105,7 +105,7 @@ public function checkAccess(RouteMatchInterface $route_match, AccountInterface $
$customer_check = $account->id() == $order->getCustomerId();
}
else {
$customer_check = $this->cartSession->hasCartId($order->id());
$customer_check = $this->cartSession->hasActiveCartId($order->id()) || $this->cartSession->hasCompletedCartId($order->id());
}

$access = AccessResult::allowedIf($customer_check)
Expand Down
25 changes: 25 additions & 0 deletions modules/checkout/tests/src/Functional/CheckoutOrderTest.php
View file
Open in desktop
Expand Up @@ -159,6 +159,7 @@ public function testGuestOrderCheckout() {
$this->drupalLogout();
$this->drupalGet($this->product->toUrl()->toString());
$this->submitForm([], 'Add to cart');
$this->assertSession()->pageTextContains('1 item');
$cart_link = $this->getSession()->getPage()->findLink('your cart');
$cart_link->click();
$this->submitForm([], 'Checkout');
Expand All @@ -179,6 +180,30 @@ public function testGuestOrderCheckout() {
$this->submitForm([], 'Pay and complete purchase');
$this->assertSession()->pageTextContains('Your order number is 1. You can view your order on your account page when logged in.');
$this->assertSession()->pageTextContains('0 items');
// Test second order.
$this->drupalGet($this->product->toUrl()->toString());
$this->submitForm([], 'Add to cart');
$this->assertSession()->pageTextContains('1 item');
$cart_link = $this->getSession()->getPage()->findLink('your cart');
$cart_link->click();
$this->submitForm([], 'Checkout');
$this->assertSession()->pageTextNotContains('Order Summary');
$this->submitForm([], 'Continue as Guest');
$this->submitForm([
'contact_information[email]' => 'guest@example.com',
'contact_information[email_confirm]' => 'guest@example.com',
'billing_information[address][0][given_name]' => $this->randomString(),
'billing_information[address][0][family_name]' => $this->randomString(),
'billing_information[address][0][organization]' => $this->randomString(),
'billing_information[address][0][address_line1]' => $this->randomString(),
'billing_information[address][0][locality]' => $this->randomString(),
], 'Continue to review');
$this->assertSession()->pageTextContains('Contact information');
$this->assertSession()->pageTextContains('Billing information');
$this->assertSession()->pageTextContains('Order Summary');
$this->submitForm([], 'Pay and complete purchase');
$this->assertSession()->pageTextContains('Your order number is 2. You can view your order on your account page when logged in.');
$this->assertSession()->pageTextContains('0 items');
}

/**
Expand Down