When fetching products and variations the returned data may be incorrect. This can happen when the store context changes, especially in an API environment.

Product and ProductVariation should add the `store` cache context in their returned getCacheContexts values.

CommentFileSizeAuthor
#4 3105238-tests-only.patch3.26 KBmglaman
#4 3105238-4.patch4.38 KBmglaman
#3 3105238-3.patch1.12 KBmglaman
#2 3105238-2.patch1.07 KBmglaman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mglaman created an issue. See original summary.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Status: Active » Needs review
Issue tags: +Needs tests
FileSize
3105238-2.patch1.07 KB

This definitely needs some tests to prove render cache is bypassed on store context. An example is price resolving by store.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
FileSize
3105238-3.patch1.12 KB 
+++ b/modules/product/src/Entity/Product.php
@@ -262,6 +262,13 @@ class Product extends CommerceContentEntityBase implements ProductInterface {
+  /**
+   * {@inheritdoc}
+   */
+  public function getCacheContexts() {
+    return Cache::mergeContexts(parent::getCacheContexts(), ['store']);
+  }

🤦🏼‍♂️this is already overridden for the variations query param.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
FileSize
3105238-4.patch4.38 KB
3105238-tests-only.patch3.26 KB

Here's an updated patch with tests.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Issue tags: -Needs tests

Status: Needs review » Needs work

The last submitted patch, 4: 3105238-tests-only.patch, failed testing. View results

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Status: Needs work » Needs review
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs review » Reviewed & tested by the community

Looks good!

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro for E.C. Barton & Co, Matsmart commented

Tagging two customers involved in this patch.

  • mglaman committed 1b123cd on 8.x-2.x 
    Issue #3105238 by mglaman, bojanz: Products and variations must have the...
mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro for E.C. Barton & Co, Matsmart commented
Status: Reviewed & tested by the community » Fixed

Committed!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.