Closed (fixed)
Project:
Commerce Core
Version:
7.x-1.x-dev
Component:
Cart
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
4 Jan 2013 at 16:13 UTC
Updated:
1 May 2013 at 09:20 UTC
Jump to comment: Most recent file
Comments
Comment #1
scotthooker commentedPatch supplied
Comment #2
scotthooker commentedComment #3
rszrama commentedHow do you end up with a cardinality other than 1? I've been told Drupal is supposed to take care of renumbering when necessary.
Comment #4
scotthooker commentedThe 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.
Comment #5
rszrama commentedSorry, 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.
Comment #6
scotthooker commentedNope, 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.
Comment #7
scotthooker commentedI've looked into this further and can confirm I can generate the same bug via the UI without any custom code.
Comment #8
scotthooker commented#2: profile_type_cardinality-1880236.patch queued for re-testing.
Comment #9
rszrama commentedFinally 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
Comment #10
scotthooker commentedThat'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 :)