Hello,

I tried adding a flag to a Commerce Product Variation. Flag module allows that but after creating that flag the products can't be viewed. The error is:

DomainException: When a #lazy_builder callback is specified, no properties can exist; all properties must be generated by the #lazy_builder callback. You specified the following properties: #attributes, #ajax_replace_class, #type, #optional, #process, #pre_render, #theme_wrappers, #defaults_loaded. in Drupal\Core\Render\Renderer->doRender() (line 335 of /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Render/Renderer.php).
Severity 	

Second part of original post (but unrelated to this issue?)

Also, if you try to delete that flag you get another crash with the following error:

TypeError: Argument 1 passed to Drupal\Core\Entity\Sql\DefaultTableMapping::requiresDedicatedTableStorage() must implement interface Drupal\Core\Field\FieldStorageDefinitionInterface, null given, called in /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php on line 331 in Drupal\Core\Entity\Sql\DefaultTableMapping->requiresDedicatedTableStorage() (line 512 of /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php)

#0 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php(331): Drupal\Core\Entity\Sql\DefaultTableMapping->requiresDedicatedTableStorage(NULL)
#1 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/Query/Sql/Tables.php(444): Drupal\Core\Entity\Sql\DefaultTableMapping->getAllColumns('flagging')
#2 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/Query/Sql/Tables.php(223): Drupal\Core\Entity\Query\Sql\Tables->getTableMapping('flagging', 'flagging')
#3 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/Query/Sql/Condition.php(52): Drupal\Core\Entity\Query\Sql\Tables->addField('entity_type', 'INNER', NULL)
#4 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/Query/Sql/Query.php(172): Drupal\Core\Entity\Query\Sql\Condition->compile(Object(Drupal\Core\Database\Driver\mysql\Select))
#5 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/Query/Sql/Query.php(80): Drupal\Core\Entity\Query\Sql\Query->compile()
#6 /opt/bitnami/apps/drupal/htdocs/modules/contrib/flag/src/FlagService.php(323): Drupal\Core\Entity\Query\Sql\Query->execute()
#7 /opt/bitnami/apps/drupal/htdocs/modules/contrib/flag/flag.module(527): Drupal\flag\FlagService->unflagAllByEntity(Object(Drupal\system\Entity\Action))
#8 [internal function]: flag_entity_predelete(Object(Drupal\system\Entity\Action), 'action')
#9 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Extension/ModuleHandler.php(403): call_user_func_array('flag_entity_pre...', Array)
#10 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php(349): Drupal\Core\Extension\ModuleHandler->invokeAll('entity_predelet...', Array)
#11 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/EntityStorageBase.php(424): Drupal\Core\Config\Entity\ConfigEntityStorage->invokeHook('predelete', Object(Drupal\system\Entity\Action))
#12 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/EntityBase.php(403): Drupal\Core\Entity\EntityStorageBase->delete(Array)
#13 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php(572): Drupal\Core\Entity\EntityBase->delete()
#14 /opt/bitnami/apps/drupal/htdocs/modules/contrib/flag/src/Entity/Flag.php(491): Drupal\Core\Config\Entity\ConfigEntityBase::preDelete(Object(Drupal\Core\Config\Entity\ConfigEntityStorage), Array)
#15 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/EntityStorageBase.php(422): Drupal\flag\Entity\Flag::preDelete(Object(Drupal\Core\Config\Entity\ConfigEntityStorage), Array)
#16 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/EntityBase.php(403): Drupal\Core\Entity\EntityStorageBase->delete(Array)
#17 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Entity/EntityDeleteFormTrait.php(122): Drupal\Core\Entity\EntityBase->delete()
#18 [internal function]: Drupal\Core\Entity\EntityDeleteForm->submitForm(Array, Object(Drupal\Core\Form\FormState))
#19 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Form/FormSubmitter.php(114): call_user_func_array(Array, Array)
#20 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Form/FormSubmitter.php(52): Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object(Drupal\Core\Form\FormState))
#21 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Form/FormBuilder.php(591): Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object(Drupal\Core\Form\FormState))
#22 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Form/FormBuilder.php(320): Drupal\Core\Form\FormBuilder->processForm('flag_delete_for...', Array, Object(Drupal\Core\Form\FormState))
#23 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Controller/FormController.php(91): Drupal\Core\Form\FormBuilder->buildForm(Object(Drupal\Core\Entity\EntityDeleteForm), Object(Drupal\Core\Form\FormState))
#24 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch))
#25 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)
#26 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/Render/Renderer.php(573): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#27 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#28 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array)
#29 /opt/bitnami/apps/drupal/vendor/symfony/http-kernel/HttpKernel.php(151): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#30 /opt/bitnami/apps/drupal/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#31 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#32 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#33 /opt/bitnami/apps/drupal/htdocs/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#34 /opt/bitnami/apps/drupal/htdocs/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#35 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#36 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#37 /opt/bitnami/apps/drupal/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#38 /opt/bitnami/apps/drupal/htdocs/core/lib/Drupal/Core/DrupalKernel.php(694): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#39 /opt/bitnami/apps/drupal/htdocs/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#40 {main}

