View Issue Details

IDProjectCategoryView StatusLast Update
0003982OXID eShop (all versions)1.02. Price calculations (discounts, coupons, additional costs etc.)public2012-12-07 15:07
Reportermanuel.reiss Assigned To 
PriorityhighSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product Version4.5.9 revision 43186 
Fixed in Version4.7.1 / 5.0.1 revision 52468 
Summary0003982: forced basket-calculation - calculateBasket(true) - messes up discount calculation
DescriptionBy calling calculateBasket(true) you might assume that all previous basket calculations are wiped and recalculated.
Unfortunately some calculations depend on _blUpdateNeeded-flag and are not affected by the $blForceUpdate flag.

Therefore oxbasket::getBasketSummary() called in oxdiscount::isForBasket() does not reset itself and stacks up its values, resulting e.g. in a huge dArticleDiscountablePrice, which messes up the discount-calculation.
Steps To Reproducecreate following discounts:

discount 1: price-range: 0 - 100€
discount 2: price-range: 100 - 99999€

add to basket::render() (or any other view)
oxSession::getInstance()->getBasket()->calculateBasket(true);

create a shopping cart < 100€ but > 50€ and call basket view.
Due to the summed up values in basketSummary, the applied discount will be discount 2.
Additional InformationAs long as you know, that setting _blUpdateNeeded = true by calling oxbasket::onUpdate() is crucial for a clean recalculation, this behaviour will not affect you.
But by referring to docs:

@param bool $blForceUpdate set this parameter to TRUE to force basket recalculation

IMO this can be classified as a bug.
TagsDiscount
ThemeBoth
BrowserAll
PHP Versionany
Database Versionany

Activities

dainius.bigelis

2012-05-07 08:53

reporter   ~0006520

@Developers:
I cannot reproduce this case according to description (by creating discounts and checking which one is applied in the basket). But:
1. The ranges of discounts written above are incorrect:
a) first discount from "0" - is incorrect, because then it discount is applied on all product prices instantly (not only in basket). That's separate feature of discount logics. So If you want discount applied only in basket, the price should be specified as "From: 0,01" or so.
b) the mid range of both discounts is 100 EUR. The range for 2nd discount then should be 100,01, because at the price =100 EUR is not clear which discount should be applied.

2. But please check the mentioned problems from the source code side, to be sure, that there is no issue with that.

manuel.reiss

2012-05-07 16:58

reporter   ~0006527

Maybe to sum up the whole issue:

method: oxbasket::getBasketSummary()

if ( $this->_blUpdateNeeded || $this->_aBasketSummary === null ) {

this condition is not valid if basket-calculation is called with its blForceUpdate flag = true, because this flag does not set the _blUpdateNeeded-Flag = true.

As a consequence, the basketSummary does not reset itself but sums up all the values from basket-contents to its current values. Since discount calculation relies on the basketSummary, those calculations will be incorrect unless a "proper" forced basket-recalculation with _blUpdateNeeded=true flag (achieved via oxbasket::onUpdate())

Proposal:
oxbasket::calculateBasket with $blForceUpdate = true should also call oxbasket::onUpdate()

Linas Kukulskis

2012-11-19 15:04

reporter   ~0007900

fixed