fix(cash): recognize GL-entry cash correctly in reconciliation and reports

- Add CountedCashLedger: one shared rule for which m_savings_txn_denomination rows are real counted cash. Replaces a magnitude-only guard (which had itself been silently excluding legitimate manual-GL-entry cash from every cash screen) with a date-plausibility test -- a stamped garbage id decodes back to the day it was written, a real GL reference to an arbitrary date -- so a hex reference that happens to land in the old guard's excluded band is no longer misclassified as timestamp garbage - Apply it consistently in the day register close preview, the cashier position/day-book reports, the cash reconciliation screen, and the withdrawal stock check, so all four agree on the same drawer - GL Entries report: join m_voucher_line to show both legs of each voucher (debit and credit account) instead of only the cash leg its own denomination row happens to carry Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-14 07:19:05 +05:30
parent 04ac233fbe
commit 6bb512e1fe
6 changed files with 148 additions and 49 deletions
@@ -43,6 +43,7 @@ import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper;
 import org.apache.fineract.infrastructure.core.service.DateUtils;
 import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
 import org.apache.fineract.portfolio.cashreport.service.VaultPositionService;
+import org.apache.fineract.portfolio.denominationmaster.service.CountedCashLedger;
 import org.apache.fineract.useradministration.domain.AppUser;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.stereotype.Component;
