Contrib.social

Ensure all calls to price getters handle NULL returns

Open on Drupal.org →
Created on 7 September 2018, almost 7 years ago
Updated 5 December 2022, over 2 years ago

Problem/Motivation

The various price getters (orders, order items) may validly return NULL (to handle that we may not know currency etc). All code calling those methods should not assume that a Price will be returned.

Proposed resolution

Fix cases that assume a Price will be returned.

Ideally see if we can get some form of static analysis in test coverage to ensure we don't get future additions.

User interface changes

None.

API changes

None.

Data model changes

None.

Original report by nikita_tt

Here is actual method:

  /**
   * {@inheritdoc}
   */
  public function isVisible() {
    if ($this->order->getTotalPrice()->isZero()) {
      // Hide the pane for free orders, since they don't need a payment.
      return FALSE;
    }
    $payment_info_pane = $this->checkoutFlow->getPane('payment_information');
    if (!$payment_info_pane->isVisible() || $payment_info_pane->getStepId() == '_disabled') {
      // Hide the pane if the PaymentInformation pane has been disabled.
      return FALSE;
    }

    return TRUE;
  }

But sometimes $this->order->getTotalPrice() could be empty. See \Drupal\commerce_order\Entity\Order::getTotalPrice().

This can happen when an Order has been processed through Availability Checker and all order items were deleted.

🐛 Bug report
Status

Needs review

Version

2.0

Component

Payment

Created by

🇺🇦Ukraine nikita_tt

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update about 2 years ago
    Patch Failed to Apply
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update about 2 years ago
    Patch Failed to Apply
  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update about 2 years ago
    run-tests.sh fatal error
  • @mrdalesmith opened merge request.
  • Comment about 2 years ago →
    🇬🇧United Kingdom MrDaleSmith

    Patch no longer applies, so I rerolled in a fork and opened an MR

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update about 2 years ago
    run-tests.sh fatal error
  • Comment about 2 years ago →
    🇬🇧United Kingdom MrDaleSmith

    Rebased against 8.x-2.x branch

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update almost 2 years ago
    run-tests.sh fatal error
  • Comment almost 2 years ago →
    🇬🇧United Kingdom MrDaleSmith

    Attempting to reroll again

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update over 1 year ago
    run-tests.sh fatal error
  • Comment over 1 year ago →
    🇨🇾Cyprus alex.bukach

    Re-rolled #18 against latest 8.x-2.x.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update over 1 year ago
    787 pass
  • Comment over 1 year ago →
    🇨🇾Cyprus alex.bukach
  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇮🇱Israel jsacksick

    Duplicating needsPayment() doesn't feel right, this should be moved to CheckoutPaneBase.

  • Comment about 2 months ago →
    🇧🇪Belgium aren33k
  • Merge request !444Rerolled patch. #2998065 @MrDaleSmith → (Open) created by Unnamed author
  • Pipeline finished with Failed
    about 2 months ago
    Total: 157s
    #496840
  • Pipeline finished with Failed
    about 2 months ago
    Total: 163s
    #496881
  • Comment 17 days ago →
    🇩🇪Germany Anybody Porta Westfalica

    Just ran into this once again, here's the backtrace:

    Error: Call to a member function isZero() on null in Drupal\commerce_payment\Plugin\Commerce\CheckoutPane\PaymentProcess->isVisible() (line 118 of /web/modules/contrib/commerce/modules/payment/src/Plugin/Commerce/CheckoutPane/PaymentProcess.php) #0 /web/modules/contrib/commerce/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowWithPanesBase..php(113): Drupal\commerce_payment\Plugin\Commerce\CheckoutPane\PaymentProcess->isVisible()
