Problem/Motivation
When using the Related Properties with a Drupal Commerce 2.x Productvariation reference, only the basic fields are listed for the mapping.
It seems that the default:commerce_producthandler, that is used for the product variation reference, is not adding information about the target_bundle. Thus the field definitions are not returned (using the related entity properties as mapping type).
Steps to reproduce
Install Drupal Commerce 2.x, create a product and product variation type, create a mapping using productvariation as base entity and then try to map the body field from the referencing product. It is not listed.
Proposed resolution
Create a patch so that the module also supports references without the target_bundle set.
Remaining tasks
Create patch.
User interface changes
None
API changes
None
Data model changes
None
Comments
Comment #2
dejan0 commentedHello!
For some reason `$settings['handler_settings']['target_bundles']` is empty for Product.
I have added `$this->entityTypeBundleInfo->getBundleInfo($entity_type)` instead, and it works fine for me.
Please find attached patch
Comment #3
aaronbaumanComment #4
aaronbaumanThis is good. A couple changes:
1. Looks like these method changes eliminate the need for try/catch
2. If target_bundles is set, I think we should still use it to limit which fields get shown
Updated patch attached - lemme know if it works for you
Comment #5
dejan0 commentedHi Aaron! Thanks for your quick reply!
1. It looks like we still need a solution for excluding 'non-fieldables'.
At least in our case, `product_variation_type` from commerce fails with an error:
```LogicException: Getting the base fields is not supported for entity type Product variation type. in Drupal\Core\Entity\EntityFieldManager->buildBaseFieldDefinitions() (line 209 of core/lib/Drupal/Core/Entity/EntityFieldManager.php).```
So, instead of try/catch I have implemented checking against FieldableEntityInterface::class. And it works fine now for me.
2. Ok, thanks for letting me know
Please find attached a new patch, let me know please how it looks...
Thanks!
Comment #6
aaronbaumanLooks good to me.
@criz does this solve the issue for you?
Comment #7
criz@AaronBauman Yes, it solves the issue well!
Comment #8
aaronbaumanThis is in, thanks folks