FINERACT-2463: Allow undo of Fixed Deposit transactions

[AshharAhmadKhan]

Description

Fixed Deposit accounts had an undo transaction endpoint that was silently broken in two ways. The API resource was calling undoSavingsAccountTransaction on the command builder, which set entityName = "SAVINGSACCOUNT" — meaning every undo request was routed to the regular savings handler, completely bypassing the Fixed Deposit command handler that already existed and was correctly wired. On top of that, undoFDTransaction in the service layer was an unconditional stub that always threw DepositAccountTransactionNotAllowedException, so even if routing had been correct, nothing would have worked.

This PR fixes both issues and makes undo transactions fully functional for Fixed Deposit accounts.

CommandWrapperBuilder.java — Added the missing undoFixedDepositAccountTransaction() builder method, correctly setting entityName = "FIXEDDEPOSITACCOUNT" and actionName = "UNDOTRANSACTION".

FixedDepositAccountTransactionsApiResource.java — Fixed the routing bug by calling the new FD-specific builder method instead of the savings one.

DepositAccountWritePlatformServiceJpaRepositoryImpl.java — Replaced the throw stub with a full implementation adapted from undoRDTransaction, with FD-specific adjustments: FixedDepositAccount cast, isTransactionsAllowed() status check for ACTIVE and MATURED states, 7-param postInterest signature, and updateMaturityDateAndAmount instead of RD-specific schedule calls.

FixedDepositAccountHelper.java — Added getFixedDepositTransactions() and undoFixedDepositTransaction() helper methods following the existing deprecated helper pattern.

FixedDepositTest.java — Added testFixedDepositAccountUndoTransaction covering the full flow: create client, open and activate FD account, post interest, undo the interest transaction, assert totalInterestPosted returns to zero.

Checklist

• Write the commit message as per our guidelines
• Create/update unit or integration tests for verifying the changes made
• Follow our coding conventions

[IOhacker]

LGTM

[adamsaghy]

@AshharAhmadKhan Please review the failing test cases!

[AshharAhmadKhan]

@adamsaghy @IOhacker

The failing test-core-5 checks have been fixed.

Root cause: After undoing an interest transaction, the DB column total_interest_posted_derived becomes 0. The DepositAccountReadPlatformServiceImpl intentionally uses getBigDecimalDefaultToNullIfZero for all summary fields, which converts 0 to null — this is consistent behaviour across the entire deposit reader. The test was calling .floatValue() directly on a potentially null Float, causing an NPE.

Fix: Added a null check in the test assertion so that a null totalInterestPosted (which semantically means zero) is treated as 0f, which is the correct expected value after an undo.

The service implementation, routing fix, and journal entry logic are all unchanged.

[AshharAhmadKhan]

@adamsaghy went through the changes again to see if I missed anything and I came out with notihng. Please let me know if there is anything else needed.

[adamsaghy]

Please use Fineract-client-feign for testing!

[adamsaghy]

Do you need any help how to rewrite this to use fineract-client-feign?

[adamsaghy]

Please see my concerns around testing.

[adamsaghy]

Please check whether transaction was reverted, balances are correct and journal entries are correct.

[AshharAhmadKhan]

@adamsaghy thanks for the review!

For the test I switched it to CASH_BASED accounting and added all three checks you mentioned — transaction marked as reversed, totalInterestPosted going back to zero, and the reversal journal entries (opposite signs on the same date).

For the feign point I had a look at the generated FixedDepositAccountApi and it only exposes handleCommands4 which hits the account level endpoint and takes a single accountId. The undo transaction endpoint needs both accountId and transactionId so there is no matching method in the generated client. I also noticed RecurringDepositAccountHelper.undoTransactionForRecurringDeposit follows the same raw HTTP pattern for the same reason, so this is consistent with the existing deposit helper approach. I have added a comment in the helper explaining the gap. Happy to extend the generated client if you would prefer that approach though.

[adamsaghy]

    Test testFixedDepositAccountUndoTransaction() FAILED
  
    org.opentest4j.AssertionFailedError: Journal Entry not found ==> expected: <true> but was: <false>
        at app//org.apache.fineract.integrationtests.FixedDepositTest.testFixedDepositAccountUndoTransaction(FixedDepositTest.java:3016)
  

[AshharAhmadKhan]

@adamsaghy the failure on 7bad2f6 was a date bug in the test, not in the service. When I computed INTEREST_POSTED_DATE I was using today's date, but Fineract actually books the interest posting at the last day of the posting period which is the end of the month containing the activation date. All the other FD tests in this file use that same end-of-activation-month pattern and I missed it. The fix in 259cc66 resets todaysDate to one month back and advances it to the end of that month before formatting the date, which is exactly what the rest of the test suite does. The journal entry assertions, the balance check and the reversed transaction check are all still there unchanged.

[AshharAhmadKhan]

@adamsaghy @IOhacker

Would either of you mind triggering the runs when you get a chance?
Happy to address anything that comes up. Thanks!

[adamsaghy]

Please use fineract-client-feign only in the integration tests.
Let me know if you need any help with that.

[AshharAhmadKhan]

@adamsaghy thanks for pushing on this, you were right.

I was looking at FixedDepositAccountApi and missed the separate FixedDepositAccountTransactionsApi which has adjustTransaction(accountId, transactionId, command, body) mapping directly to the undo endpoint. This commit wires it into FineractClient as fixedDepositAccountTransactions mirroring how recurringDepositAccountTransactions is already wired, and rewrites undoFixedDepositTransaction to use Calls.ok(getFineractClient().fixedDepositAccountTransactions.adjustTransaction(...)).

getFixedDepositTransactions stays as raw HTTP because GetFixedDepositAccountsAccountIdResponse has no transactions field in the generated model, so the typed client cannot be used there. Happy to hear if you see a better approach.

[adamsaghy]

@adamsaghy thanks for pushing on this, you were right.

I was looking at FixedDepositAccountApi and missed the separate FixedDepositAccountTransactionsApi which has adjustTransaction(accountId, transactionId, command, body) mapping directly to the undo endpoint. This commit wires it into FineractClient as fixedDepositAccountTransactions mirroring how recurringDepositAccountTransactions is already wired, and rewrites undoFixedDepositTransaction to use Calls.ok(getFineractClient().fixedDepositAccountTransactions.adjustTransaction(...)).

getFixedDepositTransactions stays as raw HTTP because GetFixedDepositAccountsAccountIdResponse has no transactions field in the generated model, so the typed client cannot be used there. Happy to hear if you see a better approach.

If a field is missing, we should add it to the swagger definition and it will be generated in the fineract-client automatically.

[AshharAhmadKhan]

@adamsaghy done — added transactions to the swagger definition for GetFixedDepositAccountsAccountIdResponse, added the associations query param to the retrieveOne endpoint so the generated client exposes it, regenerated the client, and rewrote getFixedDepositTransactions to use retrieveOne20(..., "transactions").getTransactions() via the typed client. No more raw HTTP anywhere in the test helpers for this flow.

[AshharAhmadKhan]

@adamsaghy just pushed a fix for the API backward compatibility failure.

The new GetFixedDepositAccountsTransactions inner class I added had accountNo typed as
String — swagger-brake caught this as a breaking change on
GET /v1/fixeddepositaccounts/{accountId} since the outer response class has it as Long.
Changed it to Long to match the existing pattern across all FD and RD swagger classes.