Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid issues with missing variations. #13

Open
wants to merge 1 commit into
base: 8.x-1.x
Choose a base branch
from

Conversation

grahl
Copy link
Contributor

@grahl grahl commented Nov 14, 2018

Hello

I have a case where some product data can become inconsistent due to long running import jobs from the ERP. This can lead to a missing product variation. This function causes a fatal error in that case and the following change catches it.

@WengerK
Copy link
Collaborator

WengerK commented Nov 14, 2018

Instead of altering buildProductFromProductVariation, can we prevent to call this method when no product variation exists ? Before ?

Which was the parent calling buildProductFromProductVariation ? Was is the stacktrace ?

@wanze
Copy link
Contributor

wanze commented Nov 15, 2018

@grahl

Do you know when the exception occurs? I guess it happened when viewing a commerce_product entity without a default variation? In this case, the check should go here:

https://github.com/gridonic/commerce_google_tag_manager/blob/8.x-1.x/commerce_google_tag_manager.module#L39

I would also add the check for a missing reference a higher level - not entering the method buildProductFromProductVariation at all.

@grahl
Copy link
Contributor Author

grahl commented Nov 15, 2018

Hi @wanze

I noticed this on a product detail page with an invalid variation, not quite sure if the the field there was the "add to cart" formatter or the entity renderer.

I only did a cursory check where
Both methods calling buildProductFromProductVariation() was being called and since both instances didn't look like a great place to check it I went further down. Though I understand your criticism.

Time permitting I'll try to recreate this and send the stacktrace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants