You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "Eyad-Sharkawy (via GitHub)" <gi...@apache.org> on 2026/09/06 01:03:04 UTC

[PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Eyad-Sharkawy opened a new pull request, #6405:
URL: https://github.com/apache/fineract/pull/6405

   ## Description
   Resolves [FINERACT-848](https://issues.apache.org/jira/browse/FINERACT-848).
   
   Adds the ability to override product-level interest rate charts with a custom nominal annual interest rate for Fixed Deposit (FD) and Recurring Deposit (RD) accounts:
   - Added `isRateChartOverridden` boolean flag to deposit account terms and data models.
   - Updated domain interest calculation and validation in `FixedDepositAccount` and `RecurringDepositAccount` so that when `isRateChartOverridden` is enabled, custom nominal annual interest rate takes precedence and bypasses product chart slab lookups.
   - Added API request/response parameters, Swagger documentation annotations, and Liquibase database migration.
   - Added comprehensive unit tests in `DepositAccountRateChartOverriddenTest`.
   
   ## Checklist
   - [x] Write the commit message as per our guidelines
   - [x] Acknowledge that we will not review PRs that are not passing the build ("green")
   - [x] Create/update unit or integration tests for verifying the changes made
   - [x] Follow our coding conventions
   - [x] Add required Swagger annotation and update API documentation
   - [x] This PR must not be a "code dump"
   - [ ] If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "Eyad-Sharkawy (via GitHub)" <gi...@apache.org>.
Eyad-Sharkawy commented on PR #6405:
URL: https://github.com/apache/fineract/pull/6405#issuecomment-5741888041

   I saw the announcement regarding FINERACT-2621, so I went ahead and rebased this PR to ensure full compatibility with the new Spring Boot 4.1 upgrade.


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6405:
URL: https://github.com/apache/fineract/pull/6405#issuecomment-5889078123

   > Hi @adamsaghy / @Aman-Mittal, I know the core team has been laser-focused on shipping the Working Capital epic recently! I just resolved a few minor Swagger conflicts to keep this branch perfectly up-to-date with develop. Since this is a larger domain feature ([FINERACT-848](https://issues.apache.org/jira/browse/FINERACT-848)), let me know if there is anything I can do to make the review process easier once the team has bandwidth to look at the general queue again.
   
   Sorry for being slow on PR review. I am facing an increased workload at the moment, but will try to review your PR ASAP.


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "Eyad-Sharkawy (via GitHub)" <gi...@apache.org>.
Eyad-Sharkawy commented on PR #6405:
URL: https://github.com/apache/fineract/pull/6405#issuecomment-6419531831

   Hi @galovics, I just reverted the IDE auto-formatting on DEPOSIT_ACCOUNT_REQUEST_DATA_PARAMETERS to restore the clean one-line diff (missed that on the last commit). I have also verified that the new parameter is correctly included across all response-data sets and templates. All of your review feedback has now been fully addressed, and the CI is completely green. Let me know if you need anything else to proceed with approval!


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "Eyad-Sharkawy (via GitHub)" <gi...@apache.org>.
Eyad-Sharkawy commented on PR #6405:
URL: https://github.com/apache/fineract/pull/6405#issuecomment-5559701632

   Hi @Aman-Mittal / reviewers, all 86 test and quality checks passed, but the regression-safety check was cancelled on the baseline step (Instance #1: The operation was canceled). Could someone with write permissions please re-run that failed job? Thank you


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6405:
URL: https://github.com/apache/fineract/pull/6405#issuecomment-5889140784

   @Eyad-Sharkawy Can you please review the below concerns or mine?
   
   **Gap?**
    The penalty keeps being subtracted from the stored rate
   
   To handle the override, the PR moved the premature-closure penalty outside the chart != null block. It still ends with this.nominalAnnualInterestRate = applicableInterestRate; (FixedDepositAccount.java:193, RecurringDepositAccount.java:245).
   
   - Before this PR: the rate was re-read from the chart on every call, so writing the penalised rate back did no harm.
   - With the override on: the stored rate is the only source of the rate, so each call lowers it by the penalty again.
   
   This happens in a real API flow:
   
   1. POST /fixeddepositaccounts/{id}?command=calculatePrematureAmount (the preview) runs DepositAccountPreMatureCalculationPlatformServiceImpl.calculatePreMatureAmount. That method has a plain read-write @Transactional.
   2. The call goes through calculatePreMatureInterest → calculateInterestPayable → getEffectiveInterestRateAsFraction(…, true). That changes the rate on a live database entity, and the change is saved when the transaction commits.
   3. Each preview lowers the stored rate by the penalty: with a 7.5% custom rate and a 1% penalty, it goes 7.5 → 6.5 → 5.5 and so on, until it reaches 0.
   4. The real prematureClose then subtracts the penalty once more from whatever is left, so the interest posted is lower than it should be.
   
   The RD endpoint works the same way. I traced this through the code but didn't run it. It's easy to demonstrate: calling the method from testFixedDepositEffectiveInterestRateWithOverriddenRateAndPrematureClosurePenalty a second time returns 0.055 instead of 0.065.
   
   **Suggested fix?**
   
   Don't write the penalised rate back to nominalAnnualInterestRate when the override is on. Alternatively, calculate the penalised rate in a local variable and leave the stored custom rate alone. -> Is this correct?
   
   **Change not mentioned in the description**
   
   The same move means FD accounts with no chart at all now get the premature-closure penalty. That happens when the product had no applicable chart (DepositAccountAssembler.java:308-313); before this PR, those accounts had no penalty. These accounts also get the drift bug above. This may be intended, but the description only talks about overridden accounts, so the author should either confirm it or keep the old behaviour.


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "Eyad-Sharkawy (via GitHub)" <gi...@apache.org>.
Eyad-Sharkawy commented on PR #6405:
URL: https://github.com/apache/fineract/pull/6405#issuecomment-5848280411

   Hi @adamsaghy / @Aman-Mittal, I know the core team has been laser-focused on shipping the Working Capital epic recently! I just resolved a few minor Swagger conflicts to keep this branch perfectly up-to-date with develop. Since this is a larger domain feature (FINERACT-848), let me know if there is anything I can do to make the review process easier once the team has bandwidth to look at the general queue again.


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "Eyad-Sharkawy (via GitHub)" <gi...@apache.org>.
Eyad-Sharkawy commented on PR #6405:
URL: https://github.com/apache/fineract/pull/6405#issuecomment-6430860551

   Hi @adamsaghy, thank you for the sharp review and for catching these edge cases! All points have been addressed (and I have updated the PR description above to document these domain decisions):
   
   **1. Reinvestment 0% Interest Rate:**
   * In both `FixedDepositAccount.reInvest` and `RecurringDepositAccount.reInvest`, the new account now inherits the custom interest rate when `isRateChartOverridden` is true (`interestRate = this.accountTermAndPreClosure.isRateChartOverridden() ? this.nominalAnnualInterestRate : BigDecimal.ZERO`), rather than defaulting to 0.
   * Added unit tests in `DepositAccountRateChartOverriddenTest` verifying that reinvested accounts retain their custom rate and calculate interest properly.
   
   **2. Premature Closure Penalty for Overridden Accounts:**
   * In both `FixedDepositAccount` and `RecurringDepositAccount`, the premature closure penalty is now applied to custom rates when `isPreMatureClosure && isPreClosurePenalApplicable()`.
   * The calculation is performed in a local variable and is not written back to `this.nominalAnnualInterestRate`, ensuring full idempotency and zero rate drift during previews (`calculatePrematureAmount`) or premature closure.
   * Updated unit test `testFixedDepositEffectiveInterestRateWithOverriddenRateAndPrematureClosurePenalty` to assert the penalized rate (e.g., 7.5% - 1.0% = 6.5%) and verify that multiple invocations do not mutate the stored contract rate.
   * Added equivalent unit test `testRecurringDepositEffectiveInterestRateWithOverriddenRateAndPrematureClosurePenalty`.
   
   **3. Integration Test Gaps:**
   * Added `testFixedDepositAccountWithRateChartOverriddenEndToEnd()` in `FixedDepositTest`, which keeps the override enabled through approval, activation, and interest calculation, verifying that interest is calculated at the custom 10% rate.
   * Added equivalent end-to-end integration test `testRecurringDepositAccountWithRateChartOverriddenEndToEnd()` in `RecurringDepositTest`.


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "Eyad-Sharkawy (via GitHub)" <gi...@apache.org>.
Eyad-Sharkawy commented on PR #6405:
URL: https://github.com/apache/fineract/pull/6405#issuecomment-5895110473

   Thank you @adamsaghy for catching this! You are completely right on both counts:
   
   #### Premature Closure Rate Drift
   When `isRateChartOverridden` is `true`, overwriting `this.nominalAnnualInterestRate` with `applicableInterestRate` after applying `applyPreMaturePenalty` caused each preview/calculation call (`calculatePrematureAmount`) to permanently mutate and lower the stored contract rate.
   
   #### Fix Applied
   - In both `FixedDepositAccount.java` and `RecurringDepositAccount.java`, the assignment `this.nominalAnnualInterestRate = applicableInterestRate;` has been moved strictly inside the `if (this.chart != null && !this.accountTermAndPreClosure.isRateChartOverridden())` block.
   - The premature penalty continues to be computed locally in `applicableInterestRate` and returned as the effective rate fraction without altering the persistent `nominalAnnualInterestRate` field.
   - For accounts without a chart where `preClosurePenalApplicable` is `true`, the penalty is now applied cleanly against the nominal rate as a local calculation without causing rate degradation across repeated calls.
   
   #### Unit Tests
   - Extended `DepositAccountRateChartOverriddenTest` to execute `getEffectiveInterestRateAsFraction(..., isPreMatureClosure=true)` multiple times sequentially, verifying that `nominalAnnualInterestRate` remains unaltered and the effective rate calculation is strictly idempotent.


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "Eyad-Sharkawy (via GitHub)" <gi...@apache.org>.
Eyad-Sharkawy commented on PR #6405:
URL: https://github.com/apache/fineract/pull/6405#issuecomment-6103730498

   Hi @adamsaghy, just a quick update: all 90 CI checks are now passing, and all previous review feedback has been addressed. Ready for your final review whenever you have a chance. Thank you!


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "Eyad-Sharkawy (via GitHub)" <gi...@apache.org>.
Eyad-Sharkawy commented on PR #6405:
URL: https://github.com/apache/fineract/pull/6405#issuecomment-6272525435

   Hi @galovics, thank you for the detailed and constructive review! I have addressed all points:
   
   1. **Idempotency & calculation drift**:
      - Restored the original calculation structure in [`FixedDepositAccount`](file:///c:/Users/eyadd/Documents/opensource/fineract/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/FixedDepositAccount.java) and [`RecurringDepositAccount`](file:///c:/Users/eyadd/Documents/opensource/fineract/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/RecurringDepositAccount.java) so chart lookups, premature closure penalties, and `this.nominalAnnualInterestRate` assignments only run inside `if (this.chart != null && !this.accountTermAndPreClosure.isRateChartOverridden())`. Overridden accounts return the custom nominal rate directly without entity mutation.
      - Added unit tests covering the non-overridden no-chart scenario with premature closure to verify rate idempotency.
   
   2. **Validation deduplication & behavior**:
      - Extracted shared helper method `validateRateChartOverride(...)` in [`DepositAccountDataValidator`](file:///c:/Users/eyadd/Documents/opensource/fineract/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/DepositAccountDataValidator.java).
      - Changed `.zeroOrPositiveAmount()` to `.positiveAmount()` to reject `0` upfront.
      - Allowed updates where `isRateChartOverridden = true` without re-submitting `nominalAnnualInterestRate` (falling back to existing rate).
   
   3. **Domain model & Assembler**:
      - Simplified reading of `isRateChartOverridden` using `command.booleanPrimitiveValueOfParameterNamed(isRateChartOverriddenParamName)`.
      - Changed `isRateChartOverridden` parameter in [`DepositAccountTermAndPreClosure`](file:///c:/Users/eyadd/Documents/opensource/fineract/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/DepositAccountTermAndPreClosure.java) constructor and `createNew(...)` factory to primitive `boolean`.
   
   4. **DepositsApiConstants & Swagger**:
      - Preserved original multiline formatting of `DEPOSIT_ACCOUNT_REQUEST_DATA_PARAMETERS` in [`DepositsApiConstants`](file:///c:/Users/eyadd/Documents/opensource/fineract/fineract-core/src/main/java/org/apache/fineract/portfolio/savings/DepositsApiConstants.java).
      - Added Swagger annotations for `isRateChartOverridden` and `nominalAnnualInterestRate` on PUT requests for both FD and RD resources.
   
   5. **Unit test refactoring**:
      - Refactored [`DepositAccountRateChartOverriddenTest`](file:///c:/Users/eyadd/Documents/opensource/fineract/fineract-provider/src/test/java/org/apache/fineract/portfolio/savings/domain/DepositAccountRateChartOverriddenTest.java) to eliminate repetitive account/term setup across tests.
      - Added tests covering toggling `isRateChartOverridden` back to false, no-chart premature closure idempotency, and zero-rate rejection.
   
   6. **Integration test**:
      - Added an end-to-end integration test `testFixedDepositAccountWithRateChartOverriddenLifecycle()` in [`FixedDepositTest`](file:///c:/Users/eyadd/Documents/opensource/fineract/integration-tests/src/test/java/org/apache/fineract/integrationtests/FixedDepositTest.java) exercising account creation with `isRateChartOverridden = true`, verification via GET, and updating to toggle the flag back to false.


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-848: Add ability to define custom interest rate in fixed and recurring deposit accounts [fineract]

Posted by "Eyad-Sharkawy (via GitHub)" <gi...@apache.org>.
Eyad-Sharkawy commented on code in PR #6405:
URL: https://github.com/apache/fineract/pull/6405#discussion_r4384023793


##########
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:
   Addressed in the latest commit: added `testFixedDepositAccountWithRateChartOverriddenEndToEnd()` in `FixedDepositTest` and `testRecurringDepositAccountWithRateChartOverriddenEndToEnd()` in `RecurringDepositTest`, which keep the override enabled through approval, activation, and interest calculation, asserting that interest is calculated at the custom 10% rate.



##########
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:
   Addressed in the latest commit: the premature closure penalty is now applied to custom rates in a local variable without writing it back to `this.nominalAnnualInterestRate`, ensuring full idempotency and zero rate drift during previews (`calculatePrematureAmount`) or premature closure. Updated unit tests for both FD and RD to assert the penalized rate and verify idempotency.



##########
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:
   Addressed in the latest commit: `reInvest()` in both `FixedDepositAccount` and `RecurringDepositAccount` now carries over the custom interest rate when `isRateChartOverridden` is true (`interestRate = this.accountTermAndPreClosure.isRateChartOverridden() ? this.nominalAnnualInterestRate : BigDecimal.ZERO`), preventing the 0% reinvestment. Added unit tests in `DepositAccountRateChartOverriddenTest` covering reinvestment for both FD and RD accounts.



-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org