Problem/Motivation

Currently, there's no way to allow/deny an automatic cart order refresh based on custom logic, since the cart refresh process is performance consuming, it'd be great if there was a way to turn it on/off at will.
For a project I'm currently working on, we manually trigger the refresh (after adding a product to the cart, adjusting quantities etc.), and we'd like the automatic cart refresh to only happen when landing on the checkout page (and it's currently not possible to do that).

Basically, the current custom logic will let you force a refresh on the checkout page, but won't let you "skip" it.

Proposed resolution

Introduce a hook_commerce_cart_order_can_refresh().

Remaining tasks

Propose a patch.

CommentFileSizeAuthor
#3 commerce_cart-introduce-cart-can-refresh-hook-3016616-3.patch1.77 KBjsacksick
#2 commerce_cart-introduce-cart-can-refresh-hook-3016616-2.patch2.5 KBjsacksick
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jsacksick created an issue. See original summary.

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
Status: Active » Needs review
FileSize
commerce_cart-introduce-cart-can-refresh-hook-3016616-2.patch2.5 KB

The attached patch introduces a hook_commerce_cart_order_can_refresh().

Modules implementing this hook should return one of the following values:

  • COMMERCE_CART_REFRESH_ALLOW: if the refresh is to be allowed.
  • COMMERCE_CART_REFRESH_SKIP: if the refresh is to be skipped.

I wondered between COMMERCE_CART_REFRESH_SKIP and COMMERCE_CART_REFRESH_DENY (but I think "skip" makes more sense and skip is used in 2.x as well).

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
FileSize
commerce_cart-introduce-cart-can-refresh-hook-3016616-3.patch1.77 KB

The attached patch is using booleans instead of constants, which probably makes sense since we don't have more than 2 possible values.

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
torgosPizza’s picture

torgosPizza
he/him
Primary language English
Location Portland, OR
CreditAttribution: torgosPizza commented
Status: Needs review » Reviewed & tested by the community

We've been using this patch for over a year now without any issues. Haven't had a reason to invoke the hook myself yet, but the logic looks quite good at preventing, for instance, an Administrator loading an order for reading and processing an unnecessary refresh.

  • jsacksick committed 42c16ab on 7.x-1.x 
    Issue #3016616 by jsacksick, torgosPizza: Introduce a...
jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
Status: Reviewed & tested by the community » Fixed

Committed, thanks for the RTBC!

Status: Fixed » Closed (fixed)

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