FINERACT-2455: Store and return annual effective interest rate on WC loans

[oleksii-novikov-onix]

Description

Describe the changes made and why they were made. (Ignore if these details are present on the associated Apache Fineract JIRA ticket.)

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

• Write the commit message as per our guidelines
• 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 our 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
• This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
• If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

[galovics]

Replacing the hardcoded pow(365) with npvDayCount is a genuine correctness fix, and I like the legacy-model fallback in annualEffectiveInterestRate() for schedules persisted before this change. But there's a scope-collision problem that needs sorting out before anything else here.

This directly overlaps with #6264 ("payment rate change EIR history"), on the same JIRA ticket. Both PRs touch 9 of the same files, including ProjectedAmortizationScheduleModel.java and the WC account Avro schema. Both replace the exact same hardcoded pow(365) call in WorkingCapitalLoanApplicationReadPlatformServiceImpl, but with two different helper methods living in two different classes. And the two PRs make mutually exclusive contract changes: #6264 keeps dailyEir on the response/event and documents it in a new Avro record; this PR deletes dailyEir from the response, both mappers, and the existing Avro schema. Whichever of these two merges second is going to break the other. This needs to be resolved between the two PRs - either merge them, or have one explicitly build on top of the other, before either goes further.

Two issues on the substance, both of which also apply to #6264 (already flagged there) but are worse here since this PR feeds the rounded value back into the schedule math:

Tenant money rounding mode applied to a rate, and that rate then drives the schedule. normalizedAnnualRate rounds with mc.getRoundingMode(), which is the tenant-configurable money rounding mode - and the result is fed straight back through deannualize to become effectiveInterestRate, the value the whole amortization schedule discounts on. A tenant configured FLOOR and one configured CEILING will get materially different WC schedules for identical loan inputs. A rate isn't money; this should use a fixed rounding mode (#6264 has already been corrected to RoundingMode.HALF_EVEN for the same issue - worth reusing that constant if the two PRs get reconciled).

The rate-segment's annual EIR is computed and then thrown away. computeScheduleParams returns annualEir in ScheduleParams, but applyRateChange only pulls segment.eir() and drops segment.annualEir() on the floor - so after a payment rate change, the reported annual rate goes stale while the schedule itself is segment-aware. That directly contradicts the new Swagger text ("annual effective rate the schedule runs on").

One more concrete bug: deannualize's Newton-Raphson seed (Math.pow(...)) can produce Infinity for an extreme daily IRR (reachable on a short schedule with a large discount fee relative to net disbursement), which then throws NumberFormatException out of BigDecimal.valueOf. The one existing caller of this path swallows it by accident (NumberFormatException extends IllegalArgumentException), but applyRateChange's new call site has no such guard and would surface this as a 500. Also: zero unit tests for the new TvmFunctions numerical kernel (hand-rolled Newton-Raphson, no test class at all).

Recommendation: CHANGES_REQUESTED

[oleksii-novikov-onix]

Replacing the hardcoded pow(365) with npvDayCount is a genuine correctness fix, and I like the legacy-model fallback in annualEffectiveInterestRate() for schedules persisted before this change. But there's a scope-collision problem that needs sorting out before anything else here.

This directly overlaps with #6264 ("payment rate change EIR history"), on the same JIRA ticket. Both PRs touch 9 of the same files, including ProjectedAmortizationScheduleModel.java and the WC account Avro schema. Both replace the exact same hardcoded pow(365) call in WorkingCapitalLoanApplicationReadPlatformServiceImpl, but with two different helper methods living in two different classes. And the two PRs make mutually exclusive contract changes: #6264 keeps dailyEir on the response/event and documents it in a new Avro record; this PR deletes dailyEir from the response, both mappers, and the existing Avro schema. Whichever of these two merges second is going to break the other. This needs to be resolved between the two PRs - either merge them, or have one explicitly build on top of the other, before either goes further.

Two issues on the substance, both of which also apply to #6264 (already flagged there) but are worse here since this PR feeds the rounded value back into the schedule math:

