From 6ffb0dd9c0979d23906b4caabe9021948cea2ebb Mon Sep 17 00:00:00 2001 From: Nitesh Yadav Date: Sun, 30 Aug 2026 10:37:53 +0530 Subject: [PATCH] fix(savings,cashier): auto-post pro-rata interest on closure, split Cash/Clearing hold, fix WITHDRAWN-status blind spots Closing a savings account used to only ever validate that interest had already been posted on the closing date (opt-in, off by default) -- with nothing posted beforehand the partial period's interest was simply never paid. It now always calculates and posts it before the account closes, reusing the same machinery the manual "post interest as of date" action already relied on. Savings accounts now track two separate hold totals instead of one: a manual/staff-placed Cash Hold (the existing lien mechanism, unchanged for every caller that doesn't ask for the new category) and a Clearing Hold for money still moving through the clearing process -- a Cheque/DD cashier deposit now automatically places one for the full amount at approval time, and CTS inward-clearing holds are now bucketed here too, since both are genuinely about the clearing pipeline rather than a lien someone chose to place. A new self-contained endpoint releases due Clearing Holds (wire it into an external scheduler); the existing manual release action now credits back whichever bucket the original hold actually debited. Approving a Cheque/DD deposit dispatches this automatically as the approving manager, so the checker roles that call deposit-approval needed the underlying HOLDAMOUNT/RELEASEAMOUNT permissions granted -- without it every such approval would now fail outright; a stray column-name bug in the payment- type lookup that surfaced this in testing is fixed too, guarded going forward by a try/catch so a problem in this best-effort step can never sink the deposit approval it rides along with. A clerk withdrawal's own terminal status is WITHDRAWN, not APPROVED -- deposits and disbursements use APPROVED, withdrawal is the one register that doesn't. Three places were still filtering on APPROVED alone and silently excluding every completed withdrawal: the Day Book's CR/DR totals, and the branch day register's own "cash the branch should be holding" formula (which overstated the expected figure by exactly what had been paid out, manufacturing a phantom cash-short every time a withdrawal happened that day). The GL Entries report showed a bare internal transaction id under "Voucher No" instead of the real formatted voucher stamped on the underlying deposit/withdrawal request; it now resolves and shows that instead. Denomination rows saved at deposit/withdrawal submission never recorded which payment type was used, always leaving "Payment Type" blank regardless of what the cashier actually collected -- both insert paths now carry it through, and existing rows are backfilled. FacilityChargeSeederService no longer hardcodes an income GL account onto the facility charges (ATM, UPI, Cheque Book, NEFT, ...) it seeds. Fineract's accounting engine uses a charge's own GL account ahead of its savings product's own "Income From Fees" mapping whenever one is set, so seeding all sixteen onto the same shared account (picked essentially by accident) silently overrode every product's own accounting configuration with it. Leaving it unset lets each product's own mapping decide where this income actually posts, which is the point of that mapping existing at all. Institution/Branch Profile's READ permission was scoped to manager/admin roles only, since it started as an org-setup-only screen -- now that the Passbook and Statement screens read it for their letterhead (real bank name, license number, branch IFSC/MICR) instead of a hardcoded name, and both are reachable by a cashier/clerk/teller, the read-only permission is extended to those roles too. Co-Authored-By: Claude Sonnet 5 --- .../savings/SavingsApiConstants.java | 3 + .../api/BranchDayRegisterApiResource.java | 6 +- .../api/CashierDepositRequestApiResource.java | 73 ++++++++++++++++-- .../cashreport/api/CashReportApiResource.java | 20 +++-- .../ClerkWithdrawalRequestApiResource.java | 7 +- .../CtsClearingWritePlatformServiceImpl.java | 4 +- .../InwardClearingValidationService.java | 5 +- ...avingsAccountTransactionDataValidator.java | 3 +- ...WritePlatformServiceJpaRepositoryImpl.java | 48 ++++++++---- ...SavingsClearingHoldReleaseApiResource.java | 77 +++++++++++++++++++ .../api/SavingsHoldBreakdownApiResource.java | 49 ++++++++++++ .../tenant/final-changelog-tenant.xml | 3 + .../0516_savings_account_clearing_hold.xml | 35 +++++++++ ...rant_checker_roles_hold_release_amount.xml | 31 ++++++++ ..._roles_read_institution_branch_profile.xml | 30 ++++++++ .../savings/domain/SavingsAccount.java | 42 +++++++++- .../domain/SavingsAccountTransaction.java | 14 ++++ .../service/FacilityChargeSeederService.java | 38 +++------ 18 files changed, 424 insertions(+), 64 deletions(-) create mode 100644 fineract-provider/src/main/java/org/apache/fineract/portfolio/savingshold/api/SavingsClearingHoldReleaseApiResource.java create mode 100644 fineract-provider/src/main/java/org/apache/fineract/portfolio/savingshold/api/SavingsHoldBreakdownApiResource.java create mode 100644 fineract-provider/src/main/resources/db/changelog/tenant/parts/0516_savings_account_clearing_hold.xml create mode 100644 fineract-provider/src/main/resources/db/changelog/tenant/parts/0517_grant_checker_roles_hold_release_amount.xml create mode 100644 fineract-provider/src/main/resources/db/changelog/tenant/parts/0518_grant_teller_roles_read_institution_branch_profile.xml diff --git a/fineract-core/src/main/java/org/apache/fineract/portfolio/savings/SavingsApiConstants.java b/fineract-core/src/main/java/org/apache/fineract/portfolio/savings/SavingsApiConstants.java index 8b6a61f18..8197a2477 100644 --- a/fineract-core/src/main/java/org/apache/fineract/portfolio/savings/SavingsApiConstants.java +++ b/fineract-core/src/main/java/org/apache/fineract/portfolio/savings/SavingsApiConstants.java @@ -92,6 +92,9 @@ public class SavingsApiConstants { public static final String statusParamName = "status"; public static final String subStatusParamName = "subStatus"; public static final String reasonForBlockParamName = "reasonForBlock"; + /** "CASH" (default, a manual/staff-placed lien) or "CLEARING" (money still moving through the + * clearing process — a not-yet-realized cheque/DD deposit, or a presented cheque not yet settled). */ + public static final String holdCategoryParamName = "holdCategory"; public static final String clientIdParamName = "clientId"; public static final String isRetailAccountParamName = "isRetailAccount"; public static final String autogenerateTransactionIdParamName = "autogenerateTransactionId"; diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/branchday/api/BranchDayRegisterApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/branchday/api/BranchDayRegisterApiResource.java index 29977ff75..3fc675902 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/branchday/api/BranchDayRegisterApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/branchday/api/BranchDayRegisterApiResource.java @@ -598,9 +598,13 @@ public class BranchDayRegisterApiResource { AND (office_id = ? OR office_id IS NULL) """, BigDecimal.class, date, officeId)); + // A clerk withdrawal's own terminal status is WITHDRAWN, not APPROVED (deposits and + // disbursements use APPROVED; withdrawal is the one register that doesn't) -- filtering on + // APPROVED alone silently excluded every completed withdrawal from this total, overstating + // the branch's expected cash by exactly what had actually been paid out that day. final BigDecimal withdrawals = zero(jdbcTemplate.queryForObject(""" SELECT COALESCE(SUM(amount), 0) FROM m_clerk_withdrawal_request - WHERE status = 'APPROVED' + WHERE status = 'WITHDRAWN' AND COALESCE(txn_date, CAST(submitted_at AS DATE)) = ? """, BigDecimal.class, date)); 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 2474b9c70..17115fd52 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 @@ -67,6 +67,7 @@ import com.google.common.base.Splitter; @Component @Tag(name = "Cashier Deposit Requests", description = "Maker-checker deposit request workflow: cashier submits, bank manager approves.") @RequiredArgsConstructor +@lombok.extern.slf4j.Slf4j public class CashierDepositRequestApiResource { private final JdbcTemplate jdbcTemplate; @@ -161,7 +162,7 @@ public class CashierDepositRequestApiResource { // reject() will delete these records so the vault position is reversed. if (denominationData != null && !denominationData.isBlank()) { savePendingDenomination(denominationData, newId, savingsAccountId, accountNo, clientId, - currentUser.getId(), currentUser.getUsername()); + currentUser.getId(), currentUser.getUsername(), paymentTypeName); } return gson.toJson(Map.of("resourceId", newId, "status", "PENDING", "voucherNo", voucherNo)); @@ -394,6 +395,20 @@ public class CashierDepositRequestApiResource { txnId = isCurrentAccount ? executeCurrentAccountDeposit(savingsAccountId, amount, ptId, txnDateStr, description) : 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 + // hit an edge case in the new hold-placement logic would fail entirely, where before + // this feature existed it always succeeded. Catching it keeps the Clearing Hold strictly + // best-effort: the deposit approval this whole endpoint exists for must never depend on + // it, only benefit from it when it works. + try { + placeClearingHoldIfInstrumentDeposit(savingsAccountId, ptId, amount, txnDateStr); + } catch (final RuntimeException e) { + log.warn("Could not place a Clearing Hold for cashier deposit request {} (savings account {}): {}", id, + savingsAccountId, e.getMessage(), e); + } + } } // Link voucher to the created savings txn and stamp voucher_no on it. @@ -586,6 +601,54 @@ public class CashierDepositRequestApiResource { 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 + * until the instrument clears rather than let it sit withdrawable, so this places an automatic + * Clearing Hold for the full deposited amount right after a Cheque/DD deposit posts. Cash and + * electronic-transfer payment types (NEFT/RTGS/IMPS/UPI) are untouched -- cash is available on the + * spot, and an electronic transfer has already settled by the time it's entered here. + */ + private void placeClearingHoldIfInstrumentDeposit(final Long savingsAccountId, final Object paymentTypeId, final BigDecimal amount, + final String txnDateStr) { + if (paymentTypeId == null) { + return; + } + final Long ptId = ((Number) paymentTypeId).longValue(); + // m_payment_type's display name column is "value", not "name" (the mistake that broke this + // in testing) -- name() collisions with other tables made this easy to get wrong once and not + // notice until it actually ran against the real schema. + final List names = jdbcTemplate.query("SELECT value FROM m_payment_type WHERE id = ?", + (rs, rowNum) -> rs.getString("value"), ptId); + if (names.isEmpty()) { + return; + } + final String name = names.get(0).toLowerCase(); + final boolean isInstrumentDeposit = name.contains("cheque") || name.contains("check") || name.equals("dd") + || name.contains("demand draft"); + if (!isInstrumentDeposit) { + return; + } + + 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); + payload.addProperty("reasonForBlock", "Pending clearing — " + names.get(0) + " deposit"); + payload.addProperty("holdCategory", "CLEARING"); + payload.addProperty("lienAllowed", false); + + final CommandWrapper commandRequest = new CommandWrapperBuilder() + .holdAmount(savingsAccountId) + .withJson(payload.toString()) + .build(); + commandsSourceWritePlatformService.logCommandSource(commandRequest); + } + private String executeCurrentAccountDeposit( final Long currentAccountId, final BigDecimal amount, final Object paymentTypeId, final String txnDateStr, final String note) { @@ -699,7 +762,7 @@ public class CashierDepositRequestApiResource { // approve() updates txn_id to real Fineract id; reject() deletes these records. private void savePendingDenomination(final String denominationData, final Long requestId, final Long savingsAccountId, final String accountNo, - final Long clientId, final Long cashierId, final String cashierUsername) { + final Long clientId, final Long cashierId, final String cashierUsername, final String paymentTypeName) { final JsonObject denomMap; try { denomMap = gson.fromJson(denominationData, JsonObject.class); @@ -729,12 +792,12 @@ public class CashierDepositRequestApiResource { jdbcTemplate.update(""" INSERT INTO m_savings_txn_denomination (txn_id, savings_account_id, account_no, client_id, denomination_id, - denomination_value, count, total_amount, txn_date, txn_type, officer_id, officer_username) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, CURRENT_DATE, 'DEPOSIT', ?, ?) + denomination_value, count, total_amount, txn_date, txn_type, officer_id, officer_username, payment_type) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, CURRENT_DATE, 'DEPOSIT', ?, ?, ?) """, requestId, savingsAccountId, accountNo, clientId, denomId, value, count, BigDecimal.valueOf((long) value * count), - cashierId, cashierUsername + cashierId, cashierUsername, paymentTypeName ); } } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashreport/api/CashReportApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashreport/api/CashReportApiResource.java index ec99ad3c3..a277822a0 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashreport/api/CashReportApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashreport/api/CashReportApiResource.java @@ -126,8 +126,13 @@ public class CashReportApiResource { filtered.add(row); // Only approved money is real money: a pending or rejected voucher must not inflate the - // day's totals, even though it still belongs in the listing. - if ("APPROVED".equalsIgnoreCase((String) row.get("status")) || "COMPLETED".equalsIgnoreCase((String) row.get("status"))) { + // day's totals, even though it still belongs in the listing. The three source registers + // don't share one terminal status string: deposits and disbursements land on APPROVED, + // but a clerk withdrawal's own finished state is WITHDRAWN -- missing it here meant a + // completed withdrawal moved real money out yet never showed up in the DR total at all. + final String rowStatus = (String) row.get("status"); + if ("APPROVED".equalsIgnoreCase(rowStatus) || "COMPLETED".equalsIgnoreCase(rowStatus) + || "WITHDRAWN".equalsIgnoreCase(rowStatus)) { final BigDecimal amount = (BigDecimal) row.get("amount"); if (amount != null) { if ("CR".equals(row.get("side"))) { @@ -894,9 +899,14 @@ public class CashReportApiResource { baseParams = new Object[] { txnDate }; } - // Base grouped query (reused for count, totals, and paged data) + // Base grouped query (reused for count, totals, and paged data). The two LEFT JOINs resolve + // the real, formatted voucher number (e.g. "CR/001/2908/0001") stamped on the deposit/ + // withdrawal request at approval time -- std.txn_id on its own is only the internal savings + // transaction id (e.g. "4"), never what was actually printed on the voucher. final String baseGroupSql = "FROM m_savings_txn_denomination std " + "LEFT JOIN m_client mc ON mc.id = std.client_id " + + "LEFT JOIN m_cashier_deposit_request cdr ON cdr.txn_id = CAST(std.txn_id AS VARCHAR) " + + "LEFT JOIN m_clerk_withdrawal_request cwr ON cwr.txn_id = CAST(std.txn_id AS VARCHAR) " + "WHERE std.txn_date = CAST(? AS DATE) " + officerFilter + " GROUP BY std.txn_id, std.account_no, mc.display_name, std.cif_no "; @@ -917,7 +927,7 @@ public class CashReportApiResource { // 2. Paged data query final String dataSql = "SELECT " - + " std.txn_id AS voucher_no, " + + " COALESCE(MAX(cdr.voucher_no), MAX(cwr.voucher_no), CAST(std.txn_id AS VARCHAR)) AS voucher_no, " + " std.account_no AS ac_no, " + " COALESCE(mc.display_name, std.cif_no, '') AS ac_name, " + " MAX(std.txn_date) AS txn_date, " @@ -939,7 +949,7 @@ public class CashReportApiResource { final List> pageItems = jdbcTemplate.query(dataSql, (rs, rowNum) -> { final Map row = new LinkedHashMap<>(); - row.put("voucherNo", rs.getLong("voucher_no")); + row.put("voucherNo", rs.getString("voucher_no")); row.put("acNo", rs.getString("ac_no")); row.put("acName", rs.getString("ac_name")); row.put("txnDate", rs.getString("txn_date")); diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkWithdrawalRequestApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkWithdrawalRequestApiResource.java index 736480cb9..f490750f3 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkWithdrawalRequestApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkWithdrawalRequestApiResource.java @@ -663,6 +663,7 @@ public class ClerkWithdrawalRequestApiResource { final String accountNo = (String) record.get("account_no"); final Object clientIdObj = record.get("client_id"); final Long clientId = clientIdObj != null ? Long.valueOf(clientIdObj.toString()) : null; + final String paymentTypeName = (String) record.get("payment_type_name"); for (final Map.Entry entry : denomMap.entrySet()) { final int count; @@ -686,12 +687,12 @@ public class ClerkWithdrawalRequestApiResource { jdbcTemplate.update(""" INSERT INTO m_savings_txn_denomination (txn_id, savings_account_id, account_no, client_id, denomination_id, - denomination_value, count, total_amount, txn_date, txn_type, officer_id, officer_username) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, CURRENT_DATE, 'WITHDRAWAL', ?, ?) + denomination_value, count, total_amount, txn_date, txn_type, officer_id, officer_username, payment_type) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, CURRENT_DATE, 'WITHDRAWAL', ?, ?, ?) """, txnId, savingsAccountId, accountNo, clientId, denomId, value, count, BigDecimal.valueOf((long) value * count), - cashier.getId(), cashier.getUsername() + cashier.getId(), cashier.getUsername(), paymentTypeName ); } } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/ctsclearing/service/CtsClearingWritePlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/ctsclearing/service/CtsClearingWritePlatformServiceImpl.java index 1dec7cda0..d54ba01c4 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/ctsclearing/service/CtsClearingWritePlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/ctsclearing/service/CtsClearingWritePlatformServiceImpl.java @@ -738,7 +738,7 @@ public class CtsClearingWritePlatformServiceImpl implements CtsClearingWritePlat final SavingsAccount account = this.savingsAccountRepositoryWrapper.findOneWithNotFoundDetection(item.getAccountId()); account.setHelpers(this.savingsAccountTransactionSummaryWrapper, this.savingsHelper); if (item.isHoldPlaced() && item.getHoldAmount() != null) { - account.releaseOnHoldAmount(item.getHoldAmount()); + account.releaseOnHoldAmount(item.getHoldAmount(), SavingsAccount.HOLD_CATEGORY_CLEARING); } final Office office = account.office(); final Money amountMoney = Money.of(account.getCurrency(), amount); @@ -770,7 +770,7 @@ public class CtsClearingWritePlatformServiceImpl implements CtsClearingWritePlat } if (isSavings(item)) { final SavingsAccount account = this.savingsAccountRepositoryWrapper.findOneWithNotFoundDetection(item.getAccountId()); - account.releaseOnHoldAmount(item.getHoldAmount()); + account.releaseOnHoldAmount(item.getHoldAmount(), SavingsAccount.HOLD_CATEGORY_CLEARING); this.savingsAccountRepositoryWrapper.saveAndFlush(account); } else { final CurrentAccount account = this.currentAccountRepositoryWrapper.findOneWithNotFoundDetection(item.getAccountId()); diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/ctsclearing/service/InwardClearingValidationService.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/ctsclearing/service/InwardClearingValidationService.java index 032aeeb15..098a536c9 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/ctsclearing/service/InwardClearingValidationService.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/ctsclearing/service/InwardClearingValidationService.java @@ -284,7 +284,10 @@ public class InwardClearingValidationService { if (account == null) { return; } - account.holdAmount(amount); + // A presented-but-unsettled cheque is exactly what "Clearing Hold" means -- money in the + // clearing pipeline, not a staff-placed lien -- so it shares the bucket with a pending + // cheque/DD deposit rather than the generic manual Cash Hold. + account.holdAmount(amount, SavingsAccount.HOLD_CATEGORY_CLEARING); this.savingsAccountRepository.saveAndFlush(account); } else { final CurrentAccount account = this.currentAccountRepository.findById(item.getAccountId()).orElse(null); diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountTransactionDataValidator.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountTransactionDataValidator.java index a53e5eceb..cc5e5542f 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountTransactionDataValidator.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountTransactionDataValidator.java @@ -69,7 +69,8 @@ public class SavingsAccountTransactionDataValidator { private final FromJsonHelper fromApiJsonHelper; private static final Set SAVINGS_ACCOUNT_HOLD_AMOUNT_REQUEST_DATA_PARAMETERS = new HashSet<>( Arrays.asList(transactionDateParamName, SavingsApiConstants.dateFormatParamName, SavingsApiConstants.localeParamName, - transactionAmountParamName, lienAllowedParamName, SavingsApiConstants.reasonForBlockParamName)); + transactionAmountParamName, lienAllowedParamName, SavingsApiConstants.reasonForBlockParamName, + SavingsApiConstants.holdCategoryParamName)); private final ConfigurationDomainService configurationDomainService; public void validateTransactionWithPivotDate(final LocalDate transactionDate, final SavingsAccount savingsAccount) { diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java index 10573774a..5ea7fe588 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java @@ -120,7 +120,6 @@ import org.apache.fineract.portfolio.savings.domain.SavingsAccountTransaction; import org.apache.fineract.portfolio.savings.domain.SavingsAccountTransactionRepository; import org.apache.fineract.portfolio.savings.exception.PostInterestAsOnDateException; import org.apache.fineract.portfolio.savings.exception.PostInterestAsOnDateException.PostInterestAsOnExceptionType; -import org.apache.fineract.portfolio.savings.exception.PostInterestClosingDateException; import org.apache.fineract.portfolio.savings.exception.SavingsAccountClosingNotAllowedException; import org.apache.fineract.portfolio.savings.exception.SavingsAccountTransactionNotFoundException; import org.apache.fineract.portfolio.savings.exception.SavingsOfficerAssignmentException; @@ -955,22 +954,26 @@ public class SavingsAccountWritePlatformServiceJpaRepositoryImpl implements Savi final Locale locale = command.extractLocale(); final DateTimeFormatter fmt = DateTimeFormatter.ofPattern(command.dateFormat()).withLocale(locale); final LocalDate closedDate = command.localDateValueOfParameterNamed(SavingsApiConstants.closedOnDateParamName); - final boolean isPostInterest = command.booleanPrimitiveValueOfParameterNamed(SavingsApiConstants.postInterestValidationOnClosure); - // postInterest(account,closedDate,flag); - if (isPostInterest) { - boolean postInterestOnClosingDate = false; - List savingTransactions = account.getTransactions(); - for (SavingsAccountTransaction savingTransaction : savingTransactions) { - if (savingTransaction.isInterestPosting() && savingTransaction.isNotReversed() - && DateUtils.isEqual(closedDate, savingTransaction.getTransactionDate())) { - postInterestOnClosingDate = true; - break; - } - } - if (!postInterestOnClosingDate) { - throw new PostInterestClosingDateException(); + + // Pro-rata interest for whatever partial period has accrued since the account's last posting is + // calculated and posted here, unconditionally -- closing used to only ever *validate* that + // interest had already been posted on the closing date (gated behind + // postInterestValidationOnClosure, off by default), so with nothing manually posted beforehand + // the partial period's interest was simply never paid. postInterest(...) already no-ops for a + // zero-interest product (its own rate>0 guard) and for a period with nothing newly due, so + // calling it here on every closure is safe -- it only ever adds the posting that closing an + // account on a real business day should always have included. + boolean interestAlreadyPostedOnClosingDate = false; + for (SavingsAccountTransaction savingTransaction : account.getTransactions()) { + if (savingTransaction.isInterestPosting() && savingTransaction.isNotReversed() + && DateUtils.isEqual(closedDate, savingTransaction.getTransactionDate())) { + interestAlreadyPostedOnClosingDate = true; + break; } } + if (!interestAlreadyPostedOnClosingDate) { + this.postInterest(account, true, closedDate, false); + } final Map changes = new LinkedHashMap<>(); @@ -1835,7 +1838,14 @@ public class SavingsAccountWritePlatformServiceJpaRepositoryImpl implements Savi this.savingsAccountTransactionDataValidator.validateHoldAndAssembleForm(command.json(), account, submittedBy, backdatedTxnsAllowedTill); SavingsAccountTransaction transaction = this.savingsAccountDomainService.handleHold(account, amount, transactionDate, lienAllowed); - account.holdAmount(amount); + // Defaults to Cash Hold (a manual/staff-placed lien) -- the automatic clearing hold placed on a + // Cheque/DD deposit at approval time is the only caller expected to pass CLEARING explicitly. + final String requestedHoldCategory = command.stringValueOfParameterNamed(SavingsApiConstants.holdCategoryParamName); + final String holdCategory = SavingsAccount.HOLD_CATEGORY_CLEARING.equals(requestedHoldCategory) + ? SavingsAccount.HOLD_CATEGORY_CLEARING + : SavingsAccount.HOLD_CATEGORY_CASH; + account.holdAmount(amount, holdCategory); + transaction.updateHoldCategory(holdCategory); transaction.setRunningBalance(runningBalance); final String reasonForBlock = command.stringValueOfParameterNamed(SavingsApiConstants.reasonForBlockParamName); @@ -1883,7 +1893,11 @@ public class SavingsAccountWritePlatformServiceJpaRepositoryImpl implements Savi transaction.setRunningBalance(runningBalance); this.savingsAccountTransactionDataValidator.validateTransactionWithPivotDate(transaction.getTransactionDate(), account); - account.releaseOnHoldAmount(transaction.getAmount()); + // Credit back whichever bucket the original hold debited -- a Clearing Hold released here (the + // manual "release early" action) must not accidentally credit the Cash Hold total instead. + final String holdCategory = holdTransaction.getHoldCategory() != null ? holdTransaction.getHoldCategory() + : SavingsAccount.HOLD_CATEGORY_CASH; + account.releaseOnHoldAmount(transaction.getAmount(), holdCategory); this.savingsAccountTransactionRepository.saveAndFlush(transaction); holdTransaction.updateReleaseId(transaction.getId()); diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savingshold/api/SavingsClearingHoldReleaseApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savingshold/api/SavingsClearingHoldReleaseApiResource.java new file mode 100644 index 000000000..9aaa5501b --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savingshold/api/SavingsClearingHoldReleaseApiResource.java @@ -0,0 +1,77 @@ +package org.apache.fineract.portfolio.savingshold.api; + +import com.google.gson.Gson; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.MediaType; +import java.time.LocalDate; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import lombok.RequiredArgsConstructor; +import org.apache.fineract.commands.domain.CommandWrapper; +import org.apache.fineract.commands.service.CommandWrapperBuilder; +import org.apache.fineract.commands.service.PortfolioCommandSourceWritePlatformService; +import org.apache.fineract.infrastructure.core.service.DateUtils; +import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.stereotype.Component; + +/** + * Auto-releases a Clearing Hold once its instrument's clearing period has passed — real-world CBS + * practice for a cheque/DD deposit that was held pending clearance (see + * {@code CashierDepositRequestApiResource.placeClearingHoldIfInstrumentDeposit}). This is a plain, + * self-contained REST trigger rather than a job registered in Fineract's Spring Batch scheduler: the + * batch job framework is a deep, unfamiliar subsystem to extend correctly, and a wrong registration + * there risks the whole scheduler, not just this feature. Wire this endpoint into an external + * scheduler (cron / the existing OS-level task runner) to run it daily; a manager can also call it + * directly, or release one hold early and by hand via the existing releaseAmount action, for an + * instrument confirmed cleared sooner than the default window. + */ +@Path("/v1/savingsaccounts/clearingholds/release-due") +@Component +@RequiredArgsConstructor +@Produces({ MediaType.APPLICATION_JSON }) +public class SavingsClearingHoldReleaseApiResource { + + /** Kept as a simple constant rather than a new admin-editable setting -- the clearing window this + * branch actually needs can be tuned here once real usage shows what it should be. */ + private static final int CLEARING_HOLD_DEFAULT_DAYS = 3; + + private final JdbcTemplate jdbcTemplate; + private final PlatformSecurityContext context; + private final PortfolioCommandSourceWritePlatformService commandsSourceWritePlatformService; + private final Gson gson = new Gson(); + + @POST + public String releaseDue() { + this.context.authenticatedUser(); + + final LocalDate cutoff = LocalDate.now(DateUtils.getDateTimeZoneOfTenant()).minusDays(CLEARING_HOLD_DEFAULT_DAYS); + + final List> due = this.jdbcTemplate.queryForList(""" + SELECT id, savings_account_id FROM m_savings_account_transaction + WHERE hold_category = 'CLEARING' AND release_id_of_hold_amount IS NULL + AND is_reversed = false AND transaction_date <= ? + """, java.sql.Date.valueOf(cutoff)); + + final List> released = new ArrayList<>(); + final List> failed = new ArrayList<>(); + for (final Map row : due) { + final Long savingsId = ((Number) row.get("savings_account_id")).longValue(); + final Long transactionId = ((Number) row.get("id")).longValue(); + try { + final CommandWrapper commandRequest = new CommandWrapperBuilder().releaseAmount(savingsId, transactionId) + .withJson("{}").build(); + this.commandsSourceWritePlatformService.logCommandSource(commandRequest); + released.add(Map.of("savingsId", savingsId, "transactionId", transactionId)); + } catch (final RuntimeException e) { + failed.add(Map.of("savingsId", savingsId, "transactionId", transactionId, "error", String.valueOf(e.getMessage()))); + } + } + + return this.gson.toJson( + Map.of("cutoffDate", cutoff.toString(), "releasedCount", released.size(), "released", released, "failed", failed)); + } +} diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savingshold/api/SavingsHoldBreakdownApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savingshold/api/SavingsHoldBreakdownApiResource.java new file mode 100644 index 000000000..ef91580b1 --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savingshold/api/SavingsHoldBreakdownApiResource.java @@ -0,0 +1,49 @@ +package org.apache.fineract.portfolio.savingshold.api; + +import com.google.gson.Gson; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.MediaType; +import java.math.BigDecimal; +import java.util.Map; +import lombok.RequiredArgsConstructor; +import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.stereotype.Component; + +/** + * The two-way hold breakdown behind a savings account's single combined "amount on hold" figure — + * a manual, staff-placed lien (Cash Hold) versus money still moving through the clearing process, a + * cheque/DD deposit not yet realized or a presented cheque not yet settled (Clearing Hold). + * + * Kept as a narrow, additive endpoint alongside the standard Fineract savings-account read API rather + * than adding a field to {@code SavingsAccountData} — that class has roughly a dozen constructor + * overloads threading its existing fields through every savings API response, and touching all of them + * for one screen's two extra numbers would be a wide, easy-to-regress change for a small addition. + */ +@Path("/v1/savingsaccounts/{savingsId}/holdbreakdown") +@Component +@RequiredArgsConstructor +@Produces({ MediaType.APPLICATION_JSON }) +public class SavingsHoldBreakdownApiResource { + + private final JdbcTemplate jdbcTemplate; + private final PlatformSecurityContext context; + private final Gson gson = new Gson(); + + @GET + public String retrieve(@PathParam("savingsId") final Long savingsId) { + this.context.authenticatedUser(); + + final Map row = this.jdbcTemplate.queryForMap(""" + SELECT COALESCE(total_savings_amount_on_hold, 0) AS cash_hold, + COALESCE(total_savings_amount_on_hold_clearing, 0) AS clearing_hold + FROM m_savings_account WHERE id = ? + """, savingsId); + + return this.gson.toJson(Map.of("savingsId", savingsId, "cashHold", (BigDecimal) row.get("cash_hold"), + "clearingHold", (BigDecimal) row.get("clearing_hold"))); + } +} diff --git a/fineract-provider/src/main/resources/db/changelog/tenant/final-changelog-tenant.xml b/fineract-provider/src/main/resources/db/changelog/tenant/final-changelog-tenant.xml index fff9dac05..ed25176f2 100644 --- a/fineract-provider/src/main/resources/db/changelog/tenant/final-changelog-tenant.xml +++ b/fineract-provider/src/main/resources/db/changelog/tenant/final-changelog-tenant.xml @@ -39,4 +39,7 @@ + + + diff --git a/fineract-provider/src/main/resources/db/changelog/tenant/parts/0516_savings_account_clearing_hold.xml b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0516_savings_account_clearing_hold.xml new file mode 100644 index 000000000..1ebb76e6f --- /dev/null +++ b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0516_savings_account_clearing_hold.xml @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/fineract-provider/src/main/resources/db/changelog/tenant/parts/0517_grant_checker_roles_hold_release_amount.xml b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0517_grant_checker_roles_hold_release_amount.xml new file mode 100644 index 000000000..4068ebf3e --- /dev/null +++ b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0517_grant_checker_roles_hold_release_amount.xml @@ -0,0 +1,31 @@ + + + + + + + + INSERT INTO m_role_permission (role_id, permission_id) + SELECT r.id, p.id + FROM m_role r + CROSS JOIN m_permission p + WHERE p.code IN ('HOLDAMOUNT_SAVINGSACCOUNT', 'RELEASEAMOUNT_SAVINGSACCOUNT') + AND r.name IN ('Branch Manager', 'As Manager', 'CASHIER', 'Principal Officer') + AND NOT EXISTS ( + SELECT 1 FROM m_role_permission rp + WHERE rp.role_id = r.id AND rp.permission_id = p.id + ) + + + + diff --git a/fineract-provider/src/main/resources/db/changelog/tenant/parts/0518_grant_teller_roles_read_institution_branch_profile.xml b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0518_grant_teller_roles_read_institution_branch_profile.xml new file mode 100644 index 000000000..b1ceceb41 --- /dev/null +++ b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0518_grant_teller_roles_read_institution_branch_profile.xml @@ -0,0 +1,30 @@ + + + + + + + + INSERT INTO m_role_permission (role_id, permission_id) + SELECT r.id, p.id + FROM m_role r + CROSS JOIN m_permission p + WHERE p.code IN ('READ_INSTITUTIONPROFILE', 'READ_BRANCHPROFILE') + AND r.name IN ('CASHIER', 'Clerk', 'Teller', 'Branch Manager', 'Principal Officer', 'As Manager') + AND NOT EXISTS ( + SELECT 1 FROM m_role_permission rp + WHERE rp.role_id = r.id AND rp.permission_id = p.id + ) + + + + diff --git a/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java b/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java index 9a6d1b2b6..414134b27 100644 --- a/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java +++ b/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java @@ -349,6 +349,14 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom @Column(name = "total_savings_amount_on_hold", scale = 6, precision = 19, nullable = true) private BigDecimal savingsOnHoldAmount; + + /** A hold placed because a cheque/DD/instrument is still in the clearing pipeline (a deposit not yet + * realized, or a presented cheque not yet settled) -- kept as its own running total, separate from + * {@link #savingsOnHoldAmount}'s staff-placed lien, so the two read as genuinely different figures + * rather than one bucket doing double duty. */ + @Column(name = "total_savings_amount_on_hold_clearing", scale = 6, precision = 19, nullable = true) + private BigDecimal savingsOnHoldAmountClearing; + @OneToMany(cascade = CascadeType.ALL, mappedBy = "account", orphanRemoval = true, fetch = FetchType.LAZY) protected List identifiers = new ArrayList<>(); @@ -3852,7 +3860,7 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom public BigDecimal getWithdrawableBalance() { return getAccountBalance().subtract(minRequiredBalanceDerived(getCurrency()).getAmount()).subtract(this.getOnHoldFunds()) - .subtract(this.getSavingsHoldAmount()); + .subtract(this.getSavingsHoldAmount()).subtract(this.getSavingsHoldAmountClearing()); } public BigDecimal getWithdrawableBalanceWithLien() { @@ -4189,16 +4197,44 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom return lastransactionDate; } + /** A manual, staff-placed lien (court order, minimum-balance hold, etc.) -- distinct from a + * {@link #HOLD_CATEGORY_CLEARING} hold, which exists because of the clearing process itself rather + * than a decision someone made about this specific account. */ + public static final String HOLD_CATEGORY_CASH = "CASH"; + public static final String HOLD_CATEGORY_CLEARING = "CLEARING"; + public BigDecimal getSavingsHoldAmount() { return this.savingsOnHoldAmount == null ? BigDecimal.ZERO : this.savingsOnHoldAmount; } + public BigDecimal getSavingsHoldAmountClearing() { + return this.savingsOnHoldAmountClearing == null ? BigDecimal.ZERO : this.savingsOnHoldAmountClearing; + } + + /** Defaults to the manual-lien bucket -- every call site written before the two-category split + * keeps behaving exactly as it did before. */ public void holdAmount(BigDecimal amount) { - this.savingsOnHoldAmount = getSavingsHoldAmount().add(amount); + holdAmount(amount, HOLD_CATEGORY_CASH); + } + + public void holdAmount(BigDecimal amount, String category) { + if (HOLD_CATEGORY_CLEARING.equals(category)) { + this.savingsOnHoldAmountClearing = getSavingsHoldAmountClearing().add(amount); + } else { + this.savingsOnHoldAmount = getSavingsHoldAmount().add(amount); + } } public void releaseOnHoldAmount(BigDecimal amount) { - this.savingsOnHoldAmount = getSavingsHoldAmount().subtract(amount); + releaseOnHoldAmount(amount, HOLD_CATEGORY_CASH); + } + + public void releaseOnHoldAmount(BigDecimal amount, String category) { + if (HOLD_CATEGORY_CLEARING.equals(category)) { + this.savingsOnHoldAmountClearing = getSavingsHoldAmountClearing().subtract(amount); + } else { + this.savingsOnHoldAmount = getSavingsHoldAmount().subtract(amount); + } } public AccountType getAccountType() { diff --git a/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccountTransaction.java b/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccountTransaction.java index 791a126aa..5724c699f 100644 --- a/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccountTransaction.java +++ b/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccountTransaction.java @@ -126,6 +126,12 @@ public final class SavingsAccountTransaction extends AbstractAuditableWithUTCDat @Column(name = "reason_for_block", nullable = true) private String reasonForBlock; + /** {@link SavingsAccount#HOLD_CATEGORY_CASH} or {@link SavingsAccount#HOLD_CATEGORY_CLEARING} -- + * only meaningful on a hold-amount transaction; carried here (rather than looked up) so a later + * release can credit the same running total the hold itself debited. */ + @Column(name = "hold_category", length = 20, nullable = true) + private String holdCategory; + @Column(name = "is_reversal", nullable = false) private boolean reversalTransaction; @@ -459,6 +465,14 @@ public final class SavingsAccountTransaction extends AbstractAuditableWithUTCDat this.reasonForBlock = reasonForBlock; } + public String getHoldCategory() { + return this.holdCategory; + } + + public void updateHoldCategory(String holdCategory) { + this.holdCategory = holdCategory; + } + public Long getReleaseIdOfHoldAmountTransaction() { return this.releaseIdOfHoldAmountTransaction; } diff --git a/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/service/FacilityChargeSeederService.java b/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/service/FacilityChargeSeederService.java index 6eed63afb..a8a15d08d 100644 --- a/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/service/FacilityChargeSeederService.java +++ b/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/service/FacilityChargeSeederService.java @@ -72,9 +72,6 @@ public class FacilityChargeSeederService { @Transactional public void seedFacilityCharges() { try { - // Find an income GL account to link charges (prefer DETAIL type income account) - Long incomeGlAccountId = findIncomeGlAccountId(); - for (Map.Entry entry : FACILITY_DEFAULTS.entrySet()) { final String facilityType = entry.getKey(); final Object[] def = entry.getValue(); @@ -90,12 +87,21 @@ public class FacilityChargeSeederService { Long.class, chargeName); if (chargeIds.isEmpty()) { + // income_or_liability_account_id deliberately left NULL: Fineract's own + // accounting engine (AccountingProcessorHelper.getLinkedGLAccountForSavingsCharges) + // uses a charge's own GL account, when set, ahead of the savings PRODUCT's own + // "Income From Fees" mapping. Setting one here would silently override every + // product's own configuration with the same shared account for all 16 facilities + // -- exactly what happened before this fix, all landing on whichever income + // account findIncomeGlAccountId() happened to find first. Leaving it unset lets + // each product's own accounting setup decide where this income posts, which is + // the actual point of a per-product "Income From Fees" mapping. jdbcTemplate.update( "INSERT INTO m_charge (name, currency_code, amount, charge_applies_to_enum, " + "charge_time_enum, charge_calculation_enum, is_penalty, is_active, is_deleted, " - + "is_free_withdrawal, is_payment_type, income_or_liability_account_id) " - + "VALUES (?, 'INR', ?, ?, ?, true, false, true, false, false, false, ?)", - chargeName, amount, chargeAppliesTo, chargeTimeType, incomeGlAccountId); + + "is_free_withdrawal, is_payment_type) " + + "VALUES (?, 'INR', ?, ?, ?, true, false, true, false, false, false)", + chargeName, amount, chargeAppliesTo, chargeTimeType); log.info("FacilityChargeSeeder: inserted charge '{}'", chargeName); } @@ -122,24 +128,4 @@ public class FacilityChargeSeederService { log.warn("FacilityChargeSeeder: skipped — {}", e.getMessage()); } } - - private Long findIncomeGlAccountId() { - try { - // Prefer "INCOME FROM ALL" GL account for facility charges - List ids = jdbcTemplate.queryForList( - "SELECT id FROM acc_gl_account WHERE classification_enum = 4 AND account_usage = 1 " - + "AND disabled = false AND name = 'INCOME FROM ALL' LIMIT 1", - Long.class); - if (!ids.isEmpty()) return ids.get(0); - - // Fallback: first available income detail account - ids = jdbcTemplate.queryForList( - "SELECT id FROM acc_gl_account WHERE classification_enum = 4 AND account_usage = 1 " - + "AND disabled = false ORDER BY id ASC LIMIT 1", - Long.class); - return ids.isEmpty() ? null : ids.get(0); - } catch (Exception e) { - return null; - } - } }