BF [ 1583825 ] No tax recalculation after line changes

http://sourceforge.net/tracker/?func=detail&atid=879332&aid=1583825&group_id=176962

base/src/org/compiere/model/MInvoiceLine.java

@@ -781,6 +781,29 @@ public class MInvoiceLine extends X_C_InvoiceLine
 		return true;
 	}	//	beforeSave
 	
+	/**
+	 * Recalculate invoice tax
+	 * @param oldTax true if the old C_Tax_ID should be used
+	 * @return true if success, false otherwise
+	 * 
+	 * @author teo_sarca [ 1583825 ]
+	 */
+	private boolean updateInvoiceTax(boolean oldTax) {
+		MInvoiceTax tax = MInvoiceTax.get (this, getPrecision(), oldTax, get_TrxName());
+		if (tax != null) {
+			if (!tax.calculateTaxFromLines())
+				return false;
+			if (tax.getTaxAmt().signum() != 0) {
+				if (!tax.save(get_TrxName()))
+					return false;
+			}
+			else {
+				if (!tax.is_new() && !tax.delete(false, get_TrxName()))
+					return false;
+			}
+		}
+		return true;
+	}
 	
 	/**
 	 * 	After Save
@@ -795,15 +818,8 @@ public class MInvoiceLine extends X_C_InvoiceLine
 		if (!newRecord && is_ValueChanged("C_Tax_ID"))
 		{
 			//	Recalculate Tax for old Tax
-			MInvoiceTax tax = MInvoiceTax.get (this, getPrecision(), 
+			if (!updateInvoiceTax(true))
-				true, get_TrxName());	//	old Tax
+				return false;
-			if (tax != null)
-			{
-				if (!tax.calculateTaxFromLines())
-					return false;
-				if (!tax.save(get_TrxName()))
-					return true;
-			}
 		}
 		return updateHeaderTax();
 	}	//	afterSave
@@ -827,24 +843,8 @@ public class MInvoiceLine extends X_C_InvoiceLine
 	private boolean updateHeaderTax()
 	{
 		//	Recalculate Tax for this Tax
-		MInvoiceTax tax = MInvoiceTax.get (this, getPrecision(), 
+		if (!updateInvoiceTax(false))
-			false, get_TrxName());	//	current Tax
+			return false;
-		if (tax != null)
-		{
-			if (!tax.calculateTaxFromLines())
-				return false;
-			if (!tax.save(get_TrxName()))
-				return false;
-		}
-		
-		// deathmeat: [ 1583825 ] No tax recalculation after line changes
-		if((MInvoiceTax.get(this, getPrecision(),
-				true, get_TrxName()).getTaxAmt().doubleValue() == 0.0D))
-		{
-			// Tax line total is zero, delete the line
-			MInvoiceTax.get(this, getPrecision(),
-					true, get_TrxName()).delete(true, get_TrxName());
-		}
 		
 		//	Update Invoice Header
 		String sql = "UPDATE C_Invoice i"

base/src/org/compiere/model/MInvoiceTax.java

@@ -45,9 +45,10 @@ public class MInvoiceTax extends X_C_InvoiceTax
 		if (line == null || line.getC_Invoice_ID() == 0)
 			return null;
 		int C_Tax_ID = line.getC_Tax_ID();
-		if (oldTax && line.is_ValueChanged("C_Tax_ID"))
+		boolean isOldTax = oldTax && line.is_ValueChanged(MInvoiceLine.COLUMNNAME_C_Tax_ID); 
+		if (isOldTax)
 		{
-			Object old = line.get_ValueOld("C_Tax_ID");
+			Object old = line.get_ValueOld(MInvoiceLine.COLUMNNAME_C_Tax_ID);
 			if (old == null)
 				return null;
 			C_Tax_ID = ((Integer)old).intValue();
@@ -95,6 +96,12 @@ public class MInvoiceTax extends X_C_InvoiceTax
 			s_log.fine("(old=" + oldTax + ") " + retValue);
 			return retValue;
 		}
+		// If the old tax was required and there is no MInvoiceTax for that
+		// return null, and not create another MInvoiceTax - teo_sarca [ 1583825 ]
+		else {
+			if (isOldTax)
+				return null;
+		}
 		
 		//	Create New
 		retValue = new MInvoiceTax(line.getCtx(), 0, trxName);

base/src/org/compiere/model/MOrderLine.java

@@ -866,15 +866,8 @@ public class MOrderLine extends X_C_OrderLine
 		if (!newRecord && is_ValueChanged("C_Tax_ID"))
 		{
 			//	Recalculate Tax for old Tax
-			MOrderTax tax = MOrderTax.get (this, getPrecision(), 
+			if (!updateOrderTax(true))
-				true, get_TrxName());	//	old Tax
+				return false;
-			if (tax != null)
-			{
-				if (!tax.calculateTaxFromLines())
-					return false;
-				if (!tax.save(get_TrxName()))
-					return false;
-			}
 		}
 		return updateHeaderTax();
 	}	//	afterSave
@@ -897,6 +890,30 @@ public class MOrderLine extends X_C_OrderLine
 		return updateHeaderTax();
 	}	//	afterDelete
 	
+	/**
+	 * Recalculate order tax
+	 * @param oldTax true if the old C_Tax_ID should be used
+	 * @return true if success, false otherwise
+	 * 
+	 * @author teo_sarca [ 1583825 ]
+	 */
+	private boolean updateOrderTax(boolean oldTax) {
+		MOrderTax tax = MOrderTax.get (this, getPrecision(), oldTax, get_TrxName());
+		if (tax != null) {
+			if (!tax.calculateTaxFromLines())
+				return false;
+			if (tax.getTaxAmt().signum() != 0) {
+				if (!tax.save(get_TrxName()))
+					return false;
+			}
+			else {
+				if (!tax.is_new() && !tax.delete(false, get_TrxName()))
+					return false;
+			}
+		}
+		return true;
+	}
+	
 	/**
 	 *	Update Tax & Header
 	 *	@return true if header updated
@@ -904,21 +921,8 @@ public class MOrderLine extends X_C_OrderLine
 	private boolean updateHeaderTax()
 	{
 		//	Recalculate Tax for this Tax
-		MOrderTax tax = MOrderTax.get (this, getPrecision(), 
+		if (!updateOrderTax(false))
-			false, get_TrxName());	//	current Tax
-		if (!tax.calculateTaxFromLines())
 			return false;
-		if (!tax.save(get_TrxName()))
-			return false;
-		
-		// deathmeat: [ 1583825 ] No tax recalculation after line changes
-		if((MOrderTax.get(this, getPrecision(),
-				true, get_TrxName()).getTaxAmt().doubleValue() == 0.0D))
-		{
-			// Tax line total is zero, delete the line
-			MOrderTax.get(this, getPrecision(),
-					true, get_TrxName()).delete(true, get_TrxName());
-		}
 		
 		//	Update Order Header
 		String sql = "UPDATE C_Order i"

base/src/org/compiere/model/MOrderTax.java

@@ -48,9 +48,10 @@ public class MOrderTax extends X_C_OrderTax
 			return null;
 		}
 		int C_Tax_ID = line.getC_Tax_ID();
-		if (oldTax && line.is_ValueChanged("C_Tax_ID"))
+		boolean isOldTax = oldTax && line.is_ValueChanged(MOrderTax.COLUMNNAME_C_Tax_ID); 
+		if (isOldTax)
 		{
-			Object old = line.get_ValueOld("C_Tax_ID");
+			Object old = line.get_ValueOld(MOrderTax.COLUMNNAME_C_Tax_ID);
 			if (old == null)
 			{
 				s_log.fine("No Old Tax");
@@ -99,6 +100,12 @@ public class MOrderTax extends X_C_OrderTax
 			s_log.fine("(old=" + oldTax + ") " + retValue);
 			return retValue;
 		}
+		// If the old tax was required and there is no MOrderTax for that
+		// return null, and not create another MOrderTax - teo_sarca [ 1583825 ]
+		else {
+			if (isOldTax)
+				return null;
+		}
 		
 		//	Create New
 		retValue = new MOrderTax(line.getCtx(), 0, trxName);