Tenant money rounding mode applied to a rate, and that rate then drives the schedule. normalizedAnnualRate rounds with mc.getRoundingMode(), which is the tenant-configurable money rounding mode - and the result is fed straight back through deannualize to become effectiveInterestRate, the value the whole amortization schedule discounts on. A tenant configured FLOOR and one configured CEILING will get materially different WC schedules for identical loan inputs. A rate isn't money; this should use a fixed rounding mode (#6264 has already been corrected to RoundingMode.HALF_EVEN for the same issue - worth reusing that constant if the two PRs get reconciled).

The rate-segment's annual EIR is computed and then thrown away. computeScheduleParams returns annualEir in ScheduleParams, but applyRateChange only pulls segment.eir() and drops segment.annualEir() on the floor - so after a payment rate change, the reported annual rate goes stale while the schedule itself is segment-aware. That directly contradicts the new Swagger text ("annual effective rate the schedule runs on").

One more concrete bug: deannualize's Newton-Raphson seed (Math.pow(...)) can produce Infinity for an extreme daily IRR (reachable on a short schedule with a large discount fee relative to net disbursement), which then throws NumberFormatException out of BigDecimal.valueOf. The one existing caller of this path swallows it by accident (NumberFormatException extends IllegalArgumentException), but applyRateChange's new call site has no such guard and would surface this as a 500. Also: zero unit tests for the new TvmFunctions numerical kernel (hand-rolled Newton-Raphson, no test class at all).

Recommendation: CHANGES_REQUESTED

1. Scope collision with FINERACT-2455: payment rate change EIR history #6264. We touch 10 of the same files, and a test merge gives 3 conflicts: the shared feature file, the Swagger block around dailyEir, and enrichWithRateAndTerm, where we both replace the hardcoded 365. The helpers differ - annualiseEir does not round, normalizedAnnualRate rounds to six decimals and feeds the result back into the daily rate - and ProjectedAmortizationScheduleModel merges with no conflict at all, ending up with both. The rest is not exclusive: we drop dailyEir from the account, FINERACT-2455: payment rate change EIR history #6264 adds a daily rate to each history row.

2. Tenant money rounding mode on a rate. Fixed: normalizedAnnualRate now rounds with a fixed HALF_EVEN constant, the same value FINERACT-2455: payment rate change EIR history #6264 uses.

3. Segment annual EIR dropped. The annual rate is not dropped - computeScheduleParams derives the segment's daily rate from it, so every segment does run on its own normalised rate. The wrong part was the text: the value on the account is the rate the loan was priced at, matching the period payment rate next to it, which is also left as booked because what is in force on a date is read from the rate-change history. Swagger, the Avro doc and the javadoc now say that; the segment's own annual rate belongs on a history row, which is FINERACT-2455: payment rate change EIR history #6264's subject.

4. Seed can overflow to Infinity. Fixed.

5. No unit tests. Added TvmFunctionsTest.

[adamsaghy]

