Add the cache contexts for the cart block

PR active: https://github.com/drupalcommerce/commerce/pull/341

However we need tests to verify cache is busted.

PR updated https://github.com/drupalcommerce/commerce/pull/341/files
Tests not yet added.

We'll want to add the cart cache context first, then try using that. See #2802941: Add a Cart cache context.

Crediting steveoliver for work on https://github.com/drupalcommerce/commerce/pull/528

Here is patch which fixes the cart cache context and fixes cachability of our block. Removes max age!

Interesting! The cart cache context looks fascinating. Why are there multiple cart IDs? Does this then automatically ensure that all anonymous users without carts have the same cache context value and hence get the same render cached cart block?

I am not seeing the necessary test coverage to ensure that the cart cache context works as intended/expected in all those situations…

. Why are there multiple cart IDs?

Users can have multiple carts based on order type and store. You could have a Digital and Physical order for US store. Or a Physical order for both the US and FR store. So if someone gets a new cart for some reason the cache should bust because new ID.

I suppose you're correct that anonymous users would all have the same render cached block, because no cart IDs, same cache key.

I made some comments here: https://github.com/drupalcommerce/commerce/pull/586#pullrequestreview-14.... I needed the tags in both places otherwise the tests would fail.

Our main tests are https://github.com/drupalcommerce/commerce/blob/8.x-2.x/modules/cart/tes.... You're correct it doesn't test the multiple cart, which it should.

Users can have multiple carts based on order type and store. You could have a Digital and Physical order for US store. Or a Physical order for both the US and FR store. So if someone gets a new cart for some reason the cache should bust because new ID.

Oh, interesting! That sounds complex UX-wise, but that's a different realm of concern :)

I suppose you're correct that anonymous users would all have the same render cached block, because no cart IDs, same cache key.

That sounds like it only works correctly by accident :P Or perhaps by intentional @bojanz design.

I made some comments here: https://github.com/drupalcommerce/commerce/pull/586#pullrequestreview-14.... I needed the tags in both places otherwise the tests would fail.

I left a reply there, which I'm repeating here:

A cache context's tags are only used when a cache context is "optimized away". E.g. when you have both 'user' and 'user.permissions' cache contexts, the 'user.permissions' cache context is optimized away. Only then is \Drupal\Core\Cache\Context\AccountPermissionsCacheContext::getCacheableMetadata() used.

The cart cache context cannot ever be optimized away. So it's rather pointless for that cache context to even return cache tags.

See https://www.drupal.org/docs/8/api/cache-api/cache-contexts#optimizing.

+++ b/modules/cart/src/Cache/Context/CartCacheContext.php
@@ -59,7 +59,12 @@ class CartCacheContext implements CacheContextInterface {
   public function getCacheableMetadata() {
-    return new CacheableMetadata();
+    $metadata = new CacheableMetadata();
+    // Add each cart as a cacheable dependency.
+    foreach ($this->cartProvider->getCarts($this->account) as $cart) {
+      $metadata->addCacheableDependency($cart);
+    }
+    return $metadata;
   }

In other words, this change is pointless.