Price Calculation as % of Margin - applied suggested changes

refs: #6045

de.metas.business/src/main/java/de/metas/order/impl/OrderLinePriceCalculator.java

@@ -152,7 +152,7 @@ public void updateOrderLine()
 		orderLine.setPriceStd(pricingResult.getPriceStd());
 		orderLine.setPrice_UOM_ID(UomId.toRepoId(pricingResult.getPriceUomId())); // 07090: when setting a priceActual, we also need to specify a PriceUOM
 		orderLine.setBase_Commission_Points_Per_Price_UOM( pricingResult.getBaseCommissionPointsPerPriceUOM() );
-		orderLine.setTraded_Commission_Percent( pricingResult.getTradedCommissionPercent() );
+		orderLine.setTraded_Commission_Percent( Percent.toBigDecimalOrNull( pricingResult.getTradedCommissionPercent() ) );
 
 		//
 		// C_Currency_ID, M_PriceList_Version_ID

de.metas.business/src/main/java/de/metas/pricing/IPricingResult.java

@@ -178,7 +178,7 @@ default int getCurrencyRepoId()
 
 	BigDecimal getBaseCommissionPointsPerPriceUOM();
 
-	void setTradedCommissionPercent(BigDecimal tradedCommissionPercent);
+	void setTradedCommissionPercent(Percent tradedCommissionPercent);
 
-	BigDecimal getTradedCommissionPercent();
+	Percent getTradedCommissionPercent();
 }

de.metas.business/src/main/java/de/metas/pricing/service/impl/PricingResult.java

@@ -121,7 +121,7 @@ final class PricingResult implements IPricingResult
 
 	private BigDecimal baseCommissionPointsPerPriceUOM;
 
-	private BigDecimal tradedCommissionPercent;
+	private Percent tradedCommissionPercent = Percent.ZERO;
 
 	@Builder
 	private PricingResult(

de.metas.contracts/src/main/java/de/metas/contracts/commission/commissioninstance/interceptor/C_Invoice_Candidate.java

@@ -89,20 +89,4 @@ public void deleteCommissionInstance(@NonNull final I_C_Invoice_Candidate icReco
 			invoiceCandidateService.syncSalesICToCommissionInstance(invoiceCandidateId, true/*candidateDeleted*/);
 		}
 	}
-
-	/**
-	 *  Reset {@link I_C_Invoice_Candidate.COLUMNNAME_Base_Commission_Points_Per_Price_UOM} and {@link I_C_Invoice_Candidate.COLUMNNAME_Traded_Commission_Percent}
-	 *  in case the price entered was overridden so the commission points will be calculated based on the {@link I_C_Invoice_Candidate.COLUMNNAME_PriceActual_Override}.
-	 *
-	 * @param icRecord Invoice Candidate record
-	 */
-	@ModelChange( timings = ModelValidator.TYPE_BEFORE_CHANGE, ifColumnsChanged = { I_C_Invoice_Candidate.COLUMNNAME_PriceEntered_Override } )
-	public void resetComputedCommissionPoints(@NonNull final I_C_Invoice_Candidate icRecord)
-	{
-		if ( !icRecord.getPriceEntered().equals( icRecord.getPriceEntered_Override() ) )
-		{
-			icRecord.setBase_Commission_Points_Per_Price_UOM(null);
-			icRecord.setTraded_Commission_Percent(null);
-		}
-	}
 }

de.metas.contracts/src/main/java/de/metas/contracts/commission/commissioninstance/services/CommissionInstanceRequestFactory.java

@@ -5,7 +5,7 @@
 import java.util.Optional;
 
 import de.metas.contracts.commission.Beneficiary;
-import de.metas.util.Check;
+import org.adempiere.exceptions.AdempiereException;
 import org.compiere.util.TimeUtil;
 import org.springframework.stereotype.Service;
 
