Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I believe LineItem Entity is missing the following for purchased_entity.
->setSetting('target_type', 'commerce_product_variation')
Comment | File | Size | Author |
---|---|---|---|
#32 | views_has_problems_u-2638958-31.patch | 1.02 KB | mglaman |
| |||
#32 | interdiff-2638958-27-31.txt | 1.91 KB | mglaman |
#27 | 2638958-27.default_purchasable_entity.patch | 1.1 KB | rszrama |
#7 | views_purchased_entity_relationship-2638958-7.patch | 2.52 KB | vasike |
#6 | views_purchased_entity_relationship-2638958-6.patch | 2.96 KB | vasike |
Comments
Comment #2
bojanz CreditAttribution: bojanz at Centarro commentedNo it's not, that's set by bundleFieldDefinitions(). Same trick used by the Comment entity in core.
Comment #3
powysm CreditAttribution: powysm commentedAh. I am having alot of problems with line items entity reference relationship with purchased entity when using views. It is being referenced as 'content' rather than the specific entity type, which means I get a bunch of fields but not product variation specific (sku etc.)? I thought that the above may have been the issue?
Just tried a similar simple relationship between nodes and comments and not getting the same issue.
Comment #4
bojanz CreditAttribution: bojanz at Centarro commentedI see.
I'll review our Views integration. Maybe we're missing a trick that core has for Comment. Or running into a bug.
Comment #5
bojanz CreditAttribution: bojanz at Centarro commentedConfirmed that we're missing code. Comment defines a \Drupal\comment\CommentViewsData class which has the following:
We'll need something similar, while making sure that the default relationship to node is not available.
(Note that there's also some code in comment_views_data_alter() which appears unneeded.)
Comment #6
vasikeHere is a patch that implements purchased entity relationships for all target entities types of the line item purchased entity fields defined.
Comment #7
vasikethere is an update for patch, trying to improve the code for getting the entities types for the relationships
Comment #8
bojanz CreditAttribution: bojanz at Centarro commentedTweaked and committed. Thanks!
Comment #11
niko- CreditAttribution: niko- as a volunteer commentedHi @all
purchased_entity field has view modes from node entity as target_type not seted so node is used as default.
Maybe
have sanse?
Comment #12
andypostThis needs followup to add "extra" section to relationship to make entity reference to know entity type to use ER field renderer
Comment #13
bojanz CreditAttribution: bojanz at Centarro commentedMerging back #2840386: Views assume that purchased_entity is node. into this issue and reopening.
Turns out that the relationship added by this issue doesn't work, we are missing the "extra" join information as andypost points out. However, we can't add it cause we don't actually store the purchased_entity_type that we'd need for that join to work.
Fixing the relationship would require adding a purchased_entity_type field and populating it in an update hook.
However, I am not convinced we should be allowing people to use this relationship. Our canonical way of showing purchased entity information has been to render the entity (product variation, bundle, etc) in the first column. This works for carts that have different purchased entity types (mixture of regular products, bundles, etc), unlike the relationships.
This brings us back to #2840386: Views assume that purchased_entity is node., which showed us problems with rendering the purchased entity. Due to the way Views uses fields (storage-only, no bundle-level overrides) it doesn't detect which entity type is being referenced, falling back to node.
I propose the following fix for now:
1) Remove the relationship code, document that the purchased_entity should be rendered instead.
2) Modify OrderItem::baseFieldDefinitions() and set the purchased_entity target type to "commerce_product_variation".
This is a better default than "node" since it's at least true in some cases.
That means that when configuring the rendered entity Views will always offer the product variation view modes, but that's fine since the matching view modes should exist for other puchasable entity types as well.
Comment #14
niko- CreditAttribution: niko- as a volunteer commentedComment #15
jespermb CreditAttribution: jespermb at Adapt commentedHi, we tried the patch but when installing the site we get this error:
exception 'Drupal\Component\Plugin\Exception\PluginNotFoundException' with message 'The "commerce_product_variation" entity type does not exist.' in /mnt/www/html/interflora/docroot/core/lib/Drupal/Core/Entity/EntityTypeManager.php:133
// Jesper
Comment #16
jagundez CreditAttribution: jagundez as a volunteer and commentedSame problem, the patches doesn't work for me :(
Comment #17
bradjones1I'm not entirely sure the use-cases driving all of the above, however I do have a concern around hard-coding any assumptions around the target type for the purchased_entity field, because it's not necessarily a product variation. In my case, I have an entity type implementing
PurchaseableEntityInterface
and need to access it via that field.Attached is a patch I worked up on the same code being altered in these patches, which adds a back-relationship for order items referencing a purchased entity. It might not necessarily solve everyone's concerns that led them to this issue, but I would vote against any architectural bias toward the product variation being the only type of entity you may get from this reference.
Comment #18
bojanz CreditAttribution: bojanz at Centarro commented@bradjones1
Please reread my comment. There is no fix to the Views integration that we can make without introducing another order item field and updating every order item in existence.
The assumptions we're making for the fix are this:
1) People will always render the purchased entity to show it in the cart
2) All purchasable entity types have the same required view modes
Those assumptions allow us to tell Views to assume commerce_product_variation without breaking other purchasable entity types.
Comment #19
bradjones1@bojanz
Actually I think my patch addresses a different use case entirely, I was probably distracted by the fact the patches on this issue affect the same codebase. Re: showing the purchased entity, I agree about the limitations and have run into this myself in the cart review view.
My patch, which might need to be split into a different issue after all, provides a purchased entity -> order item reference in views, which I think is another case entirely? I'm for instance using it to provide a view of a user's (custom entity type) purchaseable entities which are related to orders in fulfillment status.
Views is just a tool, of course, and I could create a custom controller to do that query and output similar data, but the opposite-direction relationship I'm providing I think is helpful, since in that case you know you're getting order item(s) always.
Comment #20
edaa CreditAttribution: edaa commentedSame problem in my tests, the 'target_type' always fallback to 'user'.
Comment #21
websiteworkspace CreditAttribution: websiteworkspace commentedThis appears to be a gnarly problem.
Comment #22
mglaman#17 belongs in its own issue.
I'm struggling to understand the bug. This seems to work fine when rendering purchased entities? We are rendering purchased entities just fine. We have variations and a custom "addon" purchased entity. I am able to render view modes in views without issue. No relationships needed or anything.
I say we close this so we can open a more succinct issue. This original issue added a relationship which works. The purchased_entity field alone with rendered entity/label formatter works.
Per #2
Can you explain? Is that on the base field? Or something in Views?
Comment #23
edaa CreditAttribution: edaa commented#22
It is on base field, but the issue is gone now (Commerce 2.2), I don't check if the issue happens on 2.1.
Comment #24
bradjones1#17 ended up being done in #3057558: Add a reverse relationship from the order item to the purchased entity though there was no reference to this ticket, adding now.
Comment #25
PlayfulWolf CreditAttribution: PlayfulWolf commentedI think the problem is that the view commerce_cart_form needs some rework to be usable, as in default configuration had no luck to display rendered variation properly. One extra relationship is enough, another is for wrapping all the cell in link to product.
p.s. commerce 2.17
Comment #26
rszrama CreditAttribution: rszrama at Centarro commentedResurrecting this issue and fixing up the metadata.
This is a failure on core's part, not our own, so I'm making this a normal task. The basic idea is that the core display formatter used here is drawing the target_type from the base field definition irrespective of any bundle specific settings. I don't know where to begin fixing that in core, but bojanz's suggestion in #13 to set a sensible default in our base field definition is the best solution I see.
However, I also think bradjones1's concern about hard-coding a specific dependency on commerce_product_variation is reasonable. We can define a new API for determining a default purchasable entity type from the available types on a site and direct people to alter the base field info if they need something other than our default choice. That's all in the attached patch.
All that said ... discussing this with Matt, it makes sense for Commerce 2.x to have a higher level API, since I basically duplicated code in the OrderItemTypeForm class. We've discussed a PurchasableEntityTypeRepository service with functions that include getAllTypes(), getDefaultType(), and getTypeOptions() that can be used both in OrderItem and OrderItemTypeForm (and elsewhere as need be). I'm going to open a separate issue for that and then we can revise the attached patch once we have that API to work with to resolve this one in similar fashion.
Comment #27
rszrama CreditAttribution: rszrama at Centarro commentedThe attached patch depends on #3146451: Add a PurchasableEntityTypeRepository service, so I'm leaving this as not tested by testbot. It manually tested locally fine, but we'll wanna let test bot ensure the new setting doesn't cause some weird unexpected conflict elsewhere.
The work in this patch is obviously much simpler than in #26, and the docblock from my previous patch is represented in the new service's interface.
Comment #28
mglamanMarking issue postponed for now, as we sort out #3146451: Add a PurchasableEntityTypeRepository service
Comment #29
mglamanBlocker merged!
Comment #31
mglamanThis failed in tests where there is no default purchasable entity type available.
EDIT: It happened here, too:
I'm going to assume it's because Order is higher in alphanumeric priority than Product, so the order fields are built before any known purchasable entity type is available.
Comment #32
mglamanThis should fix things up. The target_type is only set if there is a purchasable entity type available.
Comment #33
rszrama CreditAttribution: rszrama at Centarro commentedTested locally after updating to -dev to get the related fix, and this is workin' good for me! \o/
Comment #35
mglaman🥳 Committed, take that Views!