adamsaghy commented on code in PR #6405:
URL: https://github.com/apache/fineract/pull/6405#discussion_r4383667867
##########
fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/DepositAccountTermAndPreClosure.java:
##########
@@ -301,12 +315,13 @@ public DepositAccountTermAndPreClosure copy(BigDecimal
depositAmount) {
final DepositTermDetail depositTermDetail =
this.depositTermDetail.copy();
final LocalDate expectedFirstDepositOnDate = null;
final Boolean transferInterestToLinkedAccount = false;
+ final boolean isRateChartOverridden = this.isRateChartOverridden;
Review Comment:
Carrying the flag over to the reinvested term causes a 0% reinvestment.
`FixedDepositAccount.reInvest` and `RecurringDepositAccount.reInvest` create
the new account with `interestRate = BigDecimal.ZERO`. On develop that 0 is
harmless, because `getEffectiveInterestRateAsFraction` goes to the chart and
overwrites `nominalAnnualInterestRate`. With `isRateChartOverridden = true`
copied over, the chart is skipped and `nominalAnnualInterestRate` (0) is
returned.
`DepositAccountDomainServiceJpa` then calls
`updateMaturityDateAndAmountBeforeAccountActivation` /
`updateMaturityDateAndAmount` and activates the reinvested account without
running the domain-rule validation. So nothing rejects the 0.
Scenario: an FD opened with a custom 10% rate and "reinvest principal and
interest" matures. The reinvested deposit's maturity amount is computed at 0%
and it accrues and posts no interest.
Either carry the custom rate over to the reinvested account, or don't copy
the flag so it falls back to the product chart. Whichever is intended, please
add a test for reinvesting an overridden account.
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/FixedDepositTest.java:
##########
@@ -1620,4 +1620,61 @@ private LoanTestData.Journal debit(final Account
account, final BigDecimal amoun
private LoanTestData.Journal credit(final Account account, final
BigDecimal amount) {
return LoanTestData.Journal.credit(account.getAccountID().longValue(),
amount.doubleValue());
}
+
+ @Test
+ public void testFixedDepositAccountWithRateChartOverriddenLifecycle() {
+ DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy",
Locale.US);
+ Calendar todaysDate = Calendar.getInstance();
+ todaysDate.add(Calendar.MONTH, -3);
+ final String VALID_FROM = dateFormat.format(todaysDate.getTime());
+ todaysDate.add(Calendar.YEAR, 10);
+ final String VALID_TO = dateFormat.format(todaysDate.getTime());
+
+ todaysDate = Calendar.getInstance();
+ todaysDate.add(Calendar.MONTH, -1);
+ final String SUBMITTED_ON_DATE =
dateFormat.format(todaysDate.getTime());
+ final String APPROVED_ON_DATE =
dateFormat.format(todaysDate.getTime());
+ final String ACTIVATION_DATE = dateFormat.format(todaysDate.getTime());
+
+ Long clientId = clientHelper.createClient();
+ Assertions.assertNotNull(clientId);
+
+ Long fixedDepositProductId = createFixedDepositProduct(VALID_FROM,
VALID_TO, NONE);
+ Assertions.assertNotNull(fixedDepositProductId);
+
+ // 1. Submit application with isRateChartOverridden = true and custom
nominalAnnualInterestRate = 10.0
+ final BigDecimal customInterestRate = new BigDecimal("10.0");
+ PostFixedDepositAccountsRequest request = DepositRequestBuilders
+ .fixedDepositAccount(clientId, fixedDepositProductId,
SUBMITTED_ON_DATE, WHOLE_TERM).isRateChartOverridden(true)
+ .nominalAnnualInterestRate(customInterestRate);
+
+ Long fixedDepositAccountId =
fixedDepositHelper.submitApplication(request).getSavingsId();
+ Assertions.assertNotNull(fixedDepositAccountId);
+
+ // 2. GET and verify rate chart is overridden with custom rate
+ GetFixedDepositAccountsAccountIdResponse account =
fixedDepositHelper.getAccount(fixedDepositAccountId);
+
Assertions.assertTrue(Boolean.TRUE.equals(account.getIsRateChartOverridden()),
"isRateChartOverridden should be true");
+ Assertions.assertEquals(0,
customInterestRate.compareTo(account.getNominalAnnualInterestRate()));
+
+ // 3. Update application to toggle isRateChartOverridden back to false
Review Comment:
The flag is switched back off here before approve/activate, so step 6 only
checks the chart rate (7%). Please add a case that keeps the override on
through activation and interest calculation (asserting interest at the custom
10%), and an equivalent RD case.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/FixedDepositAccount.java:
##########
@@ -161,7 +161,7 @@ protected BigDecimal
getEffectiveInterestRateAsFraction(final MathContext mc, fi
// default it to nominalAnnualInterst rate. interest chart overrrides
// this value.
BigDecimal applicableInterestRate = this.nominalAnnualInterestRate;
- if (this.chart != null) {
+ if (this.chart != null &&
!this.accountTermAndPreClosure.isRateChartOverridden()) {
Review Comment:
The pre-closure penalty logic only exists inside this `chart != null &&
!isRateChartOverridden()` block. So for an overridden account, `prematureClose`
posts interest at the full custom rate even when the product has
`preClosurePenalApplicable` set.
`testFixedDepositEffectiveInterestRateWithOverriddenRateAndPrematureClosurePenalty`
asserts exactly that: 7.5% with a 1% penalty configured. The same applies to
`RecurringDepositAccount`.
The penalty is a product pre-closure setting, not part of the chart.
FINERACT-848 and the PR description only ask for the chart rate to be
overridden. Please make this an explicit decision: either apply the penalty to
the custom rate (in a local variable, without writing it back to
`nominalAnnualInterestRate`), or state in the PR/JIRA that overridden accounts
are exempt from the pre-closure penalty.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
