FINERACT-2081: Loan account data additional fields for summary and de… #3957

alberto-art3ch · 2024-07-03T06:45:42Z

…linquency

Description

For including new fields in the Loan account data API, we need to modify the GET Loan by ID / external-id API to return those additional fields. API : /fineract-provider/api/v1/loans/{loanId}

FINERACT-2081

Delinquency data

Summary data

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.

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/LoanSummaryData.java

adamsaghy · 2024-07-03T08:12:02Z

...loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransactionType.java

@@ -63,8 +63,7 @@ public enum LoanTransactionType {
    DOWN_PAYMENT(28, "loanTransactionType.downPayment"), //
    REAGE(29, "loanTransactionType.reAge"), //
    REAMORTIZE(30, "loanTransactionType.reAmortize"), //
-    INTEREST_PAYMENT_WAIVER(31, "loanTransactionType.interestPaymentWaiver"), //


unnecessary change... would you mind to revert?

adamsaghy · 2024-07-03T08:12:32Z

...che/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/ChargeOrTransaction.java

@@ -58,7 +58,7 @@ private LocalDate getEffectiveDate() {
    }

    private boolean isBackdatedCharge() {
-        return loanCharge.get().getDueDate().isBefore(loanCharge.get().getSubmittedOnDate());
+        return (loanCharge.get().getDueDate() != null && loanCharge.get().getDueDate().isBefore(loanCharge.get().getSubmittedOnDate()));


Unnecessary change. Would you mind to revert? If it is a fix for a bug ticket, please extract it into a new PR!

Done! removed

Nothing changed here either.... :/

...der/src/main/java/org/apache/fineract/portfolio/loanaccount/api/LoansApiResourceSwagger.java

...pache/fineract/portfolio/deliquency/DelinquencyWritePlatformServiceRangeChangeEventTest.java

adamsaghy

Please kindly check my review!

adamsaghy · 2024-07-08T15:22:52Z

...org/apache/fineract/integrationtests/AdvancedPaymentAllocationLoanRepaymentScheduleTest.java

@@ -4113,6 +4114,67 @@ public void uc137() {
        });
    }

+    // uc142: Loan Progressive processing type only with Advanced payment allocation


Please fix the description of this use case

adamsaghy · 2024-07-08T15:23:50Z

...org/apache/fineract/integrationtests/AdvancedPaymentAllocationLoanRepaymentScheduleTest.java

+            Long repayment2TransactionId = addRepaymentForLoan(loanId, 251.0, "3 January 2023");
+            assertNotNull(repayment2TransactionId);
+            loanDetails = loanTransactionHelper.getLoanDetails(loanId);
+            LOG.info("Data {}", loanDetails.getSummary().getTotalUnpaidAccruedDueInterest());


Writing out with logger is not enough! Please user assertions to validate the proper amount was calculated and retrieved!

adamsaghy · 2024-07-08T17:16:53Z

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/LoanSummaryData.java

