abhinavjha0239 opened a new pull request, #6686: URL: https://github.com/apache/fineract/pull/6686
## Description Fixes [FINERACT-1850](https://issues.apache.org/jira/browse/FINERACT-1850). After a savings transaction is reversed with the reverse command, the next run of the Post Interest For Savings job brings it back: a reversed deposit earns interest and counts in the balance again, and the reversal is lost for good. The issue is still present on develop; the reproduction with the steps of the ticket is in the Jira comments. **Cause** - The job loads accounts through `SavingAccountMapperForInterestPosting`, which did not select `tr.is_reversal`. A reversal transaction (`is_reversal` true, `is_reversed` false) was loaded as a live transaction, so the job's existing `isReversalTransaction()` checks never applied. - When the job saves an account, `SavingsSchedularInterestPoster` writes `is_reversal` from the loaded data back to the transactions, so the reversal became a normal transaction permanently, also for the savings account itself. - `SavingsAccountTransactionDataSummaryWrapper.calculateTotalDeposits` counted reversal transactions, unlike `SavingsAccountTransactionSummaryWrapper` on the savings account side. **Change** - The mapper reads `tr.is_reversal`, and the factory method it uses passes it into `SavingsAccountTransactionData`. - `calculateTotalDeposits` skips reversal transactions, as `SavingsAccountTransactionSummaryWrapper` does. - `SavingsAccountOverdraftInterestSplitTest` passes the new argument (`false`). **Test** `testPostInterestJobKeepsReversedDepositReversed`: deposit 20013 on 01 March 2019 and 1000 on 10 March, reverse the 1000 on 12 March, run the job at business date 01 April. It checks the interest (186.9708: the reversed deposit earns nothing), the total deposits (20013), the balance, and that the reversal transaction is still a reversal. On develop it fails with `expected 186.9708 but was 193.600900`; with this change it passes. **Verification** Each scenario ran on PostgreSQL 18 with `savings-interest-posting-current-period-end` disabled and enabled. In every scenario a second, identical account got its interest from the `postInterest` command (the savings account path) as a reference. | | develop | this change | |---|---|---| | Reversal scenarios: job posts exactly what the savings account path posts | 0 of 14 runs | 14 of 14 runs | | 27 other job scenarios, 52 runs (manual postings, leap years, quarterly posting, pivot mode, configuration changes) | | identical to develop | - Reversal scenarios: a reversed deposit, withdrawal and manual interest posting; a deposit reversed after its interest was posted (the savings account path then also creates reversal transactions for the interest posting); the job run three times; an overdraft account; pivot mode. - Steps of FINERACT-1850 (overdraft, daily posting, job every day): the reversed deposit stays reversed. The balance on 09 September is -1229.59 (develop: -728.67, reference: -1222.39); the remaining difference is the repeated overdraft interest posting of FINERACT-2855. - 30 savings, deposit and job integration test classes (281 tests), each version on a fresh database with the CI profile: the only difference between develop and this change is the new test. One unrelated test fails on both (`loanDisbursementToSavingsWithAutoDownPaymentAndStandingInstructionsTest`). - `fineract-core` (416) and `fineract-savings` (69) unit tests, checkstyle, spotbugs and spotless pass. **Note** #6667 (FINERACT-2957) changes the same factory method and mapper call (it reads `tr.is_manual`). Whichever of the two is merged second needs a small rebase, which I will do. ## 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. - [x] 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]
