View Issue Details

IDProjectCategoryView StatusLast Update
0007100OXID eShop (all versions)1.02. Price calculations (discounts, coupons, additional costs etc.)public2024-07-03 10:42
Reporternitin.singh Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status acknowledgedResolutionopen 
Product Version6.1.2 
Summary0007100: voucher doesn't work in case same voucher number been used for multiple voucherseries
DescriptionWe 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 ReproduceAdded 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.

TagsNo tags attached.
Attached Files
ThemeNot defined
BrowserNot defined
PHP VersionNot defined
Database VersionNot defined

Activities

nitin.singh

2020-02-21 17:11

reporter   ~0013134

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;
    }
}
oxps_patches_oxvoucher.php (6,329 bytes)   

QA

2020-02-24 08:59

administrator   ~0013135

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 -

kukulcan

2021-04-22 08:25

reporter   ~0013426

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.

QA

2021-04-22 11:21

administrator   ~0013427

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-

kukulcan

2021-04-27 07:38

reporter   ~0013430

Last edited: 2021-04-27 07:48

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)";

nitin.singh

2021-04-27 08:57

reporter   ~0013431

@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'

SvenBrunk

2024-07-03 10:42

administrator   ~0017169

https://github.com/OXID-eSales/oxideshop_ce/pull/983