Skip to content

Commit

Permalink
5522-app - Commission calculation
Browse files Browse the repository at this point in the history 
- fix bug where commission instance without shares was created if the contracts of the sales reps in question didn'T matche the commission trigger
- also some refactoring
#5522
  • Loading branch information
@metas-ts
metas-ts committed Oct 30, 2019
1 parent 736e667 commit c0b477b
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 38 deletions.
32 changes: 14 additions & 18 deletions ...s/contracts/commission/commissioninstance/businesslogic/algorithms/HierachyAlgorithm.java
View file
Expand Up @@ -88,17 +88,14 @@ private ImmutableList<SalesCommissionShare> createNewShares(
{
final Beneficiary beneficiary = node.getBeneficiary();
final HierarchyContract contract = HierarchyContract.cast(config.getContractFor(beneficiary));
if (contract == null)
if (contract != null)
{
// current hierarchy member doesn't have a contract; skip it.
continue;

final SalesCommissionShare share = SalesCommissionShare.builder()
.beneficiary(beneficiary)
.level(currentLevel)
.build();
shares.add(share);
}
final SalesCommissionShare share = SalesCommissionShare.builder()
.beneficiary(beneficiary)
.level(currentLevel)
.build();
shares.add(share);

currentLevel = currentLevel.incByOne();
}
Expand Down Expand Up @@ -162,23 +159,22 @@ private void createAndAddFacts(
}

private Optional<SalesCommissionFact> createFact(
final Instant timestamp,
final HierarchyContract contract,
final SalesCommissionState state,
final CommissionPoints base)
@NonNull final Instant timestamp,
@NonNull final HierarchyContract contract,
@NonNull final SalesCommissionState state,
@NonNull final CommissionPoints base)
{
final CommissionPoints percentage = base.computePercentageOf(
final CommissionPoints points = base.computePercentageOf(
contract.getCommissionPercent(),
contract.getPointsPrecision());

if (percentage.isZero())
if (points.isZero())
{
return Optional.empty();
return Optional.empty(); // a zero-points fact would not change anything, so don't bother creating it
}

final SalesCommissionFact fact = SalesCommissionFact.builder()
.state(state)
.points(percentage)
.points(points)
.timestamp(timestamp)
.build();
return Optional.of(fact);
Expand Down
4 changes: 4 additions & 0 deletions ...tas/contracts/commission/commissioninstance/businesslogic/algorithms/HierarchyConfig.java
View file
Expand Up @@ -89,4 +89,8 @@ public CommissionContract getContractFor(@NonNull final Beneficiary beneficiary)
return bpartnerId2HierarchyContracts.get(beneficiary.getBPartnerId().getRepoId());
}

public boolean containsContracts()
{
return !bpartnerId2HierarchyContracts.isEmpty();
}
}
20 changes: 13 additions & 7 deletions ...va/de/metas/contracts/commission/commissioninstance/services/CommissionConfigFactory.java
View file
Expand Up @@ -82,7 +82,7 @@ public CommissionConfigFactory(
this.commissionConfigStagingDataService = commissionConfigStagingDataService;
}

public ImmutableList<CommissionConfig> createForNewCommissionInstances(@NonNull final ContractRequest contractRequest)
public ImmutableList<CommissionConfig> createForNewCommissionInstances(@NonNull final ConfigRequestForNewInstance contractRequest)
{
final Hierarchy hierarchy = commissionHierarchyFactory.createFor(contractRequest.getSalesRepBPartnerId());
final Iterable<HierarchyNode> beneficiaries = hierarchy.getUpStream(Beneficiary.of(contractRequest.getSalesRepBPartnerId()));
Expand Down Expand Up @@ -119,7 +119,7 @@ private ImmutableList<CommissionConfig> createCommissionConfigsFor(
final ImmutableMap<BPartnerId, FlatrateTermId> bPartnerId2FlatrateTermIds = stagingData.getBPartnerId2FlatrateTermIds();
final ImmutableListMultimap<Integer, BPartnerId> conditionRecordId2BPartnerIds = stagingData.getConditionRecordId2BPartnerIds();

final ImmutableList.Builder<CommissionConfig> result = ImmutableList.<CommissionConfig> builder();
final ImmutableList.Builder<CommissionConfig> commissionConfigs = ImmutableList.<CommissionConfig> builder();

final ProductCategoryId salesProductCategory = productDAO.retrieveProductCategoryByProductId(salesproductId);
final BPGroupId customerGroupId = bPartnerDAO.getBPGroupIdByBPartnerId(customerBPartnerId);
Expand Down Expand Up @@ -164,9 +164,13 @@ private ImmutableList<CommissionConfig> createCommissionConfigsFor(
}
}
}
result.add(builder.build());
final HierarchyConfig config = builder.build();
if (config.containsContracts()) // discard it if there aren't any beneficiaries/contracts
{
commissionConfigs.add(config);
}
}
return result.build();
return commissionConfigs.build();
}

private boolean settingsLineRecordMatches(
Expand All @@ -188,22 +192,24 @@ private boolean settingsLineRecordMatches(

@Builder
@Value
public static class ContractRequest
public static class ConfigRequestForNewInstance
{
@NonNull
BPartnerId salesRepBPartnerId;

/** Needed because config settings can be specific to the customer's group. */
@NonNull
BPartnerId customerBPartnerId;

/** Needed because config settings can be specific to the product's category. */
@NonNull
ProductId salesProductId;

@NonNull
LocalDate date;
}

public CommissionConfig createForExisingInstance(@NonNull final CommissionConfigRequest commissionConfigRequest)
public CommissionConfig createForExisingInstance(@NonNull final ConfigRequestForExistingInstance commissionConfigRequest)
{
final ImmutableList<I_C_Flatrate_Term> commissionTermRecords = flatrateDAO
.retrieveTerms(commissionConfigRequest.getContractIds())
Expand All @@ -228,7 +234,7 @@ public CommissionConfig createForExisingInstance(@NonNull final CommissionConfig

@Builder
@Value
public static class CommissionConfigRequest
public static class ConfigRequestForExistingInstance
{
@NonNull
ImmutableList<FlatrateTermId> contractIds;
Expand Down
4 changes: 2 additions & 2 deletions ...as/contracts/commission/commissioninstance/services/CommissionInstanceRequestFactory.java
View file
Expand Up @@ -14,7 +14,7 @@
import de.metas.contracts.commission.commissioninstance.businesslogic.CreateInstanceRequest;
import de.metas.contracts.commission.commissioninstance.businesslogic.hierarchy.Hierarchy;
import de.metas.contracts.commission.commissioninstance.businesslogic.sales.CommissionTrigger;
import de.metas.contracts.commission.commissioninstance.services.CommissionConfigFactory.ContractRequest;
import de.metas.contracts.commission.commissioninstance.services.CommissionConfigFactory.ConfigRequestForNewInstance;
import de.metas.invoicecandidate.InvoiceCandidateId;
import de.metas.invoicecandidate.model.I_C_Invoice_Candidate;
import de.metas.product.ProductId;
Expand Down Expand Up @@ -74,7 +74,7 @@ private ImmutableList<CreateInstanceRequest> createRequestFor(@NonNull final I_C
return ImmutableList.of();
}

final ContractRequest contractRequest = ContractRequest.builder()
final ConfigRequestForNewInstance contractRequest = ConfigRequestForNewInstance.builder()
.customerBPartnerId(BPartnerId.ofRepoId(icRecord.getBill_BPartner_ID()))
.salesRepBPartnerId(salesRepBPartnerId)
.date(TimeUtil.asLocalDate(icRecord.getDateOrdered()))
Expand Down
5 changes: 3 additions & 2 deletions .../metas/contracts/commission/commissioninstance/services/SalesInvoiceCandidateService.java
View file
Expand Up @@ -65,11 +65,12 @@ public void syncSalesICToCommissionInstance(@NonNull final InvoiceCandidateId in
final List<CommissionInstance> instances = commissionInstanceRepository.getForInvoiceCandidateId(invoiceCandidateId);
if (instances.isEmpty())
{
if(candidateDeleted)
if (candidateDeleted)
{
return; // nothing to do
}
// initially create commission data for the given invoice candidate
// initially create commission data for the given invoice candidate;
// the list of requests might be empty, if there are no matching contracts and/or settings
final ImmutableList<CreateInstanceRequest> requests = commissionInstanceRequestFactory.createRequestsForNewSalesInvoiceCandidate(invoiceCandidateId);
for (final CreateInstanceRequest request : requests)
{
Expand Down
10 changes: 5 additions & 5 deletions .../contracts/commission/commissioninstance/services/repos/CommissionInstanceRepository.java
View file
Expand Up @@ -36,7 +36,7 @@
import de.metas.contracts.commission.commissioninstance.businesslogic.sales.SalesCommissionShare.SalesCommissionShareBuilder;
import de.metas.contracts.commission.commissioninstance.businesslogic.sales.SalesCommissionState;
import de.metas.contracts.commission.commissioninstance.services.CommissionConfigFactory;
import de.metas.contracts.commission.commissioninstance.services.CommissionConfigFactory.CommissionConfigRequest;
import de.metas.contracts.commission.commissioninstance.services.CommissionConfigFactory.ConfigRequestForExistingInstance;
import de.metas.contracts.commission.commissioninstance.services.repos.CommissionRecordStagingService.CommissionStagingRecords;
import de.metas.contracts.commission.model.I_C_Commission_Fact;
import de.metas.contracts.commission.model.I_C_Commission_Instance;
Expand Down Expand Up @@ -87,7 +87,7 @@ public CommissionInstance getForCommissionInstanceId(@NonNull final CommissionIn
final CommissionStagingRecords stagingRecords = commissionRecordStagingService.retrieveRecordsForInstanceId(ImmutableList.of(commissionInstanceId));

final I_C_Commission_Instance instanceRecord = stagingRecords.getInstanceRecordIdToInstance().get(commissionInstanceId.getRepoId());
final CommissionInstance instance = createCommissionInstance(instanceRecord, stagingRecords);
final CommissionInstance instance = loadCommissionInstance(instanceRecord, stagingRecords);
return instance;
}

Expand All @@ -104,13 +104,13 @@ public ImmutableList<CommissionInstance> getForInvoiceCandidateId(@NonNull final
final ImmutableList.Builder<CommissionInstance> result = ImmutableList.builder();
for (final I_C_Commission_Instance instanceRecord : instanceRecords)
{
final CommissionInstance instance = createCommissionInstance(instanceRecord, records);
final CommissionInstance instance = loadCommissionInstance(instanceRecord, records);
result.add(instance);
}
return result.build();
}

private CommissionInstance createCommissionInstance(
private CommissionInstance loadCommissionInstance(
@NonNull final I_C_Commission_Instance instanceRecord,
@NonNull final CommissionStagingRecords stagingRecords)
{
Expand All @@ -123,7 +123,7 @@ private CommissionInstance createCommissionInstance(
.map(FlatrateTermId::ofRepoId)
.collect(ImmutableList.toImmutableList());

final CommissionConfigRequest request = CommissionConfigRequest.builder()
final ConfigRequestForExistingInstance request = ConfigRequestForExistingInstance.builder()
.contractIds(flatrateTermIds)
.customerBPartnerId(BPartnerId.ofRepoId(instanceRecord.getBill_BPartner_ID()))
.salesProductId(ProductId.ofRepoId(instanceRecord.getM_Product_Order_ID()))
Expand Down
4 changes: 4 additions & 0 deletions ...postgresql/system/50-de.metas.contracts/5535390_cli_gh5522-app_cleanup_empty_intances.sql
View file
@@ -0,0 +1,4 @@

-- they shouldn'T have been created to start with
delete from C_Commission_Instance i where not exists (select 1 from c_commission_share s where s.C_Commission_Instance_ID=i.C_Commission_Instance_ID);

49 changes: 45 additions & 4 deletions ...e/metas/contracts/commission/commissioninstance/services/CommissionConfigFactoryTest.java
View file
Expand Up @@ -13,6 +13,7 @@
import org.junit.jupiter.api.Test;

import com.google.common.collect.ImmutableList;

import de.metas.adempiere.model.I_M_Product;
import de.metas.bpartner.BPGroupId;
import de.metas.bpartner.BPartnerId;
Expand All @@ -22,12 +23,11 @@
import de.metas.contracts.commission.commissioninstance.businesslogic.CommissionType;
import de.metas.contracts.commission.commissioninstance.businesslogic.algorithms.HierarchyConfig;
import de.metas.contracts.commission.commissioninstance.businesslogic.algorithms.HierarchyContract;
import de.metas.contracts.commission.commissioninstance.services.CommissionConfigFactory.ContractRequest;
import de.metas.contracts.commission.commissioninstance.services.CommissionConfigFactory.ConfigRequestForNewInstance;
import de.metas.contracts.commission.testhelpers.ConfigLineTestRecord;
import de.metas.contracts.commission.testhelpers.ConfigTestRecord;
import de.metas.contracts.commission.testhelpers.ConfigTestRecord.ConfigData;
import de.metas.contracts.commission.testhelpers.ContractTestRecord;

import de.metas.product.ProductCategoryId;
import de.metas.product.ProductId;

Expand Down Expand Up @@ -75,7 +75,7 @@ void beforeEach()

final I_C_BP_Group customerBPGroupRecord = newInstance(I_C_BP_Group.class);
saveRecord(customerBPGroupRecord);
customerBPGroudId = BPGroupId.ofRepoId( customerBPGroupRecord.getC_BP_Group_ID());
customerBPGroudId = BPGroupId.ofRepoId(customerBPGroupRecord.getC_BP_Group_ID());

bpartnerRecord_EndCustomer = newInstance(I_C_BPartner.class);
bpartnerRecord_EndCustomer.setC_BP_Group_ID(customerBPGroupRecord.getC_BP_Group_ID());
Expand Down Expand Up @@ -114,7 +114,7 @@ void createFor()
final BPartnerId salesRepLvl2Id = configData.getName2BPartnerId().get("headOfSales");

// invoke method under test
final ContractRequest contractRequest = ContractRequest.builder()
final ConfigRequestForNewInstance contractRequest = ConfigRequestForNewInstance.builder()
.customerBPartnerId(endCustomerId)
.salesRepBPartnerId(salesRepLvl0Id)
.salesProductId(salesProductId)
Expand Down Expand Up @@ -143,4 +143,45 @@ void createFor()
assertThat(hierarchyContractLvl2.getCommissionPercent().toBigDecimal()).isEqualTo("20");
assertThat(hierarchyContractLvl2.getPointsPrecision()).isEqualTo(3);
}

@Test
void createFor_no_lines()
{
final ConfigData configData = ConfigTestRecord.builder()
.pointsPrecision(3)
.subtractLowerLevelCommissionFromBase(true)
.contractTestRecord(ContractTestRecord.builder().name("salesRep").parentName("salesSupervisor").date(date).build())
.contractTestRecord(ContractTestRecord.builder().name("salesSupervisor").parentName("headOfSales").date(date).build())
.contractTestRecord(ContractTestRecord.builder().name("headOfSales").date(date).build())
.build()
.createConfigData();

final BPartnerId salesRepLvl0Id = configData.getName2BPartnerId().get("salesRep");
final BPartnerId salesRepLvl1Id = configData.getName2BPartnerId().get("salesSupervisor");
final BPartnerId salesRepLvl2Id = configData.getName2BPartnerId().get("headOfSales");

// invoke method under test
final ConfigRequestForNewInstance contractRequest = ConfigRequestForNewInstance.builder()
.customerBPartnerId(endCustomerId)
.salesRepBPartnerId(salesRepLvl0Id)
.salesProductId(salesProductId)
.date(date).build();
final ImmutableList<CommissionConfig> configs = commissionConfigFactory.createForNewCommissionInstances(contractRequest);

assertThat(configs).hasSize(1);
final CommissionConfig config = configs.get(0);
assertThat(config.getCommissionType()).isEqualTo(CommissionType.HIERARCHY_COMMISSION);

final HierarchyConfig hierarchyConfig = HierarchyConfig.cast(config);
assertThat(hierarchyConfig.isSubtractLowerLevelCommissionFromBase()).isTrue();

final CommissionContract contractLvl0 = hierarchyConfig.getContractFor(Beneficiary.of(salesRepLvl0Id));
assertThat(contractLvl0).isNull();

final CommissionContract contractLvl1 = hierarchyConfig.getContractFor(Beneficiary.of(salesRepLvl1Id));
assertThat(contractLvl1).isNull();

final CommissionContract contractLvl2 = hierarchyConfig.getContractFor(Beneficiary.of(salesRepLvl2Id));
assertThat(contractLvl2).isNull();
}
}

0 comments on commit c0b477b

Please sign in to comment.