<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=us-ascii"><meta name=Generator content="Microsoft Word 14 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
        {font-family:Wingdings;
        panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
        {font-family:Wingdings;
        panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:Tahoma;
        panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0cm;
        margin-bottom:.0001pt;
        font-size:12.0pt;
        font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:purple;
        text-decoration:underline;}
p.emailquote, li.emailquote, div.emailquote
        {mso-style-name:emailquote;
        mso-margin-top-alt:auto;
        margin-right:0cm;
        mso-margin-bottom-alt:auto;
        margin-left:1.0pt;
        border:none;
        padding:0cm;
        font-size:12.0pt;
        font-family:"Times New Roman","serif";}
span.EmailStyle18
        {mso-style-type:personal-reply;
        font-family:"Calibri","sans-serif";
        color:#1F497D;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@page WordSection1
        {size:612.0pt 792.0pt;
        margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=EN-AU link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I should have said I am using vtiger 6.0 This issue does not seem to apply to 6.1 <o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I have also noticed the latest version 6.1 goes to great lengths to use vtlib_purify which is excellent </span><span style='font-size:11.0pt;font-family:Wingdings;color:#1F497D'>J</span><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Chris<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><div><div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm'><p class=MsoNormal><b><span lang=EN-US style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span lang=EN-US style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> vtigercrm-developers-bounces@lists.vtigercrm.com [mailto:vtigercrm-developers-bounces@lists.vtigercrm.com] <b>On Behalf Of </b>Hamono, Chris (DPC)<br><b>Sent:</b> Friday, 23 January 2015 9:58 AM<br><b>To:</b> vtigercrm-developers@lists.vtigercrm.com<br><b>Subject:</b> [Vtigercrm-developers] long standing bug in discount calculations<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Bug: Changing discount type on an invoice does not remove the discount<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Steps to reproduce<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Create an invoice and add a line item<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Save the invoice<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Edit the invoice<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Create a discount for the invoice of 50% (at the invoice level not the line item level)<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Save the invoice<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Open the invoice<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Change the discount to ‘Zero Discount’ <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Invoice displays correct amount<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Save the invoice<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>The invoice now has the percentage reduction still enforced and the discount amount is wrong<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Solution<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Line 670 in "include\utils\InventoryUtils.php"<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Change<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>if($_REQUEST['discount_type_final'] == 'percentage')<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>        {<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>                $updatequery .= " discount_percent=?,";<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>                array_push($updateparams, $_REQUEST['discount_percentage_final']);<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>        }<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>        elseif($_REQUEST['discount_type_final'] == 'amount')<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>        {<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>                $discount_amount_final = $_REQUEST['discount_amount_final'];<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>                $updatequery .= " discount_amount=?,";<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>                array_push($updateparams, $discount_amount_final);<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>        }<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>To this <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>    switch ((isset($_REQUEST['discount_type_final']) ? $_REQUEST['discount_type_final'] : false)) {<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>        case 'percentage' :<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>            $updatequery .= " discount_percent=?,";<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>            array_push($updateparams, (float) $_REQUEST['discount_percentage_final']);<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>            break;<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>        case 'amount' :<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>            $updatequery .= " discount_percent=0, discount_amount=?,";<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>            array_push($updateparams, (float) $_REQUEST['discount_amount_final']);<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>            break;<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>        default :<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>            $updatequery .= " discount_percent=0, discount_amount=0,";<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>    }<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>In this small piece of code we see a few issues. These are some of the issues that dog vtiger.<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>The original code references the $_REQUEST object directly. <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>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.<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>The original code uses ‘==’ as a comparison operator, It should always use ‘===’ unless there is a good reason not to.<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>For those that are not aware…<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>$cat = ‘cat’;<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>0 == $cat <= TRUE<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>0 === $cat <= FALSE<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>== does a type conversion before evaluating the result, (int)’cat’ is 0.<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>=== and !== compare the type as well as the value, therefore string ‘cat’ is not equal to integer 0.<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>This can cause many hard to find bugs<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'>Chris<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:10.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div></div></body></html>