@@ -136,7 +136,15 @@ public Optional<CreateInstanceRequest> createRequestFor(final CreateForecastComm
 				    .filter(config -> config.getContractFor( Beneficiary.of( contractRequest.getSalesRepBPartnerId() ) ) != null)
 				    .collect( ImmutableList.toImmutableList() );
 
-		Check.assume(configs.size() <= 1, "There is only one active commission contract for a sales rep at a certain time!");
+		if (configs.size() > 1)
+		{
+			throw new AdempiereException("Expecting only one active commissionConfig for a sales rep at a certain time!")
+					.appendParametersToMessage()
+					.setParameter("salesRepBPartnerId", contractRequest.getSalesRepBPartnerId())
+					.setParameter("contractRequest", contractRequest)
+					.setParameter("hierarchy", hierarchy)
+					.setParameter("commissionConfigs", configs);
+		}
 
 		if ( configs.isEmpty() )
 		{

de.metas.contracts/src/main/java/de/metas/contracts/commission/commissioninstance/services/CommissionPointsService.java

@@ -70,7 +70,7 @@ public Optional<Money> getCommissionPointsValue( @NonNull final CommissionPoints
 		return Optional.of(customerTradeMarginAmount);
 	}
 
-	public IPricingResult calculateCommissionPointPriceFor(final FlatrateTermId flatrateTermId, final LocalDate requestedDate)
+	private IPricingResult calculateCommissionPointPriceFor(final FlatrateTermId flatrateTermId, final LocalDate requestedDate)
 	{
 		final IFlatrateDAO flatrateDAO = Services.get(IFlatrateDAO.class);
 		final IBPartnerDAO bPartnerDAO = Services.get(IBPartnerDAO.class);

de.metas.contracts/src/main/java/de/metas/contracts/commission/commissioninstance/services/CommissionTriggerRequest.java

@@ -26,12 +26,12 @@
 import de.metas.invoicecandidate.InvoiceCandidateId;
 import de.metas.util.lang.Percent;
 import lombok.Builder;
-import lombok.Data;
 import lombok.NonNull;
+import lombok.Value;
 
 import java.time.Instant;
 
-@Data
+@Value
 @Builder
 public class CommissionTriggerRequest
 {

de.metas.contracts/src/main/java/de/metas/contracts/commission/commissioninstance/services/CreateForecastCommissionInstanceRequest.java

@@ -27,12 +27,12 @@
 import de.metas.product.ProductPrice;
 import de.metas.quantity.Quantity;
 import lombok.Builder;
-import lombok.Data;
 import lombok.NonNull;
+import lombok.Value;
 
 import java.time.LocalDate;
 
-@Data
+@Value
 @Builder
 public class CreateForecastCommissionInstanceRequest
 {

de.metas.contracts/src/main/java/de/metas/contracts/commission/commissioninstance/services/repos/InvoiceCandidateRecordHelper.java

@@ -52,41 +52,43 @@ CommissionPoints extractForecastCommissionPoints(@NonNull final I_C_Invoice_Cand
 		final CommissionPoints forecastCommissionPoints;
 
 		final ProductPrice priceActual = Services.get(IInvoiceCandBL.class).getPriceActual(icRecord);
-		final BigDecimal baseCommissionPointsPerPriceUOM = icRecord.getBase_Commission_Points_Per_Price_UOM();
 
 		final BigDecimal forecastQtyInPriceUOM = icRecord.getQtyEntered()
 				.subtract( icRecord.getQtyToInvoiceInUOM() )
 				.subtract( icRecord.getQtyInvoicedInUOM() );
 
-		if (baseCommissionPointsPerPriceUOM.signum() > 0)
-		{
-			final BigDecimal forecastCommissionPointsAmount = baseCommissionPointsPerPriceUOM.multiply(forecastQtyInPriceUOM);
-			forecastCommissionPoints = CommissionPoints.of(forecastCommissionPointsAmount);
-		}
-		else
+		if ( useActualPriceForComputingCommissionPoints(icRecord) )
 		{
 			final Money forecastNetAmt = moneyService.multiply(
 					Quantitys.create(forecastQtyInPriceUOM, UomId.ofRepoId(icRecord.getC_UOM_ID())),
 					priceActual);
 
 			forecastCommissionPoints = CommissionPoints.of( forecastNetAmt.toBigDecimal() );
 		}
+		else
+		{
+			final BigDecimal baseCommissionPointsPerPriceUOM = icRecord.getBase_Commission_Points_Per_Price_UOM();
+
+			final BigDecimal forecastCommissionPointsAmount = baseCommissionPointsPerPriceUOM.multiply(forecastQtyInPriceUOM);
+
+			forecastCommissionPoints = CommissionPoints.of(forecastCommissionPointsAmount);
+		}
 
 		return forecastCommissionPoints;
 	}
 
 	CommissionPoints extractCommissionPointsToInvoice(@NonNull final I_C_Invoice_Candidate icRecord)
 	{
 		final CommissionPoints commissionPointsToInvoice;
-		final BigDecimal baseCommissionPointsPerPriceUOM = icRecord.getBase_Commission_Points_Per_Price_UOM();
 
-		if (baseCommissionPointsPerPriceUOM.signum() > 0)
+		if ( useActualPriceForComputingCommissionPoints(icRecord) )
 		{
-			commissionPointsToInvoice = CommissionPoints.of( baseCommissionPointsPerPriceUOM.multiply( icRecord.getQtyToInvoiceInUOM() ) );
+			commissionPointsToInvoice = CommissionPoints.of( icRecord.getNetAmtToInvoice() );
 		}
 		else
 		{
-			commissionPointsToInvoice = CommissionPoints.of( icRecord.getNetAmtToInvoice() );
+			final BigDecimal baseCommissionPointsPerPriceUOM = icRecord.getBase_Commission_Points_Per_Price_UOM();
+			commissionPointsToInvoice = CommissionPoints.of( baseCommissionPointsPerPriceUOM.multiply( icRecord.getQtyToInvoiceInUOM() ) );
 		}
 
 		return commissionPointsToInvoice;
@@ -95,22 +97,36 @@ CommissionPoints extractCommissionPointsToInvoice(@NonNull final I_C_Invoice_Can
 	CommissionPoints extractInvoicedCommissionPoints(@NonNull final I_C_Invoice_Candidate icRecord)
 	{
 		final CommissionPoints commissionPointsToInvoice;
-		final BigDecimal baseCommissionPointsPerPriceUOM = icRecord.getBase_Commission_Points_Per_Price_UOM();
 
-		if (baseCommissionPointsPerPriceUOM.signum() > 0)
+		if ( useActualPriceForComputingCommissionPoints(icRecord) )
 		{
-			commissionPointsToInvoice = CommissionPoints.of( baseCommissionPointsPerPriceUOM.multiply( icRecord.getQtyInvoicedInUOM() ) );
+			commissionPointsToInvoice = CommissionPoints.of( icRecord.getNetAmtInvoiced() );
 		}
 		else
 		{
-			commissionPointsToInvoice = CommissionPoints.of( icRecord.getNetAmtInvoiced() );
+			final BigDecimal baseCommissionPointsPerPriceUOM = icRecord.getBase_Commission_Points_Per_Price_UOM();
+			commissionPointsToInvoice = CommissionPoints.of( baseCommissionPointsPerPriceUOM.multiply( icRecord.getQtyInvoicedInUOM() ) );
 		}
 
 		return commissionPointsToInvoice;
 	}
 
 	Percent extractTradedCommissionPercent(@NonNull final I_C_Invoice_Candidate icRecord)
 	{
-		return Percent.of( icRecord.getTraded_Commission_Percent() );
+		return useActualPriceForComputingCommissionPoints(icRecord) ? Percent.ZERO : Percent.of( icRecord.getTraded_Commission_Percent() );
+	}
+
+	/**
+	 *  Use actual price when computing the base commission points sum if {@link I_C_Invoice_Candidate#COLUMN_Base_Commission_Points_Per_Price_UOM}
+	 *  was not calculated or the price was overwritten.
+	 *
+	 * @param icRecord	Invoice Candidate record
+	 * @return	true, if the actual price should be used for computing commission points, false otherwise
+	 */
+	private boolean useActualPriceForComputingCommissionPoints(@NonNull final I_C_Invoice_Candidate icRecord)
+	{
+		return  ( icRecord.getBase_Commission_Points_Per_Price_UOM().signum() == 0 )
+				|| ( icRecord.getPriceEntered_Override().signum() > 0
+						&& !icRecord.getPriceEntered_Override().equals( icRecord.getPriceEntered() ) );
 	}
 }

de.metas.contracts/src/main/java/de/metas/contracts/pricing/trade_margin/CustomerTradeMarginPricingRule.java

@@ -22,6 +22,7 @@
 
 package de.metas.contracts.pricing.trade_margin;
 
+import ch.qos.logback.classic.Level;
 import de.metas.bpartner.BPartnerId;
 import de.metas.bpartner.service.IBPartnerBL;
 import de.metas.contracts.commission.Beneficiary;
@@ -39,7 +40,9 @@
 import de.metas.pricing.rules.IPricingRule;
 import de.metas.product.ProductPrice;
 import de.metas.quantity.Quantitys;
+import de.metas.util.Loggables;
 import de.metas.util.Services;
+import de.metas.util.lang.Percent;
 import org.compiere.SpringContextHolder;
 import org.slf4j.Logger;
 
@@ -59,15 +62,15 @@ public class CustomerTradeMarginPricingRule implements IPricingRule
 	{
 		if ( !result.isCalculated() )
 		{
-			logger.debug("Not applying due to missing calculated price!");
+			Loggables.withLogger(logger, Level.DEBUG).addLog("Not applying due to missing calculated price!");
 			return false;
 		}
 		final BPartnerId customerId = pricingCtx.getBPartnerId();
 
 		final BPartnerId salesRepId = Services.get(IBPartnerBL.class).getBPartnerSalesRepId(customerId);
 
 		if (salesRepId == null) {
-			logger.debug("Not applying due to missing sales rep!");
+			Loggables.withLogger(logger, Level.DEBUG).addLog("Not applying due to missing sales rep!");
 			return false;
 		}
 
@@ -80,7 +83,7 @@ public class CustomerTradeMarginPricingRule implements IPricingRule
 
 		if ( !customerTradeMarginService.getCustomerTradeMarginForCriteria(customerTradeMarginSearchCriteria).isPresent() )
 		{
-			logger.debug("Not applying due to missing customer trade margin settings!");
+			Loggables.withLogger(logger, Level.DEBUG).addLog("Not applying due to missing customer trade margin settings!");
 			return false;
 		}
 
@@ -104,6 +107,7 @@ public class CustomerTradeMarginPricingRule implements IPricingRule
 				customerTradeMarginService.getCustomerTradeMarginForCriteria(customerTradeMarginSearchCriteria);
 
 		if ( !customerTradeMarginSettings.isPresent() ) {
+			Loggables.withLogger(logger, Level.DEBUG).addLog("Not applying due to missing customer trade margin settings!");
 			return;
 		}
 
@@ -119,6 +123,7 @@ public class CustomerTradeMarginPricingRule implements IPricingRule
 
 		if ( !forecastCommissionInstance.isPresent() )
 		{
+			Loggables.withLogger(logger, Level.DEBUG).addLog("Not applying! Forecast commission instance couldn't be created!");
 			return;
 		}
 		final CommissionContract salesRepCommissionContract = forecastCommissionInstance.get().getConfig().getContractFor( Beneficiary.of(salesRepId) );
@@ -130,6 +135,7 @@ public class CustomerTradeMarginPricingRule implements IPricingRule
 
 		if ( !tradedCommissionPointsPerPriceUOM.isPresent() )
 		{
+			Loggables.withLogger(logger, Level.DEBUG).addLog("Not applying! tradedCommissionPointsPerPriceUOM couldn't be calculated");
 			return;
 		}
 
@@ -139,17 +145,25 @@ public class CustomerTradeMarginPricingRule implements IPricingRule
 					salesRepCommissionContract.getId(),
 					pricingCtx.getPriceDate() );
 
-		if ( tradedCommissionPointsValue.isPresent() )
+		if ( !tradedCommissionPointsValue.isPresent() )
 		{
-			final Money customerTradeMarginPerPriceUOM = SpringContextHolder.instance.getBean(MoneyService.class)
-					.convertMoneyToCurrency( tradedCommissionPointsValue.get(), result.getCurrencyId() );
+			Loggables.withLogger(logger, Level.DEBUG).addLog("Not applying! tradedCommissionPointsValue couldn't be calculated");
+			return;
+		}
 
-			result.setBaseCommissionPointsPerPriceUOM( forecastCommissionInstance.get().getCurrentTriggerData().getForecastedPoints().toBigDecimal() );
-			result.setTradedCommissionPercent( customerTradeMarginSettings.get().getMarginPercent() );
+		final Money customerTradeMarginPerPriceUOM = SpringContextHolder.instance.getBean(MoneyService.class)
+				.convertMoneyToCurrency( tradedCommissionPointsValue.get(), result.getCurrencyId() );
 
-			result.setPriceStd( priceBeforeApplyingRule.toBigDecimal().subtract( customerTradeMarginPerPriceUOM.toBigDecimal() ) );
-			result.setCalculated(true);
-		}
+		result.setBaseCommissionPointsPerPriceUOM( forecastCommissionInstance.get().getCurrentTriggerData().getForecastedPoints().toBigDecimal() );
+		result.setTradedCommissionPercent( Percent.of( customerTradeMarginSettings.get().getMarginPercent() ) );
+
+		result.setPriceStd( priceBeforeApplyingRule.toBigDecimal().subtract( customerTradeMarginPerPriceUOM.toBigDecimal() ) );
+		result.setCalculated(true);
+
+		Loggables.withLogger(logger, Level.DEBUG)
+				.addLog("Price before applying rule: {} currencyID: {}, Price after applying rule: {} currencyID :{}",
+						priceBeforeApplyingRule.toBigDecimal() , priceBeforeApplyingRule.getCurrencyId().getRepoId(),
+						result.getPriceStd(), result.getCurrencyId().getRepoId() );
 	}
 
 	private Optional<CommissionInstance> createForecastCommissionInstanceForOneQtyInPriceUOM( final IPricingContext pricingCtx,

de.metas.contracts/src/main/java/de/metas/contracts/pricing/trade_margin/CustomerTradeMarginRepository.java

@@ -47,7 +47,7 @@ public class CustomerTradeMarginRepository
 			.cacheName("customerTradeMarginBySalesRepAndDate")
 			.cacheMapType(CCache.CacheMapType.LRU)
 			.initialCapacity(100)
-			.additionalTableNameToResetFor(I_C_Customer_Trade_Margin.Table_Name)
+			.tableName(I_C_Customer_Trade_Margin.Table_Name)
 			.build();
 
 	public Optional<CustomerTradeMarginSettings> getBestMatchForCriteria(final CustomerTradeMarginSearchCriteria customerTradeMarginSearchCriteria)