#1 [internal function]: Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowWithPanesBase->Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\{closure}()
#2 /web/modules/contrib/commerce/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowWithPanesBase.php(111): array_filter()
#3 /web/modules/contrib/commerce/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowWithPanesBase.php(132): Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowWithPanesBase->getVisiblePanes()
#4 /web/modules/contrib/commerce/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php(275): Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowWithPanesBase->isStepVisible()
#5 /web/modules/contrib/commerce/modules/checkout/src/Plugin/Block/CheckoutProgressBlock.php(93): Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowBase->getVisibleSteps()
#6 /web/core/modules/block/src/BlockViewBuilder.php(171): Drupal\commerce_checkout\Plugin\Block\CheckoutProgressBlock->build()
#7 [internal function]: Drupal\block\BlockViewBuilder::preRender()
#8 /web/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php(113): call_user_func_array()
#9 /web/core/lib/Drupal/Core/Render/Renderer.php(870): Drupal\Core\Render\Renderer->doTrustedCallback()
#10 /web/core/lib/Drupal/Core/Render/Renderer.php(432): Drupal\Core\Render\Renderer->doCallback()
#11 /web/core/lib/Drupal/Core/Render/Renderer.php(248): Drupal\Core\Render\Renderer->doRender()
#12 /web/core/lib/Drupal/Core/Render/Renderer.php(165): Drupal\Core\Render\Renderer->render()
#13 /web/core/lib/Drupal/Core/Render/Renderer.php(638): Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}()
#14 /web/core/lib/Drupal/Core/Render/Renderer.php(164): Drupal\Core\Render\Renderer->executeInRenderContext()
#15 /web/core/lib/Drupal/Core/Render/Renderer.php(191): Drupal\Core\Render\Renderer->renderInIsolation()
#16 /web/core/lib/Drupal/Core/Render/Renderer.php(228): Drupal\Core\Render\Renderer->doRenderPlaceholder()
#17 /web/core/modules/big_pipe/src/Render/BigPipe.php(697): Drupal\Core\Render\Renderer->renderPlaceholder()
#18 /web/core/modules/big_pipe/src/Render/BigPipe.php(524): Drupal\big_pipe\Render\BigPipe->renderPlaceholder()
#19 [internal function]: Drupal\big_pipe\Render\BigPipe->Drupal\big_pipe\Render\{closure}()
#20 /web/core/modules/big_pipe/src/Render/BigPipe.php(531): Fiber->start()
#21 /web/core/modules/big_pipe/src/Render/BigPipe.php(283): Drupal\big_pipe\Render\BigPipe->sendPlaceholders()
#22 /web/core/modules/big_pipe/src/Render/BigPipeResponse.php(113): Drupal\big_pipe\Render\BigPipe->sendContent()
#23 /vendor/symfony/http-foundation/Response.php(423): Drupal\big_pipe\Render\BigPipeResponse->sendContent()
#24 /web/index.php(20): Symfony\Component\HttpFoundation\Response->send()
#25 {main}.
#0 /web/modules/contrib/commerce/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowWithPanesBase.php(113): Drupal\commerce_payment\Plugin\Commerce\CheckoutPane\PaymentProcess->isVisible()
#1 [internal function]: Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowWithPanesBase->Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\{closure}()
#2 /web/modules/contrib/commerce/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowWithPanesBase.php(111): array_filter()
#3 /web/modules/contrib/commerce/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowWithPanesBase.php(132): Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowWithPanesBase->getVisiblePanes()
#4 /web/modules/contrib/commerce/modules/checkout/src/Plugin/Commerce/CheckoutFlow/CheckoutFlowBase.php(275): Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowWithPanesBase->isStepVisible()
#5 /web/modules/contrib/commerce/modules/checkout/src/Plugin/Block/CheckoutProgressBlock.php(93): Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowBase->getVisibleSteps()
#6 /web/core/modules/block/src/BlockViewBuilder.php(171): Drupal\commerce_checkout\Plugin\Block\CheckoutProgressBlock->build()
#7 [internal function]: Drupal\block\BlockViewBuilder::preRender()
#8 /web/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php(113): call_user_func_array()
#9 /web/core/lib/Drupal/Core/Render/Renderer.php(870): Drupal\Core\Render\Renderer->doTrustedCallback()
#10 /web/core/lib/Drupal/Core/Render/Renderer.php(432): Drupal\Core\Render\Renderer->doCallback()
#11 /web/core/lib/Drupal/Core/Render/Renderer.php(248): Drupal\Core\Render\Renderer->doRender()
#12 /web/core/lib/Drupal/Core/Render/Renderer.php(165): Drupal\Core\Render\Renderer->render()
#13 /web/core/lib/Drupal/Core/Render/Renderer.php(638): Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}()
#14 /web/core/lib/Drupal/Core/Render/Renderer.php(164): Drupal\Core\Render\Renderer->executeInRenderContext()
#15 /web/core/lib/Drupal/Core/Render/Renderer.php(191): Drupal\Core\Render\Renderer->renderInIsolation()
#16 /web/core/lib/Drupal/Core/Render/Renderer.php(228): Drupal\Core\Render\Renderer->doRenderPlaceholder()
#17 /web/core/modules/big_pipe/src/Render/BigPipe.php(697): Drupal\Core\Render\Renderer->renderPlaceholder()
#18 /web/core/modules/big_pipe/src/Render/BigPipe.php(524): Drupal\big_pipe\Render\BigPipe->renderPlaceholder()
#19 [internal function]: Drupal\big_pipe\Render\BigPipe->Drupal\big_pipe\Render\{closure}()
#20 /web/core/modules/big_pipe/src/Render/BigPipe.php(531): Fiber->start()
#21 /web/core/modules/big_pipe/src/Render/BigPipe.php(283): Drupal\big_pipe\Render\BigPipe->sendPlaceholders()
#22 /web/core/modules/big_pipe/src/Render/BigPipeResponse.php(113): Drupal\big_pipe\Render\BigPipe->sendContent()
#23 /vendor/symfony/http-foundation/Response.php(423): Drupal\big_pipe\Render\BigPipeResponse->sendContent()
#24 /web/index.php(20): Symfony\Component\HttpFoundation\Response->send()
#25 {main}

    All modules and core are up to date, so this issue clearly still exists.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024