MAGE-1434 Fix floating point precision bug in EventProcessor for v3.17 #1830
Conversation
Adding a product to the cart may trigger a PHP and Algolia error due to the discount amount being too large. ## PHP Error ``` main.CRITICAL: Unable to send add to cart event due to Algolia events model misconfiguration: Discount must be a valid decimal number and total length must be no longer than 16 characters [] [] ```
damcou
left a comment
damcou
left a comment
There was a problem hiding this comment.
Looks good ! All green ✅
I have a small comment on JIRA before merging though
|
|
||
| protected function initDecimalPrecision(): void | ||
| { | ||
| $this->decimalPrecision = $this->localeFormat->getPriceFormat()['requiredPrecision'] |
There was a problem hiding this comment.
I can see that getPriceFormat can take $localeCode and $currencyCode as parameters (that we don't pass here), and I'm wondering how it would behave in case of a multicurrency store with different currencies and precisions (USD and KWD for example) ... But I guess this is an extremely unlikely scenario which wouldn't hurt that much at Insights level in any case.
There was a problem hiding this comment.
When I looked at the implementation of \Magento\Framework\Locale\Format::getPriceFormat I noted that it fell back to resolvers on both values. I decided that rather try to get too fancy I would let Magento decide the locale and currency based on its own internals.
If that doesn't work for someone I did separate the initialization of the precision via protected method \Algolia\AlgoliaSearch\Service\Insights\EventProcessor::initDecimalPrecision. Worst case you could write a preference on that method and handle that method however you wish.
Also with the plugin available for \Algolia\AlgoliaSearch\Service\Insights\EventProcessor::applyPrecision I figured the implementation should be flexible enough for others. We shall see! 😅
Summary
This PR:
Result