org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest
  
    Test testExcessPayment_term200_discountFee1000_netDisbursement9000_pay70_80() FAILED
  
    org.opentest4j.AssertionFailedError: inst 22: discountFactor — expected: 0.97888130, actual: 0.97888129 ==> expected: <0> but was: <1>
        at app//org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest.assertValue(ProjectedAmortizationScheduleCalculatorTest.java:2464)
  
    Test testOnTimePayments_doNotMoveTheProjectionOfLaterPeriods() PASSED (1.1s)
    Test testApplyRateChange_twiceWithDateGap() PASSED
    Test testLessPayment_term10_discountFee50_netDisbursement450_pay40() PASSED
    Test testApplyRateChange_beforeDisburseDate() PASSED
    Test testOverpaidLoanProjectsNothingRatherThanUnEarningFee() PASSED
    Test testApplyRateChange_sameDayAsDisburse() PASSED
    Test testProjectedSchedule_term200_discountFee1000_netDisbursement9000() FAILED
  
    org.opentest4j.AssertionFailedError: inst 20: discountFactor — expected: 0.97888130, actual: 0.97888129 ==> expected: <0> but was: <1>
        at app//org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest.assertValue(ProjectedAmortizationScheduleCalculatorTest.java:2464)
  
    Test testNoDiscountLoan_term180_discountFee0_netDisbursement9000() PASSED
    Test testApplyRateChange_nearEndOfTerm() PASSED
    Test testAddDisbursement_term10_discountFee50_netDisbursement450_then430() PASSED
    Test testApplyRateChange_8daysAfterDisburse() PASSED
    Test testExcessPayment_term10_discountFee50_netDisbursement450_pay110() PASSED
    Test testOnTimePayment_term200_discountFee1000_netDisbursement9000_pay50_50() FAILED
  
    org.opentest4j.AssertionFailedError: inst 22: discountFactor — expected: 0.97888130, actual: 0.97888129 ==> expected: <0> but was: <1>
        at app//org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest.assertValue(ProjectedAmortizationScheduleCalculatorTest.java:2464)
  
    Test testNoPayment_term200_discountFee1000_netDisbursement9000_pay0_0_50() FAILED
  
    org.opentest4j.AssertionFailedError: inst 23: discountFactor — expected: 0.97888130, actual: 0.97888129 ==> expected: <0> but was: <1>
        at app//org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest.assertValue(ProjectedAmortizationScheduleCalculatorTest.java:2464)
  
    Test testApplyRateChange_pastEndOfTerm() PASSED
    Test testApplyRateChange_twiceWithDateGapAndPayment() PASSED
    Test testLessPayment_term200_discountFee1000_netDisbursement9000_pay40() FAILED
  
    org.opentest4j.AssertionFailedError: inst 21: discountFactor — expected: 0.97888130, actual: 0.97888129 ==> expected: <0> but was: <1>
        at app//org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest.assertValue(ProjectedAmortizationScheduleCalculatorTest.java:2464)
  
  

[oleksii-novikov-onix]

org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest
  
    Test testExcessPayment_term200_discountFee1000_netDisbursement9000_pay70_80() FAILED
  
    org.opentest4j.AssertionFailedError: inst 22: discountFactor — expected: 0.97888130, actual: 0.97888129 ==> expected: <0> but was: <1>
        at app//org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest.assertValue(ProjectedAmortizationScheduleCalculatorTest.java:2464)
  
    Test testOnTimePayments_doNotMoveTheProjectionOfLaterPeriods() PASSED (1.1s)
    Test testApplyRateChange_twiceWithDateGap() PASSED
    Test testLessPayment_term10_discountFee50_netDisbursement450_pay40() PASSED
    Test testApplyRateChange_beforeDisburseDate() PASSED
    Test testOverpaidLoanProjectsNothingRatherThanUnEarningFee() PASSED
    Test testApplyRateChange_sameDayAsDisburse() PASSED
    Test testProjectedSchedule_term200_discountFee1000_netDisbursement9000() FAILED
  
    org.opentest4j.AssertionFailedError: inst 20: discountFactor — expected: 0.97888130, actual: 0.97888129 ==> expected: <0> but was: <1>
        at app//org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest.assertValue(ProjectedAmortizationScheduleCalculatorTest.java:2464)
  
    Test testNoDiscountLoan_term180_discountFee0_netDisbursement9000() PASSED
    Test testApplyRateChange_nearEndOfTerm() PASSED
    Test testAddDisbursement_term10_discountFee50_netDisbursement450_then430() PASSED
    Test testApplyRateChange_8daysAfterDisburse() PASSED
    Test testExcessPayment_term10_discountFee50_netDisbursement450_pay110() PASSED
    Test testOnTimePayment_term200_discountFee1000_netDisbursement9000_pay50_50() FAILED
  
    org.opentest4j.AssertionFailedError: inst 22: discountFactor — expected: 0.97888130, actual: 0.97888129 ==> expected: <0> but was: <1>
        at app//org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest.assertValue(ProjectedAmortizationScheduleCalculatorTest.java:2464)
  
    Test testNoPayment_term200_discountFee1000_netDisbursement9000_pay0_0_50() FAILED
  
    org.opentest4j.AssertionFailedError: inst 23: discountFactor — expected: 0.97888130, actual: 0.97888129 ==> expected: <0> but was: <1>
        at app//org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest.assertValue(ProjectedAmortizationScheduleCalculatorTest.java:2464)
  
    Test testApplyRateChange_pastEndOfTerm() PASSED
    Test testApplyRateChange_twiceWithDateGapAndPayment() PASSED
    Test testLessPayment_term200_discountFee1000_netDisbursement9000_pay40() FAILED
  
    org.opentest4j.AssertionFailedError: inst 21: discountFactor — expected: 0.97888130, actual: 0.97888129 ==> expected: <0> but was: <1>
        at app//org.apache.fineract.portfolio.workingcapitalloan.calc.ProjectedAmortizationScheduleCalculatorTest.assertValue(ProjectedAmortizationScheduleCalculatorTest.java:2464)
  
  

