View Issue Details

IDProjectCategoryView StatusLast Update
0002133OXID eShop (all versions)1.02. Price calculations (discounts, coupons, additional costs etc.)public2012-12-07 15:07
Reporterandreas_ziethen 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version4.4.2 revision 29492 
Target VersionFixed in Version4.5.9 revision 43186 
Summary0002133: voucher reserved check is done redundantly
DescriptionThere are two places where a voucher is checked for beeing already reserved:

first one: oxvoucher::_isNotResereved()
second one: query in oxvoucher::getVoucherByNr()

This is not good cause of you want to modify the reserved logic by module you are not expecting that logic in the second place. It should be in a central place only.
TagsDiscount
Theme
BrowserAll
PHP Versionany
Database Versionany

Activities

andreas_ziethen

2010-09-27 11:43

reporter   ~0003551

I had a deeper look into this issue now and found some more things which could be done better:

oxvoucher::getVoucherByNr() does not only fetch the voucher object by voucher number - which I would expect - but it does a lot more. It does check voucher availability depending on:
- oxvoucher.oxorderid has to be NULL or empty
- oxvoucher.oxreserved has to be less then now minus 3 hours

Both checks in my opinion should be done in different functions - not in getVoucherByNr(). And the general check if a voucher is already used should be done on oxdateused field - not on oxorderid field.

E. g. we built a module to manually devalue vouchers. The shop-owner just has to fill in a voucher no. and click button. This is quite often needed if vouchers can be used on other channels as the online shop as well.
So I thought it would be enough to set the oxdateused field for such vouchers - but later I realized that I HAVE TO fill in an order id to really make in invalid. I think that should be changed. If oxdateused is != NULL, the voucher should be taken as invalid. And the checks for availability should be on a single place.

dainius.bigelis

2010-09-29 17:26

reporter   ~0003557

@Developers: please check from the source code if this is the case as described and can we optimize this check to central place.

saulius.stasiukaitis

2012-03-05 16:28

reporter   ~0005890

Checks if voucher used not only by orderid but also by useddate. If any of those fields has value, voucher counts as used.

Changed code in oxvoucher.php
add line 73 in method getVoucherByNr
            $sQ .= " and ( {$sViewName}.oxdateused is NULL || {$sViewName}.oxdateused = 0 ) ";
changed line 405 in method _isAvailableInOtherOrder
            $sSelect .= '((oxorderid is not NULL and oxorderid != "") or (oxdateused is not NULL and oxdateused != 0)) ';

Changed code in oxvoucherseria.php
changed line 150 in method countVouchers
        $sQuery = 'select count(*) as used from oxvouchers where oxvoucherserieid = ' . $oDB->quote( $this->getId() ) . ' and ((oxorderid is not NULL and oxorderid != "") or (oxdateused is not NULL and oxdateused != 0))';