From f457b9b8c7fc8ba6f79ff4fc89218f9ec6b975ba Mon Sep 17 00:00:00 2001 From: Nitesh Yadav Date: Wed, 23 Sep 2026 14:21:13 +0530 Subject: [PATCH] fix(cashier): route FD/RD cashier deposits through maker-checker approval SYMPTOM A cashier could credit an FD/RD account with cash and the money moved in instantly, with no Manager approval step - unlike a Savings/Current deposit from the same screen, which correctly creates a PENDING m_cashier_deposit_request. Discovered by tracing why a real FD funding deposit never showed up in Branch Ops -> Approvals: it had posted directly, no request record ever existed for it. ROOT CAUSE (two layers, both fixed here / in the paired frontend commit) 1. Frontend gate (transaction-entry.component.ts confirmSubmit(), see frontend commit): the maker-checker branch for a cashier deposit only matched row.acTyp in ('SD','CA','CB','GL') - 'FD' and 'RD' were never in that list, so a deposit into either fell through every other branch to the generic, unguarded direct POST at the bottom of confirmSubmit(). 2. Even with the frontend gated correctly, approve() here only knew how to tell Current apart from "everything else -> plain savings deposit command" (isCurrentAccountById()). FD/RD accounts share the m_savings_account table with plain Savings (distinguished only by deposit_type_enum 200/300 vs 100), so an FD/RD deposit request would have been posted through the generic savingsAccountDeposit command, which does not accept a term-deposit account id. FIX - resolveDepositAccountCategory(accountId): looks up the real category fresh from the DB (m_current_account / m_savings_account. deposit_type_enum) instead of trusting the request's own account_type string - same defensive pattern this class already used for Current. - New executeFixedDepositDeposit() / executeRecurringDepositDeposit(): post through Fineract's actual FD/RD deposit commands (CommandWrapperBuilder#fixedDepositAccountDeposit / #recurringAccountDeposit, hitting /fixeddepositaccounts/{id}/ transactions and /recurringdepositaccounts/{id}/transactions) instead of the generic savings command. - approve() now branches on all four categories (Current / Fixed Deposit / Recurring Deposit / Savings) instead of two. RELATED DATA CLEANUP DONE THIS SESSION (not in this commit) The one FD deposit made before this fix existed left an orphaned m_savings_txn_denomination row (physically "counted" cash with no approved request backing it), which inflated the Branch Day Register's "Vault + Cashiers" total above the approved-requests-only "book" total by exactly that amount every day going forward. That artifact row was deleted directly (not backfilled with a fabricated approval - the Claude Code safety layer correctly refused that approach as audit-log tampering). No code in this repo does that cleanup automatically. IF THIS BREAKS SOMETHING LATER - A cashier FD/RD deposit posting instantly again -> check the frontend gate list in confirmSubmit() (paired commit) still includes 'FD'/'RD'. - approve() throwing on an FD/RD deposit request -> check resolveDepositAccountCategory() is returning FIXED_DEPOSIT/ RECURRING_DEPOSIT (query m_savings_account.deposit_type_enum for the account id in question: 200 = FD, 300 = RD). - Branch Day Register showing a mysterious Vault-vs-book Excess again -> check for an m_savings_txn_denomination row with no matching m_cashier_deposit_request; that combination should be impossible now that this fix is in place, so a new one appearing means this fix (or its frontend half) regressed. Co-Authored-By: Claude Sonnet 5 --- .../api/CashierDepositRequestApiResource.java | 103 +++++++++++++++++- 1 file changed, 97 insertions(+), 6 deletions(-) diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashierdeposit/api/CashierDepositRequestApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashierdeposit/api/CashierDepositRequestApiResource.java index 7378801ec..25b268f90 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashierdeposit/api/CashierDepositRequestApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashierdeposit/api/CashierDepositRequestApiResource.java @@ -418,13 +418,20 @@ public class CashierDepositRequestApiResource { txnId = executeGlJournalEntry(glAccountId, contraGlId, glOfficeId, currencyCode, amount, ptId, txnDateStr, description, true); } else { - // Determine real account type by checking which table owns this ID. + // Determine real account type by checking which table/column owns this ID. // Do NOT rely solely on the stored account_type field — old records may have 'SAVINGS' - // even for current accounts (submitted before the frontend fix). - final boolean isCurrentAccount = isCurrentAccountById(savingsAccountId); - txnId = isCurrentAccount - ? executeCurrentAccountDeposit(savingsAccountId, amount, ptId, txnDateStr, description) - : executeSavingsDeposit(savingsAccountId, amount, ptId, txnDateStr, description); + // even for current accounts (submitted before the frontend fix), and FD/RD accounts share + // the very same m_savings_account table as a plain savings account, distinguished only by + // deposit_type_enum -- so this is looked up fresh rather than trusted from the request. + final String depositCategory = resolveDepositAccountCategory(savingsAccountId); + final boolean isCurrentAccount = "CURRENT".equals(depositCategory); + txnId = switch (depositCategory) { + case "CURRENT" -> executeCurrentAccountDeposit(savingsAccountId, amount, ptId, txnDateStr, description); + case "FIXED_DEPOSIT" -> executeFixedDepositDeposit(savingsAccountId, amount, ptId, txnDateStr, description); + case "RECURRING_DEPOSIT" -> + executeRecurringDepositDeposit(savingsAccountId, amount, ptId, txnDateStr, description); + default -> executeSavingsDeposit(savingsAccountId, amount, ptId, txnDateStr, description); + }; if (!isCurrentAccount && txnId != null) { // approve() is one @Transactional block, so an uncaught exception here would roll back // the deposit that was just posted along with it -- every Cheque/DD approval that ever @@ -718,6 +725,64 @@ public class CashierDepositRequestApiResource { return result.getResourceId() != null ? result.getResourceId().toString() : null; } + private String executeFixedDepositDeposit( + final Long accountId, final BigDecimal amount, + final Object paymentTypeId, final String txnDateStr, final String note) { + + final String today = LocalDate.now(DateUtils.getDateTimeZoneOfTenant()).format(DateTimeFormatter.ofPattern("dd MMMM yyyy")); + final String date = txnDateStr != null && !txnDateStr.isBlank() ? formatForFineract(txnDateStr) : today; + + final JsonObject payload = new JsonObject(); + payload.addProperty("locale", "en"); + payload.addProperty("dateFormat", "dd MMMM yyyy"); + payload.addProperty("transactionDate", date); + payload.addProperty("transactionAmount", amount); + if (paymentTypeId != null) { + payload.addProperty("paymentTypeId", ((Number) paymentTypeId).longValue()); + } + if (note != null && !note.isBlank()) { + payload.addProperty("note", note); + } + + final CommandWrapper commandRequest = new CommandWrapperBuilder() + .fixedDepositAccountDeposit(accountId) + .withJson(payload.toString()) + .build(); + + final CommandProcessingResult result = voucherService + .withOwnVoucher(() -> commandsSourceWritePlatformService.logCommandSource(commandRequest)); + return result.getResourceId() != null ? result.getResourceId().toString() : null; + } + + private String executeRecurringDepositDeposit( + final Long accountId, final BigDecimal amount, + final Object paymentTypeId, final String txnDateStr, final String note) { + + final String today = LocalDate.now(DateUtils.getDateTimeZoneOfTenant()).format(DateTimeFormatter.ofPattern("dd MMMM yyyy")); + final String date = txnDateStr != null && !txnDateStr.isBlank() ? formatForFineract(txnDateStr) : today; + + final JsonObject payload = new JsonObject(); + payload.addProperty("locale", "en"); + payload.addProperty("dateFormat", "dd MMMM yyyy"); + payload.addProperty("transactionDate", date); + payload.addProperty("transactionAmount", amount); + if (paymentTypeId != null) { + payload.addProperty("paymentTypeId", ((Number) paymentTypeId).longValue()); + } + if (note != null && !note.isBlank()) { + payload.addProperty("note", note); + } + + final CommandWrapper commandRequest = new CommandWrapperBuilder() + .recurringAccountDeposit(accountId) + .withJson(payload.toString()) + .build(); + + final CommandProcessingResult result = voucherService + .withOwnVoucher(() -> commandsSourceWritePlatformService.logCommandSource(commandRequest)); + return result.getResourceId() != null ? result.getResourceId().toString() : null; + } + /** * A Cheque/DD deposit is credited to the ledger immediately (same as cash) but is not really the * branch's own money yet -- it still has to clear. Real-world CBS practice is to hold that amount @@ -809,6 +874,32 @@ public class CashierDepositRequestApiResource { } } + /** + * "CURRENT", "FIXED_DEPOSIT", "RECURRING_DEPOSIT" or "SAVINGS" — checked against the database + * rather than the request's own account_type string. FD and RD accounts live in the same + * m_savings_account table as a plain savings account (deposit_type_enum 200/300 vs 100), so a + * plain "not current → savings" check would post an FD/RD deposit through the generic savings + * deposit command, which does not accept a term-deposit account id. + */ + private String resolveDepositAccountCategory(final Long accountId) { + if (accountId == null) { + return "SAVINGS"; + } + if (isCurrentAccountById(accountId)) { + return "CURRENT"; + } + final List depositTypes = jdbcTemplate.queryForList( + "SELECT deposit_type_enum FROM m_savings_account WHERE id = ?", Integer.class, accountId); + final Integer depositType = depositTypes.isEmpty() ? null : depositTypes.get(0); + if (Integer.valueOf(200).equals(depositType)) { + return "FIXED_DEPOSIT"; + } + if (Integer.valueOf(300).equals(depositType)) { + return "RECURRING_DEPOSIT"; + } + return "SAVINGS"; + } + // Parses a Fineract transaction id string to Long. // Savings/current txn ids are decimal; GL journal entry ids are hex strings. private Long parseTxnIdToLong(final String txnId) {