@@ -110,6 +116,11 @@ public static LoanSummaryData withTransactionAmountsSummary(final LoanSummaryDat
        BigDecimal totalCreditBalanceRefundReversed = BigDecimal.ZERO;
        BigDecimal totalRepaymentTransaction = BigDecimal.ZERO;
        BigDecimal totalRepaymentTransactionReversed = BigDecimal.ZERO;
+        BigDecimal totalInterestRefund = BigDecimal.ZERO;


We dont need field yet (totalInterestRefund) -> hence not implemented yet

Done! removed

Seems it was not removed, it is still there :(

adamsaghy · 2024-07-09T17:41:17Z

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/LoanSummaryData.java

@@ -191,4 +222,31 @@ private static BigDecimal computeTotalRepaymentTransactionAmount(Collection<Loan
                loanTransactions);
        return totalRepaymentTransaction.add(totalDownPaymentTransaction);
    }
+
+    private static BigDecimal computeTotalAccruedDueAmount(Collection<LoanSchedulePeriodData> periods) {


Please rename this to reflect it is calculating the accrued interest amount: computeTotalInterestAccruedDueAmount

Done! method renamed

adamsaghy · 2024-07-09T17:43:35Z

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/LoanSummaryData.java

+                .map(period -> period.getInterestPaid()).reduce(BigDecimal.ZERO, BigDecimal::add);
+    }
+
+    private static BigDecimal fetchTotalAccruedNotDueAmount(Collection<LoanSchedulePeriodData> periods) {


Please rename this to reflect it is calculating the accrued interest: computeTotalInterestAccruedNotDueAmount

Done! method renamed

adamsaghy · 2024-07-09T17:43:51Z

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/LoanSummaryData.java

+                .map(period -> period.getTotalAccruedInterest()).reduce(BigDecimal.ZERO, BigDecimal::add);
+    }
+
+    private static BigDecimal fetchTotalInterestNotDueAmount(Collection<LoanSchedulePeriodData> periods) {


Please rename this to reflect it is calculating the accrued interest: computeTotalInterestNotDueAmount

Done! method renamed

.../main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/data/LoanScheduleData.java

adamsaghy · 2024-07-09T17:46:29Z

...java/org/apache/fineract/portfolio/loanaccount/loanschedule/data/LoanSchedulePeriodData.java

    private final Boolean downPaymentPeriod;
+    private final Boolean actualPeriod;


Do we need this flag?

Done! removed

adamsaghy · 2024-07-09T17:49:37Z

...main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java

-                    + " ls.total_paid_late_derived as totalPaidLateForPeriod, ls.credits_amount as principalCredits, ls.credited_fee as feeCredits, ls.credited_penalty as penaltyCredits, ls.is_down_payment isDownPayment "
-                    + " from m_loan_repayment_schedule ls ";
+                    + " ls.total_paid_late_derived as totalPaidLateForPeriod, ls.credits_amount as principalCredits, ls.credited_fee as feeCredits, ls.credited_penalty as penaltyCredits, ls.is_down_payment isDownPayment, "
+                    + " ls.accrual_interest_derived as totalAccrualInterest " + " from m_loan_repayment_schedule ls ";


Wouldnt it be better to name InterestAccrued? (all the other interest related fields are interestDue, interestPaid, etc

Done! renamed

alberto-art3ch

Thanks!

...der/src/main/java/org/apache/fineract/portfolio/loanaccount/api/LoansApiResourceSwagger.java

alberto-art3ch · 2024-07-09T19:03:07Z

...che/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/ChargeOrTransaction.java

@@ -58,7 +58,7 @@ private LocalDate getEffectiveDate() {
    }

    private boolean isBackdatedCharge() {
-        return loanCharge.get().getDueDate().isBefore(loanCharge.get().getSubmittedOnDate());
+        return (loanCharge.get().getDueDate() != null && loanCharge.get().getDueDate().isBefore(loanCharge.get().getSubmittedOnDate()));


Done! removed

alberto-art3ch · 2024-07-09T19:05:02Z

...java/org/apache/fineract/portfolio/loanaccount/loanschedule/data/LoanSchedulePeriodData.java

    private final Boolean downPaymentPeriod;
+    private final Boolean actualPeriod;


Done! removed

alberto-art3ch · 2024-07-09T20:01:34Z

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/LoanSummaryData.java

@@ -110,6 +116,11 @@ public static LoanSummaryData withTransactionAmountsSummary(final LoanSummaryDat
        BigDecimal totalCreditBalanceRefundReversed = BigDecimal.ZERO;
        BigDecimal totalRepaymentTransaction = BigDecimal.ZERO;
        BigDecimal totalRepaymentTransactionReversed = BigDecimal.ZERO;
+        BigDecimal totalInterestRefund = BigDecimal.ZERO;


Done! removed

alberto-art3ch · 2024-07-09T20:13:23Z

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/LoanSummaryData.java

@@ -191,4 +222,31 @@ private static BigDecimal computeTotalRepaymentTransactionAmount(Collection<Loan
                loanTransactions);
        return totalRepaymentTransaction.add(totalDownPaymentTransaction);
    }
+
+    private static BigDecimal computeTotalAccruedDueAmount(Collection<LoanSchedulePeriodData> periods) {


Done! method renamed

alberto-art3ch · 2024-07-09T20:13:34Z

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/LoanSummaryData.java

+                .map(period -> period.getInterestPaid()).reduce(BigDecimal.ZERO, BigDecimal::add);
+    }
+
+    private static BigDecimal fetchTotalAccruedNotDueAmount(Collection<LoanSchedulePeriodData> periods) {


Done! method renamed

alberto-art3ch · 2024-07-09T20:13:47Z

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/LoanSummaryData.java

+                .map(period -> period.getTotalAccruedInterest()).reduce(BigDecimal.ZERO, BigDecimal::add);
+    }
+
+    private static BigDecimal fetchTotalInterestNotDueAmount(Collection<LoanSchedulePeriodData> periods) {


Done! method renamed

.../main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/data/LoanScheduleData.java

alberto-art3ch · 2024-07-09T20:16:30Z

...main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java

-                    + " ls.total_paid_late_derived as totalPaidLateForPeriod, ls.credits_amount as principalCredits, ls.credited_fee as feeCredits, ls.credited_penalty as penaltyCredits, ls.is_down_payment isDownPayment "
-                    + " from m_loan_repayment_schedule ls ";
+                    + " ls.total_paid_late_derived as totalPaidLateForPeriod, ls.credits_amount as principalCredits, ls.credited_fee as feeCredits, ls.credited_penalty as penaltyCredits, ls.is_down_payment isDownPayment, "
+                    + " ls.accrual_interest_derived as totalAccrualInterest " + " from m_loan_repayment_schedule ls ";


Done! renamed

...pache/fineract/portfolio/deliquency/DelinquencyWritePlatformServiceRangeChangeEventTest.java

adamsaghy

LGTM

…linquency

adamsaghy reviewed Jul 3, 2024

View reviewed changes

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/LoanSummaryData.java Show resolved Hide resolved

adamsaghy reviewed Jul 3, 2024

View reviewed changes

...der/src/main/java/org/apache/fineract/portfolio/loanaccount/api/LoansApiResourceSwagger.java Outdated Show resolved Hide resolved

adamsaghy reviewed Jul 3, 2024

View reviewed changes

...pache/fineract/portfolio/deliquency/DelinquencyWritePlatformServiceRangeChangeEventTest.java Show resolved Hide resolved

adamsaghy requested changes Jul 3, 2024

View reviewed changes

alberto-art3ch force-pushed the enhancement/loan_data_new_attributes branch from d14403e to 6db3dab Compare July 7, 2024 05:13

adamsaghy reviewed Jul 8, 2024

View reviewed changes

alberto-art3ch force-pushed the enhancement/loan_data_new_attributes branch from 6db3dab to 33cd782 Compare July 8, 2024 15:57

adamsaghy reviewed Jul 8, 2024

View reviewed changes

alberto-art3ch force-pushed the enhancement/loan_data_new_attributes branch from 33cd782 to 766a003 Compare July 9, 2024 16:04

adamsaghy reviewed Jul 9, 2024

View reviewed changes

.../main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/data/LoanScheduleData.java Outdated Show resolved Hide resolved

adamsaghy reviewed Jul 9, 2024

View reviewed changes

alberto-art3ch force-pushed the enhancement/loan_data_new_attributes branch from 766a003 to 2c9f868 Compare July 10, 2024 06:38

alberto-art3ch commented Jul 10, 2024

View reviewed changes

alberto-art3ch force-pushed the enhancement/loan_data_new_attributes branch 2 times, most recently from b306fd2 to 6c01694 Compare July 10, 2024 13:23

adamsaghy approved these changes Jul 10, 2024

View reviewed changes

alberto-art3ch force-pushed the enhancement/loan_data_new_attributes branch from 6c01694 to 23a096e Compare July 10, 2024 14:12

FINERACT-2081: Loan account data additional fields for summary and de…

5652354

…linquency

alberto-art3ch force-pushed the enhancement/loan_data_new_attributes branch from 23a096e to 5652354 Compare July 10, 2024 17:11

adamsaghy merged commit 79f32d3 into apache:develop Jul 11, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FINERACT-2081: Loan account data additional fields for summary and de… #3957

FINERACT-2081: Loan account data additional fields for summary and de… #3957

alberto-art3ch commented Jul 3, 2024

adamsaghy Jul 3, 2024

adamsaghy Jul 3, 2024

alberto-art3ch Jul 9, 2024

adamsaghy Jul 10, 2024

adamsaghy left a comment

adamsaghy Jul 8, 2024

adamsaghy Jul 8, 2024

adamsaghy Jul 8, 2024 •

edited

Loading

alberto-art3ch Jul 9, 2024

adamsaghy Jul 10, 2024

adamsaghy Jul 9, 2024

alberto-art3ch Jul 9, 2024

adamsaghy Jul 9, 2024

alberto-art3ch Jul 9, 2024

adamsaghy Jul 9, 2024

alberto-art3ch Jul 9, 2024

adamsaghy Jul 9, 2024

alberto-art3ch Jul 9, 2024

adamsaghy Jul 9, 2024

alberto-art3ch Jul 9, 2024

alberto-art3ch left a comment

alberto-art3ch Jul 9, 2024

alberto-art3ch Jul 9, 2024

alberto-art3ch Jul 9, 2024

alberto-art3ch Jul 9, 2024

alberto-art3ch Jul 9, 2024

alberto-art3ch Jul 9, 2024

alberto-art3ch Jul 9, 2024

adamsaghy left a comment

		private final Boolean downPaymentPeriod;
		private final Boolean actualPeriod;

FINERACT-2081: Loan account data additional fields for summary and de… #3957

FINERACT-2081: Loan account data additional fields for summary and de… #3957

Conversation

alberto-art3ch commented Jul 3, 2024

Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamsaghy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamsaghy Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alberto-art3ch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamsaghy left a comment

Choose a reason for hiding this comment

adamsaghy Jul 8, 2024 •

edited

Loading