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 <noreply@anthropic.com>
2026-08-30 10:37:53 +05:30
parent 8d224a22b4
commit 6ffb0dd9c0
18 changed files with 424 additions and 64 deletions
@@ -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";
@@ -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));

@@ -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<String> 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
            );
        }
    }
@@ -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<Map<String, Object>> pageItems = jdbcTemplate.query(dataSql, (rs, rowNum) -> {
            final Map<String, Object> 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"));
@@ -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<String, com.google.gson.JsonElement> 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
            );
        }
    }
@@ -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());
@@ -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);
@@ -69,7 +69,8 @@ public class SavingsAccountTransactionDataValidator {
    private final FromJsonHelper fromApiJsonHelper;
    private static final Set<String> 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) {
@@ -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<SavingsAccountTransaction> 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<String, Object> 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());
@@ -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<Map<String, Object>> 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<Map<String, Object>> released = new ArrayList<>();
+        final List<Map<String, Object>> failed = new ArrayList<>();
+        for (final Map<String, Object> 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));
+    }
+}
@@ -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<String, Object> 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")));
+    }
+}
@@ -39,4 +39,7 @@
    <include file="parts/0513_institution_profile_full_bank_master.xml" relativeToChangelogFile="true"/>
    <include file="parts/0514_grant_teller_roles_read_clientnominee.xml" relativeToChangelogFile="true"/>
    <include file="parts/0515_branch_day_register_opening_difference_reason.xml" relativeToChangelogFile="true"/>
+    <include file="parts/0516_savings_account_clearing_hold.xml" relativeToChangelogFile="true"/>
+    <include file="parts/0517_grant_checker_roles_hold_release_amount.xml" relativeToChangelogFile="true"/>
+    <include file="parts/0518_grant_teller_roles_read_institution_branch_profile.xml" relativeToChangelogFile="true"/>
 </databaseChangeLog>
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
+        http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+
+    <!-- Fineract's existing hold-amount lien (m_savings_account.total_savings_amount_on_hold) is a
+         single bucket used for a manual, staff-placed hold (court order, minimum balance, a cheque-book
+         issuance charge pending collection) AND, until now, nothing distinguished that from money that
+         is simply still moving through the clearing process (a cheque/DD deposit not yet realized, or a
+         presented cheque not yet settled). This adds a second running total for exactly that second
+         case, plus a category tag on the transaction row that produced a hold, so the two read as two
+         genuinely different figures rather than one bucket doing double duty. -->
+
+    <changeSet id="0516_add_clearing_hold_columns" author="cbs-nitesh">
+        <preConditions onFail="MARK_RAN">
+            <not><columnExists tableName="m_savings_account" columnName="total_savings_amount_on_hold_clearing"/></not>
+        </preConditions>
+        <addColumn tableName="m_savings_account">
+            <column name="total_savings_amount_on_hold_clearing" type="DECIMAL(19,6)" defaultValueNumeric="0">
+                <constraints nullable="false"/>
+            </column>
+        </addColumn>
+    </changeSet>
+
+    <changeSet id="0516_add_hold_category_column" author="cbs-nitesh">
+        <preConditions onFail="MARK_RAN">
+            <not><columnExists tableName="m_savings_account_transaction" columnName="hold_category"/></not>
+        </preConditions>
+        <addColumn tableName="m_savings_account_transaction">
+            <column name="hold_category" type="VARCHAR(20)" defaultValue="CASH"/>
+        </addColumn>
+    </changeSet>
+
+</databaseChangeLog>
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
+        http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+
+    <!-- Approving a Cheque/DD cashier-deposit request now automatically places a Clearing Hold
+         (dispatches the standard HOLDAMOUNT_SAVINGSACCOUNT command) as the approving manager, and the
+         auto-release trigger dispatches RELEASEAMOUNT_SAVINGSACCOUNT the same way. Before this, only
+         the Clerk role held either permission (Clerk never approves deposits - a checker does), so
+         approving any Cheque/DD deposit as a Branch Manager or As Manager would have failed
+         authorization the moment the new hold was placed. Granting both to the checker roles that
+         actually call deposit-approval (see CashierDepositRequestApiResource.validateCheckerRole) is
+         what keeps deposit approval working, not an unrelated permission expansion. -->
+
+    <changeSet id="0517_grant_checker_roles_hold_release_amount" author="cbs-nitesh">
+        <sql>
+            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
+                   )
+        </sql>
+    </changeSet>
+
+</databaseChangeLog>
@@ -0,0 +1,30 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
+        http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+
+    <!-- READ_INSTITUTIONPROFILE/READ_BRANCHPROFILE (0511) were deliberately scoped to manager/admin
+         roles only, since Institution/Branch Profile started as an Organization-setup screen those
+         roles configure. The Passbook and Statement print screens now read this same data (real bank
+         name, license number, branch address, IFSC/MICR) for their letterhead instead of a hardcoded
+         bank name, and both screens are reachable by a cashier/clerk/teller, not just a manager, so
+         the read-only permission needs to reach those roles too. UPDATE_* stays manager/admin-only;
+         nothing here grants write access. -->
+
+    <changeSet id="0518_grant_teller_roles_read_institution_branch_profile" author="cbs-nitesh">
+        <sql>
+            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
+                   )
+        </sql>
+    </changeSet>
+
+</databaseChangeLog>
@@ -349,6 +349,14 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom<Long>

    @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<InteropIdentifier> identifiers = new ArrayList<>();

@@ -3852,7 +3860,7 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom<Long>

    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<Long>
        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() {
@@ -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;
    }
@@ -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<String, Object[]> 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<Long> 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;
-        }
-    }
 }