Currently working on having different "sites" using Domain Access but don't want the carts being shared across the sites. The only way I can see to do this is to implement hook_commerce_cart_order_id to run a different query and return the relevant cart for the site the user is seeing.

The problem comes when the user first visits site B after making a cart on site A. The way the hook is used, we cannot return a "no cart" for site B without first creating a new cart which isn't a good way of doing things.

A simple change to commerce_cart_order_id() would be to allow the modules implementing hook_commerce_cart_order_id to return FALSE to say that there is no cart for the requested user.

Thoughts?

CommentFileSizeAuthor
#8 1366976-hook-commerce-cart-order-id-no-cart-8.patch1.17 KBmjpa
#1 1366976-hook-commerce-cart-order-id-no-cart.patch1.12 KBdamien tournoud

Comments

damien tournoud’s picture

damien tournoud commented
Title: Change to commerce_cart_order_id() » Cannot for a user not to have a cart using hook_commerce_cart_order_id()
Status: Active » Needs review
StatusFileSize
new 1366976-hook-commerce-cart-order-id-no-cart.patch1.12 KB

I agree, this is unfortunate. I guess we need something like this?

mjpa’s picture

mjpa commented
Title: Cannot for a user not to have a cart using hook_commerce_cart_order_id() » Cannot force a user not to have a cart using hook_commerce_cart_order_id()

That's pretty much what I was going to submit as a patch yesterday but you beat me to it :)

I'll apply the patch on Monday and give it a test and report back - should be no issues though.

maximpodorov’s picture

maximpodorov commented

BTW, is it possible to add static caching for commerce_cart_order_id()? This function produces dozens of the same queries for a single page.

mjpa’s picture

mjpa commented

The patch in #1 works fine for me.

@maximpodorov: there is static caching in commerce_cart_order_load() but I guess adding it to commerce_cart_order_id() can't hurt, quick test on a site and its running at least 10 times.

mjpa’s picture

mjpa commented
Status: Needs review » Reviewed & tested by the community

Seems to work fine.

damien tournoud’s picture

damien tournoud commented

@maximpodorov: could you please open a separate issue for the static caching, and give more information about where this function is called from in your case?

rszrama’s picture

rszrama commented
Status: Reviewed & tested by the community » Needs work

One substantive change from the patch in #1 is that 0 could now feasibly be returned as a valid cart order ID (which we know it wouldn't be). Perhaps FALSE should be checked first leaving the check on actual return values intact.

mjpa’s picture

mjpa commented
Status: Needs work » Needs review
StatusFileSize
new 1366976-hook-commerce-cart-order-id-no-cart-8.patch1.17 KB

Good point, modified the patch so that it checks for FALSE and returns NULL (the database queries return NULL if no cart is found). The original condition is checked then before returning the order_id.

rszrama’s picture

rszrama commented

Hmm, in my test the database queries actually returned FALSE if no cart is found. Either way it probably doesn't really matter, but I've split this out a little further and added more documentation to indicate that these hooks are executed in order and their return values are dealt with one at a time.

Commit: http://drupalcode.org/project/commerce.git/commitdiff/ff9bbc7

rszrama’s picture

rszrama commented
Status: Needs review » Fixed

Forgot the status change. :-/

Status: Fixed » Closed (fixed)

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