You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2022/11/11 01:40:31 UTC
[GitHub] [fineract] josehernandezfintecheandomx opened a new pull request, #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
josehernandezfintecheandomx opened a new pull request, #2739:
URL: https://github.com/apache/fineract/pull/2739
## Description
The delinquency calculation must to happen as of the latest unpaid chargeback transaction date in order to reflect the right delinquency days and amount
[FINERACT-1706](https://issues.apache.org/jira/browse/FINERACT-1706).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1022466751
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,120 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+
+public final class DelinquencyUtils {
+
+ private DelinquencyUtils() {}
+
+ public static CollectionData getOverdueCollectionData(final Loan loan, final LocalDate businessDate) {
+ final MonetaryCurrency loanCurrency = loan.getCurrency();
+ LocalDate overdueSinceDate = null;
+ CollectionData collectionData = CollectionData.template();
+ BigDecimal amountAvailable = BigDecimal.ZERO;
+ BigDecimal outstandingAmount = BigDecimal.ZERO;
+ boolean oldestOverdueInstallment = false;
+ boolean overdueSinceDateWasSet = false;
+ boolean firstNotYetDueInstallment = false;
+
+ // Get the oldest overdue installment if exists one
+ for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
+ if (!installment.isObligationsMet()) {
+ if (installment.getDueDate().isBefore(businessDate)) {
+ outstandingAmount = outstandingAmount.add(installment.getTotalOutstanding(loanCurrency).getAmount());
+ if (!oldestOverdueInstallment) {
+ oldestOverdueInstallment = true;
+ overdueSinceDate = installment.getDueDate();
+ overdueSinceDateWasSet = true;
+
+ amountAvailable = installment.getTotalPaid(loanCurrency).getAmount();
+
+ for (LoanTransactionToRepaymentScheduleMapping mappngInstallment : installment
+ .getLoanTransactionToRepaymentScheduleMappings()) {
+ final LoanTransaction loanTransaction = mappngInstallment.getLoanTransaction();
+ if (loanTransaction.isChargeback()) {
+ amountAvailable = amountAvailable.subtract(loanTransaction.getAmount());
+ if (amountAvailable.compareTo(BigDecimal.ZERO) < 0) {
+ overdueSinceDate = loanTransaction.getTransactionDate();
+ break;
+ }
+ }
+ }
+ }
+ }
+
+ } else if (!firstNotYetDueInstallment) {
+ firstNotYetDueInstallment = true;
+ amountAvailable = installment.getTotalPaid(loanCurrency).getAmount();
+
+ for (LoanTransactionToRepaymentScheduleMapping mappngInstallment : installment
+ .getLoanTransactionToRepaymentScheduleMappings()) {
+ final LoanTransaction loanTransaction = mappngInstallment.getLoanTransaction();
+ if (loanTransaction.isChargeback() && loanTransaction.getTransactionDate().isBefore(businessDate)) {
+ amountAvailable = amountAvailable.subtract(loanTransaction.getAmount());
+ if (amountAvailable.compareTo(BigDecimal.ZERO) < 0 && !overdueSinceDateWasSet) {
+ overdueSinceDate = loanTransaction.getTransactionDate();
+ overdueSinceDateWasSet = true;
+ break;
Review Comment:
We dont want to break the iteration. We need to process all the due chargeback transactions. Please remove the `break`.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1022885029
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanTransactionChargebackTest.java:
##########
@@ -160,7 +159,7 @@ public void applyLoanTransactionChargebackInLongTermLoan() {
}
loanTransactionHelper.evaluateLoanSummaryAdjustments(getLoansLoanIdResponse, Double.valueOf(baseAmount));
- DelinquencyBucketsHelper.evaluateLoanCollectionData(getLoansLoanIdResponse, 0, Double.valueOf("333.33"));
+ DelinquencyBucketsHelper.evaluateLoanCollectionData(getLoansLoanIdResponse, 0, Double.valueOf("0.00"));
Review Comment:
Would you please extend this test case to increase the business date by 1 and run the COB and check the collectiondata will be updated with the proper details?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1022467315
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java:
##########
@@ -109,52 +111,30 @@ public Collection<LoanDelinquencyTagHistoryData> retrieveDelinquencyRangeHistory
@Override
public CollectionData calculateLoanCollectionData(final Long loanId) {
- final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Optional<Loan> optLoan = this.loanRepository.findById(loanId);
CollectionData collectionData = CollectionData.template();
if (optLoan.isPresent()) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Loan loan = optLoan.get();
+ Integer graceDays = 0;
+ if (loan.loanProduct().getLoanProductConfigurableAttributes().getGraceOnArrearsAgingBoolean()) {
+ graceDays = loan.loanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
+ if (graceDays == null) {
+ graceDays = 0;
+ }
+ }
- collectionData.setAvailableDisbursementAmount(loan.getApprovedPrincipal().subtract(loan.getDisbursedAmount()));
+ collectionData.setAvailableDisbursementAmount(loan.getApprovedPrincipal().subtract(loan.getNetDisbursalAmount()));
Review Comment:
@josehernandezfintecheandomx This is still not fixed. Please share your thoughts about the issue I listed above.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1023495965
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionType;
+
+public final class DelinquencyUtils {
+
+ private DelinquencyUtils() {}
+
+ public static LocalDate getOverdueSinceDate(final Loan loan, final LocalDate businessDate, final Integer graceOnArrearAgeing) {
+ final MonetaryCurrency loanCurrency = loan.getCurrency();
+ LoanRepaymentScheduleInstallment loanRepaymentSchedule = null;
+ for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
+ if (installment.getDueDate().isBefore(businessDate) && installment.isNotFullyPaidOff()) {
+ loanRepaymentSchedule = installment;
+ break;
+ }
+ }
+
+ LocalDate overdueSinceDate = null;
+ if (loanRepaymentSchedule != null) {
+ // Default Due date
+ overdueSinceDate = loanRepaymentSchedule.getDueDate();
+ }
+
+ Map<Integer, BigDecimal> installmentBalances = new HashMap<>();
+ for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
+ installmentBalances.put(installment.getInstallmentNumber(), installment.getPrincipalCompleted(loanCurrency).getAmount());
+ }
+
+ // If there is some Loan Transaction Chargeback
+ final List<LoanTransaction> loanTransactions = loan.retrieveListOfTransactionsByType(LoanTransactionType.CHARGEBACK);
+ for (LoanTransaction loanTransaction : loanTransactions) {
Review Comment:
Done
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionType;
+
+public final class DelinquencyUtils {
+
+ private DelinquencyUtils() {}
+
+ public static LocalDate getOverdueSinceDate(final Loan loan, final LocalDate businessDate, final Integer graceOnArrearAgeing) {
+ final MonetaryCurrency loanCurrency = loan.getCurrency();
+ LoanRepaymentScheduleInstallment loanRepaymentSchedule = null;
+ for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
+ if (installment.getDueDate().isBefore(businessDate) && installment.isNotFullyPaidOff()) {
+ loanRepaymentSchedule = installment;
+ break;
+ }
+ }
+
+ LocalDate overdueSinceDate = null;
+ if (loanRepaymentSchedule != null) {
+ // Default Due date
+ overdueSinceDate = loanRepaymentSchedule.getDueDate();
+ }
+
+ Map<Integer, BigDecimal> installmentBalances = new HashMap<>();
Review Comment:
Removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1023495787
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,120 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+
+public final class DelinquencyUtils {
+
+ private DelinquencyUtils() {}
+
+ public static CollectionData getOverdueCollectionData(final Loan loan, final LocalDate businessDate) {
+ final MonetaryCurrency loanCurrency = loan.getCurrency();
+ LocalDate overdueSinceDate = null;
+ CollectionData collectionData = CollectionData.template();
+ BigDecimal amountAvailable = BigDecimal.ZERO;
+ BigDecimal outstandingAmount = BigDecimal.ZERO;
+ boolean oldestOverdueInstallment = false;
+ boolean overdueSinceDateWasSet = false;
+ boolean firstNotYetDueInstallment = false;
+
+ // Get the oldest overdue installment if exists one
+ for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
+ if (!installment.isObligationsMet()) {
+ if (installment.getDueDate().isBefore(businessDate)) {
+ outstandingAmount = outstandingAmount.add(installment.getTotalOutstanding(loanCurrency).getAmount());
+ if (!oldestOverdueInstallment) {
+ oldestOverdueInstallment = true;
+ overdueSinceDate = installment.getDueDate();
+ overdueSinceDateWasSet = true;
+
+ amountAvailable = installment.getTotalPaid(loanCurrency).getAmount();
+
+ for (LoanTransactionToRepaymentScheduleMapping mappngInstallment : installment
+ .getLoanTransactionToRepaymentScheduleMappings()) {
+ final LoanTransaction loanTransaction = mappngInstallment.getLoanTransaction();
+ if (loanTransaction.isChargeback()) {
+ amountAvailable = amountAvailable.subtract(loanTransaction.getAmount());
+ if (amountAvailable.compareTo(BigDecimal.ZERO) < 0) {
+ overdueSinceDate = loanTransaction.getTransactionDate();
+ break;
+ }
+ }
+ }
+ }
+ }
+
+ } else if (!firstNotYetDueInstallment) {
+ firstNotYetDueInstallment = true;
+ amountAvailable = installment.getTotalPaid(loanCurrency).getAmount();
+
+ for (LoanTransactionToRepaymentScheduleMapping mappngInstallment : installment
+ .getLoanTransactionToRepaymentScheduleMappings()) {
+ final LoanTransaction loanTransaction = mappngInstallment.getLoanTransaction();
+ if (loanTransaction.isChargeback() && loanTransaction.getTransactionDate().isBefore(businessDate)) {
+ amountAvailable = amountAvailable.subtract(loanTransaction.getAmount());
+ if (amountAvailable.compareTo(BigDecimal.ZERO) < 0 && !overdueSinceDateWasSet) {
+ overdueSinceDate = loanTransaction.getTransactionDate();
+ overdueSinceDateWasSet = true;
+ break;
Review Comment:
Removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1023669228
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6781,15 +6781,19 @@ public Collection<LoanCharge> getLoanCharges() {
public void initializeLazyCollections() {
checkAndFetchLazyCollection(this.charges);
checkAndFetchLazyCollection(this.trancheCharges);
- checkAndFetchLazyCollection(this.repaymentScheduleInstallments);
- checkAndFetchLazyCollection(this.loanTransactions);
+ initializeLazyBasicCollections();
checkAndFetchLazyCollection(this.disbursementDetails);
checkAndFetchLazyCollection(this.loanTermVariations);
checkAndFetchLazyCollection(this.collateral);
checkAndFetchLazyCollection(this.loanOfficerHistory);
checkAndFetchLazyCollection(this.loanCollateralManagements);
}
+ public void initializeLazyBasicCollections() {
Review Comment:
Please rename this. "Basic collections" is really non-descriptive method name. Suggestion: `fetchRepaymentScheduleAndTransactions()`
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1025226388
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java:
##########
@@ -109,89 +104,22 @@ public Collection<LoanDelinquencyTagHistoryData> retrieveDelinquencyRangeHistory
@Override
public CollectionData calculateLoanCollectionData(final Long loanId) {
- final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Optional<Loan> optLoan = this.loanRepository.findById(loanId);
CollectionData collectionData = CollectionData.template();
if (optLoan.isPresent()) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Loan loan = optLoan.get();
+ collectionData = this.loanCollectionDomainService.getOverdueCollectionData(loan, businessDate);
Review Comment:
Removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1025239837
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/LoanCollectionDomainService.java:
##########
@@ -0,0 +1,36 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.time.LocalDate;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+
+public interface LoanCollectionDomainService {
+
+ /**
+ * This method is to calculate the Overdue date and other properties, If the loan is overdue or If there is some
+ * Charge back transaction
+ *
+ * @param loan
+ * @param businessDate
+ */
+ CollectionData getOverdueCollectionData(Loan loan, LocalDate businessDate);
Review Comment:
Updated
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1023495694
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanTransactionChargebackTest.java:
##########
@@ -160,7 +159,7 @@ public void applyLoanTransactionChargebackInLongTermLoan() {
}
loanTransactionHelper.evaluateLoanSummaryAdjustments(getLoansLoanIdResponse, Double.valueOf(baseAmount));
- DelinquencyBucketsHelper.evaluateLoanCollectionData(getLoansLoanIdResponse, 0, Double.valueOf("333.33"));
+ DelinquencyBucketsHelper.evaluateLoanCollectionData(getLoansLoanIdResponse, 0, Double.valueOf("0.00"));
Review Comment:
Done!
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1023676642
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepaymentScheduleInstallment.java:
##########
@@ -140,6 +140,9 @@ public class LoanRepaymentScheduleInstallment extends AbstractAuditableWithUTCDa
@OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY, mappedBy = "installment")
private Set<LoanInstallmentCharge> installmentCharges = new HashSet<>();
+ @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER, mappedBy = "installment")
+ private Set<LoanTransactionToRepaymentScheduleMapping> loanTransactionToRepaymentScheduleMappings = new HashSet<>();
Review Comment:
I am a little hesitant whether we need to fetch the mapping eagerly. Can you tell me why did you decide to do so? Eager loading only required if we would read an association when the entity is already detached. I dont think that would happen here (or should)...i guess we need to make sure this entity is used in transactional block
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1023711751
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+
+@Slf4j
+public final class DelinquencyUtils {
Review Comment:
I don't like this. When I see the following names:
- Manager
- Utils
- Util
- etc
I'm always careful because usually it's something fishy. Not always but most of the times. I think this is rather a poor implementation for separation of concerns. On one hand, let's forbid ourselves from using static methods unless there's a good reason for it.
In this case this class is handling business logic. That's not a Util and that shouldn't be handled in static methods.
Try to generalize the class/method around single responsibilities and try to split it that way into components.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6781,15 +6781,19 @@ public Collection<LoanCharge> getLoanCharges() {
public void initializeLazyCollections() {
checkAndFetchLazyCollection(this.charges);
checkAndFetchLazyCollection(this.trancheCharges);
- checkAndFetchLazyCollection(this.repaymentScheduleInstallments);
- checkAndFetchLazyCollection(this.loanTransactions);
+ initializeLazyBasicCollections();
checkAndFetchLazyCollection(this.disbursementDetails);
checkAndFetchLazyCollection(this.loanTermVariations);
checkAndFetchLazyCollection(this.collateral);
checkAndFetchLazyCollection(this.loanOfficerHistory);
checkAndFetchLazyCollection(this.loanCollateralManagements);
}
+ public void initializeLazyBasicCollections() {
Review Comment:
Let's get rid of this method rather. It's spreading a bad pattern across the system. Let's try to force ourselves to properly define our transaction boundaries and the methods like this will never be used.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java:
##########
@@ -109,89 +103,23 @@ public Collection<LoanDelinquencyTagHistoryData> retrieveDelinquencyRangeHistory
@Override
public CollectionData calculateLoanCollectionData(final Long loanId) {
- final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Optional<Loan> optLoan = this.loanRepository.findById(loanId);
CollectionData collectionData = CollectionData.template();
if (optLoan.isPresent()) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Loan loan = optLoan.get();
+ loan.initializeLazyBasicCollections();
Review Comment:
100% agreed. TBH the initializeLazyXY method calls should be eliminated completely. They are mostly used to overcome the fact that the transaction boundaries are not properly defined.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepaymentScheduleInstallment.java:
##########
@@ -140,6 +140,9 @@ public class LoanRepaymentScheduleInstallment extends AbstractAuditableWithUTCDa
@OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY, mappedBy = "installment")
private Set<LoanInstallmentCharge> installmentCharges = new HashSet<>();
+ @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER, mappedBy = "installment")
+ private Set<LoanTransactionToRepaymentScheduleMapping> loanTransactionToRepaymentScheduleMappings = new HashSet<>();
Review Comment:
> we would read an association when the entity is already detached.
Not necessarily. Eager fetching comes handy when the parent entity - from a business point of view - can't live without the child entity.
I'm not sure if this is the case here though so I'm also waiting for some explanation here.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepaymentScheduleInstallment.java:
##########
@@ -718,6 +721,9 @@ public void addToCredits(final BigDecimal amount) {
}
public BigDecimal getTotalPaidInAdvance() {
+ if (this.totalPaidInAdvance == null) {
Review Comment:
This is kinda fishy to me again.
If the column can be null, that means there was no payment in advance, right? We introduce that logic and the users of this class will never be able to make that decision, right? Since we hide it here with this condition.
Also, if this logic has been introduced here, did you check the whole codebase for native SQLs/JPQLs/etc where the same DB table has been used and the same column has been retrieved? We'll need this same behavior there, right?
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+
+@Slf4j
+public final class DelinquencyUtils {
Review Comment:
Bonus, why don't I see unit tests for this class?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1023496166
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java:
##########
@@ -174,20 +154,27 @@ public CollectionData calculateLoanCollectionData(final Long loanId) {
}
// Calculate Delinquency Amount
- for (LoanRepaymentScheduleInstallment installment : installments) {
+ Map<Integer, BigDecimal> installmentBalances = new HashMap<>();
Review Comment:
Removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1020191376
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java:
##########
@@ -109,52 +111,30 @@ public Collection<LoanDelinquencyTagHistoryData> retrieveDelinquencyRangeHistory
@Override
public CollectionData calculateLoanCollectionData(final Long loanId) {
- final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Optional<Loan> optLoan = this.loanRepository.findById(loanId);
CollectionData collectionData = CollectionData.template();
if (optLoan.isPresent()) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Loan loan = optLoan.get();
+ Integer graceDays = 0;
+ if (loan.loanProduct().getLoanProductConfigurableAttributes().getGraceOnArrearsAgingBoolean()) {
+ graceDays = loan.loanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
+ if (graceDays == null) {
+ graceDays = 0;
+ }
+ }
- collectionData.setAvailableDisbursementAmount(loan.getApprovedPrincipal().subtract(loan.getDisbursedAmount()));
+ collectionData.setAvailableDisbursementAmount(loan.getApprovedPrincipal().subtract(loan.getNetDisbursalAmount()));
collectionData.setNextPaymentDueDate(loan.possibleNextRepaymentDate());
final MonetaryCurrency currency = loan.getCurrency();
BigDecimal delinquentAmount = BigDecimal.ZERO;
// Overdue Days calculation
Long overdueDays = 0L;
- LocalDate overdueSinceDate = null;
- final List<LoanTransaction> chargebackTransactions = loan.retrieveListOfTransactionsByType(LoanTransactionType.CHARGEBACK);
- for (LoanTransaction loanTransaction : chargebackTransactions) {
- Set<LoanTransactionToRepaymentScheduleMapping> loanTransactionToRepaymentScheduleMappings = loanTransaction
- .getLoanTransactionToRepaymentScheduleMappings();
- LoanTransactionToRepaymentScheduleMapping loanTransactionToRepaymentScheduleMapping = loanTransactionToRepaymentScheduleMappings
- .iterator().next();
- if (!loanTransactionToRepaymentScheduleMapping.getLoanRepaymentScheduleInstallment().isPrincipalCompleted(currency)) {
- overdueSinceDate = loanTransaction.getTransactionDate();
- break;
- }
- }
+ final LocalDate overdueSinceDate = DelinquencyUtils.getOverdueSinceDate(loan, businessDate, graceDays);
Review Comment:
You can get the overDueSinceDate and the amount in the same calculation. You dont need to do twice the same:
- DelinquencyUtils.getOverdueSinceDate(loan, businessDate, graceDays)
- From line 159-177, it is the same.
Please merge them together.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1020042142
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java:
##########
@@ -109,52 +111,30 @@ public Collection<LoanDelinquencyTagHistoryData> retrieveDelinquencyRangeHistory
@Override
public CollectionData calculateLoanCollectionData(final Long loanId) {
- final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Optional<Loan> optLoan = this.loanRepository.findById(loanId);
CollectionData collectionData = CollectionData.template();
if (optLoan.isPresent()) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Loan loan = optLoan.get();
+ Integer graceDays = 0;
+ if (loan.loanProduct().getLoanProductConfigurableAttributes().getGraceOnArrearsAgingBoolean()) {
+ graceDays = loan.loanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
+ if (graceDays == null) {
+ graceDays = 0;
+ }
+ }
- collectionData.setAvailableDisbursementAmount(loan.getApprovedPrincipal().subtract(loan.getDisbursedAmount()));
+ collectionData.setAvailableDisbursementAmount(loan.getApprovedPrincipal().subtract(loan.getNetDisbursalAmount()));
Review Comment:
i dont think this is correct.
The netDisbursalAmount will be overriden always by the very latest disbursement amount.
**Example:**
- Approved amount is 1000
- I do a disbursement with amount of 400
- The netDisbursalAmount will be 400, so it will set the available disbursement amount is 600
- If I do a 2nd disbursement with amount of 600
- The netDisbursalAmount will be 600, so the available disbursement amount will be set to 400, but that is not correct. We should consider all of the disbursement details to get the correct amount.
@josehernandezfintecheandomx what do you think?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1023686034
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+
+@Slf4j
+public final class DelinquencyUtils {
+
+ private DelinquencyUtils() {}
+
+ public static CollectionData getOverdueCollectionData(final Loan loan, final LocalDate businessDate) {
+ final MonetaryCurrency loanCurrency = loan.getCurrency();
+ LocalDate overdueSinceDate = null;
+ CollectionData collectionData = CollectionData.template();
+ BigDecimal amountAvailable = BigDecimal.ZERO;
+ BigDecimal outstandingAmount = BigDecimal.ZERO;
+ boolean oldestOverdueInstallment = false;
+ boolean overdueSinceDateWasSet = false;
+ boolean firstNotYetDueInstallment = false;
+
+ log.debug("Loan id {} with {} installments", loan.getId(), loan.getRepaymentScheduleInstallments().size());
+ // Get the oldest overdue installment if exists one
+ for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
+ if (installment.isObligationsMet()) {
+ continue;
+ }
+
+ if (installment.getDueDate().isBefore(businessDate)) {
+ log.debug("Loan Id: {} with installment {} due date {}", loan.getId(), installment.getInstallmentNumber(),
+ installment.getDueDate());
+ outstandingAmount = outstandingAmount.add(installment.getTotalOutstanding(loanCurrency).getAmount());
+ if (!oldestOverdueInstallment) {
+ log.debug("Oldest installment {} {}", installment.getInstallmentNumber(), installment.getDueDate());
+ oldestOverdueInstallment = true;
+ overdueSinceDate = installment.getDueDate();
+ overdueSinceDateWasSet = true;
+
+ amountAvailable = installment.getTotalPaid(loanCurrency).getAmount();
+
+ for (LoanTransactionToRepaymentScheduleMapping mappngInstallment : installment
+ .getLoanTransactionToRepaymentScheduleMappings()) {
+ final LoanTransaction loanTransaction = mappngInstallment.getLoanTransaction();
+ if (loanTransaction.isChargeback()) {
+ amountAvailable = amountAvailable.subtract(loanTransaction.getAmount());
+ if (amountAvailable.compareTo(BigDecimal.ZERO) < 0) {
+ overdueSinceDate = loanTransaction.getTransactionDate();
+ break;
+ }
+ }
+ }
+ }
+ } else if (!firstNotYetDueInstallment) {
+ log.debug("Loan Id: {} with installment {} due date {}", loan.getId(), installment.getInstallmentNumber(),
+ installment.getDueDate());
+ firstNotYetDueInstallment = true;
+ amountAvailable = installment.getTotalPaid(loanCurrency).getAmount().subtract(installment.getTotalPaidInAdvance());
Review Comment:
Are you sure we need to deduct the total paid in advance?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy merged pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy merged PR #2739:
URL: https://github.com/apache/fineract/pull/2739
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1024083766
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java:
##########
@@ -109,89 +103,23 @@ public Collection<LoanDelinquencyTagHistoryData> retrieveDelinquencyRangeHistory
@Override
public CollectionData calculateLoanCollectionData(final Long loanId) {
- final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Optional<Loan> optLoan = this.loanRepository.findById(loanId);
CollectionData collectionData = CollectionData.template();
if (optLoan.isPresent()) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Loan loan = optLoan.get();
+ loan.initializeLazyBasicCollections();
Review Comment:
Done, removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1023678632
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java:
##########
@@ -109,89 +103,23 @@ public Collection<LoanDelinquencyTagHistoryData> retrieveDelinquencyRangeHistory
@Override
public CollectionData calculateLoanCollectionData(final Long loanId) {
- final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Optional<Loan> optLoan = this.loanRepository.findById(loanId);
CollectionData collectionData = CollectionData.template();
if (optLoan.isPresent()) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Loan loan = optLoan.get();
+ loan.initializeLazyBasicCollections();
Review Comment:
I would rather put a `@Transactional` annotation on the method then manually try to fetch the lazy associations.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1020193034
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionType;
+
+public final class DelinquencyUtils {
+
+ private DelinquencyUtils() {}
+
+ public static LocalDate getOverdueSinceDate(final Loan loan, final LocalDate businessDate, final Integer graceOnArrearAgeing) {
+ final MonetaryCurrency loanCurrency = loan.getCurrency();
+ LoanRepaymentScheduleInstallment loanRepaymentSchedule = null;
+ for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
+ if (installment.getDueDate().isBefore(businessDate) && installment.isNotFullyPaidOff()) {
+ loanRepaymentSchedule = installment;
+ break;
+ }
+ }
+
+ LocalDate overdueSinceDate = null;
+ if (loanRepaymentSchedule != null) {
+ // Default Due date
+ overdueSinceDate = loanRepaymentSchedule.getDueDate();
+ }
+
+ Map<Integer, BigDecimal> installmentBalances = new HashMap<>();
Review Comment:
This is not needed. You already have the first unpaid instalment (see line 42)
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1020192526
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java:
##########
@@ -174,20 +154,27 @@ public CollectionData calculateLoanCollectionData(final Long loanId) {
}
// Calculate Delinquency Amount
- for (LoanRepaymentScheduleInstallment installment : installments) {
+ Map<Integer, BigDecimal> installmentBalances = new HashMap<>();
Review Comment:
This can be removed, the logic in DelinquencyUtils.getOverdueSinceDate can be extended to get the delinquent amount as well.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1021285475
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,117 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.util.Iterator;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+
+public final class DelinquencyUtils {
+
+ private DelinquencyUtils() {}
+
+ public static CollectionData getOverdueCollectionData(final Loan loan, final LocalDate businessDate) {
+ final MonetaryCurrency loanCurrency = loan.getCurrency();
+ LocalDate overdueSinceDate = null;
+ CollectionData collectionData = CollectionData.template();
+ LoanRepaymentScheduleInstallment nextInstallment = null;
+ BigDecimal amountAvailable = BigDecimal.ZERO;
+ BigDecimal outstandingAmount = BigDecimal.ZERO;
+ boolean oldestOverdueInstallment = false;
+
+ // Get the oldest overdue installment if exists one
+ for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
+ if (!installment.isObligationsMet()) {
+ if (installment.getDueDate().isBefore(businessDate)) {
+ if (!oldestOverdueInstallment) {
+ oldestOverdueInstallment = true;
+ overdueSinceDate = installment.getDueDate();
+ }
+ outstandingAmount = outstandingAmount.add(installment.getTotalOutstanding(loanCurrency).getAmount());
+ } else {
+ if (nextInstallment == null) {
+ nextInstallment = installment;
+ break;
+ }
+ }
+ }
+ }
+
+ if (nextInstallment == null && loan.getRepaymentScheduleInstallments().size() > 0) {
+ nextInstallment = loan.getRepaymentScheduleInstallments().get(loan.getRepaymentScheduleInstallments().size() - 1);
+ }
+
+ // If exists a next no overdue installment
+ if (nextInstallment != null) {
Review Comment:
I think this logic is not considering properly the chargeback transactions.
**Example:**
- There are 2 installments
- First one is not fully paid and overdue and contains a chargeback transaction
- The second one is not due yet
The logic here will not set the overdueSinceDate by the chargeback transaction as it was never checked.
The nextInstallment will be set to check the second installment, which is not even due and might overwrite the overdueSinceDate which is wrong, the first unpaid chargeback transaction is in the first unpaid installment
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1024083142
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepaymentScheduleInstallment.java:
##########
@@ -140,6 +140,9 @@ public class LoanRepaymentScheduleInstallment extends AbstractAuditableWithUTCDa
@OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY, mappedBy = "installment")
private Set<LoanInstallmentCharge> installmentCharges = new HashSet<>();
+ @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER, mappedBy = "installment")
+ private Set<LoanTransactionToRepaymentScheduleMapping> loanTransactionToRepaymentScheduleMappings = new HashSet<>();
Review Comment:
Done, moved to LAZY
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1024081035
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -6781,15 +6781,19 @@ public Collection<LoanCharge> getLoanCharges() {
public void initializeLazyCollections() {
checkAndFetchLazyCollection(this.charges);
checkAndFetchLazyCollection(this.trancheCharges);
- checkAndFetchLazyCollection(this.repaymentScheduleInstallments);
- checkAndFetchLazyCollection(this.loanTransactions);
+ initializeLazyBasicCollections();
checkAndFetchLazyCollection(this.disbursementDetails);
checkAndFetchLazyCollection(this.loanTermVariations);
checkAndFetchLazyCollection(this.collateral);
checkAndFetchLazyCollection(this.loanOfficerHistory);
checkAndFetchLazyCollection(this.loanCollateralManagements);
}
+ public void initializeLazyBasicCollections() {
Review Comment:
Done, removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1024091951
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepaymentScheduleInstallment.java:
##########
@@ -718,6 +721,9 @@ public void addToCredits(final BigDecimal amount) {
}
public BigDecimal getTotalPaidInAdvance() {
+ if (this.totalPaidInAdvance == null) {
Review Comment:
Removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1025229194
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -233,15 +174,9 @@ public void applyDelinquencyTagToLoan(LoanScheduleDelinquencyData loanDelinquenc
if (loan.hasDelinquencyBucket()) {
final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final DelinquencyBucket delinquencyBucket = loan.getLoanProduct().getDelinquencyBucket();
- final Integer graceOnArrearAgeing = loan.getLoanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
-
- LocalDate overdueSinceDate = loanDelinquencyData.getOverdueSinceDate();
- if (overdueSinceDate == null) {
- getOverdueSinceDate(loan, businessDate, graceOnArrearAgeing);
- }
-
- final Long overdueDays = calculateOverdueDays(businessDate, overdueSinceDate);
- lookUpDelinquencyRange(loan, delinquencyBucket, overdueDays);
+ final CollectionData collectionData = this.loanCollectionDomainService.getOverdueCollectionData(loan, businessDate);
+ log.debug("Delinquency {}", collectionData.toString());
Review Comment:
Removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1025228461
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -217,12 +162,8 @@ public CommandProcessingResult applyDelinquencyTagToLoan(Long loanId, JsonComman
final DelinquencyBucket delinquencyBucket = loan.getLoanProduct().getDelinquencyBucket();
if (delinquencyBucket != null) {
final LocalDate businessDate = DateUtils.getBusinessLocalDate();
-
- final Integer graceOnArrearAgeing = loan.getLoanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
- final LocalDate overdueSinceDate = getOverdueSinceDate(loan, businessDate, graceOnArrearAgeing);
-
- final Long overdueDays = calculateOverdueDays(businessDate, overdueSinceDate);
- changes = lookUpDelinquencyRange(loan, delinquencyBucket, overdueDays);
+ final CollectionData collectionData = this.loanCollectionDomainService.getOverdueCollectionData(loan, businessDate);
Review Comment:
Removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1020197414
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionType;
+
+public final class DelinquencyUtils {
+
+ private DelinquencyUtils() {}
+
+ public static LocalDate getOverdueSinceDate(final Loan loan, final LocalDate businessDate, final Integer graceOnArrearAgeing) {
+ final MonetaryCurrency loanCurrency = loan.getCurrency();
+ LoanRepaymentScheduleInstallment loanRepaymentSchedule = null;
+ for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
+ if (installment.getDueDate().isBefore(businessDate) && installment.isNotFullyPaidOff()) {
+ loanRepaymentSchedule = installment;
+ break;
+ }
+ }
+
+ LocalDate overdueSinceDate = null;
+ if (loanRepaymentSchedule != null) {
+ // Default Due date
+ overdueSinceDate = loanRepaymentSchedule.getDueDate();
+ }
+
+ Map<Integer, BigDecimal> installmentBalances = new HashMap<>();
+ for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
+ installmentBalances.put(installment.getInstallmentNumber(), installment.getPrincipalCompleted(loanCurrency).getAmount());
+ }
+
+ // If there is some Loan Transaction Chargeback
+ final List<LoanTransaction> loanTransactions = loan.retrieveListOfTransactionsByType(LoanTransactionType.CHARGEBACK);
+ for (LoanTransaction loanTransaction : loanTransactions) {
Review Comment:
This can be simplified:
- You already knows which is the first unpaid instalment
- Put the total paid principal for that instalment into a temporary variable
- If there are chargeback transactions for that instalment (fetch those loantransactions through the repayment mapping or simply iterate through the loan transactions and check the from- & due date of instalment against the transaction date), as iterating through those transaction, deduct the transaction amount from this temporary variable. And check after the deduction it went to negative. If yes, that transaction is not fully paid yet, so the delinquent amount can be collected from that moment and the overdue since date can be set with the transaction date of that transaction.
BTW question: only chargeback transaction should increase the delinquent amount?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1023676642
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepaymentScheduleInstallment.java:
##########
@@ -140,6 +140,9 @@ public class LoanRepaymentScheduleInstallment extends AbstractAuditableWithUTCDa
@OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY, mappedBy = "installment")
private Set<LoanInstallmentCharge> installmentCharges = new HashSet<>();
+ @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER, mappedBy = "installment")
+ private Set<LoanTransactionToRepaymentScheduleMapping> loanTransactionToRepaymentScheduleMappings = new HashSet<>();
Review Comment:
I am a little hesitant whether we need to fetch the mapping eagerly. Can you tell me why did you decide to do so? Eager loading only required if we would read an association when the entity is already detached. I dont think that would happen here.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1024879572
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java:
##########
@@ -109,89 +104,22 @@ public Collection<LoanDelinquencyTagHistoryData> retrieveDelinquencyRangeHistory
@Override
public CollectionData calculateLoanCollectionData(final Long loanId) {
- final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Optional<Loan> optLoan = this.loanRepository.findById(loanId);
CollectionData collectionData = CollectionData.template();
if (optLoan.isPresent()) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Loan loan = optLoan.get();
+ collectionData = this.loanCollectionDomainService.getOverdueCollectionData(loan, businessDate);
Review Comment:
`this.` not needed.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -152,61 +149,9 @@ public LoanScheduleDelinquencyData calculateDelinquencyData(LoanScheduleDelinque
if (loan == null) {
loan = this.loanRepository.findOneWithNotFoundDetection(loanScheduleDelinquencyData.getLoanId());
}
-
- final Integer graceOnArrearAgeing = loan.getLoanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
- final LocalDate overdueSinceDate = getOverdueSinceDate(loan, businessDate, graceOnArrearAgeing);
- final Long overdueDays = calculateOverdueDays(businessDate, overdueSinceDate);
- return new LoanScheduleDelinquencyData(loan.getId(), overdueSinceDate, overdueDays, loan);
- }
-
- @Override
- public LocalDate getOverdueSinceDate(final Loan loan, final LocalDate businessDate, final Integer graceOnArrearAgeing) {
- LoanRepaymentScheduleInstallment loanRepaymentSchedule = null;
- for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
- if (installment.getDueDate().isBefore(businessDate) && installment.isNotFullyPaidOff()) {
- loanRepaymentSchedule = installment;
- break;
- }
- }
-
- LocalDate overdueSinceDate = null;
- final MonetaryCurrency loanCurrency = loan.getCurrency();
- final List<LoanTransaction> loanTransactions = loan.retrieveListOfTransactionsByType(LoanTransactionType.CHARGEBACK);
- if (loanRepaymentSchedule != null) {
- // Default Due date
- overdueSinceDate = loanRepaymentSchedule.getDueDate();
- }
-
- // If there is some Loan Transaction Chargeback
- for (LoanTransaction loanTransaction : loanTransactions) {
- if (loanTransaction.getLoanTransactionToRepaymentScheduleMappings().iterator().hasNext()) {
- final LoanTransactionToRepaymentScheduleMapping transactionMapping = loanTransaction
- .getLoanTransactionToRepaymentScheduleMappings().iterator().next();
-
- if (transactionMapping != null
- && !transactionMapping.getLoanRepaymentScheduleInstallment().isPrincipalCompleted(loanCurrency)) {
- overdueSinceDate = loanTransaction.getTransactionDate();
- break;
- }
- }
- }
-
- // Include grace days If It's defined
- if (overdueSinceDate != null && graceOnArrearAgeing != null) {
- overdueSinceDate = overdueSinceDate.plusDays(graceOnArrearAgeing.longValue());
- }
- return overdueSinceDate;
- }
-
- private Long calculateOverdueDays(final LocalDate businessDate, LocalDate overdueSinceDate) {
- Long overdueDays = 0L;
- if (overdueSinceDate != null) {
- overdueDays = DateUtils.getDifferenceInDays(overdueSinceDate, businessDate);
- if (overdueDays < 0) {
- overdueDays = 0L;
- }
- }
- return overdueDays;
+ final CollectionData collectionData = this.loanCollectionDomainService.getOverdueCollectionData(loan, businessDate);
+ log.debug("Delinquency {}", collectionData.toString());
Review Comment:
No need for the explicit toString call.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -233,15 +174,9 @@ public void applyDelinquencyTagToLoan(LoanScheduleDelinquencyData loanDelinquenc
if (loan.hasDelinquencyBucket()) {
final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final DelinquencyBucket delinquencyBucket = loan.getLoanProduct().getDelinquencyBucket();
- final Integer graceOnArrearAgeing = loan.getLoanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
-
- LocalDate overdueSinceDate = loanDelinquencyData.getOverdueSinceDate();
- if (overdueSinceDate == null) {
- getOverdueSinceDate(loan, businessDate, graceOnArrearAgeing);
- }
-
- final Long overdueDays = calculateOverdueDays(businessDate, overdueSinceDate);
- lookUpDelinquencyRange(loan, delinquencyBucket, overdueDays);
+ final CollectionData collectionData = this.loanCollectionDomainService.getOverdueCollectionData(loan, businessDate);
+ log.debug("Delinquency {}", collectionData.toString());
Review Comment:
No need for toString
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -217,12 +162,8 @@ public CommandProcessingResult applyDelinquencyTagToLoan(Long loanId, JsonComman
final DelinquencyBucket delinquencyBucket = loan.getLoanProduct().getDelinquencyBucket();
if (delinquencyBucket != null) {
final LocalDate businessDate = DateUtils.getBusinessLocalDate();
-
- final Integer graceOnArrearAgeing = loan.getLoanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
- final LocalDate overdueSinceDate = getOverdueSinceDate(loan, businessDate, graceOnArrearAgeing);
-
- final Long overdueDays = calculateOverdueDays(businessDate, overdueSinceDate);
- changes = lookUpDelinquencyRange(loan, delinquencyBucket, overdueDays);
+ final CollectionData collectionData = this.loanCollectionDomainService.getOverdueCollectionData(loan, businessDate);
Review Comment:
No need for `this.`
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -233,15 +174,9 @@ public void applyDelinquencyTagToLoan(LoanScheduleDelinquencyData loanDelinquenc
if (loan.hasDelinquencyBucket()) {
final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final DelinquencyBucket delinquencyBucket = loan.getLoanProduct().getDelinquencyBucket();
- final Integer graceOnArrearAgeing = loan.getLoanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
-
- LocalDate overdueSinceDate = loanDelinquencyData.getOverdueSinceDate();
- if (overdueSinceDate == null) {
- getOverdueSinceDate(loan, businessDate, graceOnArrearAgeing);
- }
-
- final Long overdueDays = calculateOverdueDays(businessDate, overdueSinceDate);
- lookUpDelinquencyRange(loan, delinquencyBucket, overdueDays);
+ final CollectionData collectionData = this.loanCollectionDomainService.getOverdueCollectionData(loan, businessDate);
Review Comment:
As above.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -152,61 +149,9 @@ public LoanScheduleDelinquencyData calculateDelinquencyData(LoanScheduleDelinque
if (loan == null) {
loan = this.loanRepository.findOneWithNotFoundDetection(loanScheduleDelinquencyData.getLoanId());
}
-
- final Integer graceOnArrearAgeing = loan.getLoanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
- final LocalDate overdueSinceDate = getOverdueSinceDate(loan, businessDate, graceOnArrearAgeing);
- final Long overdueDays = calculateOverdueDays(businessDate, overdueSinceDate);
- return new LoanScheduleDelinquencyData(loan.getId(), overdueSinceDate, overdueDays, loan);
- }
-
- @Override
- public LocalDate getOverdueSinceDate(final Loan loan, final LocalDate businessDate, final Integer graceOnArrearAgeing) {
- LoanRepaymentScheduleInstallment loanRepaymentSchedule = null;
- for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
- if (installment.getDueDate().isBefore(businessDate) && installment.isNotFullyPaidOff()) {
- loanRepaymentSchedule = installment;
- break;
- }
- }
-
- LocalDate overdueSinceDate = null;
- final MonetaryCurrency loanCurrency = loan.getCurrency();
- final List<LoanTransaction> loanTransactions = loan.retrieveListOfTransactionsByType(LoanTransactionType.CHARGEBACK);
- if (loanRepaymentSchedule != null) {
- // Default Due date
- overdueSinceDate = loanRepaymentSchedule.getDueDate();
- }
-
- // If there is some Loan Transaction Chargeback
- for (LoanTransaction loanTransaction : loanTransactions) {
- if (loanTransaction.getLoanTransactionToRepaymentScheduleMappings().iterator().hasNext()) {
- final LoanTransactionToRepaymentScheduleMapping transactionMapping = loanTransaction
- .getLoanTransactionToRepaymentScheduleMappings().iterator().next();
-
- if (transactionMapping != null
- && !transactionMapping.getLoanRepaymentScheduleInstallment().isPrincipalCompleted(loanCurrency)) {
- overdueSinceDate = loanTransaction.getTransactionDate();
- break;
- }
- }
- }
-
- // Include grace days If It's defined
- if (overdueSinceDate != null && graceOnArrearAgeing != null) {
- overdueSinceDate = overdueSinceDate.plusDays(graceOnArrearAgeing.longValue());
- }
- return overdueSinceDate;
- }
-
- private Long calculateOverdueDays(final LocalDate businessDate, LocalDate overdueSinceDate) {
- Long overdueDays = 0L;
- if (overdueSinceDate != null) {
- overdueDays = DateUtils.getDifferenceInDays(overdueSinceDate, businessDate);
- if (overdueDays < 0) {
- overdueDays = 0L;
- }
- }
- return overdueDays;
+ final CollectionData collectionData = this.loanCollectionDomainService.getOverdueCollectionData(loan, businessDate);
Review Comment:
`this.` not needed
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/LoanCollectionDomainServiceImpl.java:
##########
@@ -0,0 +1,135 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import javax.transaction.Transactional;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+import org.springframework.stereotype.Service;
+
+@Slf4j
+@Service
+public class LoanCollectionDomainServiceImpl implements LoanCollectionDomainService {
Review Comment:
Don't like the naming still, especially the terminology CollectionData because it doesn't reflect anything about the data it represents.
But I can live with it for now.
The other thing though which I can't live without is proper unit test coverage for this class.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanEventApiJsonValidator.java:
##########
@@ -237,7 +237,7 @@ public void validateChargebackTransaction(final String json) {
final BigDecimal transactionAmount = this.fromApiJsonHelper
.extractBigDecimalWithLocaleNamed(LoanApiConstants.TRANSACTION_AMOUNT_PARAMNAME, element);
baseDataValidator.reset().parameter(LoanApiConstants.TRANSACTION_AMOUNT_PARAMNAME).value(transactionAmount).notNull()
- .zeroOrPositiveAmount();
+ .positiveAmount();
Review Comment:
This is a separate bugfix. Please extract it to a different PR so it can be reviewed separately along with its tests.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/LoanCollectionDomainService.java:
##########
@@ -0,0 +1,36 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.time.LocalDate;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+
+public interface LoanCollectionDomainService {
+
+ /**
+ * This method is to calculate the Overdue date and other properties, If the loan is overdue or If there is some
+ * Charge back transaction
+ *
+ * @param loan
+ * @param businessDate
+ */
+ CollectionData getOverdueCollectionData(Loan loan, LocalDate businessDate);
Review Comment:
If we're always passing the current business date, why is it a parameter? Can't we just resolve the current business date from within the implementation?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1023496337
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java:
##########
@@ -109,52 +111,30 @@ public Collection<LoanDelinquencyTagHistoryData> retrieveDelinquencyRangeHistory
@Override
public CollectionData calculateLoanCollectionData(final Long loanId) {
- final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Optional<Loan> optLoan = this.loanRepository.findById(loanId);
CollectionData collectionData = CollectionData.template();
if (optLoan.isPresent()) {
+ final LocalDate businessDate = DateUtils.getBusinessLocalDate();
final Loan loan = optLoan.get();
+ Integer graceDays = 0;
+ if (loan.loanProduct().getLoanProductConfigurableAttributes().getGraceOnArrearsAgingBoolean()) {
+ graceDays = loan.loanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
+ if (graceDays == null) {
+ graceDays = 0;
+ }
+ }
- collectionData.setAvailableDisbursementAmount(loan.getApprovedPrincipal().subtract(loan.getDisbursedAmount()));
+ collectionData.setAvailableDisbursementAmount(loan.getApprovedPrincipal().subtract(loan.getNetDisbursalAmount()));
Review Comment:
Updared
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1024086328
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+
+@Slf4j
+public final class DelinquencyUtils {
+
+ private DelinquencyUtils() {}
+
+ public static CollectionData getOverdueCollectionData(final Loan loan, final LocalDate businessDate) {
+ final MonetaryCurrency loanCurrency = loan.getCurrency();
+ LocalDate overdueSinceDate = null;
+ CollectionData collectionData = CollectionData.template();
+ BigDecimal amountAvailable = BigDecimal.ZERO;
+ BigDecimal outstandingAmount = BigDecimal.ZERO;
+ boolean oldestOverdueInstallment = false;
+ boolean overdueSinceDateWasSet = false;
+ boolean firstNotYetDueInstallment = false;
+
+ log.debug("Loan id {} with {} installments", loan.getId(), loan.getRepaymentScheduleInstallments().size());
+ // Get the oldest overdue installment if exists one
+ for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
+ if (installment.isObligationsMet()) {
+ continue;
+ }
+
+ if (installment.getDueDate().isBefore(businessDate)) {
+ log.debug("Loan Id: {} with installment {} due date {}", loan.getId(), installment.getInstallmentNumber(),
+ installment.getDueDate());
+ outstandingAmount = outstandingAmount.add(installment.getTotalOutstanding(loanCurrency).getAmount());
+ if (!oldestOverdueInstallment) {
+ log.debug("Oldest installment {} {}", installment.getInstallmentNumber(), installment.getDueDate());
+ oldestOverdueInstallment = true;
+ overdueSinceDate = installment.getDueDate();
+ overdueSinceDateWasSet = true;
+
+ amountAvailable = installment.getTotalPaid(loanCurrency).getAmount();
+
+ for (LoanTransactionToRepaymentScheduleMapping mappngInstallment : installment
+ .getLoanTransactionToRepaymentScheduleMappings()) {
+ final LoanTransaction loanTransaction = mappngInstallment.getLoanTransaction();
+ if (loanTransaction.isChargeback()) {
+ amountAvailable = amountAvailable.subtract(loanTransaction.getAmount());
+ if (amountAvailable.compareTo(BigDecimal.ZERO) < 0) {
+ overdueSinceDate = loanTransaction.getTransactionDate();
+ break;
+ }
+ }
+ }
+ }
+ } else if (!firstNotYetDueInstallment) {
+ log.debug("Loan Id: {} with installment {} due date {}", loan.getId(), installment.getInstallmentNumber(),
+ installment.getDueDate());
+ firstNotYetDueInstallment = true;
+ amountAvailable = installment.getTotalPaid(loanCurrency).getAmount().subtract(installment.getTotalPaidInAdvance());
Review Comment:
Removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1024596855
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+
+@Slf4j
+public final class DelinquencyUtils {
Review Comment:
It is now in a service, I can't put in the current Read service because It generates circular dependency
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1025228167
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java:
##########
@@ -152,61 +149,9 @@ public LoanScheduleDelinquencyData calculateDelinquencyData(LoanScheduleDelinque
if (loan == null) {
loan = this.loanRepository.findOneWithNotFoundDetection(loanScheduleDelinquencyData.getLoanId());
}
-
- final Integer graceOnArrearAgeing = loan.getLoanProduct().getLoanProductRelatedDetail().getGraceOnArrearsAgeing();
- final LocalDate overdueSinceDate = getOverdueSinceDate(loan, businessDate, graceOnArrearAgeing);
- final Long overdueDays = calculateOverdueDays(businessDate, overdueSinceDate);
- return new LoanScheduleDelinquencyData(loan.getId(), overdueSinceDate, overdueDays, loan);
- }
-
- @Override
- public LocalDate getOverdueSinceDate(final Loan loan, final LocalDate businessDate, final Integer graceOnArrearAgeing) {
- LoanRepaymentScheduleInstallment loanRepaymentSchedule = null;
- for (LoanRepaymentScheduleInstallment installment : loan.getRepaymentScheduleInstallments()) {
- if (installment.getDueDate().isBefore(businessDate) && installment.isNotFullyPaidOff()) {
- loanRepaymentSchedule = installment;
- break;
- }
- }
-
- LocalDate overdueSinceDate = null;
- final MonetaryCurrency loanCurrency = loan.getCurrency();
- final List<LoanTransaction> loanTransactions = loan.retrieveListOfTransactionsByType(LoanTransactionType.CHARGEBACK);
- if (loanRepaymentSchedule != null) {
- // Default Due date
- overdueSinceDate = loanRepaymentSchedule.getDueDate();
- }
-
- // If there is some Loan Transaction Chargeback
- for (LoanTransaction loanTransaction : loanTransactions) {
- if (loanTransaction.getLoanTransactionToRepaymentScheduleMappings().iterator().hasNext()) {
- final LoanTransactionToRepaymentScheduleMapping transactionMapping = loanTransaction
- .getLoanTransactionToRepaymentScheduleMappings().iterator().next();
-
- if (transactionMapping != null
- && !transactionMapping.getLoanRepaymentScheduleInstallment().isPrincipalCompleted(loanCurrency)) {
- overdueSinceDate = loanTransaction.getTransactionDate();
- break;
- }
- }
- }
-
- // Include grace days If It's defined
- if (overdueSinceDate != null && graceOnArrearAgeing != null) {
- overdueSinceDate = overdueSinceDate.plusDays(graceOnArrearAgeing.longValue());
- }
- return overdueSinceDate;
- }
-
- private Long calculateOverdueDays(final LocalDate businessDate, LocalDate overdueSinceDate) {
- Long overdueDays = 0L;
- if (overdueSinceDate != null) {
- overdueDays = DateUtils.getDifferenceInDays(overdueSinceDate, businessDate);
- if (overdueDays < 0) {
- overdueDays = 0L;
- }
- }
- return overdueDays;
+ final CollectionData collectionData = this.loanCollectionDomainService.getOverdueCollectionData(loan, businessDate);
+ log.debug("Delinquency {}", collectionData.toString());
Review Comment:
Removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1025230005
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanEventApiJsonValidator.java:
##########
@@ -237,7 +237,7 @@ public void validateChargebackTransaction(final String json) {
final BigDecimal transactionAmount = this.fromApiJsonHelper
.extractBigDecimalWithLocaleNamed(LoanApiConstants.TRANSACTION_AMOUNT_PARAMNAME, element);
baseDataValidator.reset().parameter(LoanApiConstants.TRANSACTION_AMOUNT_PARAMNAME).value(transactionAmount).notNull()
- .zeroOrPositiveAmount();
+ .positiveAmount();
Review Comment:
Removed
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] josehernandezfintecheandomx commented on a diff in pull request #2739: FINERACT-1706: Fix Loan Delinquency Classification after Repayment
Posted by GitBox <gi...@apache.org>.
josehernandezfintecheandomx commented on code in PR #2739:
URL: https://github.com/apache/fineract/pull/2739#discussion_r1025977302
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyUtils.java:
##########
@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.delinquency.service;
+
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.apache.fineract.organisation.monetary.domain.MonetaryCurrency;
+import org.apache.fineract.portfolio.loanaccount.data.CollectionData;
+import org.apache.fineract.portfolio.loanaccount.domain.Loan;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanRepaymentScheduleInstallment;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction;
+import org.apache.fineract.portfolio.loanaccount.domain.LoanTransactionToRepaymentScheduleMapping;
+
+@Slf4j
+public final class DelinquencyUtils {
Review Comment:
Done in class `LoanDelinquencyDomainServiceTest`
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org