Fixed

[adamsaghy]

@oleksii-novikov-onix Please rebase

[oleksii-novikov-onix]

@oleksii-novikov-onix Please rebase

Done

[galovics]

Four of the five things from last round are genuinely fixed, and well - the rounding mode is now a documented fixed constant (RoundingMode.HALF_EVEN) rather than the tenant's money mode, the Newton-Raphson overflow is fixed by splitting the decimal exponent instead of going through double, and TvmFunctions now has 16 unit tests covering round-trips and both overflow directions. The rate-segment staleness is now a documented design decision rather than an accident, though it does depend on #6264 for a way to see the current in-force rate.

The collision with #6264 is still completely open though, and it's sharper than before: both PRs were pushed within the same minute today, both still touch the same 9 files, and now they define the same field incompatibly - this PR makes calculatedAnnualEir a rounded percentage while #6264 keeps it an unrounded fraction, on the same endpoint. #6264's new Swagger text ("Note: periodPaymentRateHistory[].calculatedAnnualEir is a percentage" contrasted with the fraction at the top level) would be actively wrong the moment this PR lands. This needs to be resolved between the two PRs - ideally by having this PR read the annual rate off #6264's Solved record instead of maintaining a second annualization helper.

Beyond the collision, CI is actually red right now, and it looks like the same root cause across all of the failures: the rate round-trip (solve IRR -> round to 6dp annual -> spread back to a periodic rate) moves real cents in the amortization, and the expected values in the test suite were only partially updated to match - ProjectedAmortizationScheduleCalculatorTest and one e2e scenario both show the sweep stopped partway through. And the dailyEir removal is actually failing the project's own run-api-backward-compatibility gate right now (4 confirmed R014 errors), not just something that will show up as a warning.

