Label Printing Error

Could you share what types of fields you have on commerce_product that are set as attributes?

Looks like commerce_pos_label is assuming that the value of the field is either an array or a scalar value, but in this case it's getting an object (a taxonomy term).

Commerce Cart allows any field defined by a module that implements hook_options_list() to be an attribute. It looks like we might need to use that function to get a list of human-readable values, then use only the value that applies to the particular product - unless there is a more direct way to render a human-friendly value from any field that would be accepted by commerce_cart_field_attribute_eligible().

Fix for this issue https://github.com/AcroMedia/commerce_pos/pull/12

As per discussion with @tbradbury:

That's not the way I had been thinking of because it'd only fix it for taxonomy term reference fields and could still be a problem for others (maybe some other entity reference fields).
The label module is trying to display values for any fields that are attribute fields according to commerce cart, and that module allows any field to be an attribute if it has a hook_options_list implementation. So I think to solve the problem for any field that could be an attribute field we'd have to use the options similar to how the cart module does.
I could see if I can write a patch with my idea on the weekend to see if it's actually an idea that would work or not, but I don't think I'd be able to do it during the week due to project work.

Pull request is updated https://github.com/AcroMedia/commerce_pos/pull/12

This patch is what I was thinking of in #4. Let me know what you think.

It seems a bit klunky because it involves getting a whole list of values and only selecting one, but we don't shouldn't have to do any special logic for taxonomy fields or any other types that might come up.

I tested it successfully on a Kickstart install with single and multi-value taxonomy term fields.

I'm not sure if we can collaborate on a single pull request so I made another one:
https://github.com/AcroMedia/commerce_pos/pull/13

1. +++ b/modules/label/commerce_pos_label.module
   @@ -212,12 +212,21 @@ function commerce_pos_label_attributes_string($product) {
   +      $options = _options_get_options($field, $field_info, $properties, 'commerce_product', $product);
   

   We just need the available options. Do we still need to get the _options_properties(). Although we are hardcoding the widget type here, but I have tested this using radio buttons, and it worked alright. Which means, the widget type and its properties are not important here, and we can skip them.

2. +++ b/modules/label/commerce_pos_label.module
   @@ -212,12 +212,21 @@ function commerce_pos_label_attributes_string($product) {
   +          $attribute_value = array_map(function ($item) use ($options) {
   

   I thought, the idea was to make it compatible with commerce_cart_field_attribute_eligible(). But we are adding support for multivalued fields as well. Also, if we are adding multivalued field support, can you please put a comment here, the comment should tell the purpose of using array_map().

Other that that, the code looks alright to me.

Do we still need to get the _options_properties().

I think so.$properties is a required parameter in _options_get_options(). It looks like it's a pretty low-overhead function that just sets some settings for escaping the values of the fields that will be displayed.

I thought, the idea was to make it compatible with commerce_cart_field_attribute_eligible(). But we are adding support for multivalued fields as well.

Good catch. Only a single-value field should be eligible. However, when I tested it, it's really easy to set an attribute field to be multilple-value. It looks like commerce_cart deals with that by calling commerce_cart_field_attribute_eligible() and commerce_cart_field_instance_is_attribute(). I think it's OK for us to support multi-value fields, but the alternative is probably to add a call to commerce_cart_field_attribute_eligible() in commerce_pos_label_attribute_fields() so that we never get a multi-value field in commerce_pos_label_attributes_string().

I've updated the pull request with some comments. I also moved some function calls inside the condition on the field value so that we don't bother calling them if the field has a null value.

https://github.com/AcroMedia/commerce_pos/pull/13

Code looks great! Tested locally, worked without any errors.

There are two commits here https://github.com/AcroMedia/commerce_pos/pull/13, make sure that there is only one commit when you push the code.

Turned the PR back into a patch since it passed testing, mostly just for easy of use on this issue should someone want to apply the patch.