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

always return numeric value in Isotope::calculatePrice #2007

Closed
wants to merge 1 commit into from

Conversation

fritzmg
Copy link
Contributor

@fritzmg fritzmg commented Jan 12, 2019

The Isotope::calculatePrice function promises to always return a float. However, right at the beginning of the function, there is the following check:

if (!is_numeric($fltPrice)) {
return $fltPrice;
}

So if an empty string was passed to the function (for example when an AttributeOption does not have a price change), the following error will occur:

ErrorException:
Warning: A non-numeric value encountered

  at vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Frontend.php:685
  at Isotope\Frontend->addOptionsPrice('729.00', object(ProductPrice), 'price', '1', array('color' => '1', 'storage' => '22', 'cover' => '19'))
     (vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Isotope.php:247)
  at Isotope\Isotope::calculatePrice('729.00', object(ProductPrice), 'price', '1', null, array('color' => '1', 'storage' => '22', 'cover' => '19'))
     (vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Model\ProductPrice.php:77)
  at Isotope\Model\ProductPrice->getAmount(1, array('color' => '1', 'storage' => '22', 'cover' => '19'))
     (vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Collection\ProductPrice.php:82)
  at Isotope\Collection\ProductPrice->getAmount(1, array('color' => '1', 'storage' => '22', 'cover' => '19'))
     (vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Model\ProductCollectionItem.php:285)
  at Isotope\Model\ProductCollectionItem->getPrice()
     (vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Model\ProductCollection.php:1644)
  at Isotope\Model\ProductCollection->generateItem(object(ProductCollectionItem))
     (vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Model\ProductCollection.php:1603)
  at Isotope\Model\ProductCollection->addItemsToTemplate(object(Template), object(Closure))
     (vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Model\ProductCollection.php:1449)
  at Isotope\Model\ProductCollection->addToTemplate(object(Template), array('module' => object(Cart), 'gallery' => '2', 'sorting' => object(Closure)))
     (vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Module\AbstractProductCollection.php:87)
  at Isotope\Module\AbstractProductCollection->compile()
     (vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Module\Cart.php:51)
  at Isotope\Module\Cart->compile()
     (vendor\contao\core-bundle\src\Resources\contao\modules\Module.php:214)
  at Contao\Module->generate()
     (vendor\codefog\contao-haste\library\Haste\Frontend\AbstractFrontendModule.php:52)
  at Haste\Frontend\AbstractFrontendModule->generate()
     (vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Module\Module.php:111)
  at Isotope\Module\Module->generate()
     (vendor\isotope\isotope-core\system\modules\isotope\library\Isotope\Module\AbstractProductCollection.php:56)
  at Isotope\Module\AbstractProductCollection->generate()
     (vendor\contao\core-bundle\src\Resources\contao\elements\ContentModule.php:78)
  at Contao\ContentModule->generate()
     (vendor\contao\core-bundle\src\Resources\contao\library\Contao\Controller.php:483)
  at Contao\Controller::getContentElement(object(ContentModel), 'main')
     (vendor\contao\core-bundle\src\Resources\contao\modules\ModuleArticle.php:193)
  at Contao\ModuleArticle->compile()
     (vendor\contao\core-bundle\src\Resources\contao\modules\Module.php:214)
  at Contao\Module->generate()
     (vendor\contao\core-bundle\src\Resources\contao\modules\ModuleArticle.php:75)
  at Contao\ModuleArticle->generate(false)
     (vendor\contao\core-bundle\src\Resources\contao\library\Contao\Controller.php:423)
  at Contao\Controller::getArticle(object(ArticleModel), false, false, 'main')
     (vendor\contao\core-bundle\src\Resources\contao\library\Contao\Controller.php:282)
  at Contao\Controller::getFrontendModule('0', 'main')
     (vendor\contao\core-bundle\src\Resources\contao\pages\PageRegular.php:175)
  at Contao\PageRegular->prepare(object(PageModel))
     (vendor\contao\core-bundle\src\Resources\contao\pages\PageRegular.php:48)
  at Contao\PageRegular->getResponse(object(PageModel), true)
     (vendor\contao\core-bundle\src\Resources\contao\controllers\FrontendIndex.php:351)
  at Contao\FrontendIndex->renderPage(object(Collection))
     (vendor\contao\core-bundle\src\Resources\contao\controllers\FrontendIndex.php:78)
  at Contao\FrontendIndex->run()
     (vendor\contao\core-bundle\src\Controller\FrontendController.php:39)
  at Contao\CoreBundle\Controller\FrontendController->indexAction()
     (vendor\symfony\http-kernel\HttpKernel.php:149)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor\symfony\http-kernel\HttpKernel.php:66)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor\symfony\http-kernel\Kernel.php:188)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (web\app_dev.php:84)

This PR simply casts the non-numeric value to (float). Alternatively, the function could also simply return 0.0 or 0 there.

@aschempp
Copy link
Member

I think the point of this check is to return the original value, if it is not numeric. Casting it to a float would break this idea. What did you configure to get an error?

@fritzmg
Copy link
Contributor Author

fritzmg commented Mar 19, 2019

🤔 I'll try to create an accurate reproduction again.

@aschempp aschempp added the bug label Mar 25, 2019
@aschempp aschempp added this to the 2.5.12 milestone Mar 25, 2019
@aschempp
Copy link
Member

Fixed in version 2.5.12

@aschempp aschempp closed this Mar 25, 2019
@fritzmg fritzmg deleted the patch-4 branch March 25, 2019 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants