View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0007100 | OXID eShop (all versions) | 1.02. Price calculations (discounts, coupons, additional costs etc.) | public | 2020-02-21 17:08 | 2024-07-03 10:42 |
Reporter | nitin.singh | Assigned To | |||
Priority | normal | Severity | minor | Reproducibility | always |
Status | acknowledged | Resolution | open | ||
Product Version | 6.1.2 | ||||
Summary | 0007100: voucher doesn't work in case same voucher number been used for multiple voucherseries | ||||
Description | We had one use case where customer used some voucher number for multiple voucherseries and one voucherseries got expired and second voucherseries is still live. Then, if you use related voucher then it doesn't work and give you error. | ||||
Steps To Reproduce | Added by QA - SG - : 1. Add Serie "Test 1" with code "test" 2. Add Serie "Test 2" with code "test" 3. Set Test 1 to be deactivated 4. the code "test" won't be accepted. | ||||
Tags | No tags attached. | ||||
Attached Files | |||||
Theme | Not defined | ||||
Browser | Not defined | ||||
PHP Version | Not defined | ||||
Database Version | Not defined | ||||
|
We have fixed this issue with attached patch. oxps_patches_oxvoucher.php (6,329 bytes)
<?php /** * This Software is the property of OXID eSales and is protected * by copyright law - it is NOT Freeware. * * Any unauthorized use of this software without a valid license key * is a violation of the license agreement and will be prosecuted by * civil and criminal law. * * @link http://www.oxid-esales.com * @copyright (C) OXID eSales AG 2003-2017 * @version OXID eShop EE */ /** * Voucher manager. * Performs deletion, generating, assigning to group and other voucher * managing functions. * */ class oxps_patches_oxvoucher extends oxps_patches_oxvoucher_parent { /** * Gets voucher from db by given number. * * @param string $sVoucherNr Voucher number * @param array $aVouchers Array of available vouchers (default array()) * @param bool $blCheckavalability check if voucher is still reserver od not * * @throws oxVoucherException exception * * @return mixed */ public function getVoucherByNr($sVoucherNr, $aVouchers = array(), $blCheckavalability = false) { $oRet = null; if (!is_null($sVoucherNr)) { $sViewName = $this->getViewName(); $sSeriesViewName = getViewName('oxvoucherseries'); $oDb = oxDb::getDb(); $sQ = "select {$sViewName}.* from {$sViewName}, {$sSeriesViewName} where {$sSeriesViewName}.oxid = {$sViewName}.oxvoucherserieid and {$sViewName}.oxvouchernr = " . $oDb->quote($sVoucherNr) . " and "; if (is_array($aVouchers)) { foreach ($aVouchers as $sVoucherId => $sSkipVoucherNr) { $sQ .= "{$sViewName}.oxid != " . $oDb->quote($sVoucherId) . " and "; } } $sQ .= "( {$sViewName}.oxorderid is NULL || {$sViewName}.oxorderid = '' ) "; $sQ .= " and ( {$sViewName}.oxdateused is NULL || {$sViewName}.oxdateused = 0 ) "; // Validate end date of voucher (To prevent issue in case same voucher number been used for multiple voucherseries) $sQ .= " and {$sSeriesViewName}.oxenddate >= NOW()"; //voucher timeout for 3 hours if ($blCheckavalability) { $iTime = time() - $this->_getVoucherTimeout(); $sQ .= " and {$sViewName}.oxreserved < '{$iTime}' "; } $sQ .= " limit 1"; if (!($oRet = $this->assignRecord($sQ))) { $oEx = oxNew('oxVoucherException'); $oEx->setMessage('ERROR_MESSAGE_VOUCHER_NOVOUCHER'); $oEx->setVoucherNr($sVoucherNr); throw $oEx; } } return $oRet; } /** * Checks if calculation with vouchers of the same series possible. Returns * true on success. * * @param array $aVouchers array of vouchers * * @throws oxVoucherException exception * * @return bool * */ protected function _isAvailableWithSameSeries($aVouchers) { if (is_array($aVouchers)) { $sId = $this->getId(); if (isset($aVouchers[$sId])) { unset($aVouchers[$sId]); } $oSeries = $this->getSerie(); if (!$oSeries->oxvoucherseries__oxallowsameseries->value) { $oBasket = $this->getSession()->getBasket(); foreach ($aVouchers as $voucherId => $voucherNr) { $oVoucher = oxNew('oxVoucher'); $oVoucher->load($voucherId); if ($this->oxvouchers__oxvoucherserieid->value == $oVoucher->oxvouchers__oxvoucherserieid->value) { // removing voucher on error $oBasket->removeVoucher($voucherId); $oEx = oxNew('oxVoucherException'); $oEx->setMessage('ERROR_MESSAGE_VOUCHER_NOTALLOWEDSAMESERIES'); $oEx->setVoucherNr($this->oxvouchers__oxvouchernr->value); throw $oEx; } } } } return true; } /** * Checks if calculation with vouchers from the other series possible. * Returns true on success. * * @param array $aVouchers array of vouchers * * @throws oxVoucherException exception * * @return bool */ protected function _isAvailableWithOtherSeries($aVouchers) { if (is_array($aVouchers) && count($aVouchers)) { $oSeries = $this->getSerie(); $sIds = implode(',', oxDb::getInstance()->quoteArray(array_keys($aVouchers))); $blAvailable = true; $oDb = oxDb::getDb(); if (!$oSeries->oxvoucherseries__oxallowotherseries->value) { // just search for vouchers with different series $sSql = "select 1 from oxvouchers where oxvouchers.oxid in ($sIds) and "; $sSql .= "oxvouchers.oxvoucherserieid != " . $oDb->quote($this->oxvouchers__oxvoucherserieid->value); $blAvailable &= !$oDb->getOne($sSql); } else { // search for vouchers with different series and those vouchers do not allow other series $sSql = "select 1 from oxvouchers left join oxvoucherseries on oxvouchers.oxvoucherserieid=oxvoucherseries.oxid "; $sSql .= "where oxvouchers.oxid in ($sIds) and oxvouchers.oxvoucherserieid != " . $oDb->quote($this->oxvouchers__oxvoucherserieid->value); $sSql .= "and not oxvoucherseries.oxallowotherseries"; $blAvailable &= !$oDb->getOne($sSql); } if (!$blAvailable) { // removing voucher on error if (is_array($aVouchers)) { $oBasket = $this->getSession()->getBasket(); foreach ($aVouchers as $key => $value) { $oBasket->removeVoucher($key); } } $oEx = oxNew('oxVoucherException'); $oEx->setMessage('ERROR_MESSAGE_VOUCHER_NOTALLOWEDOTHERSERIES'); $oEx->setVoucherNr($this->oxvouchers__oxvouchernr->value); throw $oEx; } } return true; } } |
|
For me personally it is more a bug to allow the same code to be used. If both series are active, how can the shop know with series should be assigned? Therefore to prevent such behavior, I recommend forbidding usage of the same code. QA - SG - |
|
Sorry, QA, but that makes not much sense. This is a bug that should be fixed. Marketing stuff like coupon codes is often handled by external and changing service providers and they won't keep lists or check those of predecessors. If you believe using the same code again is bad style then please fix the bug by providing the user with a warning. |
|
Dear kukulcan, that there is a issue software side, i acknowledge. therefore the bug was set to status acknowledged and not to the status closed, so it will be fixed and the whole issue will be solved in the future. Best Regards QA -SG- |
|
Thanks for the clarification, QA. @nitin.singh - The supposed patch does not work if vouchers have no start and no end date. I think it should be: $sQ .= " and ({$sSeriesViewName}.oxenddate >= NOW() OR {$sSeriesViewName}.oxenddate IS NULL)"; |
|
@kukulcan - Thanks for noticing edge case. OXENDDATE field is not null but having default value '0000-00-00 00:00:00'. So, i think you can write query like this. $sQ .= " and ({$sSeriesViewName}.oxenddate >= NOW() OR {$sSeriesViewName}.oxenddate = '0000-00-00 00:00:00')"; Field Type: `OXENDDATE` datetime NOT NULL DEFAULT '0000-00-00 00:00:00' COMMENT 'Valid to' |
|
https://github.com/OXID-eSales/oxideshop_ce/pull/983 |