abhinavjha0239 opened a new pull request, #6667: URL: https://github.com/apache/fineract/pull/6667
## Description Fixes [FINERACT-2957](https://issues.apache.org/jira/browse/FINERACT-2957). When a savings account has a manual ("post interest as on") interest posting, the Post Interest For Savings job ignores it and posts the interest of the whole posting period, so the days before the manual posting are credited twice. This happens with `savings-interest-posting-current-period-end` disabled (the default) and enabled. **Cause** The job loads accounts through `SavingAccountMapperForInterestPosting`. Its query selects `tr.is_manual as manualTransaction`, but the mapper passed a hard-coded `false` instead (since FINERACT-1495), and the factory method it calls always set `isManualTransaction` to false. `SavingsAccountInterestPostingServiceImpl.getManualPostingDates` therefore never found a manual posting, and the job did not split the posting period at it. **Change** - The mapper reads `manualTransaction`, and the factory method passes it into `SavingsAccountTransactionData`. The mapper is the only caller of that overload. - With the manual dates loaded, `SavingsAccountInterestPostingServiceImpl.calculateInterestUsing` needs the same one day shift that FINERACT-752 added to `SavingsAccount.calculateInterestUsing`: with the configuration enabled, a manual posting is dated on its period end, not on the day after it. - The job is the only caller of `SavingsAccountInterestPostingService`, so the savings account path is not affected. **Test** `testPostInterestJobDoesNotPostManuallyPostedInterestAgain` follows the steps of the Jira issue with the configuration disabled and enabled: deposit 20013 on 01 March 2019, post interest as on 25 March at business date 25 March, run the job at business date 01 April. It checks both postings: 144.7516 for 1 to 24 March and 42.2192 for 25 to 31 March. On develop both cases fail with `expected 42.2192 but was 186.970800` (all of March); with this change they pass. **Verification** I compared develop and this change in 23 API scenarios on PostgreSQL 18, each with the configuration disabled and enabled. In every scenario a second, identical account got its interest from the `postInterest` command (the savings account path) instead of the job, as a reference. | | develop | this change | |---|---|---| | Job posts exactly what the savings account path posts | 8 of 44 runs | 44 of 44 runs | The scenarios cover: as on the first, a middle and the last day of a month; two manual postings in one period; quarterly posting; daily compounding; a deposit, a withdrawal or a backdated deposit around the manual posting; an overdraft-enabled product; the job run three times (no reversals); the job a month later; 28/29 February in leap and non-leap years; a year end; recent dates; an undone manual posting; and the configuration changed between the manual posting and the job. - Accounts without manual postings, and all pivot mode scenarios (`allow-backdated-transaction-before-interest-posting` disabled), give identical results on develop and with this change. In pivot mode the job drops the last interest posting before calculating, so it never sees a manual posting. - 30 savings, deposit and job integration test classes (282 tests), each version on a fresh database with the CI profile: the only differences between develop and this change are the two new test cases. One unrelated test fails on both (`loanDisbursementToSavingsWithAutoDownPaymentAndStandingInstructionsTest`). - `fineract-core` and `fineract-savings` unit tests, checkstyle, spotbugs and spotless pass. **Note** A manual posting reversed with the reverse command (not undo) is still not handled correctly by the job, on develop and with this change, because the job does not load `is_reversal` at all. That affects every reversed transaction, not only manual postings, so I will raise it separately. ## Checklist - [x] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests) - [x] 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. - [x] Create/update [unit or integration tests](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made. - [x] Follow our [coding conventions](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 (not applicable, no API change) - [x] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute). - [x] If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately. - [ ] I followed the [AI Policy](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#ai-policy). -- 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]
