fix(core): Add product translation to product variant entity resolver #2644
Conversation
✅ Deploy Preview for effervescent-donut-4977b2 canceled.
|
oliverqx
changed the title
There was a problem hiding this comment.
See my comment on the resolver code. Also, do you think you'd be able to create a failing e2e test in this suite that demonstrates the bug & fix?
| return this.requestContextCache.get( | ||
| ctx, | ||
| `ProductVariantEntityResolver.product(${productVariant.productId})`, | ||
| () => this.productVariantService.translateProduct(ctx, productVariant.product), |
There was a problem hiding this comment.
I don't think we need to create a brand new method .translateProduct() for this purpose. We can reuse getProductForVariant() which is already doing the translation step.
Which then would make both branches do the same thing, which also doesn't make sense. So there must be a logical branch in which this additional lookup/translation does not need to be done - that would be if the product is already translated. This can be tested by inspecting productVariant.product.name to see if it is defined. If so, we should be ok to just return it as-is like in the original version.
Tests if productVariant entity resolves its product property properly
michaelbromley
merged commit 9289a1c
into
vendure-ecommerce:master
|
@oliverqx thank you! Can you contact me at m.bromley [at] vendure.io about the bounty payment? |
|
@michaelbromley will do, thanks! |
/claim #2171
Description
This fixes any custom fields of type LocaleString to return null on a collection query. Bug comes from product variant entity resolver not translating the product when its given productVariant argument has product already defined
productVariant gets its product property from a call to listQueryBuilder, which means its not translated.
(sorry about the double pr, wont happen again!)
Breaking changes
No breaking changes
Screenshots
You can add screenshots here if applicable.
Checklist
📌 Always:
👍 Most of the time: