abraham-menyhart commented on code in PR #3220:
URL: https://github.com/apache/fineract/pull/3220#discussion_r1219238103
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -4484,6 +4486,147 @@ public BigDecimal getProposedPrincipal() {
return this.proposedPrincipal;
}
+ public List<Map<String, Object>>
deriveAccountingBridgeDataForChargeOff(final String currencyCode,
Review Comment:
For readability, I would suggest to break this big method into smaller parts
(10-20 lines/method).
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -4484,6 +4486,147 @@ public BigDecimal getProposedPrincipal() {
return this.proposedPrincipal;
}
+ public List<Map<String, Object>>
deriveAccountingBridgeDataForChargeOff(final String currencyCode,
+ final List<Long> existingTransactionIds, final List<Long>
existingReversedTransactionIds, boolean isAccountTransfer) {
+
+ final List<Map<String, Object>> accountingBridgeData = new
ArrayList<>();
+
+ final Map<String, Object> accountingBridgeDataGenericAttributes = new
LinkedHashMap<>();
+ accountingBridgeDataGenericAttributes.put("loanId", getId());
+ accountingBridgeDataGenericAttributes.put("loanProductId",
productId());
+ accountingBridgeDataGenericAttributes.put("officeId", getOfficeId());
+ accountingBridgeDataGenericAttributes.put("currencyCode",
currencyCode);
+ accountingBridgeDataGenericAttributes.put("calculatedInterest",
this.summary.getTotalInterestCharged());
+
accountingBridgeDataGenericAttributes.put("cashBasedAccountingEnabled",
isCashBasedAccountingEnabledOnLoanProduct());
+
accountingBridgeDataGenericAttributes.put("upfrontAccrualBasedAccountingEnabled",
isUpfrontAccrualAccountingEnabledOnLoanProduct());
+
accountingBridgeDataGenericAttributes.put("periodicAccrualBasedAccountingEnabled",
+ isPeriodicAccrualAccountingEnabledOnLoanProduct());
+ accountingBridgeDataGenericAttributes.put("isAccountTransfer",
isAccountTransfer);
+
+ // get map before charge-off
+ final Map<String, Object> accountingBridgeDataBeforeChargeOff = new
LinkedHashMap<>(accountingBridgeDataGenericAttributes);
Review Comment:
You added 3 comments here, in this method, they are perfect markers what you
should extract into smaller methods.
Like this first one, you commented "get map before charge-off", and this
could be extracted into getAccountingBridgeDataBeforeChargeOff(), or something
like this.
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -4484,6 +4486,147 @@ public BigDecimal getProposedPrincipal() {
return this.proposedPrincipal;
}
+ public List<Map<String, Object>>
deriveAccountingBridgeDataForChargeOff(final String currencyCode,
+ final List<Long> existingTransactionIds, final List<Long>
existingReversedTransactionIds, boolean isAccountTransfer) {
+
+ final List<Map<String, Object>> accountingBridgeData = new
ArrayList<>();
+
+ final Map<String, Object> accountingBridgeDataGenericAttributes = new
LinkedHashMap<>();
Review Comment:
For example extract this part (creating the
accountingBridgeDataGenericAttributes) into a method, like
createAccountingBridgeDataGenericAttributes, or something like this.
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -4484,6 +4486,147 @@ public BigDecimal getProposedPrincipal() {
return this.proposedPrincipal;
}
+ public List<Map<String, Object>>
deriveAccountingBridgeDataForChargeOff(final String currencyCode,
+ final List<Long> existingTransactionIds, final List<Long>
existingReversedTransactionIds, boolean isAccountTransfer) {
+
+ final List<Map<String, Object>> accountingBridgeData = new
ArrayList<>();
+
+ final Map<String, Object> accountingBridgeDataGenericAttributes = new
LinkedHashMap<>();
+ accountingBridgeDataGenericAttributes.put("loanId", getId());
+ accountingBridgeDataGenericAttributes.put("loanProductId",
productId());
+ accountingBridgeDataGenericAttributes.put("officeId", getOfficeId());
+ accountingBridgeDataGenericAttributes.put("currencyCode",
currencyCode);
+ accountingBridgeDataGenericAttributes.put("calculatedInterest",
this.summary.getTotalInterestCharged());
+
accountingBridgeDataGenericAttributes.put("cashBasedAccountingEnabled",
isCashBasedAccountingEnabledOnLoanProduct());
+
accountingBridgeDataGenericAttributes.put("upfrontAccrualBasedAccountingEnabled",
isUpfrontAccrualAccountingEnabledOnLoanProduct());
+
accountingBridgeDataGenericAttributes.put("periodicAccrualBasedAccountingEnabled",
+ isPeriodicAccrualAccountingEnabledOnLoanProduct());
+ accountingBridgeDataGenericAttributes.put("isAccountTransfer",
isAccountTransfer);
+
+ // get map before charge-off
+ final Map<String, Object> accountingBridgeDataBeforeChargeOff = new
LinkedHashMap<>(accountingBridgeDataGenericAttributes);
+ accountingBridgeDataBeforeChargeOff.put("isChargeOff", false);
+ accountingBridgeDataBeforeChargeOff.put("isFraud", false);
+
+ Predicate<LoanTransaction> isBeforeChargeOff = transaction ->
transaction.getTransactionDate().isBefore(getChargedOffOnDate());
+ final List<Map<String, Object>> newLoanTransactionsBeforeChargeOff =
new ArrayList<>();
+
+ getTransactionsForAccountingBridgeData(currencyCode,
existingTransactionIds, existingReversedTransactionIds,
+ newLoanTransactionsBeforeChargeOff, isBeforeChargeOff);
+
+ // get map after charge-off
+ final Map<String, Object> accountingBridgeDataAfterChargeOff = new
LinkedHashMap<>(accountingBridgeDataGenericAttributes);
+ accountingBridgeDataAfterChargeOff.put("isChargeOff", isChargedOff());
+ accountingBridgeDataAfterChargeOff.put("isFraud", isFraud());
+
+ Predicate<LoanTransaction> isAfterChargeOff = transaction ->
transaction.getTransactionDate().isAfter(getChargedOffOnDate());
+ final List<Map<String, Object>> newLoanTransactionsAfterChargeOff =
new ArrayList<>();
+
+ getTransactionsForAccountingBridgeData(currencyCode,
existingTransactionIds, existingReversedTransactionIds,
+ newLoanTransactionsAfterChargeOff, isAfterChargeOff);
+
+ // get map onCharge off date
+ Predicate<LoanTransaction> isOnChargeOff = transaction ->
transaction.getTransactionDate().isEqual(getChargedOffOnDate());
+ getTransactionsForAccountingBridgeDataOnChargeOffDate(currencyCode,
existingTransactionIds, existingReversedTransactionIds,
+ newLoanTransactionsBeforeChargeOff,
newLoanTransactionsAfterChargeOff, isOnChargeOff);
+
+ accountingBridgeDataBeforeChargeOff.put("newLoanTransactions",
newLoanTransactionsBeforeChargeOff);
+ accountingBridgeData.add(accountingBridgeDataBeforeChargeOff);
+
+ accountingBridgeDataAfterChargeOff.put("newLoanTransactions",
newLoanTransactionsAfterChargeOff);
+ accountingBridgeData.add(accountingBridgeDataAfterChargeOff);
+
+ return accountingBridgeData;
+ }
+
+ private void getTransactionsForAccountingBridgeDataOnChargeOffDate(String
currencyCode, List<Long> existingTransactionIds,
+ List<Long> existingReversedTransactionIds, List<Map<String,
Object>> newLoanTransactionsBeforeChargeOff,
+ List<Map<String, Object>> newLoanTransactionsAfterChargeOff,
Predicate<LoanTransaction> chargeOffDateCriteria) {
+ List<LoanTransaction> transactionsOnChargeOffDate =
this.loanTransactions.stream().filter(chargeOffDateCriteria)
+ .collect(Collectors.toList());
+ /**
+ *
+ * TODO: Modify logic to retrieve correct charge-off transaction once
reverse replay of charge-off is
+ * implemented
+ */
+ LoanTransaction chargeOffTransaction =
this.loanTransactions.stream().filter(LoanTransaction::isChargeOff).findFirst().get();
+ Predicate<LoanTransactionRelation> isReplayed = transactionRelation ->
transactionRelation.getRelationType()
+ .equals(LoanTransactionRelationTypeEnum.REPLAYED);
+ for (final LoanTransaction transaction : transactionsOnChargeOffDate) {
+ if (!transaction.isChargeOff()) {
+ if (transaction.isReversed() &&
existingTransactionIds.contains(transaction.getId())
+ &&
!existingReversedTransactionIds.contains(transaction.getId())) {
+ // transaction on charge-off date got reversed
+
compareWithChargeOffIdAndAddTransactionForAccountingData(currencyCode,
newLoanTransactionsBeforeChargeOff,
+ newLoanTransactionsAfterChargeOff, transaction,
chargeOffTransaction.getId());
+
+ } else if
(!existingTransactionIds.contains(transaction.getId())) {
+ // new transaction,backdated transaction or reverse replay
on chargeoff date
+
+ // check transaction relations
+ if (!transaction.getLoanTransactionRelations().isEmpty()) {
+ // if relations has REPLAYED relation then it is
reverse replay scenario
+ List<LoanTransactionRelation>
replayedTransactionRelations =
transaction.getLoanTransactionRelations().stream()
+
.filter(isReplayed).collect(Collectors.toList());
+ if (!replayedTransactionRelations.isEmpty()) {
+ // get transaction Id for first reversed
transaction relation
+ Long transactionIdForReversedTransaction =
replayedTransactionRelations.get(0).getToTransaction().getId();
+ if (transactionIdForReversedTransaction >
chargeOffTransaction.getId()) {
Review Comment:
I'm not sure about this, but are we using transaction id here, to determine
whether the transaction happened before/after the chargeOff? I saw
transactionDate in the code above, I think that is a better approach to use
that.
But I'm not familiar with the business logic here. Maybe @adamsaghy could
help me with this.
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -4484,6 +4486,147 @@ public BigDecimal getProposedPrincipal() {
return this.proposedPrincipal;
}
+ public List<Map<String, Object>>
deriveAccountingBridgeDataForChargeOff(final String currencyCode,
+ final List<Long> existingTransactionIds, final List<Long>
existingReversedTransactionIds, boolean isAccountTransfer) {
+
+ final List<Map<String, Object>> accountingBridgeData = new
ArrayList<>();
+
+ final Map<String, Object> accountingBridgeDataGenericAttributes = new
LinkedHashMap<>();
+ accountingBridgeDataGenericAttributes.put("loanId", getId());
+ accountingBridgeDataGenericAttributes.put("loanProductId",
productId());
+ accountingBridgeDataGenericAttributes.put("officeId", getOfficeId());
+ accountingBridgeDataGenericAttributes.put("currencyCode",
currencyCode);
+ accountingBridgeDataGenericAttributes.put("calculatedInterest",
this.summary.getTotalInterestCharged());
+
accountingBridgeDataGenericAttributes.put("cashBasedAccountingEnabled",
isCashBasedAccountingEnabledOnLoanProduct());
+
accountingBridgeDataGenericAttributes.put("upfrontAccrualBasedAccountingEnabled",
isUpfrontAccrualAccountingEnabledOnLoanProduct());
+
accountingBridgeDataGenericAttributes.put("periodicAccrualBasedAccountingEnabled",
+ isPeriodicAccrualAccountingEnabledOnLoanProduct());
+ accountingBridgeDataGenericAttributes.put("isAccountTransfer",
isAccountTransfer);
+
+ // get map before charge-off
+ final Map<String, Object> accountingBridgeDataBeforeChargeOff = new
LinkedHashMap<>(accountingBridgeDataGenericAttributes);
+ accountingBridgeDataBeforeChargeOff.put("isChargeOff", false);
+ accountingBridgeDataBeforeChargeOff.put("isFraud", false);
+
+ Predicate<LoanTransaction> isBeforeChargeOff = transaction ->
transaction.getTransactionDate().isBefore(getChargedOffOnDate());
+ final List<Map<String, Object>> newLoanTransactionsBeforeChargeOff =
new ArrayList<>();
+
+ getTransactionsForAccountingBridgeData(currencyCode,
existingTransactionIds, existingReversedTransactionIds,
+ newLoanTransactionsBeforeChargeOff, isBeforeChargeOff);
+
+ // get map after charge-off
+ final Map<String, Object> accountingBridgeDataAfterChargeOff = new
LinkedHashMap<>(accountingBridgeDataGenericAttributes);
+ accountingBridgeDataAfterChargeOff.put("isChargeOff", isChargedOff());
+ accountingBridgeDataAfterChargeOff.put("isFraud", isFraud());
+
+ Predicate<LoanTransaction> isAfterChargeOff = transaction ->
transaction.getTransactionDate().isAfter(getChargedOffOnDate());
+ final List<Map<String, Object>> newLoanTransactionsAfterChargeOff =
new ArrayList<>();
+
+ getTransactionsForAccountingBridgeData(currencyCode,
existingTransactionIds, existingReversedTransactionIds,
+ newLoanTransactionsAfterChargeOff, isAfterChargeOff);
+
+ // get map onCharge off date
+ Predicate<LoanTransaction> isOnChargeOff = transaction ->
transaction.getTransactionDate().isEqual(getChargedOffOnDate());
+ getTransactionsForAccountingBridgeDataOnChargeOffDate(currencyCode,
existingTransactionIds, existingReversedTransactionIds,
+ newLoanTransactionsBeforeChargeOff,
newLoanTransactionsAfterChargeOff, isOnChargeOff);
+
+ accountingBridgeDataBeforeChargeOff.put("newLoanTransactions",
newLoanTransactionsBeforeChargeOff);
+ accountingBridgeData.add(accountingBridgeDataBeforeChargeOff);
+
+ accountingBridgeDataAfterChargeOff.put("newLoanTransactions",
newLoanTransactionsAfterChargeOff);
+ accountingBridgeData.add(accountingBridgeDataAfterChargeOff);
+
+ return accountingBridgeData;
+ }
+
+ private void getTransactionsForAccountingBridgeDataOnChargeOffDate(String
currencyCode, List<Long> existingTransactionIds,
+ List<Long> existingReversedTransactionIds, List<Map<String,
Object>> newLoanTransactionsBeforeChargeOff,
+ List<Map<String, Object>> newLoanTransactionsAfterChargeOff,
Predicate<LoanTransaction> chargeOffDateCriteria) {
+ List<LoanTransaction> transactionsOnChargeOffDate =
this.loanTransactions.stream().filter(chargeOffDateCriteria)
+ .collect(Collectors.toList());
+ /**
+ *
+ * TODO: Modify logic to retrieve correct charge-off transaction once
reverse replay of charge-off is
+ * implemented
+ */
+ LoanTransaction chargeOffTransaction =
this.loanTransactions.stream().filter(LoanTransaction::isChargeOff).findFirst().get();
+ Predicate<LoanTransactionRelation> isReplayed = transactionRelation ->
transactionRelation.getRelationType()
Review Comment:
Generally, it is better to use the `equals` on the constant, not on the
object. If the `getRelationType()` returns a null, then you got
`NullPointException`. But if you wrote
`REPLAYED.equals(transactionRelation.getRelationType())` , then you are safe.
Or you could use `Objects.equals()`, and the order wont matter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