@@ -72,13 +73,6 @@ public class BranchDayRegisterApiResource {
    private final VaultPositionService vaultPositionService;
    private final Gson gson = new Gson();

-    // Mirrors CashReportApiResource.MAX_PLAUSIBLE_TXN_ID: a genuine m_savings_account_transaction id
-    // is always small. Some cashier-GL deposit legs stamp a synthetic, implausibly large txn_id (a
-    // millisecond timestamp, decoded back out of hex) that never links to a real transaction and is
-    // never corrected -- without this same filter here, the close-preview's counted-in/counted-out
-    // figures would include that phantom amount, on top of the real cash the branch actually counted.
-    private static final long MAX_PLAUSIBLE_TXN_ID = 1_000_000_000_000L;
-
    // ── GET /v1/branchday ── the register itself ──────────────────────────────

    /**
@@ -197,28 +191,32 @@ public class BranchDayRegisterApiResource {
        // cross-check was being compared against every branch's counted cash at once, which makes it
        // wrong at every office but the first. A row whose officer maps to no cashier is still
        // counted, for the same reason as the withdrawal legs in bookBreakdown.
+        //
+        // Which rows are real counted cash is CountedCashLedger's rule, not this screen's -- see the
+        // note there on why a manual GL entry's decoded reference is kept and a stamped millisecond
+        // timestamp is not.
        final BigDecimal countedIn = zero(jdbcTemplate.queryForObject("""
                SELECT COALESCE(SUM(std.total_amount), 0) FROM m_savings_txn_denomination std
                 WHERE std.txn_date = ? AND std.txn_type = 'DEPOSIT'
-                   AND (std.txn_id IS NULL OR std.txn_id < ?)
+                   AND """ + CountedCashLedger.REAL_TXN_ID + """
                   AND (std.officer_id IS NULL
                        OR NOT EXISTS (SELECT 1 FROM m_appuser au JOIN m_cashiers c ON c.staff_id = au.staff_id
                                        WHERE au.id = std.officer_id)
                        OR EXISTS (SELECT 1 FROM m_appuser au JOIN m_cashiers c ON c.staff_id = au.staff_id
                                     JOIN m_tellers t ON t.id = c.teller_id
                                    WHERE au.id = std.officer_id AND t.office_id = ?))
-                """, BigDecimal.class, Date.valueOf(businessDate), MAX_PLAUSIBLE_TXN_ID, officeId));
+                """, BigDecimal.class, Date.valueOf(businessDate), officeId));
        final BigDecimal countedOut = zero(jdbcTemplate.queryForObject("""
                SELECT COALESCE(SUM(std.total_amount), 0) FROM m_savings_txn_denomination std
                 WHERE std.txn_date = ? AND std.txn_type = 'WITHDRAWAL'
-                   AND (std.txn_id IS NULL OR std.txn_id < ?)
+                   AND """ + CountedCashLedger.REAL_TXN_ID + """
                   AND (std.officer_id IS NULL
                        OR NOT EXISTS (SELECT 1 FROM m_appuser au JOIN m_cashiers c ON c.staff_id = au.staff_id
                                        WHERE au.id = std.officer_id)
                        OR EXISTS (SELECT 1 FROM m_appuser au JOIN m_cashiers c ON c.staff_id = au.staff_id
                                     JOIN m_tellers t ON t.id = c.teller_id
                                    WHERE au.id = std.officer_id AND t.office_id = ?))
-                """, BigDecimal.class, Date.valueOf(businessDate), MAX_PLAUSIBLE_TXN_ID, officeId));
+                """, BigDecimal.class, Date.valueOf(businessDate), officeId));
        result.put("denominationIn", countedIn);
        result.put("denominationOut", countedOut);

@@ -87,9 +87,9 @@ public class CashierDepositRequestApiResource {
    // DELETE) would silently rewrite or erase an unrelated real transaction's own denomination
    // breakdown whenever the two ids happened to match. Offsetting the temporary id into a range no
    // real transaction id will reach in this system's lifetime makes every such match unambiguous,
-    // without a schema change. Kept well under CashReportApiResource.MAX_PLAUSIBLE_TXN_ID (10^12) so
-    // a request still sitting PENDING continues to count in cashier-position reports -- the cash is
-    // physically in the drawer already, approval or not.
+    // without a schema change. Kept well under CashReportApiResource's own implausible-millisecond-id
+    // band (10^12) so a request still sitting PENDING continues to count in cashier-position reports
+    // -- the cash is physically in the drawer already, approval or not.
    private static final long PENDING_DEPOSIT_TXN_ID_OFFSET = 500_000_000L;

    private final JdbcTemplate jdbcTemplate;
@@ -28,6 +28,7 @@ import org.apache.fineract.portfolio.cashreconciliation.domain.CashReconciliatio
 import org.apache.fineract.portfolio.cashreconciliation.domain.CashReconciliationDetail;
 import org.apache.fineract.portfolio.cashreconciliation.domain.CashReconciliationDetailRepository;
 import org.apache.fineract.portfolio.cashreconciliation.domain.CashReconciliationRepository;
+import org.apache.fineract.portfolio.denominationmaster.service.CountedCashLedger;
 import org.apache.fineract.useradministration.domain.AppUser;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.stereotype.Component;
@@ -47,13 +48,6 @@ public class CashReconciliationApiResource {
    private final JdbcTemplate jdbcTemplate;
    private final Gson gson = new Gson();

-    // Mirrors CashReportApiResource.MAX_PLAUSIBLE_TXN_ID: a genuine m_savings_account_transaction id
-    // is always small. Some cashier-GL deposit legs stamp a synthetic, implausibly large txn_id (a
-    // millisecond timestamp, decoded back out of hex) that never links to a real transaction and is
-    // never corrected -- without this same filter here, this endpoint (unlike the cashier-position
-    // report) would count that phantom figure into a cashier's expected cash.
-    private static final long MAX_PLAUSIBLE_TXN_ID = 1_000_000_000_000L;
-
    /**
     * GET /cashreconciliation/expected?cashierId=X&asOfDate=yyyy-MM-dd
     * Returns denomination-wise expected balance for a cashier.
@@ -96,7 +90,7 @@ public class CashReconciliationApiResource {
                + "LEFT JOIN m_denomination_master dm ON dm.id = std.denomination_id "
                + "WHERE std.txn_type = 'DEPOSIT' "
                + "AND std.txn_date <= CAST(? AS DATE) "
-                + "AND (std.txn_id IS NULL OR std.txn_id < " + MAX_PLAUSIBLE_TXN_ID + ") "
+                + "AND " + CountedCashLedger.REAL_TXN_ID
                + "AND std.officer_id IN ("
                + "  SELECT au.id FROM m_appuser au "
                + "  JOIN m_cashiers mc ON mc.staff_id = au.staff_id WHERE mc.id = ?) "
@@ -126,7 +120,7 @@ public class CashReconciliationApiResource {
                + "LEFT JOIN m_denomination_master dm ON dm.id = std.denomination_id "
                + "WHERE std.txn_type = 'WITHDRAWAL' "
                + "AND std.txn_date <= CAST(? AS DATE) "
-                + "AND (std.txn_id IS NULL OR std.txn_id < " + MAX_PLAUSIBLE_TXN_ID + ") "
+                + "AND " + CountedCashLedger.REAL_TXN_ID
                + "AND std.officer_id IN ("
                + "  SELECT au.id FROM m_appuser au "
                + "  JOIN m_cashiers mc ON mc.staff_id = au.staff_id WHERE mc.id = ?) "
@@ -17,6 +17,7 @@ import lombok.extern.slf4j.Slf4j;
 import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
 import org.apache.fineract.portfolio.cashreport.service.CashierRoleHelper;
 import org.apache.fineract.portfolio.cashreport.service.VaultPositionService;
+import org.apache.fineract.portfolio.denominationmaster.service.CountedCashLedger;
 import org.apache.fineract.portfolio.denominationmaster.service.DenominationBreakdownWriter;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.stereotype.Component;
@@ -59,18 +60,14 @@ public class CashReportApiResource {
     * </ul>
     *
     * <p>
-     * The timestamp test is a magnitude test, not a lookup, because a missing transaction is not by
-     * itself wrong: the Demand Draft register keys its breakdowns by its own request id, and those
-     * rows are genuine cash that has to keep counting. Request ids and transaction ids are both
-     * small; a millisecond epoch is thirteen digits and cannot be either. So anything at or above
-     * 10^12 is rejected as not-an-id, and everything smaller is kept.
+     * Which ids are real and which are stamped timestamps is {@link CountedCashLedger}'s rule, shared
+     * with the day register, the reconciliation screen and the withdrawal stock check, so a drawer
+     * reads the same on all four.
     */
-    private static final long MAX_PLAUSIBLE_TXN_ID = 1_000_000_000_000L;
-
    private static final String COUNTER_NOT_REVERSED = "AND NOT EXISTS ("
            + "  SELECT 1 FROM m_savings_account_transaction rt "
            + "   WHERE rt.id = std.txn_id AND rt.is_reversed = true) "
-            + "AND (std.txn_id IS NULL OR std.txn_id < " + MAX_PLAUSIBLE_TXN_ID + ") "
+            + "AND " + CountedCashLedger.REAL_TXN_ID
            // A breakdown has to add up to the transaction it belongs to. Where it does not, the
            // notes were mis-keyed and the transaction amount is the fact -- ₹1,60,000 of notes
            // recorded against a ₹918 deposit is not ₹1,60,000 of cash in anybody's drawer. Seven
@@ -83,8 +80,9 @@ public class CashReportApiResource {
            // everything counted after it) out of the cashier's position.
            //
            // Compared per transaction inside a correlated subquery rather than as a join, so a
-            // breakdown with no transaction behind it -- a Demand Draft keyed by its request id --
-            // is untouched by this test and still handled by the id-magnitude test above.
+            // breakdown with no transaction behind it -- a Demand Draft keyed by its request id, or a
+            // manual GL entry keyed by its decoded reference -- is untouched by this test and left to
+            // CountedCashLedger's rule above.
            + "AND NOT EXISTS ("
            + "  SELECT 1 FROM m_savings_account_transaction mt "
            + "   WHERE mt.id = std.txn_id"
@@ -1341,21 +1339,42 @@ public class CashReportApiResource {
        final java.math.BigDecimal totalDr = (java.math.BigDecimal) totalsRow.get("total_dr");
        final java.math.BigDecimal totalCr = (java.math.BigDecimal) totalsRow.get("total_cr");

-        // 2. Paged data query
-        final String dataSql = "SELECT "
+        // 2. Paged data query. Each row here is one physical-cash leg -- the only one a manual GL
+        // entry ever has a denomination breakdown for -- so on its own it names only the cash side
+        // (e.g. "CASH AT VAULT") and neither the voucher's other leg nor which two accounts the
+        // entry actually moved money between. Every voucher raised through VoucherService (both this
+        // one and an ordinary counter deposit) has its full pair of legs recorded in m_voucher_line,
+        // the same table the My Work register reads its own debit/credit columns from -- joined here
+        // by voucher_no so this report shows the identical pair rather than only the half this
+        // table's own denomination row happens to know about.
+        final String baseSql = "SELECT "
                + "  COALESCE(MAX(cdr.voucher_no), MAX(cwr.voucher_no), MAX(ccr.voucher_no), "
                + "           CAST(std.txn_id AS VARCHAR)) AS voucher_no, "
                + "  std.account_no AS ac_no, "
                + "  COALESCE(mc.display_name, std.cif_no, MAX(gla.name), '') AS ac_name, "
                + "  MAX(std.txn_date) AS txn_date, "
                + "  TO_CHAR(MAX(std.created_at), 'HH24:MI:SS') AS txn_time, "
+                + "  MAX(std.created_at) AS sort_key, "
                + "  MAX(std.officer_username) AS teller_name, "
                + "  MAX(std.payment_type) AS payment_type, "
                + "  CASE WHEN MAX(std.client_id) IS NULL THEN 'GL' ELSE 'SAVINGS' END AS txn_source, "
                + "  SUM(CASE WHEN std.txn_type = 'WITHDRAWAL' THEN std.total_amount ELSE 0 END) AS dr_amount, "
                + "  SUM(CASE WHEN std.txn_type = 'DEPOSIT' THEN std.total_amount ELSE 0 END) AS cr_amount "
-                + baseGroupSql
-                + "ORDER BY MAX(std.created_at) "
+                + baseGroupSql;
+
+        final String dataSql = "SELECT b.*, "
+                + "  dr.party_name AS debit_party, dr.account_no AS debit_account, "
+                + "  cr.party_name AS credit_party, cr.account_no AS credit_account "
+                + "FROM (" + baseSql + ") b "
+                + "LEFT JOIN LATERAL ("
+                + "     SELECT party_name, account_no FROM m_voucher_line"
+                + "      WHERE voucher_no = b.voucher_no AND entry_type = 'DR'"
+                + "      ORDER BY line_no LIMIT 1) dr ON TRUE "
+                + "LEFT JOIN LATERAL ("
+                + "     SELECT party_name, account_no FROM m_voucher_line"
+                + "      WHERE voucher_no = b.voucher_no AND entry_type = 'CR'"
+                + "      ORDER BY line_no LIMIT 1) cr ON TRUE "
+                + "ORDER BY b.sort_key "
                + "LIMIT ? OFFSET ?";

        // Append limit and offset to params
@@ -1376,6 +1395,12 @@ public class CashReportApiResource {
            row.put("txnSource", rs.getString("txn_source"));
            row.put("dr", rs.getBigDecimal("dr_amount"));
            row.put("cr", rs.getBigDecimal("cr_amount"));
+            // Falls back to null rather than the cash-leg-only ac_no/ac_name above when a voucher
+            // was raised before m_voucher_line existed, or belongs to no register that writes one.
+            row.put("debitParty", rs.getString("debit_party"));
+            row.put("debitAccount", rs.getString("debit_account"));
+            row.put("creditParty", rs.getString("credit_party"));
+            row.put("creditAccount", rs.getString("credit_account"));
            return row;
        }, pagedParams);

@@ -0,0 +1,90 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.denominationmaster.service;
+
+/**
+ * The one definition of which {@code m_savings_txn_denomination} rows are real counted cash.
+ *
+ * <p>
+ * Written here once because it is read from four places -- the branch day register's close preview,
+ * the cashier position report, the cash reconciliation screen and the withdrawal stock check -- and
+ * those places disagreeing is what makes a drawer hold one figure on one screen and another on the
+ * next. A change to the rule belongs in this file and nowhere else.
+ *
+ * <h3>What the rule has to separate</h3>
+ *
+ * <p>
+ * The {@code txn_id} column is a bigint shared by every register that counts notes, and each writes
+ * a different kind of id into it:
+ *
+ * <ul>
+ * <li>a savings, current or deposit transaction writes its own transaction id -- always small;
+ * <li>a cashier deposit request writes its request id plus a fixed offset -- also small;
+ * <li>a Demand Draft issue writes its own request id -- small;
+ * <li>a manual GL entry has no numeric id at all, so its hex transaction reference is decoded into
+ * this column (see {@code CashierGlDenominationApiResource}, matching the encoding
+ * {@code CashReportApiResource}'s voucher lookup decodes back out of hex) -- almost always very
+ * large, but a hex string beginning with low digits can land anywhere;
+ * <li>and older code once wrote {@code System.currentTimeMillis()} here, leaving rows belonging to
+ * no transaction at all. Those are phantom cash: a cashier's position is cumulative, so they are
+ * carried forward for ever, and one database held ₹7.29 lakh of them.
+ * </ul>
+ *
+ * <h3>Why this is not a magnitude test</h3>
+ *
+ * <p>
+ * A magnitude test cannot tell the last two apart, and a wider version of this rule -- "reject
+ * anything at or above 10^12" -- rejected every manual GL entry's cash along with the phantom rows.
+ * A narrower band still misses: one live row carries a decoded GL reference of 6,547,106,798,661,
+ * which sits squarely inside the 13-digit band a millisecond epoch occupies.
+ *
+ * <p>
+ * What actually separates them is that a millisecond epoch stamped at write time decodes back to
+ * approximately the day the row itself belongs to, while a hex reference that happens to land in the
+ * same band decodes to an arbitrary date -- the row above reads as June 2177 against a June 2026
+ * transaction date. So the test reads the id as a date and asks whether it lands on the row's own
+ * day. That is self-maintaining: it needs no constant moved forward as real timestamps grow.
+ */
+public final class CountedCashLedger {
+
+    private CountedCashLedger() {}
+
+    /** Widest gap still read as "this id is the moment the row was written", in days. */
+    private static final int SAME_DAY_TOLERANCE_DAYS = 7;
+
+    /**
+     * SQL predicate keeping only rows whose {@code txn_id} is a real reference, as an expression over
+     * the alias {@code std} ({@code m_savings_txn_denomination}).
+     *
+     * <p>
+     * The date decoding is confined to the 13-digit band, where {@code to_timestamp} is always inside
+     * PostgreSQL's own timestamp range; ids outside that band are kept without decoding, so no value
+     * this column can hold can overflow the comparison.
+     *
+     * <p>
+     * Padded with a space at each end so it reads the same spliced into a text block -- which strips
+     * the trailing space off the {@code AND } it follows -- as concatenated onto a plain string.
+     */
+    public static final String REAL_TXN_ID = " (std.txn_id IS NULL"
+            + " OR std.txn_id < 1000000000000 OR std.txn_id >= 10000000000000"
+            + " OR COALESCE(std.txn_date, CAST(std.created_at AS DATE)) IS NULL"
+            + " OR CAST(to_timestamp(std.txn_id / 1000.0) AS DATE) NOT BETWEEN"
+            + "    COALESCE(std.txn_date, CAST(std.created_at AS DATE)) - " + SAME_DAY_TOLERANCE_DAYS
+            + " AND COALESCE(std.txn_date, CAST(std.created_at AS DATE)) + " + SAME_DAY_TOLERANCE_DAYS + ") ";
+}
@@ -40,14 +40,6 @@ public class DenominationBreakdownWriter {
    private final PlatformSecurityContext context;
    private final JdbcTemplate jdbcTemplate;

-    // Mirrors CashReportApiResource.MAX_PLAUSIBLE_TXN_ID: a genuine m_savings_account_transaction id
-    // is always small. Some cashier-GL deposit legs stamp a synthetic, implausibly large txn_id (a
-    // millisecond timestamp, decoded back out of hex) that never links to a real transaction and is
-    // never corrected -- without this same filter here, a cashier's computed stock (and the
-    // insufficient-stock check that guards a withdrawal against it) would count that phantom figure
-    // as real notes and coins in the drawer.
-    private static final long MAX_PLAUSIBLE_TXN_ID = 1_000_000_000_000L;
-
    // ── Public API ────────────────────────────────────────────────────────────

    /**
@@ -294,7 +286,7 @@ public class DenominationBreakdownWriter {
                + "    SELECT std.denomination_id, CAST(SUM(std.count) AS BIGINT)"
                + "    FROM m_savings_txn_denomination std"
                + "    WHERE std.txn_type = 'DEPOSIT'"
-                + "    AND (std.txn_id IS NULL OR std.txn_id < " + MAX_PLAUSIBLE_TXN_ID + ")"
+                + "    AND " + CountedCashLedger.REAL_TXN_ID
                + "    AND std.officer_id IN (SELECT au.id FROM m_appuser au"
                + "      JOIN m_cashiers mc ON mc.staff_id = au.staff_id WHERE mc.id = ?)"
                + "    GROUP BY std.denomination_id"
@@ -318,7 +310,7 @@ public class DenominationBreakdownWriter {
                + "    SELECT std.denomination_id, CAST(-SUM(std.count) AS BIGINT)"
                + "    FROM m_savings_txn_denomination std"
                + "    WHERE std.txn_type = 'WITHDRAWAL'"
-                + "    AND (std.txn_id IS NULL OR std.txn_id < " + MAX_PLAUSIBLE_TXN_ID + ")"
+                + "    AND " + CountedCashLedger.REAL_TXN_ID
                + "    AND std.officer_id IN (SELECT au.id FROM m_appuser au"
                + "      JOIN m_cashiers mc ON mc.staff_id = au.staff_id WHERE mc.id = ?)"
                + "    GROUP BY std.denomination_id"
@@ -415,7 +407,7 @@ public class DenominationBreakdownWriter {
                + "  SELECT std.denomination_id, CAST(SUM(std.count) AS BIGINT)"
                + "  FROM m_savings_txn_denomination std"
                + "  WHERE std.txn_type = 'DEPOSIT'"
-                + "  AND (std.txn_id IS NULL OR std.txn_id < " + MAX_PLAUSIBLE_TXN_ID + ")"
+                + "  AND " + CountedCashLedger.REAL_TXN_ID
                + "  AND std.officer_id IN (SELECT au.id FROM m_appuser au"
                + "    JOIN m_cashiers mc ON mc.staff_id = au.staff_id WHERE mc.id = ?)"
                + "  GROUP BY std.denomination_id"
@@ -439,7 +431,7 @@ public class DenominationBreakdownWriter {
                + "  SELECT std.denomination_id, CAST(-SUM(std.count) AS BIGINT)"
                + "  FROM m_savings_txn_denomination std"
                + "  WHERE std.txn_type = 'WITHDRAWAL'"
-                + "  AND (std.txn_id IS NULL OR std.txn_id < " + MAX_PLAUSIBLE_TXN_ID + ")"
+                + "  AND " + CountedCashLedger.REAL_TXN_ID
                + "  AND std.officer_id IN (SELECT au.id FROM m_appuser au"
                + "    JOIN m_cashiers mc ON mc.staff_id = au.staff_id WHERE mc.id = ?)"
                + "  GROUP BY std.denomination_id"