[Vtigercrm-developers] long standing bug in discount calculations

[Uma S]

Hi,

Thanks! for the notification. we have raised trac
<http://trac.vtiger.com/cgi-bin/trac.cgi/ticket/8404> ticket for same will
check this against latest source again and update response.

Please leave your observations here in above trac.

On Fri, Jan 23, 2015 at 7:28 AM, Hamono, Chris (DPC) <Chris.Hamono at sa.gov.au
> wrote:

> I should have said I am using vtiger 6.0 This issue does not seem to apply
> to 6.1
>
>
>
> I have also noticed the latest version 6.1 goes to great lengths to use
> vtlib_purify which is excellent J
>
>
>
> Chris
>
>
>
>
>
> *From:* vtigercrm-developers-bounces at lists.vtigercrm.com [mailto:
> vtigercrm-developers-bounces at lists.vtigercrm.com] *On Behalf Of *Hamono,
> Chris (DPC)
> *Sent:* Friday, 23 January 2015 9:58 AM
> *To:* vtigercrm-developers at lists.vtigercrm.com
> *Subject:* [Vtigercrm-developers] long standing bug in discount
> calculations
>
>
>
> Bug: Changing discount type on an invoice does not remove the discount
>
>
>
> Steps to reproduce
>
>
>
> Create an invoice and add a line item
>
> Save the invoice
>
> Edit the invoice
>
> Create a discount for the invoice of 50% (at the invoice level not the
> line item level)
>
> Save the invoice
>
> Open the invoice
>
> Change the discount to ‘Zero Discount’
>
> Invoice displays correct amount
>
> Save the invoice
>
>
>
> The invoice now has the percentage reduction still enforced and the
> discount amount is wrong
>
>
>
> Solution
>
> Line 670 in "include\utils\InventoryUtils.php"
>
>
>
> Change
>
>
>
> if($_REQUEST['discount_type_final'] == 'percentage')
>
>         {
>
>                 $updatequery .= " discount_percent=?,";
>
>                 array_push($updateparams,
> $_REQUEST['discount_percentage_final']);
>
>         }
>
>         elseif($_REQUEST['discount_type_final'] == 'amount')
>
>         {
>
>                 $discount_amount_final =
> $_REQUEST['discount_amount_final'];
>
>                 $updatequery .= " discount_amount=?,";
>
>                 array_push($updateparams, $discount_amount_final);
>
>         }
>
>
>
> To this
>
>
>
>     switch ((isset($_REQUEST['discount_type_final']) ?
> $_REQUEST['discount_type_final'] : false)) {
>
>         case 'percentage' :
>
>             $updatequery .= " discount_percent=?,";
>
>             array_push($updateparams, (float)
> $_REQUEST['discount_percentage_final']);
>
>             break;
>
>         case 'amount' :
>
>             $updatequery .= " discount_percent=0, discount_amount=?,";
>
>             array_push($updateparams, (float)
> $_REQUEST['discount_amount_final']);
>
>             break;
>
>         default :
>
>             $updatequery .= " discount_percent=0, discount_amount=0,";
>
>     }
>
>
>
>
>
> In this small piece of code we see a few issues. These are some of the
> issues that dog vtiger.
>
>
>
> The original code references the $_REQUEST object directly.
>
> The original code does not test for the variables existence before trying
> to use it which is bad form as well as a potential source of unexplained
> errors.
>
> The original code uses ‘==’ as a comparison operator, It should always use
> ‘===’ unless there is a good reason not to.
>
>
>
> For those that are not aware…
>
>
>
> $cat = ‘cat’;
>
>
>
> 0 == $cat <= TRUE
>
> 0 === $cat <= FALSE
>
>
>
> == does a type conversion before evaluating the result, (int)’cat’ is 0.
>
> === and !== compare the type as well as the value, therefore string ‘cat’
> is not equal to integer 0.
>
>
>
> This can cause many hard to find bugs
>
>
>
> Chris
>
>
>
> _______________________________________________
> http://www.vtiger.com/
>



-- 
With
Best Regards
Uma.S
Vtiger Team
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.vtigercrm.com/pipermail/vtigercrm-developers/attachments/20150123/c7f0313e/attachment-0001.html>