Posted by joachim on October 10, 2012 at 3:08pm
5 followers
| Project: | Commerce Physical Product |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
This module assumes products have only one weight field. But it's feasible for products to have a weight field for their actual weight, and another weight field to describe some other aspect of the product.
For example:
- maximum weight for a child's bike seat / baby carrier / etc
- suitable body weight for certain sports products
- weight of individual components, such as dry weight for food
Comments
#1
@joachim, IMO this weight (+ measures) is for the box /tin/can/bag the product is in, to be used for storage and shipping calculations. The weight you are referring to seems more like a 'normal' product attribute.
#2
Precisely.
But this module is making the assumption that there is only one field of type 'physical_weight' on the product when calculating how much a product weighs for shipping purposes.
Whereas the product can have more than one weight field: one for its shipping weight, and others which describe other things for the benefit of the consumer.
#3
@joachim In your example, the other weight fields (e.g., maximum weight, suitable body weight, etc.) won't affect shipping. Because of that, you could simply use integer fields. Weight fields are pretty much strictly for shipping (in this context).
#4
> Because of that, you could simply use integer fields
Of course I could. But I don't want to! I would like the benefit of the weight field type for these: selection of units, conversion, etc etc.
> Weight fields are pretty much strictly for shipping (in this context).
Yes, and that is my point: they should not be!
#5
Ahh, gotcha. I hadn't considered that. Why can't you simply add another weight field to your product type?
#6
Because this module assumes there is only one, and uses that as the order weight. Which is what this bug report is about.
#7
If the UI was implemented in this module #1499538: Commerce Physical UI the the calculation can use the default defined field rather than just the first physical field it comes across.
#8
Marking this as fixed as the issue has been resolved.
#9
I don't understand how this has been fixed -- the other issue pointed to is not fixed either, and it doesn't yet implement what is needed here.
#10
Indeed, once the UI has been implimented the code that does these calculations will need to changed to target on those fields only.
#11
Woops, sorry. When I first scanned over this issue I had the impression that it wasn't really needed anymore. I probably read over the comments too quickly.
#12
After reviewing everything I don't necessarily agree with the approach taken in #1499538: Commerce Physical UI, for me it seems that the Field UI does a good enough job and the code contained in that sandbox only makes things more complicated.
Attached you can find a patch that implements the functionality requested in this issue. It's based upon a previous patch I wrote for this module: #1390190: hide shipping pane for non-shipping product. You first need to apply the patch in that issue before applying this one. The reason for this is that the previous patch already adds an admin UI to Commerce Physical.