Problem/Motivation

The custom filter for product attribute values, commerce_product_field_views_data_alter, causes a fatal error when the attribute field storage exists but the field instance does not exist. This situation will occur during a migration because the field storage and the field instance are two different migrations so it would be very helpful to check that the field instance exists before using it.

$ drush cr
PHP Fatal error:  Uncaught Error: Call to a member function getSetting() on null in /opt/sites/d8/modules/contrib/commerce/modules/product/commerce_product.views.inc:28
Stack trace:
#0 /opt/sites/d8/core/lib/Drupal/Core/Extension/ModuleHandler.php(539): commerce_product_field_views_data_alter(Array, Object(Drupal\field\Entity\FieldStorageConfig), NULL)
#1 /opt/sites/d8/core/modules/views/views.views.inc(187): Drupal\Core\Extension\ModuleHandler->alter('field_views_dat...', Array, Object(Drupal\field\Entity\FieldStorageConfig))
#2 [internal function]: views_views_data()
#3 /opt/sites/d8/core/lib/Drupal/Core/Extension/ModuleHandler.php(391): call_user_func_array('views_views_dat...', Array)
#4 /opt/sites/d8/core/modules/views/src/ViewsData.php(245): Drupal\Core\Extension\ModuleHandler->invoke('views', 'views_data')
#5 /opt/sites/d8/core/modules/views/src/ViewsData.php(162): Drupal\views\ViewsData->getData()
#6 /opt/sites/d8/core/modules/views/src/Plugin/Derivative/ViewsEntityRow.php(91): Drupal\views\ViewsData->get('block_conten in /opt/sites/d8/modules/contrib/commerce/modules/product/commerce_product.views.inc on line 28

Proposed resolution

Check that the field exists before using it.
Maybe throw an error with a nice message if it does not exist.

Remaining tasks

CommentFileSizeAuthor
#2 2944310-2.patch1.61 KBquietone
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
FileSize
2944310-2.patch1.61 KB

I do this to workaround the fatal.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Active » Needs review

Missed this because of the status.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs review » Needs work

In this situation adding an if () like this introduces too much nesting. It makes more sense to introduce a guard clause (early return), with an explanation about when it's going to be triggered.

  • bojanz committed ac9f16b on 8.x-2.x authored by quietone 
    Issue #2944310 by quietone, bojanz: Fatal error from views_data_alter...
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs work » Fixed

Done. Thanks!

Status: Fixed » Closed (fixed)

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