Tax crashes if the customer profile has no address [#3009618]

Problem/Motivation
I am getting an error on the order screen when trying to sort orders. I have a bunch of orders that were migrated from a different platform and not all of the orders in draft have addresses.

TypeError: Argument 1 passed to Drupal\commerce_tax\TaxZone::match() must implement interface CommerceGuys\Addressing\AddressInterface, null given, called in /var/www/html/drupal/web/modules/contrib/commerce/modules/tax/src/Plugin/Commerce/TaxType/LocalTaxTypeBase.php on line 254 in Drupal\commerce_tax\TaxZone->match() (line 138 of /var/www/html/drupal/web/modules/contrib/commerce/modules/tax/src/TaxZone.php) #0 /var/www/html/drupal/web/modules/contrib/commerce/modules/tax/src/Plugin/Commerce/TaxType/LocalTaxTypeBase.php(254): Drupal\commerce_tax\TaxZone->match(NULL) #1 /var/www/html/drupal/web/modules/contrib/commerce/modules/tax/src/Plugin/Commerce/TaxType/LocalTaxTypeBase.php(229): Drupal\commerce_tax\Plugin\Commerce\TaxType\LocalTaxTypeBase->resolveZones(Object(Drupal\commerce_order\Entity\OrderItem), Object(Drupal\profile\Entity\Profile)) #2 /var/www/html/drupal/web/modules/contrib/commerce/modules/tax/src/Plugin/Commerce/TaxType/LocalTaxTypeBase.php(112): Drupal\commerce_tax\Plugin\Commerce\TaxType\LocalTaxTypeBase->resolveRates(Object(Drupal\commerce_order\Entity\OrderItem), Object(Drupal\profile\Entity\Profile)) #3 /var/www/html/drupal/web/modules/contrib/commerce/modules/tax/src/TaxOrderProcessor.php(85): Drupal\commerce_tax\Plugin\Commerce\TaxType\LocalTaxTypeBase->apply(Object(Drupal\commerce_order\Entity\Order)) #4 /var/www/html/drupal/web/modules/contrib/commerce/modules/order/src/OrderRefresh.php(155): Drupal\commerce_tax\TaxOrderProcessor->process(Object(Drupal\commerce_order\Entity\Order)) #5 /var/www/html/drupal/web/modules/contrib/commerce/modules/order/src/OrderStorage.php(99): Drupal\commerce_order\OrderRefresh->refresh(Object(Drupal\commerce_order\Entity\Order)) #6 /var/www/html/drupal/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php(426): Drupal\commerce_order\OrderStorage->doPreSave(Object(Drupal\commerce_order\Entity\Order)) #7 /var/www/html/drupal/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php(774): Drupal\Core\Entity\EntityStorageBase->save(Object(Drupal\commerce_order\Entity\Order)) #8 /var/www/html/drupal/web/core/lib/Drupal/Core/Entity/Entity.php(390): Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object(Drupal\commerce_order\Entity\Order)) #9 /var/www/html/drupal/web/modules/contrib/commerce/modules/order/src/OrderStorage.php(133): Drupal\Core\Entity\Entity->save() #10 /var/www/html/drupal/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php(286): Drupal\commerce_order\OrderStorage->postLoad(Array) #11 /var/www/html/drupal/web/core/modules/views/src/Plugin/views/query/Sql.php(1610): Drupal\Core\Entity\EntityStorageBase->loadMultiple(Array) #12 /var/www/html/drupal/web/core/modules/views/src/Plugin/views/query/Sql.php(1535): Drupal\views\Plugin\views\query\Sql->loadEntities(Array) #13 /var/www/html/drupal/web/core/modules/views/src/ViewExecutable.php(1426): Drupal\views\Plugin\views\query\Sql->execute(Object(Drupal\views\ViewExecutable)) #14 /var/www/html/drupal/web/core/modules/views/src/ViewExecutable.php(1454): Drupal\views\ViewExecutable->execute(NULL) #15 /var/www/html/drupal/web/core/modules/views/src/Plugin/views/display/Page.php(183): Drupal\views\ViewExecutable->render() #16 /var/www/html/drupal/web/core/modules/views/src/ViewExecutable.php(1630): Drupal\views\Plugin\views\display\Page->execute() #17 /var/www/html/drupal/web/core/modules/views/src/Element/View.php(77): Drupal\views\ViewExecutable->executeDisplay('page_1', Array) #18 [internal function]: Drupal\views\Element\View::preRenderViewElement(Array) #19 /var/www/html/drupal/web/core/lib/Drupal/Core/Render/Renderer.php(378): call_user_func(Array, Array) #20 /var/www/html/drupal/web/core/lib/Drupal/Core/Render/Renderer.php(195): Drupal\Core\Render\Renderer->doRender(Array, false) #21 /var/www/html/drupal/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(226): Drupal\Core\Render\Renderer->render(Array, false) #22 /var/www/html/drupal/web/core/lib/Drupal/Core/Render/Renderer.php(582): Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() #23 /var/www/html/drupal/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(227): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure)) #24 /var/www/html/drupal/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(117): Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch)) #25 /var/www/html/drupal/web/core/lib/Drupal/Core/EventSubscriber/MainContentViewSubscriber.php(90): Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch)) #26 [internal function]: Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object(Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent), 'kernel.view', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher)) #27 /var/www/html/drupal/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(111): call_user_func(Array, Object(Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent), 'kernel.view', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher)) #28 /var/www/html/drupal/vendor/symfony/http-kernel/HttpKernel.php(156): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object(Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent)) #29 /var/www/html/drupal/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1) #30 /var/www/html/drupal/web/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #31 /var/www/html/drupal/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #32 /var/www/html/drupal/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(99): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #33 /var/www/html/drupal/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(78): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true) #34 /var/www/html/drupal/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #35 /var/www/html/drupal/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #36 /var/www/html/drupal/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #37 /var/www/html/drupal/web/core/lib/Drupal/Core/DrupalKernel.php(665): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #38 /var/www/html/drupal/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request)) #39 {main}.

Proposed resolution
Just check if there is a customer address before trying to resolve zones to them.

Attaching patch for the work.

Thanks

Comment	File	Size	Author
#21	3009618-21-fix-tax-crash-with-broken-profile.patch	3.73 KB	bojanz
#16	3009618-resolve-customer-profile-null-if-no-address-15.diff	1.03 KB	dobe
#10	3009618-resolve-customer-profile-null-if-no-address-10.diff	1 KB	dobe
#9	3009618-resolve-customer-profile-null-if-no-address-8.diff	1.48 KB	dobe
#7	3009618-resolve-customer-profile-null-if-no-address-5.diff	991 bytes	dobe
#2	3009618-do-not-match-empty-address-1.diff	956 bytes	dobe

Comments

Comment #1

26 October 2018 at 16:44

dobe created an issue. See original summary.

Comment #2

dobe commented 26 October 2018 at 16:53

Status	File	Size
new	3009618-do-not-match-empty-address-1.diff	956 bytes

Comment #3

dobe commented 26 October 2018 at 17:03

Status:

Active

» Closed (duplicate)

Address variable should be optional in TaxZone::match() (2925470) Is a better solution.

Comment #4

mglaman

English

WI, USA

commented 26 October 2018 at 17:37

Status:

Closed (duplicate)

» Active

Let's work on this here instead of the other issue. We need to see why orders without addresses are being passed.

Comment #5

mglaman

English

WI, USA

commented 26 October 2018 at 17:42

See: https://github.com/drupalcommerce/commerce/blob/8.x-2.x/modules/tax/src/...

    foreach ($order->getItems() as $order_item) {
      $customer_profile = $this->resolveCustomerProfile($order_item);
      if (!$customer_profile) {
        continue;
      }
      $rates = $this->resolveRates($order_item, $customer_profile);

So you have a billing profile with an empty address. Tax assumes that if a profile exists it will have an address.

Comment #6

mglaman

English

WI, USA

commented 26 October 2018 at 17:44

resolveCustomerProfile needs to verify the address is not empty.

  /**
   * Resolves the customer profile for the given order item.
   *
   * @param \Drupal\commerce_order\Entity\OrderItemInterface $order_item
   *   The order item.
   *
   * @return \Drupal\profile\Entity\ProfileInterface|null
   *   The customer profile, or NULL if not yet known.
   */
  protected function resolveCustomerProfile(OrderItemInterface $order_item) {
    $order = $order_item->getOrder();
    $customer_profile = $order->getBillingProfile();
    // A shipping profile is preferred, when available.
    $event = new CustomerProfileEvent($customer_profile, $order_item);
    $this->eventDispatcher->dispatch(TaxEvents::CUSTOMER_PROFILE, $event);
    $customer_profile = $event->getCustomerProfile();
    if (!$customer_profile && $this->isDisplayInclusive()) {
      // The customer is still unknown, but prices are displayed tax-inclusive
      // (VAT scenario), better to show the store's default tax than nothing.
      $customer_profile = $this->buildStoreProfile($order->getStore());
    }
    return $customer_profile;
  }

Comment #7

dobe commented 26 October 2018 at 18:00

Status:

Active

» Needs review

Status	File	Size
new	3009618-resolve-customer-profile-null-if-no-address-5.diff	991 bytes

Based on conversation with @mglaman. Here is a patch.

Comment #8

mglaman

English

WI, USA

commented 26 October 2018 at 18:04

Issue tags:

+Needs tests

+1 thank you! We have missing test coverage here that we should add.

Comment #9

dobe commented 26 October 2018 at 18:13

Issue tags:

-Needs tests

Status	File	Size
new	3009618-resolve-customer-profile-null-if-no-address-8.diff	1.48 KB

Was running into same issue with other patch. Even though there was a store and the store has an address.

Comment #10

dobe commented 26 October 2018 at 18:14

Status	File	Size
new	3009618-resolve-customer-profile-null-if-no-address-10.diff	1 KB

This one is better. Had some syntax from other patch.

Comment #11

26 October 2018 at 18:31

The last submitted patch, 7: 3009618-resolve-customer-profile-null-if-no-address-5.diff, failed testing. View results

Comment #12

26 October 2018 at 18:41

The last submitted patch, 9: 3009618-resolve-customer-profile-null-if-no-address-8.diff, failed testing. View results

Comment #13

26 October 2018 at 18:42

Status:

Needs review

» Needs work

The last submitted patch, 10: 3009618-resolve-customer-profile-null-if-no-address-10.diff, failed testing. View results

Comment #14

mglaman

English

WI, USA

commented 26 October 2018 at 19:04

Status:

Needs work

» Needs review

Unrelated test fails

Comment #15

26 October 2018 at 19:30

The last submitted patch, 2: 3009618-do-not-match-empty-address-1.diff, failed testing. View results

Comment #16

dobe commented 26 October 2018 at 22:19

Status	File	Size
new	3009618-resolve-customer-profile-null-if-no-address-15.diff	1.03 KB

Fixing if $customer_profile is already null.

Comment #17

26 October 2018 at 22:49

Status:

Needs review

» Needs work

The last submitted patch, 16: 3009618-resolve-customer-profile-null-if-no-address-15.diff, failed testing. View results

Comment #18

facine commented 30 April 2019 at 13:27

Status:

Needs work

» Reviewed & tested by the community

#16 works fine!

Comment #19

mglaman

English

WI, USA

commented 30 April 2019 at 14:29

Queued some tests to see what's up, looks like a random failure.

Comment #20

bojanz commented 21 May 2019 at 11:53

Title:	LocalTax resolving Zones without Address	» Tax crashes if the customer profile has no address
Status:	Reviewed & tested by the community	» Needs work

A bug fix can't be RTBC if it has no test.

Wouldn't it make more sense to perform the check before the store profile defaulting, so that if the current profile has no address, we calculate tax with the store address?

"If the customer profile has no address, calculate tax with the store address" sounds like a better rule than "If the customer profile has no address, calculate no tax'.

Comment #21

bojanz commented 21 May 2019 at 12:15

Status:

Needs work

» Needs review

Status	File	Size
new	3009618-21-fix-tax-crash-with-broken-profile.patch	3.73 KB

How about this?

Comment #22

21 May 2019 at 17:44

bojanz committed 823e3e5 on 8.x-2.x

Issue #3009618 by dobe, bojanz, mglaman: Tax crashes if the customer...

Comment #23

bojanz commented 21 May 2019 at 17:44

Status:

Needs review

» Fixed

Committed #21. Thanks, everyone!

Comment #24

4 June 2019 at 17:44

Status:

Fixed

» Closed (fixed)

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

Tax crashes if the customer profile has no address

Comments

Comment #1

Comment #2

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

Comment #8

Comment #9

Comment #10

Comment #11

Comment #12

Comment #13

Comment #14

Comment #15

Comment #16

Comment #17

Comment #18

Comment #19

Comment #20

Comment #21

Comment #22

Comment #23

Comment #24

Related issues

News items

Our community

Documentation

Drupal code base

Governance of community