You can't also uninstall the module after that. The only solution is removing the config object for that flag.

I know it's unusual to add a flag to a commerce product variation, but I'm doing that on a decoupled project.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

introfini created an issue. See original summary.

tarikflz’s picture

Facing with same issue any fix ?

localnetwork’s picture

Having this issue too.

jcontreras’s picture

My work around if it helps anyone.
So the problem it seems, is that in the array of the flag field there is an "#attributes".. I took it out of the array and not it works.

Because I was using twig to print it out I used:

{% set flag_add_to_wishlist = flag_add_to_wishlist | filter((v, k) => k != '#attributes') %}
{{ flag_add_to_wishlist }}

if you are not using twig you will need to take out out via hooks in the them.module or some custom module.

Neograph734’s picture

Title: Crash when Flags are added to Commerce Product Variations » ProductVariationFieldRenderer's AJAXifiying breaks lazy_built fields (such as flags)
Project: Flag » Commerce Core
Version: 8.x-4.0-beta1 » 8.x-2.x-dev
Component: Flag core » Product
Issue summary: View changes

I ran into this is well, but flag never sets the #attributes property in the render array. This is actually a commerce issue.

More specifically, it is caused by using the Inject product variation fields into the rendered product. option. That option makes that every product variation field (so also a flag field) is passed to ProductVariationFieldRenderer.

This class has a function prepareForAjax() that adds the #attributes and #ajax_replace_class for updating the fields with AJAX once the production variation options are changed.

Moving this issue to the commerce issue queue and changing the title. Not sure on how to proceed, perhaps prepareForAjax could append a wrapper class instead of fiddling with the original rendered field?

Neograph734’s picture

Issue summary: View changes
Neograph734’s picture

This should resolve the problem.

Status: Needs review » Needs work

The last submitted patch, 7: 3120117-7-ProductVariationFieldRenderer-and-lazy_builders.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Neograph734’s picture

Some cleaning up and updated the test. ProductVariationFieldRendererTest should pass now.

The promotion tests are unrelated and can be seen in other issues' tests too.

jsacksick’s picture

Status: Needs review » Fixed

Thank you! Committed!

jsacksick’s picture

This introduced a regression that I should have spotted out before committing... Will most probably have to revert that... See #3191798: First item of a product variation image field not rendered.

jsacksick’s picture

Status: Fixed » Needs work

Reverted the fix, setting this back to "needs work".

  • jsacksick committed 18ddf4e on 8.x-2.x
    Revert "Issue #3120117 by Neograph734: ProductVariationFieldRenderer's...
Neograph734’s picture

Ah, that is unfortunate... I did not think of it either because I did not use twig to render the fields...

It will make finding a solution more difficult though. Theoretically we could only apply the wrapper when a lazy built field is found, but that would result in different treatment of fields and I am not sure if that is desirable. (Or even understandable at twig level.)

Any other ideas?

I am curious if product.variation_*.rendered_field.0 would have worked and (with proper communication) would be a viable option?

jsacksick’s picture

I am curious if product.variation_*.rendered_field.0 would have worked and (with proper communication) would be a viable option?

This would have worked... But there are probably thousands of sites who would need to update their Twig templates, so I don't think this is a viable option.

jsacksick’s picture

