[
https://issues.apache.org/jira/browse/FINERACT-2492?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ralph Hopman resolved FINERACT-2492.
------------------------------------
Resolution: Fixed
> AprCalculator produces incorrect annual interest rate when DaysInYearType is
> ACTUAL with WHOLE_TERM frequency
> -------------------------------------------------------------------------------------------------------------
>
> Key: FINERACT-2492
> URL: https://issues.apache.org/jira/browse/FINERACT-2492
> Project: Apache Fineract
> Issue Type: Bug
> Components: Loan
> Reporter: Ralph Hopman
> Assignee: Ralph Hopman
> Priority: Major
>
> When calculating annual nominal interest rates for loans using
> {{interestRateFrequencyType = WHOLE_TERM}} combined with {{{}daysInYearType =
> ACTUAL{}}}, the {{AprCalculator}} produces dramatically incorrect results.
> The calculator multiplies the per-period rate by
> {{daysInYearType.getValue()}} which returns {{1}} for ACTUAL instead of
> {{{}365{}}}, resulting in interest charges that are ~365x lower than expected.
> h2. Steps to Reproduce
> # Create a loan product with:
> ** {{daysInYearType = ACTUAL}}
> ** {{interestType = FLAT}}
> ** {{interestRateFrequencyType = WHOLE_TERM}}
> # Create a loan with:
> ** {{principal = 3,400}}
> ** {{interestRatePerPeriod = 10}} (expecting 10% total interest = 340)
> ** {{numberOfRepayments = 3}}
> ** {{repaymentEvery = 1}}
> ** {{repaymentFrequencyType = DAYS}}
> ** {{loanTermFrequency = 3}}
> ** {{loanTermFrequencyType = DAYS}}
> # Calculate loan schedule via {{POST /loans?command=calculateLoanSchedule}}
> h2. Expected Behavior
> *Annual rate calculation:*
> {code:java}
> ratePerPeriod = 10% ÷ 3 repayments = 3.333%
> annualRate = 3.333% × 365 days = 1,216.67%
> {code}
> *Interest for 3-day term:*
> {code:java}
> interestRate = (1,216.67% / 365 / 100) × 3 days = 10%
> totalInterest = 3,400 × 10% = 340.00
> {code}
> *Result:* Each of 3 installments should have *113.33* interest charge.
> h2. Actual Behavior
> *Annual rate calculation:*
> {code:java}
> ratePerPeriod = 10% ÷ 3 repayments = 3.333%
> annualRate = 3.333% × 1 = 3.333% ← WRONG!
> {code}
> *Interest for 3-day term:*
> {code:java}
> interestRate = (3.333% / 365 / 100) × 3 days = 0.0274%
> totalInterest = 3,400 × 0.0274% = 0.93
> {code}
> *Result:* Total interest charged is only *0.93* instead of *340.00* (365x too
> low).
> h2. Root Cause
> In {{{}AprCalculator.java{}}}, both the {{DAYS}} case (line 40) and the
> {{WHOLE_TERM → DAYS}} case (line 56) use {{daysInYearType.getValue()}}
> directly:
> {code:java}
> case DAYS:
> defaultAnnualNominalInterestRate = interestRatePerPeriod
> .multiply(BigDecimal.valueOf(daysInYearType.getValue())); //
> getValue() returns 1 for ACTUAL!
> break;
> {code}
> The {{DaysInYearType}} enum defines:
> {code:java}
> ACTUAL(1, "DaysInYearType.actual"),
> DAYS_360(360, "DaysInYearType.days360"),
> DAYS_364(364, "DaysInYearType.days364"),
> DAYS_365(365, "DaysInYearType.days365");
> {code}
> The value {{1}} for ACTUAL is a sentinel value intended to signal "use actual
> days calculation" (handled properly in other parts of the codebase like
> {{{}LoanApplicationTerms.calculatePeriodsInOneYear(){}}}). However, in
> {{{}AprCalculator{}}}, this sentinel value is incorrectly used directly as a
> multiplier for annualization.
> The correct pattern (used elsewhere in Fineract) is to delegate to
> {{PaymentPeriodsInOneYearCalculator}} when {{{}daysInYearType == ACTUAL{}}},
> rather than using the enum's {{getValue()}} directly.
> h2. Solution
> Inject {{PaymentPeriodsInOneYearCalculator}} into {{AprCalculator}} and add a
> helper method {{getDaysInYear()}} that delegates to the calculator when
> {{{}daysInYearType == ACTUAL{}}}:
> {code:java}
> @Component
> @RequiredArgsConstructor
> public class AprCalculator {
> private final PaymentPeriodsInOneYearCalculator
> paymentPeriodsInOneYearCalculator;
> public BigDecimal calculateFrom(..., final DaysInYearType daysInYearType)
> {
> // ... existing code ...
> case DAYS:
> defaultAnnualNominalInterestRate = interestRatePerPeriod
> .multiply(BigDecimal.valueOf(getDaysInYear(daysInYearType)));
> break;
> // ... similar for WHOLE_TERM → DAYS case ...
> }
> /**
> * Helper method to get the number of days in a year, handling ACTUAL
> appropriately.
> *
> * When daysInYearType is ACTUAL, this delegates to the
> PaymentPeriodsInOneYearCalculator
> * (consistent with how Fineract handles ACTUAL elsewhere). For other
> types (DAYS_360,
> * DAYS_364, DAYS_365), it returns the configured value.
> */
> private int getDaysInYear(final DaysInYearType daysInYearType) {
> // When ACTUAL, delegate to calculator (consistent with
> LoanApplicationTerms.calculatePeriodsInOneYear)
> if (daysInYearType == DaysInYearType.ACTUAL) {
> return
> paymentPeriodsInOneYearCalculator.calculate(PeriodFrequencyType.DAYS);
> }
> // For DAYS_360, DAYS_364, DAYS_365: use configured value
> return daysInYearType.getValue();
> }
> }
> {code}
> This approach:
> * Delegates to
> {{PaymentPeriodsInOneYearCalculator.calculate(PeriodFrequencyType.DAYS)}}
> which returns {{365}}
> * Follows the established pattern used in
> {{LoanApplicationTerms.calculatePeriodsInOneYear()}}
> * Maintains architectural consistency across the codebase
> * Automatically benefits from any future enhancements to the calculator
> (e.g., leap year support)
> h2. Testing Recommendations
> # Add unit tests for {{AprCalculator.calculateFrom()}} covering:
> ** {{DAYS}} frequency with {{DaysInYearType.ACTUAL}}
> ** {{WHOLE_TERM}} frequency with {{DAYS}} repayment and
> {{DaysInYearType.ACTUAL}}
> ** Verify {{{}DAYS_360{}}}, {{{}DAYS_364{}}}, {{DAYS_365}} continue to work
> correctly
> ** Mock {{PaymentPeriodsInOneYearCalculator}} to verify delegation behavior
> # Add integration test for loan schedule calculation with the reproduction
> steps above
> # Verify existing loans with {{DaysInYearType.ACTUAL}} produce correct
> interest after fix
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
