If the cardinality of the commerce_customer_profile is not 1 then the EntityMetadataWrapper is not EnityDrupalWrapper therefore we need to interate over the metadata and update any of the the uids of any referenced customer profiles.

Line 769 of commerce_cart.module converts an order over when logging in. It makes several presumptions about the Entity Metadata Wrapper and therefore if the cardinality of the field on the order is not 1 the user can not login.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

scotthooker’s picture

Patch supplied

scotthooker’s picture

Status: Active » Needs review
FileSize
2.4 KB
rszrama’s picture

How do you end up with a cardinality other than 1? I've been told Drupal is supposed to take care of renumbering when necessary.

scotthooker’s picture

The field settings for my custom customer profile type has the number of values as unlimited (my order can have multiple 'attendees' associated with it - my use case is that I'm selling tickets that require names/ a customer profile for each ticket). Therefore the field on my order has a cardinality of -1 for unlimited values. As such the wrapper is different.

rszrama’s picture

Sorry, I had confused cardinality with delta when I read your post.

Do you not run into other problems with unlimited, tho? Commerce isn't exactly setup to support multiple value fields like this.

scotthooker’s picture

Nope, that's the only issue we've had. Seeing as the fields cardinality/ multiple value settings can be changed from the UI I suspect this problem could be stumbled upon unintentionally at some stage.

Took me a bit of debugging to track the problem down and this is the only issue:- if I create an order as anon and then login it breaks.

scotthooker’s picture

I've looked into this further and can confirm I can generate the same bug via the UI without any custom code.

scotthooker’s picture

rszrama’s picture

Status: Needs review » Fixed

Finally had a chance to look into this, and your patch will not work with the way Commerce is designed. The customer profile reference field that links an order to a particular customer profile type is variable (configured in the checkout pane settings form). Even though it initializes to commerce_customer_[type name], we can't assume it always will be. I'm honestly not sure why you changed the logic there - it looks like it was supposed to be a bit of optimization, but it introduces that regression.

Additionally, we can just do a foreach() over the list wrapper. No need to do the while() thing. EntityListWrappers implement PHP's ArrayInterface, so you can basically treat them as arrays with various functions / language constructs.

If I read you right, you created your own customer profile reference field widget type. The Customer module prevents the customer profile manager widget from using anything other than 1 as the cardinality. I'd assume that's why no one has encountered this issue before. Still, I'm happy to fix it. See the commit diff for the final change.

I don't expect I'll be making any changes to the Customer code itself, so I'm going to assume developers are on their own if they are adding custom customer profile reference field widgets to also supply their own checkout panes and other things to work with the multi-value field structure.

Commit: http://drupalcode.org/project/commerce.git/commitdiff/59caa20

scotthooker’s picture

That's perfect. I've read your comment and can explain why we did the above ^^ when I get chance.

I'm not sure "The Customer module prevents the customer profile manager widget from using anything other than 1 as the cardinality" is entirely correct though...

Alas that change fixes my problem so thats brilliant :)

Status: Fixed » Closed (fixed)

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