what about skipping altering the array in prepareForAjax() if we detect that a field is lazy built? (If we can easily detect that?

Neograph734’s picture

what about skipping altering the array in prepareForAjax() if we detect that a field is lazy built? (If we can easily detect that?

It should have a #lazy_builder property (flag has), I am not sure if there are other ways?

But this would make that this specific variation field would not update when you change an attribute...

jsacksick’s picture

I guess our only option in this case is what you've suggested (i.e wrap only lazy built fields), which kind of sucks but I don't have a better idea at this point.

Neograph734’s picture

I suppose it becomes something like this then. But commerce does not have any lazy built fields suitable for testing, so this currently has no test coverage anymore...

UPDATE
The most elegant solution I could think of is to make the prepareForAjax method a static one and then also calling if from an alter hook or event that would fire after a lazy built content has been generated. But I do not believe such an event exists... So then we would have to hook into a very generic build_alter hook and see if the element came from a lazy builder (contains #lazy_builder_built = TRUE.
But then how would we know this is a product variation field and not something else...

Neograph734’s picture

I just ran into another issue with the original approach:

if (!Element::children($rendered_field)) {
  $rendered_field['#type'] = 'container';
}

This will also alter EVA fields (and possible other views) added to the variation, causing them not to render. A view does not have children because all its properties start with a hash. So instead of '#type' => 'view', the original approach now changes this into '#type' => 'container' which obviously does not render a view anymore.

This can not be solved by only wrapping lazy built fields anymore, more fields will need to be wrapped. But also #12 needs to be avoided.

Attaching a patch, but this is nowhere near a solution and possible other issues might arise in the future.

Status: Needs review » Needs work
Neograph734’s picture

Also inclusing the weight to properly inherit the positions chosen in the Display mode.

mgstables’s picture

The patch #23 works fine for me.
Although I couldn't patch it with Composer.

With this patch, the Flag module worked fine on Productvariation. Even on the productpage display.

Drupal 9.3.0
Commerce Core 8.x-2.28
Flag 8.x-4.0-beta3

jsacksick’s picture

@Neograph734: I'm a bit confused by your comment in #21, can we commit #23 or not? If this alter only Lazy built fields, considering that the current situation is a WSOD, I think we can live with the workaround, what do you think?

Neograph734’s picture

@jsacksick, the original commerce approach is to check if the render element has any children and then change the type:

if (!Element::children($rendered_field)) {
  $rendered_field['#type'] = 'container';
}

A child is considered to be any propoerty that does not start with a hash (#). When adding EVA views to product variations, their render array consists of only hashed properties. Effectively the above code will then replace ['#type'] = 'view' with ['#type'] = 'container' and the whole view won't render anymore. As discovered in #21.

So rather than checking for lazy built fields only, we should then include an exception for views (patch #23) and I don't know how many other types that do not contain any children. I believe #23 is safe to commit, but at the risk of more exceptions having to be added later.

Neograph734’s picture

Version: 8.x-2.x-dev » 3.0.x-dev

@jsacksick, is this a candidate for follow-up in 3.x? We've had the regression in #12. But a new major could make this less of an issue?

I still believe that the solution from #9 to always add a wrapper would avoid most of the concerns mentioned in the later comments, which were mere workarounds.
Looking at it, I only doubt if the name 'rendered_field' makes sense. Perhaps just use array key 0 instead, so that the impact is limited to a minimal. Key 0 should show all fields in case of multi value and $field[0][0] (adding a second [0]) should again show the first.

What are your thoughts?

Neograph734’s picture

Setting NR status to consider the patch from #9 for Commerce 3.x.

jsacksick’s picture

@Neograph734: Even though we're technically allowed to do breaking changes in a major branch. I think this is too annoying to be committed as is, just for the purpose of supporting lazy built fields... I'd rather still commit the workaround for these specific fields... If it's working and not too hacking... So perhaps the patch from #23?

Neograph734’s picture

@jsacksick The patch from #23 is what I am using now and it works good for me with flags and views (via EVA module) being added. For now these are the troublemakers I have ran into.

To summarize the proposal, regular fields will remain as-is and can be rendered with {{product.variation_field_product_image}}. A single value is available via {{product.variation_field_product_image.0}}. Lazy built fields and views will be wrapped in an extra container div and will be rendered with {{product.variation_field_product_image.rendered_field}} or a single value (does that even make sense for such fields?) with {{product.variation_field_product_image.rendered_field.0}}
Taking the suggestion from #27 into account these will respectively become {{product.variation_field_product_image.0}} and {{product.variation_field_product_image.0.0}}.

I am ok with anything and would be happy to see an out-of-the-box solution. I am only wondering if it is logical for themers.

Neograph734’s picture

Status: Needs work » Reviewed & tested by the community

Changing to RTBC (I know I should not) but @jsacksick mentioned multiple times he can live with the current workaround. In #24 mgstables also RTBC'd. I think this will not get any better.

Neograph734’s picture

Status: Reviewed & tested by the community » Needs work
Neograph734’s picture

Great, the new tests helped uncovering a logic flaw. I assume the type for empty elements was not set correctly causing them not to render.

Neograph734’s picture

Status: Needs review » Reviewed & tested by the community

Setting RTBC as per #31.

  • jsacksick committed 1a8736a on 8.x-2.x authored by Neograph734
    Issue #3120117 by Neograph734, jsacksick: ProductVariationFieldRenderer'...

  • jsacksick committed 53a7164 on 3.0.x authored by Neograph734
    Issue #3120117 by Neograph734, jsacksick: ProductVariationFieldRenderer'...
jsacksick’s picture

Status: Reviewed & tested by the community » Fixed

Committed, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.