One more: this modifies the WorkingCapitalLoanAccountDataV1 avro schema in place (deleting dailyEir, redefining calculatedAnnualEir's meaning) rather than creating a V2, which the project's own reliable-event-framework doc says should happen for exactly this kind of change to a published V1 record.

There's also an unrelated commit in here (SavingsInterestPostingJobIntegrationTest business-date pinning) that has nothing to do with WC or annual EIR - should be its own PR.

To get this to green: resolve the collision with #6264 (one unit, one helper), fix the CI failures (either the amortization drift is intentional and every expected value needs updating, or the round-trip shouldn't be perturbing the schedule at all), restore dailyEir or get sign-off on removing it, and split out the unrelated commit.

Recommendation: CHANGES_REQUESTED

[oleksii-novikov-onix]

Four of the five things from last round are genuinely fixed, and well - the rounding mode is now a documented fixed constant (RoundingMode.HALF_EVEN) rather than the tenant's money mode, the Newton-Raphson overflow is fixed by splitting the decimal exponent instead of going through double, and TvmFunctions now has 16 unit tests covering round-trips and both overflow directions. The rate-segment staleness is now a documented design decision rather than an accident, though it does depend on #6264 for a way to see the current in-force rate.

The collision with #6264 is still completely open though, and it's sharper than before: both PRs were pushed within the same minute today, both still touch the same 9 files, and now they define the same field incompatibly - this PR makes calculatedAnnualEir a rounded percentage while #6264 keeps it an unrounded fraction, on the same endpoint. #6264's new Swagger text ("Note: periodPaymentRateHistory[].calculatedAnnualEir is a percentage" contrasted with the fraction at the top level) would be actively wrong the moment this PR lands. This needs to be resolved between the two PRs - ideally by having this PR read the annual rate off #6264's Solved record instead of maintaining a second annualization helper.

Beyond the collision, CI is actually red right now, and it looks like the same root cause across all of the failures: the rate round-trip (solve IRR -> round to 6dp annual -> spread back to a periodic rate) moves real cents in the amortization, and the expected values in the test suite were only partially updated to match - ProjectedAmortizationScheduleCalculatorTest and one e2e scenario both show the sweep stopped partway through. And the dailyEir removal is actually failing the project's own run-api-backward-compatibility gate right now (4 confirmed R014 errors), not just something that will show up as a warning.

One more: this modifies the WorkingCapitalLoanAccountDataV1 avro schema in place (deleting dailyEir, redefining calculatedAnnualEir's meaning) rather than creating a V2, which the project's own reliable-event-framework doc says should happen for exactly this kind of change to a published V1 record.

There's also an unrelated commit in here (SavingsInterestPostingJobIntegrationTest business-date pinning) that has nothing to do with WC or annual EIR - should be its own PR.

To get this to green: resolve the collision with #6264 (one unit, one helper), fix the CI failures (either the amortization drift is intentional and every expected value needs updating, or the round-trip shouldn't be perturbing the schedule at all), restore dailyEir or get sign-off on removing it, and split out the unrelated commit.

Recommendation: CHANGES_REQUESTED

Collision with #6264. Nine files overlap, and that line in
#6264
's Swagger ("unlike the top-level calculatedAnnualEir, which is a fraction") does become wrong once this lands. But the two PRs define the field the same way:
#6264
already has ANNUAL_EIR_SCALE = 6, ANNUAL_EIR_ROUNDING = HALF_EVEN and annualEirPercentage = annualiseEir(dailyEir, npvDayCount) × 100 rounded to six decimals. That is exactly what our AmortizationParams.calculatedAnnualEir does — same base, same scale, same rounding, same unit. The only real difference is timing: we have already moved the top-level field to a percentage, and #6264 still describes the old fraction. Once both land, both surfaces are percentages at scale 6 and they agree. The unit is not ours to change either — "store the annualized calculated eir as percentage, so we got a higher precision" is the instruction this PR implements.

On reading the value off #6264's Solved, it works the other way round. Solved already exists in develop, and this PR is the one adding calculatedAnnualEir to it.
#6264
adds RateChangeSolve, a map keyed by rate change date, and that map is empty for a loan that never had a rate change, so the account-level value cannot come from there.

What we suggest instead:
#6264
reads Solved.calculatedAnnualEir() from this PR for its history rows. Both of its helpers then become unused, and the fraction sentence goes away with them.

CI. Fixed, and green on the current head.

dailyEir and the R014 gate. Confirmed. The four errors are one field on four endpoints: the template, the paged list, {loanId} and external-id/{loanExternalId}. Removing it is what the AC asks for: "let's return annual effective interest rate (only) on loan details".

Avro V1 versus V2. The rule is in the docs, but it has never been used for WC: there is no *V2.avsc file anywhere in the WC module. This schema was created by this ticket in July and has already been amended in place once since, and both changes were merged. I'm fine with creating a V2 if you want the rule applied to WC. @adamsaghy What do you think?

The savings commit. Agreed, and done. It is out of this branch (three commits now, all WC) and raised separately as #6422.

[adamsaghy]

@oleksii-novikov-onix Please rebase

We dont need v2, Working capital is not released officially.