- Issue created by @harismalik
- Merge request !1Issue #3423688 by HarisMalik: Enable media parent entity links for commerce products. → (Open) created by harismalik
- 🇩🇪Germany stefan.korn Jossgrund
@harismalik. Thanks for pointing this out. Interesting problem. I tried it out too.
First of all, I suppose the steps to reproduce would be
1. Create a commerce product variation with a media image.
5. Click on a product variation image.Because just with a product this seem to work with the canonical link.
Now looking at the product variation entity, I suppose it doesn't especially matter if the collection link template does exist. It just matters that the product variation entity does not provide a canonical link template and it would be sufficient here to just check if it is product variation entity.
So something like
if (!$parententity->isNew() && ($parententity->hasLinkTemplate('canonical') || $parententity->getEntityTypeId() === 'commerce_product_variation'))
would probably do it as well.But it seems a bit odd to just check for the entity type, a more generic solution would be better imho.
The product variation entity provides a method toUrl which handles the creation of a "canonical" link. So basically the product variation has a canonical link, but not provided as link template.
So for a more generic solution I am thinking if
if (!$parententity->isNew() && method_exists($parententity, 'toUrl'))
could do it. It works for the case given with the product variation. But at this moment I am not exactly sure if this has some undesired implications.On the other side, I am not sure why product variation does not provide a canonical link template (probably because it just uses query params to alter from product url) and whether maybe commerce could provide a canonical link template for product variation entity as well.
- Assigned to stefan.korn
- 🇩🇪Germany stefan.korn Jossgrund
There seem to be related issues. The check for "canonical" template was introduced to mitigate a problem with non linking entities (paragraphs). So there is probably an issue if just checking for method toUrl, probably any entity has this method.
The paragraph entity is throwing an exception for the canonical link template. Probably a solution would be to get away from checking the "canonical" template and just getting toUrl on the parent entity but put this in try/catch to avoid exception and just do not put out a link in case the exception is caught. That way behavior would be like before, non error on non linkable parent entities, but the toUrl would work independently whether the template is canonical or something else. That will be more flexible and would solve this issue in a generic way probably.
Thinking about this and testing, but seems like a good approach.
-
stefan.korn →
committed f717b193 on 8.x-1.x
Issue #3423688: Media entity parent link does not support Commerce...
-
stefan.korn →
committed f717b193 on 8.x-1.x
- Issue was unassigned.
- Status changed to Needs review
7 months ago 8:46pm 3 May 2024 - 🇩🇪Germany stefan.korn Jossgrund
So I have committed a more generic approach to the dev branch, that should also handle commerce product variant correctly.
On my end it is working.
Would be great if you can test it too. @harismalik
- Status changed to RTBC
3 months ago 1:13pm 15 August 2024