From b779d045713dfbf568083f0384a3428bce843a63 Mon Sep 17 00:00:00 2001 From: Nitesh Yadav Date: Thu, 17 Sep 2026 12:54:04 +0530 Subject: [PATCH] fix(accounting): ledger transaction-id integrity, share GL correctness, and validation fixes Ledger transaction ids: - New LedgerTransactionId generator (PREFIX-YYMMDD-NNNN, e.g. DIVRND-260916-0001) backed by a dedicated m_ledger_txn_sequence counter (migration 0563) that belongs to no business table, so no table's sequence reset can make an id repeat. Migrated 18 hand-rolled id sites (dividend, branch day, closure, year-end, share collection/advance) off ad hoc nanoTime()/PK-concatenation strings. - ShareAccount.getShareAccountTransaction() matched a just-saved transaction by date+shares+price+ type and took the first hit -- two identical transactions on the same day resolved to the wrong one and posted under the older transaction's id. Now matches by id where one exists. Share accounting -- Cash at Vault fully retired from the share account/product lifecycle: - New ShareAccountingSuppression lets a register that posts its own accounting tell the platform not to post its native version (mirrors VoucherService.withOwnVoucher). Wired into share withdrawal, additional-shares apply/approve/reject and account close, so every governed flow now posts one balanced entry against the real counterparty instead of the platform's fixed Shares Reference placeholder, reversed and reposted. - Modify Application and Reject (the account's first application, pre-approval) no longer post via the native bridge at all -- no money has moved at that stage, so there was nothing to post. - The native "close" command no longer force-redeems a share account's remaining holding in one unguarded step; closing with a balance outstanding is now refused, so redemption can only happen through the governed Share Withdrawal flow the frontend already restricts it to. - "Share Reference (Cash Vault or Suspense)" is optional on the product, not mandatory: validator, GL mapping write/merge (including removing a stale mapping when the field is cleared on edit), and AccountingProcessorHelper's resolver (now throws a named error instead of NPEing when a product carries no mapping) all updated together. Removed from the product UI entirely (create, edit, preview, view). - AdditionalShareTrancheFundingService: submit posts nothing; settle() posts the whole entry itself, debiting whichever GL a voucher receipt actually landed in (read back from its own posting) or the member's savings account, crediting Share Capital. A voucher-funded tranche whose receipts total more than it costs now credits the difference to the member's linked savings account -- every named receipt is visited, not just enough to cover the tranche, since one left untouched here was still marked APPLIED and its money left untraceable. - ShareAccountDataSerializer: redeeming every share down to zero is exempt from the product's minimum-shares check (the check protects an account staying open with a reduced holding, not one being closed via a full exit); an over-payment on a voucher-funded tranche is refused at submit when no savings account is linked to receive it, rather than accepted and left unable to settle; six failWithCode(code, message) calls in the tranche validator were silently discarding their own message for the generic fallback text (failWithCode's second argument is a template arg, not a displayed string) -- switched to failWithCodeAndMessage. Removed: ShareCollectionApiResource, a direct-cash counter-collection endpoint that read m_share_account.cashier_collected, a column that has never existed in this schema -- no call site in this codebase's frontend reached it, and no entry it could have produced ever has. Co-Authored-By: Claude Opus 5 (1M context) --- ...AccountMappingFromApiJsonDeserializer.java | 8 +- .../ShareProductToGLAccountMappingHelper.java | 38 +- .../service/AccountingProcessorHelper.java | 7 + ...WritePlatformServiceJpaRepositoryImpl.java | 7 +- .../service/LedgerTransactionId.java | 157 +++++ .../journalentry/service/LedgerTxnPrefix.java | 87 +++ .../service/ShareAccountingSuppression.java | 83 +++ .../service/IncomeProfitTransferService.java | 6 +- .../api/BranchDayRegisterApiResource.java | 5 +- .../ShareAdvanceCollectionApiResource.java | 59 +- .../api/ShareCollectionApiResource.java | 439 -------------- .../api/ClerkClosureRequestApiResource.java | 4 +- ...lerkShareWithdrawalRequestApiResource.java | 486 +++++++++------ .../api/DividendClaimRequestApiResource.java | 4 +- .../SavingsPooledInterestPostingService.java | 8 +- .../shareaccounts/domain/ShareAccount.java | 23 + .../ShareAccountDataSerializer.java | 51 +- .../AdditionalShareTrancheFundingService.java | 564 +++++++++++------- .../ShareAccountSchedularServiceImpl.java | 4 +- ...WritePlatformServiceJpaRepositoryImpl.java | 78 ++- ...WritePlatformServiceJpaRepositoryImpl.java | 14 +- .../api/VoucherVerificationApiResource.java | 90 +-- .../tenant/final-changelog-tenant.xml | 1 + .../tenant/parts/0563_ledger_txn_sequence.xml | 48 ++ 24 files changed, 1281 insertions(+), 990 deletions(-) create mode 100644 fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/LedgerTransactionId.java create mode 100644 fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/LedgerTxnPrefix.java create mode 100644 fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/ShareAccountingSuppression.java delete mode 100644 fineract-provider/src/main/java/org/apache/fineract/portfolio/cashtransfer/api/ShareCollectionApiResource.java create mode 100644 fineract-provider/src/main/resources/db/changelog/tenant/parts/0563_ledger_txn_sequence.xml diff --git a/fineract-accounting/src/main/java/org/apache/fineract/accounting/producttoaccountmapping/serialization/ProductToGLAccountMappingFromApiJsonDeserializer.java b/fineract-accounting/src/main/java/org/apache/fineract/accounting/producttoaccountmapping/serialization/ProductToGLAccountMappingFromApiJsonDeserializer.java index fff2ccc74..d32c6b0eb 100644 --- a/fineract-accounting/src/main/java/org/apache/fineract/accounting/producttoaccountmapping/serialization/ProductToGLAccountMappingFromApiJsonDeserializer.java +++ b/fineract-accounting/src/main/java/org/apache/fineract/accounting/producttoaccountmapping/serialization/ProductToGLAccountMappingFromApiJsonDeserializer.java @@ -279,9 +279,15 @@ public final class ProductToGLAccountMappingFromApiJsonDeserializer { if (AccountingValidations.isCashBasedAccounting(accountingRuleType)) { + // Optional, not mandatory like the three below: it names an account for cash physically + // received at the counter for a share purchase, which every register in this bank now + // books against the account the money actually reached instead. Left absent, a product + // simply carries no such mapping, and any code path that still needs one (a genuine + // over-the-counter collection) fails with a named "GL not mapped" error rather than + // silently assuming the till. final Long shareReferenceId = this.fromApiJsonHelper.extractLongNamed(SharesProductAccountingParams.SHARES_REFERENCE.getValue(), element); - baseDataValidator.reset().parameter(SharesProductAccountingParams.SHARES_REFERENCE.getValue()).value(shareReferenceId).notNull() + baseDataValidator.reset().parameter(SharesProductAccountingParams.SHARES_REFERENCE.getValue()).value(shareReferenceId) .integerGreaterThanZero(); final Long shareSuspenseId = this.fromApiJsonHelper.extractLongNamed(SharesProductAccountingParams.SHARES_SUSPENSE.getValue(), diff --git a/fineract-accounting/src/main/java/org/apache/fineract/accounting/producttoaccountmapping/service/ShareProductToGLAccountMappingHelper.java b/fineract-accounting/src/main/java/org/apache/fineract/accounting/producttoaccountmapping/service/ShareProductToGLAccountMappingHelper.java index b7314f693..c17857d76 100644 --- a/fineract-accounting/src/main/java/org/apache/fineract/accounting/producttoaccountmapping/service/ShareProductToGLAccountMappingHelper.java +++ b/fineract-accounting/src/main/java/org/apache/fineract/accounting/producttoaccountmapping/service/ShareProductToGLAccountMappingHelper.java @@ -27,6 +27,7 @@ import org.apache.fineract.accounting.common.AccountingRuleType; import org.apache.fineract.accounting.glaccount.domain.GLAccountRepository; import org.apache.fineract.accounting.glaccount.domain.GLAccountRepositoryWrapper; import org.apache.fineract.accounting.glaccount.domain.GLAccountType; +import org.apache.fineract.accounting.producttoaccountmapping.domain.ProductToGLAccountMapping; import org.apache.fineract.accounting.producttoaccountmapping.domain.ProductToGLAccountMappingRepository; import org.apache.fineract.infrastructure.codes.domain.CodeValueRepository; import org.apache.fineract.infrastructure.core.api.JsonCommand; @@ -166,9 +167,9 @@ public class ShareProductToGLAccountMappingHelper extends ProductToGLAccountMapp case NONE: break; case CASH_BASED: - // asset - mergeSharesToAssetAccountMappingChanges(element, SharesProductAccountingParams.SHARES_REFERENCE.getValue(), sharesProductId, - CashAccountsForShares.SHARES_REFERENCE.getValue(), CashAccountsForShares.SHARES_REFERENCE.toString(), changes); + // asset -- the one optional mapping here, so unlike the three below it needs its own + // handling for the case the field is now absent (see mergeOrRemoveSharesReferenceMapping). + mergeOrRemoveSharesReferenceMapping(element, sharesProductId, changes); // income mergeSharesToIncomeAccountMappingChanges(element, SharesProductAccountingParams.INCOME_FROM_FEES.getValue(), @@ -194,4 +195,35 @@ public class ShareProductToGLAccountMappingHelper extends ProductToGLAccountMapp public void deleteSharesProductToGLAccountMapping(final Long sharesProductId) { deleteProductToGLAccountMapping(sharesProductId, PortfolioProductType.SHARES); } + + /** + * Handles the one optional share-product mapping, Shares Reference, on an edit that does not change + * the accounting rule. + * + *

+ * The other three mappings (Suspense, Income From Fees, Equity) are mandatory, so + * {@code mergeProductToAccountMappingChanges} only ever needs to handle "present and possibly + * changed" for them -- a null value there cannot happen without failing validation first. Shares + * Reference can legitimately be absent, and the shared merge helper (used across loan, savings and + * share products alike) only handles "present": it silently leaves an existing mapping row alone + * when the incoming value is null, rather than clearing it. Editing this one field out of an + * existing product would otherwise leave the product still carrying its old GL underneath, invisible + * on the edit form but still resolvable by any code path that looks it up directly. + */ + private void mergeOrRemoveSharesReferenceMapping(final JsonElement element, final Long sharesProductId, + final Map changes) { + final Long shareReferenceId = this.fromApiJsonHelper.extractLongNamed(SharesProductAccountingParams.SHARES_REFERENCE.getValue(), + element); + if (shareReferenceId != null) { + mergeSharesToAssetAccountMappingChanges(element, SharesProductAccountingParams.SHARES_REFERENCE.getValue(), sharesProductId, + CashAccountsForShares.SHARES_REFERENCE.getValue(), CashAccountsForShares.SHARES_REFERENCE.toString(), changes); + return; + } + final ProductToGLAccountMapping existing = this.accountMappingRepository.findCoreProductToFinAccountMapping(sharesProductId, + PortfolioProductType.SHARES.getValue(), CashAccountsForShares.SHARES_REFERENCE.getValue()); + if (existing != null) { + this.accountMappingRepository.delete(existing); + changes.put(SharesProductAccountingParams.SHARES_REFERENCE.getValue(), null); + } + } } diff --git a/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java b/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java index aaa9143aa..2c6e8888c 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java +++ b/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java @@ -1468,6 +1468,13 @@ public class AccountingProcessorHelper { accountMapping = paymentChannelSpecificAccountMapping; } } + // Matches getLinkedGLAccountForSavingsProduct's own guard just above: a share product can + // now genuinely have no SHARES_REFERENCE mapping (it stopped being a mandatory field), so + // this has to fail with a clean, named error instead of NPEing on accountMapping. + if (accountMapping == null) { + throw new ProductToGLAccountMappingNotFoundException(PortfolioProductType.SHARES, shareProductId, + CashAccountsForShares.fromInt(accountMappingTypeId).toString()); + } glAccount = accountMapping.getGlAccount(); } return glAccount; diff --git a/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/JournalEntryWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/JournalEntryWritePlatformServiceJpaRepositoryImpl.java index 195582cf4..93f8d9e80 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/JournalEntryWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/JournalEntryWritePlatformServiceJpaRepositoryImpl.java @@ -631,7 +631,12 @@ public class JournalEntryWritePlatformServiceJpaRepositoryImpl implements Journa final boolean cashBasedAccountingEnabled = (Boolean) accountingBridgeData.get("cashBasedAccountingEnabled"); final boolean accrualBasedAccountingEnabled = (Boolean) accountingBridgeData.get("accrualBasedAccountingEnabled"); - if (cashBasedAccountingEnabled) { + // A register that knows the transaction's real counterparty posts the entry itself, in one + // balanced pair against that account, and says so by wrapping the command in + // ShareAccountingSuppression.withOwnShareEntries(). Posting here as well would add a second + // entry for the same money, booked against the product's fixed Shares Reference account -- + // which is what that register is avoiding in the first place. + if (cashBasedAccountingEnabled && ShareAccountingSuppression.shouldPostShareEntries()) { final SharesDTO sharesDTO = this.helper.populateSharesDtoFromMap(accountingBridgeData, cashBasedAccountingEnabled, accrualBasedAccountingEnabled); final AccountingProcessorForShares accountingProcessorForShares = this.accountingProcessorForSharesFactory diff --git a/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/LedgerTransactionId.java b/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/LedgerTransactionId.java new file mode 100644 index 000000000..f07414670 --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/LedgerTransactionId.java @@ -0,0 +1,157 @@ +/** + * 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.accounting.journalentry.service; + +import java.sql.Date; +import java.time.LocalDate; +import java.time.format.DateTimeFormatter; +import org.apache.fineract.infrastructure.core.service.DateUtils; +import org.springframework.jdbc.core.JdbcTemplate; + +/** + * Mints the transaction id that ties the rows of one posting together in acc_gl_journal_entry. + * + *

+ * Every feature here that posts a journal entry by direct JDBC used to invent this id by hand, and the two + * shapes that emerged were each wrong in their own way. Some concatenated a business row's own primary key + * ({@code "DIVROUND" + dividendPayOutId}); a primary key is only unique while its sequence keeps climbing, + * so a reset hands the same number out twice - which is exactly how two unrelated share withdrawals came to + * share one transaction id and render as a single doubled voucher. Others concatenated + * {@code System.nanoTime()}, which is unique but says nothing to whoever reads it off a screen + * ({@code "CASHMIG-1-20260913-923935527932300"}). + * + *

+ * This generator replaces both with one readable shape, {@code PREFIX-YYMMDD-NNNN}: + * + *

+ *   SHWGL-260916-0001   the first share-withdrawal GL settlement posted on 16 Sep 2026
+ *   DIVRND-260916-0007  the seventh dividend rounding posted that day
+ * 
+ * + * The prefix names the event, the date is the tenant's business date, and the serial restarts at 0001 each + * day. That makes a transaction id self-describing: a reader can tell what it was and when it happened + * without looking anything up. + * + *

+ * Uniqueness does not depend on any business table. The serial comes from {@code m_ledger_txn_sequence}, a + * counter that belongs to no entity, so no entity's sequence can make it repeat. The counter is bumped by a + * single {@code INSERT ... ON CONFLICT DO UPDATE}, which takes and releases the row lock atomically, so two + * users posting at the same instant are serialised by the database rather than handed the same number - the + * same mechanism {@code m_voucher_sequence} already uses for voucher numbers. + * + *

+ * The counter alone would still repeat if that table were ever truncated or rewound by hand, which is + * precisely the accident being defended against, so the minted id is also checked against the ledger before + * being handed back and the counter is advanced past anything already in use. That makes a duplicate + * impossible by construction rather than by convention: the id is only returned once no row holds it. + * + *

+ * Ids already in the ledger keep their old shape. Nothing reconstructs a custom id to find its rows again + * (every call site mints one, posts with it, and never derives it a second time), so old and new shapes + * coexist without anything needing to read both. + * + *

+ * Postings made by Fineract's own command path are not covered here and keep their native + * {@code "S"/"L"/"SH"/"C" + transaction id} form, which {@code AccountingProcessorHelper} derives and + * several readers reconstruct. Those readers are hardened at the query instead, by resolving to the most + * recently posted rows rather than assuming the text is unique. + */ +public final class LedgerTransactionId { + + /** Guards against an impossible-in-practice runaway if the ledger and the counter disagree. */ + private static final int MAX_PROBES = 50; + + private static final DateTimeFormatter YYMMDD = DateTimeFormatter.ofPattern("yyMMdd"); + + private LedgerTransactionId() {} + + /** + * Returns the next transaction id for this kind of posting, e.g. {@code SHWGL-260916-0001}. + * + * @param prefix + * short uppercase code naming the event, from {@link LedgerTxnPrefix} + */ + public static String next(final JdbcTemplate jdbcTemplate, final String prefix) { + // Tenant timezone, not the server's - the day the counter resets on has to be the branch's + // business day, otherwise a late-evening posting lands in the wrong series. + final LocalDate today = LocalDate.now(DateUtils.getDateTimeZoneOfTenant()); + final String datePart = today.format(YYMMDD); + + for (int probe = 0; probe < MAX_PROBES; probe++) { + final int serial = nextSerial(jdbcTemplate, prefix, today); + final String candidate = String.format("%s-%s-%04d", prefix, datePart, serial); + if (!isTaken(jdbcTemplate, candidate)) { + return candidate; + } + // Only reachable if the counter was rewound behind the ledger by hand. Bumping again walks + // it forward until it clears whatever is already posted, which self-heals the counter. + } + throw new IllegalStateException("Could not mint a free ledger transaction id for prefix " + prefix + " after " + MAX_PROBES + + " attempts. The m_ledger_txn_sequence counter for this prefix is far behind the entries already posted today."); + } + + /** + * Returns the id for a posting that has to be reproducible, e.g. {@code SAVINT-260916-P0002}. + * + *

+ * Some postings are not one-shot: a pooled interest run pays hundreds of members, and a run that + * stops halfway is resumed rather than restarted. Those resumed legs belong to the posting that + * started them, so the id has to come out the same on the second pass - a counter would hand the + * remainder a different id and split one posting across two transactions. This builds the id from + * what identifies the run instead: the same prefix, date and discriminator always produce the same + * string, and no counter is consumed. + * + *

+ * Uniqueness rests on the caller's own discriminator being unique for that prefix and date (the + * product being posted, say), which is what makes the id reproducible in the first place. Use + * {@link #next} for everything else - a posting that happens once wants a counter, not a formula. + * The {@code P} marks the tail as a discriminator so it is not misread as a serial. + * + * @param date + * the date the posting belongs to, which is not always today - interest can be posted + * for an earlier day + * @param discriminator + * what makes this run distinct from another on the same date, e.g. the product id + */ + public static String forDatedSeries(final String prefix, final LocalDate date, final long discriminator) { + return String.format("%s-%s-P%04d", prefix, date.format(YYMMDD), discriminator); + } + + /** + * Reserves the next counter value for one (prefix, day) series. + * + * The INSERT ... ON CONFLICT DO UPDATE both creates the series on its first use of the day and + * increments it thereafter. Being a single statement it is atomic, so no explicit lock is needed. + */ + private static int nextSerial(final JdbcTemplate jdbcTemplate, final String prefix, final LocalDate day) { + return jdbcTemplate.queryForObject(""" + INSERT INTO m_ledger_txn_sequence (prefix, series_date, last_number) + VALUES (?,?,1) + ON CONFLICT (prefix, series_date) + DO UPDATE SET last_number = m_ledger_txn_sequence.last_number + 1 + RETURNING last_number + """, Integer.class, prefix, Date.valueOf(day)); + } + + private static boolean isTaken(final JdbcTemplate jdbcTemplate, final String transactionId) { + final Boolean taken = jdbcTemplate.queryForObject("SELECT EXISTS (SELECT 1 FROM acc_gl_journal_entry WHERE transaction_id = ?)", + Boolean.class, transactionId); + return Boolean.TRUE.equals(taken); + } +} diff --git a/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/LedgerTxnPrefix.java b/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/LedgerTxnPrefix.java new file mode 100644 index 000000000..d31549dbb --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/LedgerTxnPrefix.java @@ -0,0 +1,87 @@ +/** + * 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.accounting.journalentry.service; + +/** + * The event codes that open a transaction id minted by {@link LedgerTransactionId}. + * + *

+ * They live together in one place so a reader can decode any transaction id on a screen from a single list, + * and so no two features can drift into naming the same event differently - which had already happened: + * dividend forfeiture was posting under {@code DIVFORF} from one service and {@code DIVFORFEIT} from + * another, for the same event. + * + *

+ * Codes are uppercase, up to eight characters, and grouped by the module that raises them. A code is part of + * the audit trail once it has been posted, so treat these as append-only: add a new one for a new event + * rather than renaming an existing one, which would leave already-posted entries speaking a language nothing + * else uses any more. + */ +public final class LedgerTxnPrefix { + + private LedgerTxnPrefix() {} + + // A clerk's share withdrawal has no code here on purpose: it posts under the transaction id the + // native redeem command already minted ("SH" + share transaction id), because its debit leg is + // that command's own and the two have to group as one balanced entry. + + // ── Share tranche funding ───────────────────────────────────────────────── + /** Additional share tranche: reversal of the stock reference posting. */ + public static final String SHARE_TRANCHE_REFERENCE = "SHTREF"; + /** Additional share tranche funded from the member's savings account. */ + public static final String SHARE_TRANCHE_SAVINGS = "SHTSAV"; + + // ── Share collection and advances ───────────────────────────────────────── + /** Share application money collected. */ + public static final String SHARE_COLLECTION = "SHCOL"; + /** Share advance collected against a future allotment. */ + public static final String SHARE_ADVANCE = "SHADV"; + /** Share advance applied to an allotment. */ + public static final String SHARE_ADVANCE_APPLIED = "SHAPP"; + + // ── Dividend ────────────────────────────────────────────────────────────── + /** Dividend claim paid out to a member. */ + public static final String DIVIDEND_CLAIM = "DIVCLM"; + /** Dividend declaration posted. */ + public static final String DIVIDEND_DECLARED = "DIVDCL"; + /** Dividend declaration approved. */ + public static final String DIVIDEND_APPROVED = "DIVAPR"; + /** Dividend payout closed off. */ + public static final String DIVIDEND_CLOSED = "DIVCLS"; + /** Rounding difference on a dividend payout. */ + public static final String DIVIDEND_ROUNDING = "DIVRND"; + /** Unclaimed dividend forfeited. */ + public static final String DIVIDEND_FORFEIT = "DIVFRF"; + + // ── Branch day and cash ─────────────────────────────────────────────────── + /** Opening cash migrated into the branch day register. */ + public static final String CASH_MIGRATION = "CSHMIG"; + /** Rounding difference on a clerk's day closure. */ + public static final String CLOSURE_ROUNDING = "CLSRND"; + + // ── Year end ────────────────────────────────────────────────────────────── + /** Year-end transfer of income into the profit account. */ + public static final String YEAR_END_PROFIT = "YEPFT"; + /** Year-end closure of an income or expense account. */ + public static final String YEAR_END_CLOSE = "YECLS"; + + // ── Savings ─────────────────────────────────────────────────────────────── + /** Pooled savings interest posting for one product and date. */ + public static final String SAVINGS_INTEREST_POOLED = "SAVINT"; +} diff --git a/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/ShareAccountingSuppression.java b/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/ShareAccountingSuppression.java new file mode 100644 index 000000000..2efbd1239 --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/ShareAccountingSuppression.java @@ -0,0 +1,83 @@ +/** + * 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.accounting.journalentry.service; + +import java.util.function.Supplier; + +/** + * Lets a register that posts a share transaction's accounting itself tell the platform not to post its + * own. + * + *

Why this exists

+ * + *

+ * Fineract books every share transaction against a fixed "Shares Reference" account taken from the + * product's GL mapping: a redemption posts Dr Shares Equity / Cr Shares Reference, whatever the money + * actually did. That account is a placeholder for a counterparty the platform does not know, and in a + * real core banking ledger there is no such thing — a redemption paid in cash credits the cash + * account, and one settled by transfer credits the account it was transferred to. Naming a single + * asset account for both leaves every redemption sitting against a GL the money never touched, which + * then has to be corrected by hand. + * + *

+ * The governed clerk registers already know the real counterparty, because the clerk keyed it: the + * linked savings account, or the GL picked on the transfer's credit row. They post the redemption + * themselves, correctly, in one balanced entry. Before this flag existed they had to let the platform + * post its version first and then reverse the half that was wrong, which left a dead Cr Shares + * Reference row behind on every redemption and made the ledger read as though cash had moved. + * + *

Why a flag and not the product's accounting rule

+ * + *

+ * Turning the share product's accounting off entirely ({@code m_share_product.accounting_type = NONE}) + * would silence the platform everywhere, including the share account screen's own Redeem action, which + * no register covers — those redemptions would post nothing at all and go unnoticed, because that + * branch fails silently by design. Scoping the suppression to the one call that is about to post its + * own entry leaves every other path exactly as it is. + * + *

+ * A ThreadLocal rather than a request attribute, and restoring the previous value rather than clearing + * it, for the same reasons as {@code VoucherService.withOwnVoucher} — a batch job has no request to + * hang the flag on, and nesting must not switch the suppression off early for an outer caller. + */ +public final class ShareAccountingSuppression { + + private static final ThreadLocal CALLER_POSTS_SHARE_ENTRIES = ThreadLocal.withInitial(() -> Boolean.FALSE); + + private ShareAccountingSuppression() {} + + /** + * Runs {@code command} with the platform's own share journal entries suppressed, for a caller that + * posts the transaction's accounting itself. + */ + public static T withOwnShareEntries(final Supplier command) { + final boolean previous = CALLER_POSTS_SHARE_ENTRIES.get(); + CALLER_POSTS_SHARE_ENTRIES.set(Boolean.TRUE); + try { + return command.get(); + } finally { + CALLER_POSTS_SHARE_ENTRIES.set(previous); + } + } + + /** False only while a caller inside {@link #withOwnShareEntries} is posting its own entries. */ + public static boolean shouldPostShareEntries() { + return !CALLER_POSTS_SHARE_ENTRIES.get(); + } +} diff --git a/fineract-provider/src/main/java/org/apache/fineract/accounting/yearend/service/IncomeProfitTransferService.java b/fineract-provider/src/main/java/org/apache/fineract/accounting/yearend/service/IncomeProfitTransferService.java index 5bb6f385d..6873aa9eb 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/accounting/yearend/service/IncomeProfitTransferService.java +++ b/fineract-provider/src/main/java/org/apache/fineract/accounting/yearend/service/IncomeProfitTransferService.java @@ -12,6 +12,8 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.apache.fineract.accounting.common.DividendGlAccountResolver; import org.apache.fineract.accounting.glaccount.validation.GLAccountTransactionGuard; +import org.apache.fineract.accounting.journalentry.service.LedgerTransactionId; +import org.apache.fineract.accounting.journalentry.service.LedgerTxnPrefix; import org.apache.fineract.infrastructure.core.exception.PlatformDataIntegrityException; import org.apache.fineract.infrastructure.core.service.DateUtils; import org.springframework.dao.EmptyResultDataAccessException; @@ -117,7 +119,7 @@ public class IncomeProfitTransferService { final String currencyCode = resolveCurrency(incomeGlId); final LocalDateTime now = LocalDateTime.now(DateUtils.getDateTimeZoneOfTenant()); final LocalDate postingDate = range[1]; - final String txnId = "YEPROFIT-" + System.nanoTime(); + final String txnId = LedgerTransactionId.next(this.jdbcTemplate, LedgerTxnPrefix.YEAR_END_PROFIT); final String desc = "Year-end closing " + financialYear + " — " + incomeGl.get("name") + " to profit" + (note != null ? " | " + note : ""); @@ -210,7 +212,7 @@ public class IncomeProfitTransferService { final String currencyCode = resolveCurrency(glId); final LocalDateTime now = LocalDateTime.now(DateUtils.getDateTimeZoneOfTenant()); final LocalDate postingDate = range[1]; - final String txnId = "YECLOSE-" + financialYear + "-" + gl.get("gl_code") + "-" + System.nanoTime(); + final String txnId = LedgerTransactionId.next(this.jdbcTemplate, LedgerTxnPrefix.YEAR_END_CLOSE); final String descr = "Year-end closing " + financialYear + " — " + gl.get("name") + " to profit" + (note != null ? " | " + note : ""); 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 701e9e4e8..cbfc2ee8e 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 @@ -38,6 +38,8 @@ import java.util.Map; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.apache.fineract.accounting.glaccount.validation.GLAccountTransactionGuard; +import org.apache.fineract.accounting.journalentry.service.LedgerTransactionId; +import org.apache.fineract.accounting.journalentry.service.LedgerTxnPrefix; import org.apache.fineract.infrastructure.core.exception.PlatformDataIntegrityException; import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper; import org.apache.fineract.infrastructure.core.service.DateUtils; @@ -513,8 +515,7 @@ public class BranchDayRegisterApiResource { final Long contraId = contraIds.get(0); final String currencyCode = jdbcTemplate.queryForObject( "SELECT code FROM m_organisation_currency ORDER BY code LIMIT 1", String.class); - final String transactionId = "CASHMIG-" + officeId + "-" + businessDate.toString().replace("-", "") + "-" - + System.nanoTime(); + final String transactionId = LedgerTransactionId.next(jdbcTemplate, LedgerTxnPrefix.CASH_MIGRATION); insertJournalEntry(officeId, cashGlAccountId, currencyCode, transactionId, businessDate, 2, amount, currentUser); insertJournalEntry(officeId, contraId, currencyCode, transactionId, businessDate, 1, amount, currentUser); diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashtransfer/api/ShareAdvanceCollectionApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashtransfer/api/ShareAdvanceCollectionApiResource.java index b45a7fe52..c0c72ff3b 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashtransfer/api/ShareAdvanceCollectionApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashtransfer/api/ShareAdvanceCollectionApiResource.java @@ -24,6 +24,8 @@ import java.util.List; import java.util.Map; import lombok.RequiredArgsConstructor; import org.apache.fineract.accounting.glaccount.validation.GLAccountTransactionGuard; +import org.apache.fineract.accounting.journalentry.service.LedgerTransactionId; +import org.apache.fineract.accounting.journalentry.service.LedgerTxnPrefix; import org.apache.fineract.commands.domain.CommandWrapper; import org.apache.fineract.commands.service.CommandWrapperBuilder; import org.apache.fineract.commands.service.PortfolioCommandSourceWritePlatformService; @@ -150,7 +152,7 @@ public class ShareAdvanceCollectionApiResource { final long userId = currentUser.getId(); final LocalDateTime now = LocalDateTime.now(DateUtils.getDateTimeZoneOfTenant()); - final String txnId = "SHADV-" + System.nanoTime(); + final String txnId = LedgerTransactionId.next(this.jdbcTemplate, LedgerTxnPrefix.SHARE_ADVANCE); final String receiptNo = nextReceiptNo(); final String desc = "Share advance collected from " + clientName + " | Receipt " + receiptNo + (note != null ? " | " + note : ""); @@ -472,14 +474,26 @@ public class ShareAdvanceCollectionApiResource { final long userId = currentUser.getId(); final LocalDateTime now = LocalDateTime.now(DateUtils.getDateTimeZoneOfTenant()); final LocalDate entryDate = parseDate(getString(json, "transactionDate")); - final String txnId = "SHAPP-" + System.nanoTime(); + final String txnId = LedgerTransactionId.next(this.jdbcTemplate, LedgerTxnPrefix.SHARE_ADVANCE_APPLIED); final String accountNo = (String) acct.get("account_no"); - // Dr Sundry for the share-side receipts, including any over-payment on them — the liability - // those receipts created is cleared in full. A verified fee receipt is deliberately excluded: - // it was never in Sundry, so nothing here should debit Sundry for it. - insertEntry(officeId, glSuspense, DEBIT, receiptAmount, txnId, entryDate, shareAccountId, userId, now, currencyCode, - "Share application settled from Sundry | Receipt " + receiptNos + " | " + accountNo); + // Dr whichever account each receipt's money is actually sitting in, receipt by receipt, + // including any over-payment on them — the liability those receipts created is cleared in + // full. A verified fee receipt is deliberately excluded: its money was never held for the + // shares, so nothing here should move it. + // + // Read back from the collection's own posting rather than resolved from the product's Sundry + // mapping. The cashier does not always deposit into Sundry -- an entry fee goes straight to + // its income account, and instalments taken on different days can land in different accounts + // -- so debiting the mapped Sundry account for every receipt took money out of an account + // that had never received it, and left the account that had holding the balance for ever. + // Debiting each receipt's own account is also what the clerk sees: the GL picked against the + // voucher on the apply screen is the account being cleared here. + for (final Map row : collections) { + final Long receiptGl = receiptGlOf((String) row.get("gl_transaction_id"), glSuspense); + insertEntry(officeId, receiptGl, DEBIT, (BigDecimal) row.get("amount"), txnId, entryDate, shareAccountId, userId, now, + currencyCode, "Share application settled | Receipt " + row.get("receipt_no") + " | " + accountNo); + } // Cr Share Capital for the share value. insertEntry(officeId, glEquity, CREDIT, shareAmount, txnId, entryDate, shareAccountId, userId, now, currencyCode, @@ -809,6 +823,37 @@ public class ShareAdvanceCollectionApiResource { } } + /** + * The account a receipt's money is actually sitting in, read back from the posting that put it + * there — the credit leg of the collection's own journal transaction. + * + *

+ * Whoever took the money decided where it went: this register's own collection credits Sundry, a + * cashier deposit credits whichever account the cashier posted against, and an entry fee usually + * goes straight to fee income. Reading the account back is therefore the only way to know it, and + * it matches what the clerk picks against the voucher on the apply screen. + * + *

+ * Falls back to the product's mapped Sundry account for a collection with no posting recorded + * against it, which is where anything taken before this register started stamping its transaction + * id was held. + * + *

+ * Scoped to the live credit leg, newest first: a transaction id is only as unique as whatever + * minted it, and picking the most recent match keeps this correct even if one is ever reused. + */ + private Long receiptGlOf(final String glTransactionId, final Long fallbackGlId) { + if (glTransactionId == null || glTransactionId.isBlank()) { + return fallbackGlId; + } + final List rows = jdbcTemplate.queryForList(""" + SELECT account_id FROM acc_gl_journal_entry + WHERE transaction_id = ? AND type_enum = ? AND reversed = false + ORDER BY id DESC LIMIT 1 + """, Long.class, glTransactionId, CREDIT); + return rows.isEmpty() ? fallbackGlId : rows.get(0); + } + private Long requireGl(final Long productId, final int financialAccountType, final String label) { final Long glId = resolveGl(productId, financialAccountType); if (glId == null) { diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashtransfer/api/ShareCollectionApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashtransfer/api/ShareCollectionApiResource.java deleted file mode 100644 index 1ee09f3a3..000000000 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/cashtransfer/api/ShareCollectionApiResource.java +++ /dev/null @@ -1,439 +0,0 @@ -package org.apache.fineract.portfolio.cashtransfer.api; - -import com.google.gson.Gson; -import com.google.gson.JsonObject; -import com.google.gson.JsonParser; -import jakarta.ws.rs.Consumes; -import jakarta.ws.rs.GET; -import jakarta.ws.rs.POST; -import jakarta.ws.rs.Path; -import jakarta.ws.rs.PathParam; -import jakarta.ws.rs.Produces; -import jakarta.ws.rs.core.MediaType; -import jakarta.ws.rs.core.Response; -import java.math.BigDecimal; -import java.time.LocalDate; -import java.time.LocalDateTime; -import java.time.ZoneOffset; -import java.time.format.DateTimeParseException; -import java.util.ArrayList; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import lombok.RequiredArgsConstructor; -import org.apache.fineract.accounting.glaccount.validation.GLAccountTransactionGuard; -import org.apache.fineract.infrastructure.core.exception.PlatformDataIntegrityException; -import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; -import org.apache.fineract.useradministration.domain.AppUser; -import org.springframework.dao.EmptyResultDataAccessException; -import org.springframework.dao.IncorrectResultSizeDataAccessException; -import org.springframework.jdbc.core.JdbcTemplate; -import org.springframework.stereotype.Component; -import org.springframework.transaction.annotation.Transactional; -import org.apache.fineract.infrastructure.core.service.DateUtils; - -/** - * Cashier Share Collection API — POST /v1/cashier/sharecollection - * - * Records cash received from a member for share purchase and posts GL entries: - * Dr SHARES_REFERENCE (Cash at Vault) = shareAmount + chargeAmount - * Cr SHARES_SUSPENSE (Share Sundry) = shareAmount - * Cr INCOME_FROM_FEES = chargeAmount (if > 0) - * - * On account activation Fineract posts: - * Dr SHARES_SUSPENSE → Cr SHARES_EQUITY - * Activation skips charge GL if is_paid_derived = true (set here). - */ -@Path("/v1/cashier/sharecollection") -@Component -@RequiredArgsConstructor -@Consumes({ MediaType.APPLICATION_JSON }) -@Produces({ MediaType.APPLICATION_JSON }) -public class ShareCollectionApiResource { - - // ── acc_gl_journal_entry.type_enum ───────────────────────────────────── - // Fineract convention: 1 = CREDIT (Cr), 2 = DEBIT (Dr) - // Opposite of intuition — verified from Fineract AccountingProcessorHelper. - private static final int DEBIT = 2; - private static final int CREDIT = 1; - - // ── CashAccountsForShares financial_account_type ─────────────────────── - private static final int FA_SHARES_REFERENCE = 1; - private static final int FA_SHARES_SUSPENSE = 2; - private static final int FA_INCOME_FROM_FEES = 3; - - // ── acc_gl_journal_entry.entity_type_enum ───────────────────────────── - // Manual cashier entries use NULL — avoids NPE in PortfolioProductType.fromInt() - // which only handles values 1-5 and returns null for unknown values. - - // ── acc_product_mapping.product_type for shares ──────────────────────── - private static final int PRODUCT_TYPE_SHARES = 4; - - // ── m_share_account.status_enum ─────────────────────────────────────── - private static final int STATUS_SUBMITTED_PENDING = 100; - - // ── Maximum days a collection record is considered valid ────────────── - private static final int MAX_COLLECTION_AGE_DAYS = 30; - - private final PlatformSecurityContext context; - private final JdbcTemplate jdbcTemplate; - private final Gson gson = new Gson(); - - // ══════════════════════════════════════════════════════════════════════ - // POST — Collect cash for a share account - // ══════════════════════════════════════════════════════════════════════ - @POST - @Transactional - public Response collect(final String body) { - - final AppUser currentUser = this.context.authenticatedUser(); - currentUser.validateHasPermissionTo("CREATE_JOURNALENTRY"); - - // ── Parse request ────────────────────────────────────────────────── - if (body == null || body.isBlank()) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.request.empty", - "Request body is required."); - } - final JsonObject json; - try { - json = JsonParser.parseString(body).getAsJsonObject(); - } catch (Exception e) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.request.invalid", - "Request body must be valid JSON."); - } - - final Long shareAccountId = getLong(json, "shareAccountId"); - final String transactionDateStr = getString(json, "transactionDate"); - final BigDecimal shareAmount = getBigDecimal(json, "shareAmount", "shareAmount"); - final BigDecimal chargeAmountRaw = getBigDecimal(json, "chargeAmount", "chargeAmount"); - final String note = sanitiseNote(getString(json, "note")); - - // ── Validate inputs ──────────────────────────────────────────────── - if (shareAccountId == null) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.accountid.required", - "shareAccountId is required."); - } - if (shareAmount.compareTo(BigDecimal.ZERO) < 0) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.shareamount.negative", - "shareAmount cannot be negative."); - } - final BigDecimal safeCharge = chargeAmountRaw.compareTo(BigDecimal.ZERO) > 0 - ? chargeAmountRaw : BigDecimal.ZERO; - final BigDecimal totalAmount = shareAmount.add(safeCharge); - if (totalAmount.compareTo(BigDecimal.ZERO) <= 0) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.total.zero", - "Total collection amount must be greater than zero."); - } - - final LocalDate entryDate; - try { - entryDate = transactionDateStr != null - ? LocalDate.parse(transactionDateStr) : LocalDate.now(DateUtils.getDateTimeZoneOfTenant()); - } catch (DateTimeParseException e) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.date.invalid", - "transactionDate must be in YYYY-MM-DD format. Received: " + transactionDateStr); - } - - // ── Load account + client details ────────────────────────────────── - final Map acct = loadAccount(shareAccountId); - - // ── Account must be in 'Submitted and pending approval' ─────────── - final int statusEnum = toInt(acct.get("status_enum")); - if (statusEnum != STATUS_SUBMITTED_PENDING) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.account.invalid.status", - "Cash collection is only allowed for accounts in 'Submitted and pending approval' " - + "status. Account " + acct.get("account_no") + " has status code " + statusEnum + "."); - } - - // ── Prevent double collection ────────────────────────────────────── - if (Boolean.TRUE.equals(acct.get("cashier_collected"))) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.already.collected", - "Cash has already been collected for share account " - + acct.get("account_no") + ". Re-collection is not allowed."); - } - - final Long officeId = toLong(acct.get("office_id")); - final Long productId = toLong(acct.get("product_id")); - final String currencyCode = (String) acct.get("currency_code"); - final String clientName = (String) acct.get("client_name"); - final String accountNo = (String) acct.get("account_no"); - - // ── Resolve GL accounts from product mapping ─────────────────────── - final Long glReference = resolveGl(productId, FA_SHARES_REFERENCE); - final Long glSuspense = resolveGl(productId, FA_SHARES_SUSPENSE); - final Long glIncome = resolveGl(productId, FA_INCOME_FROM_FEES); - - if (glReference == null) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.gl.reference.notmapped", - "Share Reference (Cash at Vault) GL is not mapped for product " + productId - + ". Configure Share Product accounting mappings first."); - } - if (glSuspense == null) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.gl.suspense.notmapped", - "Share Suspense GL is not mapped for product " + productId - + ". Configure Share Product accounting mappings first."); - } - if (safeCharge.compareTo(BigDecimal.ZERO) > 0 && glIncome == null) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.gl.income.notmapped", - "Income from Fees GL is not mapped for product " + productId - + " but charge amount " + safeCharge + " is present. " - + "Configure Share Product accounting mappings first."); - } - - // ── Generate unique transaction ID ───────────────────────────────── - final String txnId = "SC-" + shareAccountId + "-" + System.currentTimeMillis(); - final long userId = currentUser.getId(); - final String collectorName = currentUser.getUsername(); - final LocalDateTime now = LocalDateTime.now(DateUtils.getDateTimeZoneOfTenant()); - - // ── Post GL journal entries ──────────────────────────────────────── - // Dr Cash at Vault (SHARES_REFERENCE) = totalAmount - insertEntry(officeId, glReference, DEBIT, totalAmount, txnId, - entryDate, shareAccountId, userId, now, currencyCode, - "Share collection received for " + accountNo - + (note != null ? " | " + note : "")); - - // Cr Share Sundry (SHARES_SUSPENSE) = shareAmount - insertEntry(officeId, glSuspense, CREDIT, shareAmount, txnId, - entryDate, shareAccountId, userId, now, currencyCode, - "Share amount held in suspense - " + accountNo); - - // Cr Income from Fees = chargeAmount (only if > 0) - if (safeCharge.compareTo(BigDecimal.ZERO) > 0) { - insertEntry(officeId, glIncome, CREDIT, safeCharge, txnId, - entryDate, shareAccountId, userId, now, currencyCode, - "Charge collected upfront for " + accountNo); - - // Mark charges paid → activation will skip duplicate charge GL entry - jdbcTemplate.update( - "UPDATE m_share_account_charge " - + "SET is_paid_derived = true, " - + " amount_paid_derived = amount, " - + " amount_outstanding_derived = 0 " - + "WHERE account_id = ? AND is_active = true AND is_paid_derived = false", - shareAccountId); - } - - // ── Flip cashier_collected flag ──────────────────────────────────── - jdbcTemplate.update( - "UPDATE m_share_account SET cashier_collected = true WHERE id = ?", - shareAccountId); - - // ── Insert audit / validation record ────────────────────────────── - jdbcTemplate.update( - "INSERT INTO m_share_cashier_collection " - + "(share_account_id, collected_by_user, collected_by_name, collection_date, " - + " share_amount, charge_amount, total_amount, gl_transaction_id, " - + " office_id, currency_code, note) " - + "VALUES (?,?,?,?,?,?,?,?,?,?,?)", - shareAccountId, userId, collectorName, - java.sql.Date.valueOf(entryDate), - shareAmount, safeCharge, totalAmount, - txnId, officeId, currencyCode, note); - - // ── Build GL entries summary for response ────────────────────────── - final List> glEntries = new ArrayList<>(); - glEntries.add(glEntryMap(resolveGlName(glReference), "Dr", totalAmount, currencyCode)); - glEntries.add(glEntryMap(resolveGlName(glSuspense), "Cr", shareAmount, currencyCode)); - if (safeCharge.compareTo(BigDecimal.ZERO) > 0) { - glEntries.add(glEntryMap(resolveGlName(glIncome), "Cr", safeCharge, currencyCode)); - } - - // ── Response ─────────────────────────────────────────────────────── - final Map resp = new LinkedHashMap<>(); - resp.put("transactionId", txnId); - resp.put("shareAccountId", shareAccountId); - resp.put("accountNo", accountNo); - resp.put("clientName", clientName); - resp.put("collectionDate", entryDate.toString()); - resp.put("collectedBy", collectorName); - resp.put("shareAmount", shareAmount); - resp.put("chargeAmount", safeCharge); - resp.put("totalAmount", totalAmount); - resp.put("currencyCode", currencyCode); - resp.put("glEntries", glEntries); - - return Response.ok(gson.toJson(resp)).build(); - } - - // ══════════════════════════════════════════════════════════════════════ - // GET — Check collection status for a share account - // ══════════════════════════════════════════════════════════════════════ - @GET - @Path("/{shareAccountId}") - public Response getStatus(@PathParam("shareAccountId") final Long shareAccountId) { - this.context.authenticatedUser(); - - try { - final Map rec = jdbcTemplate.queryForMap( - "SELECT scc.share_account_id, sa.account_no, " - + " mc.display_name AS client_name, " - + " scc.collected_by_name, scc.collection_date, " - + " scc.share_amount, scc.charge_amount, scc.total_amount, " - + " scc.gl_transaction_id, scc.currency_code, scc.note " - + "FROM m_share_cashier_collection scc " - + "JOIN m_share_account sa ON sa.id = scc.share_account_id " - + "JOIN m_client mc ON mc.id = sa.client_id " - + "WHERE scc.share_account_id = ?", - shareAccountId); - - final Map resp = new LinkedHashMap<>(); - resp.put("collected", true); - resp.put("shareAccountId", shareAccountId); - resp.put("accountNo", rec.get("account_no")); - resp.put("clientName", rec.get("client_name")); - resp.put("collectedBy", rec.get("collected_by_name")); - resp.put("collectionDate", rec.get("collection_date") != null - ? rec.get("collection_date").toString() : null); - resp.put("shareAmount", rec.get("share_amount")); - resp.put("chargeAmount", rec.get("charge_amount")); - resp.put("totalAmount", rec.get("total_amount")); - resp.put("transactionId", rec.get("gl_transaction_id")); - resp.put("currencyCode", rec.get("currency_code")); - resp.put("note", rec.get("note")); - return Response.ok(gson.toJson(resp)).build(); - - } catch (EmptyResultDataAccessException e) { - final Map resp = new LinkedHashMap<>(); - resp.put("collected", false); - resp.put("shareAccountId", shareAccountId); - return Response.ok(gson.toJson(resp)).build(); - } - } - - // ══════════════════════════════════════════════════════════════════════ - // Private helpers - // ══════════════════════════════════════════════════════════════════════ - - private Map loadAccount(final Long shareAccountId) { - try { - return jdbcTemplate.queryForMap( - "SELECT sa.id, sa.account_no, sa.status_enum, sa.product_id, " - + " sa.cashier_collected, sa.currency_code, " - + " mc.office_id, mc.display_name AS client_name " - + "FROM m_share_account sa " - + "JOIN m_client mc ON mc.id = sa.client_id " - + "WHERE sa.id = ?", - shareAccountId); - } catch (EmptyResultDataAccessException e) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.account.notfound", - "Share account with id " + shareAccountId + " does not exist."); - } - } - - private Long resolveGl(final Long productId, final int financialAccountType) { - try { - return jdbcTemplate.queryForObject( - "SELECT gl_account_id FROM acc_product_mapping " - + "WHERE product_id = ? AND product_type = ? AND financial_account_type = ?", - Long.class, productId, PRODUCT_TYPE_SHARES, financialAccountType); - } catch (IncorrectResultSizeDataAccessException e) { - // covers both EmptyResultDataAccessException (0 rows) and multiple rows - return null; - } - } - - private String resolveGlName(final Long glAccountId) { - if (glAccountId == null) return "Unknown GL"; - try { - return jdbcTemplate.queryForObject( - "SELECT name FROM acc_gl_account WHERE id = ?", - String.class, glAccountId); - } catch (EmptyResultDataAccessException e) { - return "GL-" + glAccountId; - } - } - - private void insertEntry(final Long officeId, final Long glAccountId, - final int typeEnum, final BigDecimal amount, final String txnId, - final LocalDate entryDate, final Long shareAccountId, - final long userId, final LocalDateTime now, final String currencyCode, - final String description) { - GLAccountTransactionGuard.assertPostable(jdbcTemplate, glAccountId); - jdbcTemplate.update( - "INSERT INTO acc_gl_journal_entry " - + "(account_id, office_id, currency_code, transaction_id, reversed, manual_entry, " - + " entry_date, type_enum, amount, description, entity_id, " - + " created_by, last_modified_by, created_date, lastmodified_date, " - + " is_running_balance_calculated, office_running_balance, " - + " organization_running_balance, submitted_on_date, transaction_date, " - + " created_on_utc, last_modified_on_utc) " - + "VALUES (?,?,?,?,false,true,?,?,?,?,?,?,?,?,?,false,0,0,?,?,?,?)", - glAccountId, officeId, currencyCode, txnId, - java.sql.Date.valueOf(entryDate), typeEnum, amount, description, - shareAccountId, // entity_id — links entry to the share account - userId, userId, now, now, - java.sql.Date.valueOf(entryDate), - java.sql.Date.valueOf(entryDate), - toUtc(now), - toUtc(now)); - } - - /** - * A tenant-local timestamp converted to the real UTC instant, for the {@code *_on_utc} columns. - * - * {@code atOffset(ZoneOffset.UTC)} would only relabel it — stamping 18:34 IST as "18:34 UTC", which reads back as - * 00:04 the next day and made an evening posting appear as just after midnight. The instant has to be resolved in - * the tenant's own zone first, which is the zone {@code now} was taken in. - */ - private java.time.OffsetDateTime toUtc(final LocalDateTime tenantLocalTime) { - return tenantLocalTime.atZone(DateUtils.getDateTimeZoneOfTenant()).toOffsetDateTime() - .withOffsetSameInstant(ZoneOffset.UTC); - } - - private Map glEntryMap(final String account, final String type, - final BigDecimal amount, final String currency) { - final Map m = new LinkedHashMap<>(); - m.put("account", account); - m.put("type", type); - m.put("amount", amount); - m.put("currency", currency); - return m; - } - - private String sanitiseNote(final String raw) { - if (raw == null || raw.isBlank()) return null; - final String trimmed = raw.trim(); - return trimmed.length() > 490 ? trimmed.substring(0, 490) : trimmed; - } - - private Long toLong(final Object o) { - return o instanceof Number ? ((Number) o).longValue() : null; - } - - private int toInt(final Object o) { - return o instanceof Number ? ((Number) o).intValue() : 0; - } - - private Long getLong(final JsonObject json, final String key) { - return json.has(key) && !json.get(key).isJsonNull() ? json.get(key).getAsLong() : null; - } - - private BigDecimal getBigDecimal(final JsonObject json, final String key, - final String fieldLabel) { - if (!json.has(key) || json.get(key).isJsonNull()) return BigDecimal.ZERO; - try { - return json.get(key).getAsBigDecimal(); - } catch (NumberFormatException e) { - throw new PlatformDataIntegrityException( - "error.msg.sharecollection.field.invalid.number", - "Field '" + fieldLabel + "' must be a valid number."); - } - } - - private String getString(final JsonObject json, final String key) { - return json.has(key) && !json.get(key).isJsonNull() ? json.get(key).getAsString() : null; - } -} diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkClosureRequestApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkClosureRequestApiResource.java index e8f286fb1..5495eee7b 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkClosureRequestApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkClosureRequestApiResource.java @@ -50,6 +50,8 @@ import java.util.Map; import java.util.Optional; import lombok.RequiredArgsConstructor; import org.apache.fineract.accounting.glaccount.validation.GLAccountTransactionGuard; +import org.apache.fineract.accounting.journalentry.service.LedgerTransactionId; +import org.apache.fineract.accounting.journalentry.service.LedgerTxnPrefix; import org.apache.fineract.commands.domain.CommandWrapper; import org.apache.fineract.commands.service.CommandWrapperBuilder; import org.apache.fineract.commands.service.PortfolioCommandSourceWritePlatformService; @@ -856,7 +858,7 @@ public class ClerkClosureRequestApiResource { // Same business date as the closure this is adjusting, so the two never land on // different days in the day book. final LocalDate entryDate = DateUtils.getBusinessLocalDate(); - final String transactionId = "CLOSEROUND-" + officeId + "-" + entryDate.toString().replace("-", "") + "-" + System.nanoTime(); + final String transactionId = LedgerTransactionId.next(jdbcTemplate, LedgerTxnPrefix.CLOSURE_ROUNDING); final Long cashGlId = ((Number) cashGl.get("id")).longValue(); // type_enum: 2 = DEBIT, 1 = CREDIT. diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkShareWithdrawalRequestApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkShareWithdrawalRequestApiResource.java index e2db00cf3..dc9690edf 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkShareWithdrawalRequestApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkShareWithdrawalRequestApiResource.java @@ -38,18 +38,23 @@ import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import java.math.BigDecimal; import java.sql.Date; +import java.sql.PreparedStatement; import java.sql.Timestamp; import java.time.Instant; import java.time.LocalDate; import java.time.LocalDateTime; +import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; +import java.util.UUID; import lombok.RequiredArgsConstructor; import org.apache.fineract.accounting.glaccount.validation.GLAccountTransactionGuard; +import org.apache.fineract.accounting.journalentry.service.ShareAccountingSuppression; import org.apache.fineract.commands.domain.CommandWrapper; import org.apache.fineract.commands.service.CommandWrapperBuilder; import org.apache.fineract.commands.service.PortfolioCommandSourceWritePlatformService; @@ -64,6 +69,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.support.GeneratedKeyHolder; +import org.springframework.jdbc.support.KeyHolder; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; @@ -104,22 +111,25 @@ public class ClerkShareWithdrawalRequestApiResource { "settlement_amount", "settlement_savings_transaction_id", "settlement_row_snapshot", "settlement_mode", "settlement_gl_account_id" }; - // "Share Redemption Transfer" (a dedicated, teller-hidden payment type) was retired in favour of - // the plain "Transfer" type every settlement now uses -- this fallback code name follows it, so a - // pre-settlement_payment_type_id-column legacy request still resolves to a real payment type - // instead of silently posting its settlement with none at all. - private static final String SHARE_REDEMPTION_PAYMENT_TYPE_CODE = "TRANSFER"; - - // ── Sundry pass-through for the redemption settlement ─────────────────── + // ── GL posting for the redemption and its settlement ──────────────────── // Fineract convention: 1 = CREDIT, 2 = DEBIT. private static final int CREDIT = 1; private static final int DEBIT = 2; + // m_savings_account.status_enum: 300 = active. + private static final int SAVINGS_STATUS_ACTIVE = 300; + // acc_product_mapping.financial_account_type for share products. - private static final int FA_SHARES_REFERENCE = 1; private static final int FA_SHARES_SUSPENSE = 2; + private static final int FA_INCOME_FROM_FEES = 3; + private static final int FA_SHARES_EQUITY = 4; private static final int PRODUCT_TYPE_SHARES = 4; + // What Fineract's own bridge prefixes a share transaction's journal entries with + // (AccountingProcessorHelper.SHARE_TRANSACTION_IDENTIFIER). Reused so an entry this class posts + // in the platform's place groups under the id everything else already looks for. + private static final String SHARE_TXN_ID_PREFIX = "SH"; + // acc_product_mapping for savings: product_type 2, financial_account_type 2 = Savings Control. private static final int PRODUCT_TYPE_SAVINGS = 2; private static final int FA_SAVINGS_CONTROL = 2; @@ -146,8 +156,8 @@ public class ClerkShareWithdrawalRequestApiResource { final Long settlementPaymentTypeId = fromJsonHelper.extractLongNamed("settlementPaymentTypeId", root); // 'SAVINGS' (default, and what every request submitted before this field existed carries) // settles by book-transfer to the linked savings account; 'GL' settles by book-transfer to - // whichever GL account the clerk picked on the CR row instead (a Sundry-style settlement that - // never touches a member's own account) — see postGlSettlement() in approve(). 'NONE' is not + // whichever GL account the clerk picked on the CR row instead (a settlement that never + // touches a member's own account) — see the settlement branch in approve(). 'NONE' is not // accepted from the client at all; it is what 'GL' downgrades to below when the clerk's own // chosen GL account turns out to be the redemption's own Sundry account. final String settlementModeRaw = fromJsonHelper.extractStringNamed("settlementMode", root); @@ -189,10 +199,10 @@ public class ClerkShareWithdrawalRequestApiResource { throw apiError(400, "Settlement GL Account is required for a GL settlement"); } GLAccountTransactionGuard.assertPostable(jdbcTemplate, settlementGlAccountId); - // The redemption's own leg 1 (routeRedeemIntoSundry(), unconditional) always credits the - // product's own Sundry GL -- picking that same account as the settlement target here - // would make postGlSettlement() debit and credit the identical account, a self-cancelling - // entry with no real movement that no real CBS ledger allows. Rather than reject this + // A redemption that settles nowhere further credits the product's own Sundry GL -- + // picking that same account as the settlement target here would name Sundry as both the + // source and the destination of the one entry, a self-cancelling posting with no real + // movement that no real CBS ledger allows. Rather than reject this // outright, treat it as what the clerk actually means: "stop here, the value stays in // Sundry" -- downgrade to 'NONE' so approve() posts nothing further and raises a voucher // for exactly leg 1 (Dr Share / Cr Sundry) on its own, standalone. @@ -524,8 +534,8 @@ public class ClerkShareWithdrawalRequestApiResource { // Only meaningful for a SAVINGS settlement: creditLinkedSavingsAccount() posts through a // plain savings deposit command, which resolves its GL leg from the savings product's own // Fund Source mapping -- the exact bug class this check exists to catch. A GL settlement - // never goes near that mapping at all; postGlSettlement() resolves Sundry and the clerk's - // chosen GL account directly, so this check is reported not-applicable instead. + // never goes near that mapping at all; it credits the clerk's chosen GL account directly, + // so this check is reported not-applicable instead. if (glSettlementMode) { final Object glAccountIdObj = record.get("settlement_gl_account_id"); final boolean settlementGlPresent = glAccountIdObj != null; @@ -709,7 +719,13 @@ public class ClerkShareWithdrawalRequestApiResource { final CommandWrapper commandRequest = new CommandWrapperBuilder().createAccountCommand("share", shareAccountId, "redeemshares") .withJson(payload.toString()).build(); - final CommandProcessingResult result = commandsSourceWritePlatformService.logCommandSource(commandRequest); + // The command is run for what it does to the share account -- the holding it reduces and the + // transaction it records -- not for its accounting. Left to itself it would credit the + // product's fixed "Shares Reference" account, which is a stand-in for a counterparty it has + // no way of knowing; this register does know it, because the clerk keyed it, and posts the + // entry itself below against the account the money actually reached. + final CommandProcessingResult result = ShareAccountingSuppression + .withOwnShareEntries(() -> commandsSourceWritePlatformService.logCommandSource(commandRequest)); // redeemShares() puts the newly-created share transaction's id under "requestedShares" in // the changes map — NOT in getResourceId(), which for this command is the share ACCOUNT id // instead. Reading getResourceId() here would silently look up the wrong row downstream. @@ -717,52 +733,65 @@ public class ClerkShareWithdrawalRequestApiResource { final Long shareTransactionId = shareTxnIdRaw instanceof Number ? ((Number) shareTxnIdRaw).longValue() : null; final String shareTxnId = shareTransactionId != null ? shareTransactionId.toString() : null; - // Redirects the native redeem posting's Cr Shares Reference (Cash at Vault) leg into - // Cr Sundry instead — see routeRedeemIntoSundry() for why: every other way money reaches or - // leaves share capital in this codebase passes through Sundry, and leaving redeem on its own - // "give cash back over the counter" assumption breaks that the moment the settlement below - // lands on a different GL than this leg did. - if (shareTransactionId != null) { - routeRedeemIntoSundry(shareAccountId, shareTransactionId, LocalDate.parse(requestedDateObj.toString().substring(0, 10)), - currentUser.getId()); - } - - // Settlement: within this SAME approval transaction, immediately credit the redemption value - // to the member's linked savings account, routed Dr Sundry / Cr Savings Control -- explicitly, - // not by relying on the settlement payment type's own GL Fund Source mapping to coincidentally - // land on the same account the leg above credited. See creditLinkedSavingsAccount(). - final Object storedSettlementPaymentTypeId = record.get("settlement_payment_type_id"); - final Long settlementPaymentTypeId = storedSettlementPaymentTypeId instanceof Number - ? ((Number) storedSettlementPaymentTypeId).longValue() - : null; + // Settlement: within this SAME approval transaction, the redemption value is booked to + // wherever the clerk said it should go. Where that destination is known here -- a linked + // savings account, or a GL the clerk picked -- the native redeem posting's own Cr Shares + // Reference (Cash at Vault) leg is redirected straight onto it below, so the ledger reads as + // the single entry the clerk actually keyed: Dr Share Capital / Cr . final String settlementMode = Optional.ofNullable((String) record.get("settlement_mode")).orElse("SAVINGS"); final Settlement settlement; final GlSettlement glSettlement; final String settlementNote; + // Where the redemption credit lands, and what to call it on the journal line. Resolved per + // mode, then applied once below. + final Long destinationGl; + final String destinationNarration; if ("NONE".equals(settlementMode)) { - // The clerk deliberately chose to stop at leg 1 -- the redemption value stays in Share - // Sundry (already credited by routeRedeemIntoSundry() above), with no second leg routing - // it anywhere else. raiseSundryOnlyVoucher() below gives that leg its own standalone + // The clerk deliberately chose to settle nowhere further: Share Sundry is the + // destination, not a staging post, and the value genuinely rests there until some later + // entry moves it. raiseSundryOnlyVoucher() below gives that entry its own standalone // voucher instead of leaving it visible only as "Also posted" under some other voucher. settlement = null; glSettlement = null; + final Map noneAcct = loadShareAccountGlContext(shareAccountId); + destinationGl = requireShareGl(toLong(noneAcct.get("product_id")), FA_SHARES_SUSPENSE, "Share Sundry (Suspense)"); + destinationNarration = "Share redemption held in Sundry pending settlement"; settlementNote = "Redemption booked to Share Sundry — no further settlement leg."; } else if ("GL".equals(settlementMode)) { settlement = null; final Object glAccountIdObj = record.get("settlement_gl_account_id"); final Long settlementGlAccountId = glAccountIdObj instanceof Number ? ((Number) glAccountIdObj).longValue() : null; - glSettlement = postGlSettlement(shareAccountId, shareTransactionId, requestedDateObj.toString(), settlementGlAccountId); + glSettlement = resolveGlSettlement(shareTransactionId, settlementGlAccountId); + destinationGl = glSettlement != null ? glSettlement.glAccountId() : null; + destinationNarration = "Share redemption settled to GL account"; settlementNote = glSettlement != null ? "Settled " + glSettlement.amount() + " to GL account " + glCodeOf(glSettlement.glAccountId()) : "Share transaction not found — GL settlement could not be booked."; } else { glSettlement = null; - settlement = creditLinkedSavingsAccount(shareAccountId, shareTransactionId, requestedDateObj.toString(), resolutionNumber, - settlementPaymentTypeId); + settlement = creditLinkedSavingsAccount(shareAccountId, shareTransactionId, requestedDateObj.toString(), resolutionNumber); + destinationGl = settlement != null ? settlement.savingsControlGlId() : null; + destinationNarration = "Share redemption settled to savings"; settlementNote = settlement != null ? "Settled " + settlement.amount() + " to linked savings account (txn #" + settlement.savingsTransactionId() + ")" : "No linked savings account on this share account — settlement must be completed manually."; } + + // One posting, not two: the native redeem's Cr Cash at Vault leg is replaced by a credit to + // the destination itself, under the native posting's own transaction_id. The ledger then + // holds exactly the entry the clerk keyed -- Dr Share Capital / Cr -- instead + // of routing it through Sundry and back out again, which left two transaction groups and a + // pass-through balance that always netted to zero. + // + // Where no destination could be resolved (no linked savings account, or the share + // transaction could not be read back), the native Cr leg is left exactly as Fineract posted + // it rather than being reversed with nothing put in its place, which would leave the Dr + // Share Capital leg standing alone and the entry out of balance. + if (shareTransactionId != null && destinationGl != null) { + postRedemptionEntry(shareAccountId, shareTransactionId, LocalDate.parse(requestedDateObj.toString().substring(0, 10)), + currentUser.getId(), destinationGl, destinationNarration); + } + final String updatedLog = buildActivityLog((String) record.get("activity_log"), "APPROVED", currentUser.getUsername(), settlementNote); @@ -791,8 +820,8 @@ public class ClerkShareWithdrawalRequestApiResource { // redemption settled into a member's savings account left nothing on the voucher register to // read it back from. The GL entries were always written — it is the slip that was missing. final String voucherNo = "NONE".equals(settlementMode) ? raiseSundryOnlyVoucher(id, shareTransactionId, currentUser) - : "GL".equals(settlementMode) ? raiseGlShareWithdrawalVoucher(id, shareAccountId, record, glSettlement, currentUser) - : raiseShareWithdrawalVoucher(id, shareAccountId, record, settlement, currentUser); + : "GL".equals(settlementMode) ? raiseGlShareWithdrawalVoucher(id, shareTransactionId, record, glSettlement, currentUser) + : raiseShareWithdrawalVoucher(id, shareAccountId, shareTransactionId, record, settlement, currentUser); final JsonObject res = new JsonObject(); res.addProperty("resourceId", id); @@ -836,7 +865,11 @@ public class ClerkShareWithdrawalRequestApiResource { closePayload.addProperty("note", "Closed automatically — all shares withdrawn on approval of this Share Withdrawal Request."); final CommandWrapper closeCommand = new CommandWrapperBuilder().createAccountCommand("share", shareAccountId, "close") .withJson(closePayload.toString()).build(); - commandsSourceWritePlatformService.logCommandSource(closeCommand); + // Suppressed for the same reason as the redemption above: anything this posts would go + // against the product's fixed "Shares Reference" account. There is nothing left for it to + // post in any case -- this runs only once the holding has reached zero, so the redemption + // that emptied it has already been booked, in full, against the account the money reached. + ShareAccountingSuppression.withOwnShareEntries(() -> commandsSourceWritePlatformService.logCommandSource(closeCommand)); return true; } @@ -853,17 +886,18 @@ public class ClerkShareWithdrawalRequestApiResource { * savings credit have all already succeeded in this transaction, and losing the slip is worth * less than rolling back a member's money. */ - private String raiseShareWithdrawalVoucher(final Long requestId, final Long shareAccountId, final Map record, - final Settlement settlement, final AppUser currentUser) { + private String raiseShareWithdrawalVoucher(final Long requestId, final Long shareAccountId, final Long shareTransactionId, + final Map record, final Settlement settlement, final AppUser currentUser) { if (!voucherService.shouldRaiseVoucher() || settlement == null || settlement.amount() == null || settlement.amount().compareTo(BigDecimal.ZERO) <= 0) { return null; } try { - final Long productId = jdbcTemplate.queryForObject("SELECT product_id FROM m_share_account WHERE id = ?", Long.class, - shareAccountId); - final Long glSundry = resolveGl(productId, FA_SHARES_SUSPENSE); + final Long glShare = redeemDebitGlOf(shareTransactionId); + if (glShare == null) { + return null; + } final Long savingsAccountId = jdbcTemplate .queryForList("SELECT savings_account_id FROM m_share_account WHERE id = ?", Long.class, shareAccountId).stream() @@ -875,7 +909,7 @@ public class ClerkShareWithdrawalRequestApiResource { .stream().findFirst().orElse(null); final List lines = new ArrayList<>(); - lines.add(VoucherLine.debitGl(glSundry, glNameOf(glSundry), glCodeOf(glSundry), settlement.amount())); + lines.add(VoucherLine.debitGl(glShare, glNameOf(glShare), glCodeOf(glShare), settlement.amount())); // This leg is the member's own savings account receiving the redemption proceeds, not a // second GL posting -- creditGl(glSavingsControl, ...) named the shared Savings Control GL // account here, mismatched against the member's own account number on the same line. @@ -905,24 +939,52 @@ public class ClerkShareWithdrawalRequestApiResource { } } - private record Settlement(BigDecimal amount, Long savingsTransactionId) {} + private record Settlement(BigDecimal amount, Long savingsTransactionId, Long savingsControlGlId) {} private record GlSettlement(BigDecimal amount, Long glAccountId) {} /** - * Books the redemption value to whichever GL account the clerk picked on the CR row of the Fund - * Transfer, routed Dr Share Sundry / Cr that GL account — the GL counterpart to - * {@link #creditLinkedSavingsAccount} for a settlement that never touches a member's own account - * (e.g. crediting a "DD Payable" holding account, so a second, independent manual entry can later - * move the value out to the actual demand draft). Posted in full at approval time, exactly like - * the Sundry leg before it. + * The GL account the native redeem command debited — the share capital side of the redemption, + * which is what a voucher's Dr line has to name now that the credit goes straight to its + * destination and no Sundry leg stands between them. + * + *

+ * Read back from what was actually posted rather than resolved from a mapping constant: this + * class has never posted that leg itself (only Fineract's own redeem command does), so reading it + * back is correct whichever GL the product's Shares Equity mapping happens to point at. + * + *

+ * Scoped to the live Dr row for this share transaction, newest first: the "SH"+id transaction_id + * text is only unique so long as {@code m_share_account_transactions.id} never repeats, which a + * reset id sequence in a dev environment can violate. + */ + private Long redeemDebitGlOf(final Long shareTransactionId) { + if (shareTransactionId == null) { + return null; + } + return jdbcTemplate.queryForList(""" + SELECT account_id FROM acc_gl_journal_entry + WHERE share_transaction_id = ? AND type_enum = ? AND reversed = false + ORDER BY id DESC LIMIT 1 + """, Long.class, shareTransactionId, DEBIT).stream().findFirst().orElse(null); + } + + /** + * Reads back the amount to settle and names the GL account the clerk picked on the CR row of the + * Fund Transfer — the GL counterpart to {@link #creditLinkedSavingsAccount} for a settlement that + * never touches a member's own account (e.g. crediting a "DD Payable" holding account, so a + * second, independent manual entry can later move the value out to the actual demand draft). + * + *

+ * Nothing is posted here. The caller redirects the native redeem posting's own credit leg onto + * this account, so the redemption reads as the one entry the clerk keyed — Dr Share Capital / Cr + * that GL — rather than as a Sundry hop that nets to zero. * *

* Unlike a cash settlement, nothing physical changes hands here, so the amount is not rounded — * the redemption value posts to the paisa, same as the linked-savings path. */ - private GlSettlement postGlSettlement(final Long shareAccountId, final Long shareTransactionId, final String requestedDateStr, - final Long settlementGlAccountId) { + private GlSettlement resolveGlSettlement(final Long shareTransactionId, final Long settlementGlAccountId) { if (shareTransactionId == null) { return null; } @@ -935,55 +997,36 @@ public class ClerkShareWithdrawalRequestApiResource { if (row == null) { return null; } - final BigDecimal settlementAmount = (BigDecimal) row.get("settlementAmount"); - - final Map acct = loadShareAccountGlContext(shareAccountId); - final Long productId = toLong(acct.get("product_id")); - final Long officeId = toLong(acct.get("office_id")); - final String currencyCode = (String) acct.get("currency_code"); - final String accountNo = (String) acct.get("account_no"); - - final Long glSundry = requireShareGl(productId, FA_SHARES_SUSPENSE, "Share Sundry (Suspense)"); - - final LocalDate entryDate = LocalDate.parse(requestedDateStr.substring(0, 10)); - final long userId = context.authenticatedUser().getId(); - final LocalDateTime now = LocalDateTime.now(DateUtils.getDateTimeZoneOfTenant()); - final String txnId = "SHRGLSTL-" + System.nanoTime(); - - insertEntry(officeId, glSundry, DEBIT, settlementAmount, txnId, entryDate, shareAccountId, userId, now, currencyCode, - "Share redemption settled to GL account | " + accountNo); - insertEntry(officeId, settlementGlAccountId, CREDIT, settlementAmount, txnId, entryDate, shareAccountId, userId, now, currencyCode, - "Share redemption settlement credited to GL account | " + accountNo); - - return new GlSettlement(settlementAmount, settlementGlAccountId); + return new GlSettlement((BigDecimal) row.get("settlementAmount"), settlementGlAccountId); } /** - * The voucher for an approved GL redemption: Dr Share Sundry, Cr the clerk-chosen settlement GL - * account — the same two accounts {@link #postGlSettlement} actually posted to, both named and - * coded from the ledger since both are real GL accounts here (unlike the linked-savings path, - * there is no member-account leg on this side at all). + * The voucher for an approved GL redemption: Dr Share Capital, Cr the clerk-chosen settlement + * GL account — the same two accounts the redemption actually posted to, both named and coded + * from the ledger since both are real GL accounts here (unlike the linked-savings path, there is + * no member-account leg on this side at all). */ - private String raiseGlShareWithdrawalVoucher(final Long requestId, final Long shareAccountId, final Map record, + private String raiseGlShareWithdrawalVoucher(final Long requestId, final Long shareTransactionId, final Map record, final GlSettlement glSettlement, final AppUser currentUser) { if (!voucherService.shouldRaiseVoucher() || glSettlement == null || glSettlement.amount() == null || glSettlement.amount().compareTo(BigDecimal.ZERO) <= 0) { return null; } try { - final Long productId = jdbcTemplate.queryForObject("SELECT product_id FROM m_share_account WHERE id = ?", Long.class, - shareAccountId); - final Long glSundry = resolveGl(productId, FA_SHARES_SUSPENSE); + final Long glShare = redeemDebitGlOf(shareTransactionId); final Long glTarget = glSettlement.glAccountId(); + if (glShare == null) { + return null; + } final List lines = new ArrayList<>(); - lines.add(VoucherLine.debitGl(glSundry, glNameOf(glSundry), glCodeOf(glSundry), glSettlement.amount())); + lines.add(VoucherLine.debitGl(glShare, glNameOf(glShare), glCodeOf(glShare), glSettlement.amount())); lines.add(VoucherLine.creditGl(glTarget, glNameOf(glTarget), glCodeOf(glTarget), glSettlement.amount())); // PREFIX_DEBIT, not CREDIT: unlike the linked-savings path, this settlement never credits - // a member's own account -- the value is moving out of Sundry toward an eventual payout - // (e.g. a second, independent manual entry crediting a "DD Payable" account out to the - // actual draft), which is the payment series, not a receipt. + // a member's own account -- the value is moving out of share capital toward an eventual + // payout (e.g. a second, independent manual entry crediting a "DD Payable" account out to + // the actual draft), which is the payment series, not a receipt. final String voucherNo = voucherService.createVoucherWithLines(VoucherService.TYPE_WITHDRAWAL, VoucherService.REF_CLERK_SHARE_WITHDRAWAL_REQUEST, requestId, currentUser, VoucherService.PREFIX_DEBIT, lines); // Every other request type that raises a voucher stores its own voucher_no back on @@ -1002,7 +1045,7 @@ public class ClerkShareWithdrawalRequestApiResource { /** * The voucher for a request that settles nowhere further than leg 1 itself (settlement mode - * 'NONE'): Dr Share Capital, Cr Share Sundry — exactly what {@link #routeRedeemIntoSundry} just + * 'NONE'): Dr Share Capital, Cr Share Sundry — exactly what {@link #postRedemptionEntry} just * posted, given its own standalone voucher instead of only being visible as "Also posted" under * some other request's voucher (see {@code VoucherVerificationApiResource.relatedGlEntries()}). * @@ -1020,7 +1063,7 @@ public class ClerkShareWithdrawalRequestApiResource { * dev/test environment can violate — an older, unrelated redemption that happened to land on the * same id would otherwise be pulled in as a second, phantom copy of this leg, doubling the total * shown here even though only one pair was ever posted for this approval. This call runs - * immediately after {@link #routeRedeemIntoSundry} in the same approve(), so the highest-id pair + * immediately after {@link #postRedemptionEntry} in the same approve(), so the highest-id pair * is always the one it just inserted. */ private String raiseSundryOnlyVoucher(final Long requestId, final Long shareTransactionId, final AppUser currentUser) { @@ -1079,14 +1122,19 @@ public class ClerkShareWithdrawalRequestApiResource { * Posted through Fineract's own native savings deposit command first, so the transaction is fully audited and the * member's passbook/statement carry it exactly as an over-the-counter deposit would. That command's own GL entry * (Dr whichever GL the settlement payment type's Fund Source maps to / Cr Savings Control) is then - * reversed in full and replaced with an explicit Dr Sundry / Cr Savings Control pair — the debit side used - * to be left to the payment type's own mapping in the hope it would land on the same account - * {@link #routeRedeemIntoSundry} just credited, netting to zero; when that mapping pointed anywhere else (as it - * did the day this was found — Sundry, when it should have matched Cash at Vault) the two legs never actually - * met, leaving Sundry debited for money that was never credited to it. Explicit beats coincidental. + * reversed in full, and nothing is posted in its place here: the caller redirects the redemption's own + * credit leg straight onto the Savings Control GL returned below, so the ledger carries one entry — Dr Share + * Capital / Cr Savings Control — instead of a pair routed through Sundry and back out again. + * + *

+ * The debit side used to be left to the payment type's own Fund Source mapping in the hope it would land on the + * account the redemption leg had credited, netting to zero; when that mapping pointed anywhere else (as it did the + * day this was found — Sundry, when it should have matched Cash at Vault) the two legs never actually met, leaving + * Sundry debited for money that was never credited to it. Naming the destination explicitly, and posting only + * once, removes that class of mismatch entirely. */ private Settlement creditLinkedSavingsAccount(final Long shareAccountId, final Long shareTransactionId, final String requestedDateStr, - final String resolutionNumber, final Long settlementPaymentTypeId) { + final String resolutionNumber) { if (shareTransactionId == null) { return null; } @@ -1105,57 +1153,102 @@ public class ClerkShareWithdrawalRequestApiResource { final BigDecimal settlementAmount = (BigDecimal) row.get("settlementAmount"); - final Long paymentTypeId = settlementPaymentTypeId != null ? settlementPaymentTypeId - : jdbcTemplate.queryForList("SELECT id FROM m_payment_type WHERE code_name = ?", Long.class, - SHARE_REDEMPTION_PAYMENT_TYPE_CODE).stream().findFirst().orElse(null); - - final JsonObject payload = new JsonObject(); - payload.addProperty("locale", "en"); - payload.addProperty("dateFormat", "dd MMMM yyyy"); - payload.addProperty("transactionDate", formatForFineract(requestedDateStr)); - payload.addProperty("transactionAmount", settlementAmount); - if (paymentTypeId != null) { - payload.addProperty("paymentTypeId", paymentTypeId); - } - payload.addProperty("note", "Share redemption settlement — Resolution No. " + resolutionNumber); - - final CommandWrapper commandRequest = new CommandWrapperBuilder().savingsAccountDeposit(savingsAccountId) - .withJson(payload.toString()).build(); - final CommandProcessingResult result = commandsSourceWritePlatformService.logCommandSource(commandRequest); - final Long savingsTransactionId = result.getResourceId(); - - // The deposit's own GL entries (whatever they turned out to be) are reversed wholesale rather - // than picked apart leg-by-leg — the correct Dr Sundry / Cr Savings Control pair below stands - // on its own regardless of which GL the payment type's Fund Source mapping happened to hit. - // Guarded by reversed = false: if the "S"+id transaction_id text is ever reused (e.g. - // after a sequence reset), this can only flip rows that are currently live, never - // re-touch an already-reversed unrelated transaction's rows. - jdbcTemplate.update( - "UPDATE acc_gl_journal_entry SET reversed = true WHERE transaction_id = ? AND reversed = false", - "S" + savingsTransactionId); - - final Map acct = loadShareAccountGlContext(shareAccountId); - final Long productId = toLong(acct.get("product_id")); - final Long officeId = toLong(acct.get("office_id")); - final String currencyCode = (String) acct.get("currency_code"); - final Long glSundry = requireShareGl(productId, FA_SHARES_SUSPENSE, "Share Sundry (Suspense)"); final Long glSavingsControl = savingsControlGlOf(savingsAccountId); if (glSavingsControl == null) { throw apiError(409, "The Savings Control GL is not mapped for this savings account's product."); } - final LocalDate entryDate = LocalDate.parse(requestedDateStr.substring(0, 10)); + // Office comes from the share account the redemption was posted against, not from a join + // through the savings account's client -- a savings account can be held by a group, where + // that join yields nothing and the insert below would fail on a null office. + final Long officeId = toLong(loadShareAccountGlContext(shareAccountId).get("office_id")); + if (officeId == null) { + throw apiError(409, "The share account has no office on it, so the settlement transaction cannot be booked."); + } + + final Long savingsTransactionId = directCreditSavingsAccount(savingsAccountId, officeId, settlementAmount, requestedDateStr, + resolutionNumber); + + return new Settlement(settlementAmount, savingsTransactionId, glSavingsControl); + } + + /** + * Credits the member's savings account as a book transfer: the transaction row and the balance, + * written directly, with no journal entry of its own. + * + *

+ * The obvious way to do this is Fineract's own deposit command, and that is what this used to + * call. But that command posts its own Dr Cash at Vault / Cr Savings Control pair on the + * assumption that notes came over the counter, which they did not — the money is moving out of + * share capital inside one approval. Reversing that pair afterwards worked, yet left two dead + * rows in the ledger for every redemption and a Cash at Vault leg that had to be explained to + * anyone reading the table. Writing the transaction directly means those rows are never created + * at all, and the redemption's own entry — Dr Share Capital / Cr Savings Control, posted by the + * caller — is the only accounting the event produces. + * + *

+ * This is the same technique the disbursement register already uses for its own book transfers + * ({@code ClerkDisbursementRequestApiResource.directCreditSavingsAccount()}), for the same + * reason, and carries the same trade-off: the native command's interest recalculation, lien and + * overdraft checks and business events are all skipped. That is acceptable here because a + * redemption settlement is a plain credit of a known amount into an active account on the + * approval date, which is exactly the case those extras do not apply to. + * + *

+ * The passbook and statement are unaffected: both read {@code m_savings_account_transaction}, + * which is what this writes. + */ + private Long directCreditSavingsAccount(final Long savingsAccountId, final Long officeId, final BigDecimal amount, + final String requestedDateStr, final String resolutionNumber) { + final Map acct = jdbcTemplate + .queryForList("SELECT status_enum, account_balance_derived FROM m_savings_account WHERE id = ?", savingsAccountId).stream() + .findFirst().orElse(null); + if (acct == null) { + throw apiError(409, "The linked savings account " + savingsAccountId + " no longer exists."); + } + final int statusEnum = ((Number) acct.get("status_enum")).intValue(); + if (statusEnum != SAVINGS_STATUS_ACTIVE) { + throw apiError(409, "The linked savings account is not active, so the redemption cannot be settled into it."); + } + final BigDecimal currentBalance = acct.get("account_balance_derived") != null + ? (BigDecimal) acct.get("account_balance_derived") + : BigDecimal.ZERO; + + final java.sql.Date sqlDate = java.sql.Date.valueOf(requestedDateStr.substring(0, 10)); + final OffsetDateTime now = OffsetDateTime.now(DateUtils.getDateTimeZoneOfTenant()); + final String refNo = UUID.randomUUID().toString().replace("-", "").substring(0, 20); final long userId = context.authenticatedUser().getId(); - final LocalDateTime now = LocalDateTime.now(DateUtils.getDateTimeZoneOfTenant()); - final String txnId = "SHRDMSTL-" + System.nanoTime(); - final String accountNo = (String) acct.get("account_no"); - insertEntry(officeId, glSundry, DEBIT, settlementAmount, txnId, entryDate, shareAccountId, userId, now, currencyCode, - "Share redemption settled to savings | " + accountNo); - insertEntry(officeId, glSavingsControl, CREDIT, settlementAmount, txnId, entryDate, shareAccountId, userId, now, currencyCode, - "Share redemption settlement credited to savings | " + accountNo); + final KeyHolder kh = new GeneratedKeyHolder(); + jdbcTemplate.update(con -> { + // transaction_type_enum 1 = deposit. + final PreparedStatement ps = con.prepareStatement("INSERT INTO m_savings_account_transaction " + + "(savings_account_id, office_id, payment_detail_id, transaction_type_enum, " + + "is_reversed, transaction_date, amount, submitted_on_date, ref_no, " + + "running_balance_derived, is_lien_transaction, " + + "created_by, last_modified_by, created_on_utc, last_modified_on_utc) " + + "VALUES (?, ?, NULL, 1, false, ?, ?, ?, ?, ?, false, ?, ?, ?, ?)", new String[] { "id" }); + ps.setLong(1, savingsAccountId); + ps.setLong(2, officeId); + ps.setDate(3, sqlDate); + ps.setBigDecimal(4, amount); + ps.setDate(5, sqlDate); + ps.setString(6, refNo); + ps.setBigDecimal(7, currentBalance.add(amount)); + ps.setLong(8, userId); + ps.setLong(9, userId); + ps.setObject(10, now); + ps.setObject(11, now); + return ps; + }, kh); + final long savingsTransactionId = Objects.requireNonNull(kh.getKey()).longValue(); - return new Settlement(settlementAmount, savingsTransactionId); + jdbcTemplate.update("UPDATE m_savings_account SET account_balance_derived = account_balance_derived + ?, " + + "total_deposits_derived = total_deposits_derived + ? WHERE id = ?", amount, amount, savingsAccountId); + + LOG.info("Share redemption settled into savings account {} as transaction {} (Resolution No. {})", savingsAccountId, + savingsTransactionId, resolutionNumber); + return savingsTransactionId; } // ── POST /v1/clerk/share-withdrawal-requests/{id}/reject ── Manager rejects ─ @@ -1226,58 +1319,65 @@ public class ClerkShareWithdrawalRequestApiResource { } } - // ── Sundry pass-through helpers ────────────────────────────────────────── + // ── The redemption's own accounting ────────────────────────────────────── /** - * Redirects the native {@code redeemshares} posting's Cr Shares Reference (Cash at Vault) leg into Cr Sundry. + * Posts the redemption as one balanced entry against the account the money actually reached. * *

- * Fineract's own accounting bridge assumes a redemption hands cash back over the counter (Dr Share Equity / Cr - * Shares Reference) — right for a walk-in cash redemption, wrong here, where the value is heading to the - * member's linked savings instead. Left alone, that Cr Shares Reference leg has no partner: the settlement - * posted in {@link #creditLinkedSavingsAccount} debits Sundry, not Shares Reference, so Sundry would be debited - * for money it was never credited with. Reversing just this one leg (matched by the specific "Shares Reference" - * GL account, not merely "the credit leg" — a chargeable redemption also credits Income from Fees, which must be - * left standing) and replacing it with Cr Sundry closes that gap: the account genuinely passes through Sundry - * the same way every other share-capital movement in this codebase does. + * Share capital is debited and the destination credited, and that is the whole entry: Dr Share + * Equity / Cr the linked savings account's control GL, or the GL the clerk picked, or Share Sundry + * where the clerk chose to settle nowhere further. No "Shares Reference" account appears anywhere + * in it. Fineract's own bridge would have credited that account instead — a placeholder for a + * counterparty it cannot know — so the command that creates the share transaction runs inside + * {@link ShareAccountingSuppression#withOwnShareEntries} and posts nothing of its own. + * + *

+ * A redemption charge, where the product has one, is income and is credited separately: share + * capital carries the full amount plus the charge, the member receives the amount, and the + * difference lands in Income from Fees. With no charge configured the third line is simply absent. + * + *

+ * The transaction_id is the one the native command would have used ({@code "SH" + id}), so the + * entry groups and reconciles exactly as every other share posting in the platform does, and + * anything reading a share transaction's entries by that id keeps working unchanged. */ - private void routeRedeemIntoSundry(final Long shareAccountId, final Long shareTransactionId, final LocalDate entryDate, - final long userId) { + private void postRedemptionEntry(final Long shareAccountId, final Long shareTransactionId, final LocalDate entryDate, + final long userId, final Long destinationGlId, final String narration) { + final Map txn = jdbcTemplate + .queryForList("SELECT amount, COALESCE(charge_amount, 0) AS charge_amount FROM m_share_account_transactions WHERE id = ?", + shareTransactionId) + .stream().findFirst().orElse(null); + if (txn == null) { + return; + } + final BigDecimal amount = (BigDecimal) txn.get("amount"); + final BigDecimal charge = (BigDecimal) txn.get("charge_amount"); + if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) { + return; + } + final Map acct = loadShareAccountGlContext(shareAccountId); final Long productId = toLong(acct.get("product_id")); final Long officeId = toLong(acct.get("office_id")); final String currencyCode = (String) acct.get("currency_code"); final String accountNo = (String) acct.get("account_no"); - final Long glReference = requireShareGl(productId, FA_SHARES_REFERENCE, "Shares Reference"); - final Long glSundry = requireShareGl(productId, FA_SHARES_SUSPENSE, "Share Sundry (Suspense)"); - - final Map reversedLeg = jdbcTemplate - .queryForList( - "SELECT amount, transaction_id FROM acc_gl_journal_entry WHERE share_transaction_id = ? AND account_id = ? " - + " AND type_enum = ? AND reversed = false", - shareTransactionId, glReference, CREDIT) - .stream().findFirst().orElse(null); - if (reversedLeg == null) { - // Nothing to redirect -- the native posting didn't credit Shares Reference at all (an - // unexpected accounting configuration), so there is no leg here to reroute. - return; - } - final BigDecimal amount = (BigDecimal) reversedLeg.get("amount"); - // Reuses the native posting's OWN transaction_id rather than minting a new one -- entries are - // grouped and checked for balance by transaction_id, so a fresh id here would leave the Dr - // Share Equity leg standing alone as "unbalanced" and this new Cr Sundry leg equally alone, - // instead of the two reading as the one balanced entry they actually are. - final String nativeTxnId = (String) reversedLeg.get("transaction_id"); - - jdbcTemplate.update( - "UPDATE acc_gl_journal_entry SET reversed = true WHERE share_transaction_id = ? AND account_id = ? " - + " AND type_enum = ? AND reversed = false", - shareTransactionId, glReference, CREDIT); + final Long glEquity = requireShareGl(productId, FA_SHARES_EQUITY, "Share Equity"); + final boolean hasCharge = charge != null && charge.compareTo(BigDecimal.ZERO) > 0; + final String nativeTxnId = SHARE_TXN_ID_PREFIX + shareTransactionId; final LocalDateTime now = LocalDateTime.now(DateUtils.getDateTimeZoneOfTenant()); - insertEntry(officeId, glSundry, CREDIT, amount, nativeTxnId, entryDate, shareAccountId, userId, now, currencyCode, - "Share redemption held in Sundry pending settlement | " + accountNo); + + insertEntry(officeId, glEquity, DEBIT, hasCharge ? amount.add(charge) : amount, nativeTxnId, entryDate, shareAccountId, + shareTransactionId, userId, now, currencyCode, "Share redemption | " + accountNo); + insertEntry(officeId, destinationGlId, CREDIT, amount, nativeTxnId, entryDate, shareAccountId, shareTransactionId, userId, now, + currencyCode, narration + " | " + accountNo); + if (hasCharge) { + final Long glFees = requireShareGl(productId, FA_INCOME_FROM_FEES, "Income from Fees"); + insertEntry(officeId, glFees, CREDIT, charge, nativeTxnId, entryDate, shareAccountId, shareTransactionId, userId, now, + currencyCode, "Share redemption charge | " + accountNo); + } } private Map loadShareAccountGlContext(final Long shareAccountId) { @@ -1349,19 +1449,31 @@ public class ClerkShareWithdrawalRequestApiResource { return rows.isEmpty() ? null : toLong(rows.get(0).get("gl_account_id")); } + /** + * {@code share_transaction_id} and {@code entity_type_enum} are written, not left null as a manual + * entry normally would: these rows stand in for the ones the platform's own bridge would have + * posted, so they have to be indistinguishable from them to everything that goes looking. + * + *

+ * The voucher builders here match on {@code share_transaction_id}; Fineract's own + * {@code revertShareAccountJournalEntries} matches on {@code transaction_id} and + * {@code entity_type_enum = SHARES}, and would silently skip a row missing that column, leaving + * the accounting standing after the transaction behind it had been reverted. + */ private void insertEntry(final Long officeId, final Long glAccountId, final int typeEnum, final BigDecimal amount, final String txnId, - final LocalDate entryDate, final Long shareAccountId, final long userId, final LocalDateTime now, final String currencyCode, - final String description) { + final LocalDate entryDate, final Long shareAccountId, final Long shareTransactionId, final long userId, + final LocalDateTime now, final String currencyCode, final String description) { GLAccountTransactionGuard.assertPostable(jdbcTemplate, glAccountId); jdbcTemplate.update( "INSERT INTO acc_gl_journal_entry " + "(account_id, office_id, currency_code, transaction_id, reversed, manual_entry, " - + " entry_date, type_enum, amount, description, entity_id, " + + " entry_date, type_enum, amount, description, entity_id, entity_type_enum, share_transaction_id, " + " created_by, last_modified_by, created_date, lastmodified_date, " + " is_running_balance_calculated, office_running_balance, " + " organization_running_balance, submitted_on_date, transaction_date, " + " created_on_utc, last_modified_on_utc) " - + "VALUES (?,?,?,?,false,true,?,?,?,?,?,?,?,?,?,false,0,0,?,?,?,?)", + + "VALUES (?,?,?,?,false,true,?,?,?,?,?,?,?,?,?,?,?,false,0,0,?,?,?,?)", glAccountId, officeId, currencyCode, txnId, java.sql.Date.valueOf(entryDate), typeEnum, amount, description, shareAccountId, - userId, userId, now, now, java.sql.Date.valueOf(entryDate), java.sql.Date.valueOf(entryDate), toUtc(now), toUtc(now)); + PRODUCT_TYPE_SHARES, shareTransactionId, userId, userId, now, now, java.sql.Date.valueOf(entryDate), + java.sql.Date.valueOf(entryDate), toUtc(now), toUtc(now)); } /** diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/dividenddashboard/api/DividendClaimRequestApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/dividenddashboard/api/DividendClaimRequestApiResource.java index 71ca09351..2c1d1e584 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/dividenddashboard/api/DividendClaimRequestApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/dividenddashboard/api/DividendClaimRequestApiResource.java @@ -47,6 +47,8 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.apache.fineract.accounting.common.DividendGlAccountResolver; import org.apache.fineract.accounting.glaccount.validation.GLAccountTransactionGuard; +import org.apache.fineract.accounting.journalentry.service.LedgerTransactionId; +import org.apache.fineract.accounting.journalentry.service.LedgerTxnPrefix; import org.apache.fineract.commands.domain.CommandWrapper; import org.apache.fineract.commands.service.CommandWrapperBuilder; import org.apache.fineract.commands.service.PortfolioCommandSourceWritePlatformService; @@ -520,7 +522,7 @@ public class DividendClaimRequestApiResource { final Long ddPayableAccountId = ddPayableAccount(); final java.sql.Date entryDate = java.sql.Date.valueOf(today); - final String transactionId = "DIVDD" + claimId; + final String transactionId = LedgerTransactionId.next(this.jdbcTemplate, LedgerTxnPrefix.DIVIDEND_CLAIM); // Stamped with the financial year the dividend was DECLARED for, not the year it is being // claimed in — a member can collect a 2026-27 dividend in 2028. Without it the ledger cannot // say how much of each year's declaration has actually been claimed. diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/dailyinterest/SavingsPooledInterestPostingService.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/dailyinterest/SavingsPooledInterestPostingService.java index d5e81a559..d2fc838bb 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/dailyinterest/SavingsPooledInterestPostingService.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/dailyinterest/SavingsPooledInterestPostingService.java @@ -27,6 +27,8 @@ import java.util.Map; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.apache.fineract.accounting.glaccount.validation.GLAccountTransactionGuard; +import org.apache.fineract.accounting.journalentry.service.LedgerTransactionId; +import org.apache.fineract.accounting.journalentry.service.LedgerTxnPrefix; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -225,7 +227,11 @@ public class SavingsPooledInterestPostingService { final Long officeId = ((Number) due.get(0).get("office_id")).longValue(); final String currency = (String) due.get(0).get("currency_code"); - final String poolReference = "SIP-" + productId + "-" + postingDate; + // Deterministic, not counter-based: a run that stops halfway is resumed rather than restarted, + // and the resumed legs have to carry the same reference as the legs already posted, or one + // interest posting ends up split across two transaction ids. + final String poolReference = LedgerTransactionId.forDatedSeries(LedgerTxnPrefix.SAVINGS_INTEREST_POOLED, postingDate, + productId); // Stage 1: the debt the accrual has been building moves out of the payable and into the // account the payout is made from, so what is still merely owed and what is on its way to diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/domain/ShareAccount.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/domain/ShareAccount.java index bf3465113..ffb0ce4ef 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/domain/ShareAccount.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/domain/ShareAccount.java @@ -562,8 +562,31 @@ public class ShareAccount extends AbstractPersistableCustom { this.totalSharesPending = shares; } + /** + * Finds this account's own instance of a transaction, after a save has replaced the caller's copy. + * + *

+ * Matched on identity where there is one. The callers run this right after {@code saveAndFlush}, + * by which point the transaction has been assigned its id, and matching on that is exact. The + * field comparison below is the fallback for a transaction that has not been persisted yet. + * + *

+ * The fallback cannot stand on its own, because the fields it compares do not identify a + * transaction: two redemptions of the same number of shares, at the same price, on the same day + * are identical in every one of them. When that happened the loop returned the earlier of + * the two and the caller went on to treat the new transaction as that one -- posting the second + * redemption's journal entries under the first one's transaction id, so one share transaction + * appeared to carry two redemptions and its voucher showed double the money that actually moved. + */ public ShareAccountTransaction getShareAccountTransaction(final ShareAccountTransaction transaction) { ShareAccountTransaction returnTrans = null; + if (transaction.getId() != null) { + for (ShareAccountTransaction tran : this.shareAccountTransactions) { + if (transaction.getId().equals(tran.getId())) { + return tran; + } + } + } for (ShareAccountTransaction tran : this.shareAccountTransactions) { // A charge/fee transaction carries no share count or unit price at all (both columns are // null), so this account's own transaction list is not uniformly non-null the way a diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/serialization/ShareAccountDataSerializer.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/serialization/ShareAccountDataSerializer.java index ff3afb640..448f14fc3 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/serialization/ShareAccountDataSerializer.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/serialization/ShareAccountDataSerializer.java @@ -867,7 +867,7 @@ public class ShareAccountDataSerializer { advanceCollectionId); if (rows.isEmpty()) { baseDataValidator.reset().parameter(ShareAccountApiConstants.advancecollectionid_paramname).value(advanceCollectionId) - .failWithCode("share.advance.collection.not.found", "A voucher for this payment could not be found."); + .failWithCodeAndMessage("share.advance.collection.not.found", "A voucher for this payment could not be found."); continue; } final Map collection = rows.get(0); @@ -878,7 +878,7 @@ public class ShareAccountDataSerializer { final Number collectionClientId = (Number) collection.get("client_id"); if (collectionClientId == null || collectionClientId.longValue() != account.getClientId().longValue()) { baseDataValidator.reset().parameter(ShareAccountApiConstants.advancecollectionid_paramname).value(advanceCollectionId) - .failWithCode("share.advance.collection.client.mismatch", + .failWithCodeAndMessage("share.advance.collection.client.mismatch", "Voucher " + label + " belongs to a different member."); } @@ -887,13 +887,13 @@ public class ShareAccountDataSerializer { final String status = (String) collection.get("status"); if (!"UNAPPLIED".equals(status)) { baseDataValidator.reset().parameter(ShareAccountApiConstants.advancecollectionid_paramname).value(advanceCollectionId) - .failWithCode("share.advance.collection.not.unapplied", + .failWithCodeAndMessage("share.advance.collection.not.unapplied", "Voucher " + label + " is no longer available — it reads " + status + "."); } if (isReservedByAnotherTranche(advanceCollectionId)) { baseDataValidator.reset().parameter(ShareAccountApiConstants.advancecollectionid_paramname).value(advanceCollectionId) - .failWithCode("share.advance.collection.already.reserved", + .failWithCodeAndMessage("share.advance.collection.already.reserved", "Voucher " + label + " is already reserved against another pending share request."); } @@ -907,8 +907,21 @@ public class ShareAccountDataSerializer { .multiply(BigDecimal.valueOf(sharesRequested)); if (receiptTotal.compareTo(trancheValue) < 0) { baseDataValidator.reset().parameter(ShareAccountApiConstants.advancecollectionid_paramname).value(receiptTotal) - .failWithCode("share.advance.collection.insufficient", "The vouchers selected come to " + receiptTotal + .failWithCodeAndMessage("share.advance.collection.insufficient", "The vouchers selected come to " + receiptTotal + ", which does not cover the " + trancheValue + " value of the shares requested."); + } else if (receiptTotal.compareTo(trancheValue) > 0 && account.getSavingsAccount() == null) { + // An over-payment is settled by crediting the difference to the member's linked savings + // account (see AdditionalShareTrancheFundingService.postSharesIssued) -- with none linked, + // approval would have nowhere to put it. Caught here, at submit, rather than left to fail + // once the maker has already gone: short of a member paying the exact figure, this is + // refused up front instead of accepted and then unable to settle. + final BigDecimal excessAmount = receiptTotal.subtract(trancheValue); + baseDataValidator.reset().parameter(ShareAccountApiConstants.advancecollectionid_paramname).value(receiptTotal) + .failWithCodeAndMessage("share.advance.collection.excess.no.savings", + "The vouchers selected come to " + receiptTotal + ", which is " + excessAmount + " more than the " + + trancheValue + " value of the shares requested. The extra " + excessAmount + + " must be credited to the member's savings account, but none is linked to this share account. " + + "Link one, or collect the exact amount."); } } @@ -1212,7 +1225,12 @@ public class ShareAccountDataSerializer { private void validateMinimumSharesAndTenure(final ShareAccount account, final ShareAccountTransaction redeemTransaction, final Long remainingShares, final DataValidatorBuilder baseDataValidator, final List dataValidationErrors) { final Long minimumClientShares = account.getShareProduct().getMinimumClientShares(); - if (minimumClientShares == null || remainingShares >= minimumClientShares) { + // The minimum only protects an account that is staying open with a reduced holding -- it has + // nothing to say about one being emptied out entirely. A member redeeming every share they + // hold is exiting, not falling short of a floor; the account closes right behind this + // transaction (see closeShareAccountIfSharesZero), and there is no remaining balance left for + // "at least the minimum" to mean anything about. + if (minimumClientShares == null || remainingShares == 0 || remainingShares >= minimumClientShares) { return; } final Integer minimumActivePeriod = account.getMinimumActivePeriodFrequency(); @@ -1317,17 +1335,20 @@ public class ShareAccountDataSerializer { } AppUser approvedUser = this.platformSecurityContext.authenticatedUser(); - // Only synthesize a redeem transaction (and its GL posting) when shares actually remain to - // be redeemed — an account closed after every share was already withdrawn has nothing left - // to force-redeem, and posting a zero-share/zero-value transaction just clutters the GL with - // a meaningless ₹0.00/₹0.00 entry pair. + // A close with shares still outstanding used to force-redeem them right here -- one native + // command, no maker-checker, liquidating the member's holding and crediting cash back to the + // GL Fineract's own bridge names, all in the single step of closing the account. That is + // exactly the governed Share Withdrawal flow's job: a clerk raises the redemption, a manager + // approves it, and it settles to wherever the clerk actually keyed -- the member's savings, a + // chosen GL, or Sundry. Refusing here instead of quietly redeeming is what makes closing an + // account with a balance impossible to do any way other than through that flow; the frontend + // already hid its own "Close" button for the same reason, but the command underneath was still + // reachable directly and this is what actually closes that gap. final Long remainingShares = account.getTotalApprovedShares(); if (remainingShares != null && remainingShares > 0) { - final BigDecimal unitPrice = account.getShareProduct().deriveMarketPrice(DateUtils.getBusinessLocalDate()); - ShareAccountTransaction transaction = ShareAccountTransaction.createRedeemTransaction(closedDate, remainingShares, unitPrice); - account.addAdditionalPurchasedShares(transaction); - handleRedeemSharesChargeTransactions(account, transaction); - actualChanges.put(ShareAccountApiConstants.requestedshares_paramname, transaction); + baseDataValidator.reset().parameter(ShareAccountApiConstants.closeddate_paramname).value(closedDate) + .failWithCodeNoParameterAddedToErrorCode("share.account.cannot.be.closed.with.shares.outstanding"); + throw new PlatformApiDataValidationException(dataValidationErrors); } account.close(closedDate, approvedUser); return actualChanges; diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/AdditionalShareTrancheFundingService.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/AdditionalShareTrancheFundingService.java index 68b73f81b..fcf1b502a 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/AdditionalShareTrancheFundingService.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/AdditionalShareTrancheFundingService.java @@ -18,20 +18,18 @@ */ package org.apache.fineract.portfolio.shareaccounts.service; -import com.google.gson.JsonObject; import java.math.BigDecimal; +import java.sql.PreparedStatement; import java.time.LocalDate; import java.time.LocalDateTime; import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.UUID; import lombok.RequiredArgsConstructor; import org.apache.fineract.accounting.glaccount.validation.GLAccountTransactionGuard; -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.data.CommandProcessingResult; import org.apache.fineract.infrastructure.core.exception.PlatformDataIntegrityException; import org.apache.fineract.infrastructure.core.service.DateUtils; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; @@ -41,11 +39,13 @@ import org.apache.fineract.portfolio.shareaccounts.domain.ShareAccountTransactio import org.apache.fineract.useradministration.domain.AppUser; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.support.GeneratedKeyHolder; +import org.springframework.jdbc.support.KeyHolder; import org.springframework.stereotype.Component; /** - * Funds one additional-share tranche — moves the member's money into the Share Sundry holding - * account, where it waits for the tranche to be approved. + * Funds one additional-share tranche — records how the member is paying for it, and turns that + * money into share capital once the tranche is approved. * *

* Why this is separate from the account-level settlement resource. {@code ShareAdvanceCollectionApiResource} @@ -55,35 +55,33 @@ import org.springframework.stereotype.Component; * valued and recorded per transaction, and leaves that account-level state alone. * *

- * Where the money ends up. Both routes leave it in Share Sundry (SHARES_SUSPENSE), so approving the tranche posts - * the same Dr Sundry / Cr Share Capital pair either way — that pair is Fineract's own, posted by - * {@code CashBasedAccountingProcessorForShares} when the transaction turns APPROVED. Nothing here posts it; this only - * gets the money into Sundry in the first place. + * What is posted, and when. Nothing is posted when a tranche is raised — a request that is never approved + * never touches anybody's money. {@code settle()} books the whole thing at approval, and the entry is the transfer the + * clerk actually keyed: * *

* *

- * Note that Fineract's stock accounting also posts an APPLIED leg of its own for a new purchase - * (Dr Shares Reference / Cr Shares Suspense), assuming cash is being received at the counter right now. Neither route - * here works that way, so both reverse that pair in full: on the cash route the cashier already posted exactly those - * entries when the voucher was issued, and on the savings route the money came from the member's account rather than the - * till. Reversing only the reference debit and keeping the stock Sundry credit would leave Sundry credited twice — once - * by the stock leg and once by the pair posted here — so the whole pair goes, and Sundry ends up credited exactly once. + * Fineract's stock accounting would post an APPLIED leg of its own when the tranche is raised + * (Dr Shares Reference / Cr Shares Suspense), assuming cash is being received at the counter right now, and an APPROVED + * leg out of the product's mapped Sundry account. Neither matches either route here, so neither is made for a funded + * tranche (see {@code ShareAccountWritePlatformServiceJpaRepositoryImpl}). They used to be posted and reversed a moment + * later, which left dead rows in the ledger on every tranche and a Shares Reference leg that had to be explained to + * anyone reading the account. */ @Component @RequiredArgsConstructor @@ -94,22 +92,21 @@ public class AdditionalShareTrancheFundingService { private static final int DEBIT = 2; /** acc_product_mapping.financial_account_type for share products. */ - private static final int FA_SHARES_REFERENCE = 1; private static final int FA_SHARES_SUSPENSE = 2; + private static final int FA_SHARES_EQUITY = 4; private static final int PRODUCT_TYPE_SHARES = 4; + /** What Fineract's own bridge prefixes a share transaction's journal entries with. */ + private static final String SHARE_TXN_ID_PREFIX = "SH"; + /** acc_product_mapping for savings: product_type 2, financial_account_type 2 = Savings Control. */ private static final int PRODUCT_TYPE_SAVINGS = 2; private static final int FA_SAVINGS_CONTROL = 2; private static final int SAVINGS_STATUS_ACTIVE = 300; - /** m_payment_type.value seeded by changelog 0483, shared with the account-level settlement. */ - private static final String SETTLEMENT_PAYMENT_TYPE = "Share Application Settlement"; - private final PlatformSecurityContext context; private final JdbcTemplate jdbcTemplate; - private final PortfolioCommandSourceWritePlatformService commandsSourceWritePlatformService; /** * Funds a freshly raised tranche according to the route recorded on it. @@ -125,20 +122,26 @@ public class AdditionalShareTrancheFundingService { if (ShareAccountApiConstants.FUNDING_SOURCE_CASH.equals(fundingSource)) { reserveAdvanceCollection(account, transaction); } else if (ShareAccountApiConstants.FUNDING_SOURCE_SAVINGS.equals(fundingSource)) { - debitSavingsIntoSundry(account, transaction); + validateSavingsFunding(account, transaction); } } /** - * Settles a tranche's funding once the tranche has been approved and its value released from - * Share Sundry into share capital. + * Turns an approved tranche's funding into share capital, and closes the funding record behind it. * - * No journal entries of its own — Fineract's APPROVED posting already moved the money. This only - * closes the funding record, so a voucher whose money is now share capital cannot be presented - * again: until this point the receipt was merely reserved, because approval was still refusable. + * Posts the whole entry itself rather than leaving it to the platform, because only this knows + * where the money actually is. Closing the record last means a voucher whose money is now share + * capital cannot be presented again: until this point the receipt was merely reserved, because + * approval was still refusable. */ public void settle(final ShareAccount account, final ShareAccountTransaction transaction) { - if (!ShareAccountApiConstants.FUNDING_SOURCE_CASH.equals(transaction.getFundingSource())) { + final String fundingSource = transaction.getFundingSource(); + if (fundingSource == null) { + return; + } + postSharesIssued(account, transaction); + + if (!ShareAccountApiConstants.FUNDING_SOURCE_CASH.equals(fundingSource)) { return; } // Every receipt behind the tranche, not just the lead one: a member's later instalments have @@ -153,21 +156,170 @@ public class AdditionalShareTrancheFundingService { } /** - * Unwinds the funding of a tranche that was rejected, so the member's money does not sit in - * Share Sundry against shares that will never be issued. + * Issues the share capital for an approved tranche: the money leaves wherever it has been sitting + * and becomes equity. * - * Fineract's own REJECTED posting already takes the value back out of Sundry — but it credits - * Shares Reference, the till, which is only correct for the cash route. The routes part company - * here: + *

+ * The credit is always Share Capital. What the debit is depends on where the money actually is, + * which is the whole point of posting this here rather than letting the platform do it: * *

+ * + *

+ * Receipts are drawn down in order, each capped at what it actually holds, so a tranche paid for + * in instalments clears each receipt for its own share. Anything the receipts do not cover falls + * back to Sundry, which keeps the entry balanced rather than silently posting a one-sided one. + * + *

+ * Whatever a receipt holds beyond what the tranche needed — a member overpaying the lead receipt, + * or naming a further instalment that turns out not to be needed at all — is credited to their + * linked savings account, receipt by receipt, rather than left standing: every named receipt is + * marked APPLIED at the end of {@code settle()} regardless of whether the drawdown below actually + * reached it, so any part of one that this method does not move becomes untraceable money the + * moment that happens — still sitting in whatever GL the cashier banked it to, but with no + * UNAPPLIED receipt left pointing back at it. + */ + private void postSharesIssued(final ShareAccount account, final ShareAccountTransaction transaction) { + final BigDecimal amount = transaction.amount(); + if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) { + return; + } + final Long productId = account.getShareProduct().getId(); + final Long officeId = account.getOfficeId(); + final String currencyCode = account.getCurrency().getCode(); + final Long glSundry = requireShareGl(productId, FA_SHARES_SUSPENSE, "Share Sundry (Suspense)"); + final Long glEquity = requireShareGl(productId, FA_SHARES_EQUITY, "Share Capital (Equity)"); + + final AppUser currentUser = this.context.authenticatedUser(); + final long userId = currentUser.getId(); + final LocalDateTime now = LocalDateTime.now(DateUtils.getDateTimeZoneOfTenant()); + final LocalDate entryDate = DateUtils.getBusinessLocalDate(); + final String txnId = SHARE_TXN_ID_PREFIX + transaction.getId(); + final String accountNo = account.getAccountNumber(); + + if (ShareAccountApiConstants.FUNDING_SOURCE_SAVINGS.equals(transaction.getFundingSource())) { + // The member pays for the shares at the moment they are issued, not when the request was + // raised: their balance is checked on submit but nothing is taken, so a tranche that is + // never approved never touches their account. One pair, Dr their savings / Cr share + // capital, which is exactly the transfer the clerk keyed. + final Long savingsAccountId = account.getSavingsAccount().getId(); + final Long glSavingsControl = savingsControlGlOf(savingsAccountId); + if (glSavingsControl == null) { + throw new PlatformDataIntegrityException("error.msg.share.tranche.savings.gl.not.mapped", + "The Savings Control GL is not mapped for this savings account's product."); + } + final Long savingsTxnId = debitSavingsAccount(savingsAccountId, officeId, amount, entryDate); + transaction.updateSavingsDebitRequestId(savingsTxnId); + insertEntry(officeId, glSavingsControl, DEBIT, amount, txnId, entryDate, account.getId(), transaction.getId(), userId, now, + currencyCode, + "Additional shares paid from savings | " + accountNo); + insertEntry(officeId, glEquity, CREDIT, amount, txnId, entryDate, account.getId(), transaction.getId(), userId, now, + currencyCode, + "Share capital issued for " + accountNo); + return; + } + + BigDecimal remaining = amount; + BigDecimal excess = BigDecimal.ZERO; + if (ShareAccountApiConstants.FUNDING_SOURCE_CASH.equals(transaction.getFundingSource())) { + // Every named receipt is visited, not just enough of them to cover the tranche: one left + // untouched here would still be marked APPLIED at the end of settle(), so this is the only + // chance to account for its money before that happens. + for (final Long collectionId : transaction.getAllAdvanceCollectionIds()) { + final Map coll = this.jdbcTemplate + .queryForList("SELECT amount, gl_transaction_id FROM m_share_advance_collection WHERE id = ?", collectionId) + .stream().findFirst().orElse(null); + if (coll == null) { + continue; + } + final BigDecimal held = (BigDecimal) coll.get("amount"); + final BigDecimal take = held.min(remaining.max(BigDecimal.ZERO)); + final BigDecimal leftover = held.subtract(take); + final Long receiptGl = receiptGlOf((String) coll.get("gl_transaction_id"), glSundry); + if (take.compareTo(BigDecimal.ZERO) > 0) { + insertEntry(officeId, receiptGl, DEBIT, take, txnId, entryDate, account.getId(), transaction.getId(), userId, now, + currencyCode, "Additional shares issued | " + accountNo); + remaining = remaining.subtract(take); + } + if (leftover.compareTo(BigDecimal.ZERO) > 0) { + // This receipt held more than the tranche needed. Its own GL still has to be + // cleared for the leftover -- the receipt is about to read APPLIED in full -- so + // that leftover is drawn down here too, and handed back below as a savings credit + // rather than left to vanish along with the receipt's UNAPPLIED status. + insertEntry(officeId, receiptGl, DEBIT, leftover, txnId, entryDate, account.getId(), transaction.getId(), userId, now, + currencyCode, "Excess on additional shares receipt refunded | " + accountNo); + excess = excess.add(leftover); + } + } + } + if (remaining.compareTo(BigDecimal.ZERO) > 0) { + insertEntry(officeId, glSundry, DEBIT, remaining, txnId, entryDate, account.getId(), transaction.getId(), userId, now, + currencyCode, + "Additional shares issued from Share Sundry | " + accountNo); + } + + insertEntry(officeId, glEquity, CREDIT, amount, txnId, entryDate, account.getId(), transaction.getId(), userId, now, + currencyCode, + "Share capital issued for " + accountNo); + + if (excess.compareTo(BigDecimal.ZERO) > 0) { + // Validated at submit (see ShareAccountDataSerializer.validateAdvanceCollectionsForTranche): + // a tranche cannot be raised with receipts totalling more than its value unless a savings + // account is linked to receive the difference. Checked again here rather than trusted, + // since this also runs for a tranche raised before that guard existed. + if (account.getSavingsAccount() == null) { + throw new PlatformDataIntegrityException("error.msg.share.tranche.excess.no.savings", + "This tranche's receipts total " + excess + + " more than the shares cost, but no savings account is linked to credit it to."); + } + final Long savingsAccountId = account.getSavingsAccount().getId(); + final Long glSavingsControl = savingsControlGlOf(savingsAccountId); + if (glSavingsControl == null) { + throw new PlatformDataIntegrityException("error.msg.share.tranche.savings.gl.not.mapped", + "The Savings Control GL is not mapped for this savings account's product."); + } + // Not recorded on transaction.savingsDebitRequestId: that field means the savings-funded + // route's own debit, a different transaction to a different end -- overloading it here for + // a cash-funded tranche's incidental credit would misname what it actually is. + creditSavingsAccount(savingsAccountId, officeId, excess, entryDate); + insertEntry(officeId, glSavingsControl, CREDIT, excess, txnId, entryDate, account.getId(), transaction.getId(), userId, now, + currencyCode, "Excess on additional shares receipts credited to savings | " + accountNo); + } + } + + /** + * The account a receipt's money is actually sitting in, read back from the posting that put it + * there - the credit leg of that receipt's own journal transaction. Falls back to the product's + * Sundry account for a receipt with no posting recorded against it. + */ + private Long receiptGlOf(final String glTransactionId, final Long fallbackGlId) { + if (glTransactionId == null || glTransactionId.isBlank()) { + return fallbackGlId; + } + final List rows = this.jdbcTemplate.queryForList(""" + SELECT account_id FROM acc_gl_journal_entry + WHERE transaction_id = ? AND type_enum = ? AND reversed = false + ORDER BY id DESC LIMIT 1 + """, Long.class, glTransactionId, CREDIT); + return rows.isEmpty() ? fallbackGlId : rows.get(0); + } + + /** + * Unwinds the funding of a rejected tranche. + * + *

+ * Nothing was ever debited on submit — {@link #postSharesIssued} is where the money actually + * moves, and that never ran, because approval is what would have called it — so there is nothing + * to give back. The only route with a submit-time side effect is cash, where the receipt was + * earmarked against this account; releasing that lets the clerk spend the voucher on another + * tranche or refund it at the counter. Savings has none, so it is a no-op here. */ public void unwind(final ShareAccount account, final ShareAccountTransaction transaction) { final String fundingSource = transaction.getFundingSource(); @@ -176,9 +328,9 @@ public class AdditionalShareTrancheFundingService { } if (ShareAccountApiConstants.FUNDING_SOURCE_CASH.equals(fundingSource)) { releaseAdvanceCollection(transaction); - } else if (ShareAccountApiConstants.FUNDING_SOURCE_SAVINGS.equals(fundingSource)) { - refundSavings(account, transaction); } + // A savings-funded tranche has nothing to unwind: the member is only debited when the shares + // are issued, so a rejected one never took their money in the first place. } /** @@ -192,90 +344,129 @@ public class AdditionalShareTrancheFundingService { } /** - * Returns a rejected tranche's money to the savings account it was taken from. - */ - private void refundSavings(final ShareAccount account, final ShareAccountTransaction transaction) { - if (account.getSavingsAccount() == null) { - return; - } - final BigDecimal amount = transaction.amount(); - if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) { - return; - } - final Long savingsAccountId = account.getSavingsAccount().getId(); - final Long productId = account.getShareProduct().getId(); - final Long officeId = account.getOfficeId(); - final String currencyCode = account.getCurrency().getCode(); - - final Long glSavingsControl = savingsControlGlOf(savingsAccountId); - if (glSavingsControl == null) { - throw new PlatformDataIntegrityException("error.msg.share.tranche.savings.gl.not.mapped", - "The Savings Control GL is not mapped for this savings account's product."); - } - - final LocalDate entryDate = DateUtils.getBusinessLocalDate(); - final String note = "Additional shares rejected, refunded to savings | share account " + account.getAccountNumber(); - - // Fineract's own deposit command, so the refund reaches the member's passbook and statement - // as a real entry. Its Dr Cash / Cr Savings Control pair is half wrong — no cash came in - // over the counter — so the cash debit is replaced by a Sundry debit below. - final Long savingsTxnId = executeSavingsDeposit(savingsAccountId, amount, entryDate, note); - // Guarded by reversed = false: if the "S"+id transaction_id text is ever reused (e.g. - // after a sequence reset), this can only flip rows that are currently live, never - // re-touch an already-reversed unrelated transaction's rows. - this.jdbcTemplate.update( - "UPDATE acc_gl_journal_entry SET reversed = true WHERE transaction_id = ? AND reversed = false", - "S" + savingsTxnId); - - // Fineract's stock REJECTED posting credits Shares Reference. Nothing is going back to the - // till, so that credit is reversed; the Sundry debit it raised alongside is correct and - // stands. - reverseStockReferenceCredit(transaction.getId(), productId, officeId); - - final AppUser currentUser = this.context.authenticatedUser(); - final long userId = currentUser.getId(); - final LocalDateTime now = LocalDateTime.now(DateUtils.getDateTimeZoneOfTenant()); - final String txnId = "SHTRREF-" + transaction.getId(); - - insertEntry(officeId, glSavingsControl, CREDIT, amount, txnId, entryDate, account.getId(), userId, now, currencyCode, - "Rejected additional shares refunded to savings " + account.getSavingsAccountNo() + " | " + account.getAccountNumber()); - } - - /** - * Reverses the Shares Reference credit that Fineract's stock REJECTED posting raised for this - * tranche, leaving its Sundry debit standing. - */ - private void reverseStockReferenceCredit(final Long shareTransactionId, final Long productId, final Long officeId) { - final Long glReference = requireShareGl(productId, FA_SHARES_REFERENCE, "Shares Reference"); - reverseTrancheEntries(shareTransactionId, officeId, glReference, CREDIT); - } - - /** - * Reverses this tranche's own unreversed entries on one GL account and one side. + * Takes the tranche value out of the member's savings account as a book transfer: the transaction + * row and the balance, written directly, with no journal entry of its own. * - * Matched on {@code share_transaction_id} rather than the {@code SH} transaction_id string, - * because the APPLIED, APPROVED and REJECTED postings all reuse that same string; narrowing to a - * single GL and side is what keeps each reversal to the posting it is meant to undo. + *

+ * Fineract's withdrawal command would post its own Dr Savings Control / Cr Cash pair, and no cash + * leaves the branch here - the money becomes share capital. That pair used to be posted and then + * reversed a moment later, leaving two dead rows in the ledger on every tranche. The caller books + * the real pair itself; this only moves the member's own balance. + * + *

+ * The passbook and statement are unaffected: both read {@code m_savings_account_transaction}, + * which is what this writes. Interest recalculation, lien and overdraft checks and business events + * are the trade-off, as on the disbursement register's own book transfers. */ - private void reverseTrancheEntries(final Long shareTransactionId, final Long officeId, final Long glAccountId, final int typeEnum) { - this.jdbcTemplate.update(""" - UPDATE acc_gl_journal_entry SET reversed = true - WHERE share_transaction_id = ? AND account_id = ? AND office_id = ? AND type_enum = ? AND reversed = false - """, shareTransactionId, glAccountId, officeId, typeEnum); + private Long debitSavingsAccount(final Long savingsAccountId, final Long officeId, final BigDecimal amount, + final LocalDate entryDate) { + final Map acct = this.jdbcTemplate + .queryForList("SELECT status_enum, account_balance_derived FROM m_savings_account WHERE id = ?", savingsAccountId).stream() + .findFirst().orElse(null); + if (acct == null) { + throw new PlatformDataIntegrityException("error.msg.share.tranche.savings.missing", + "The linked savings account " + savingsAccountId + " no longer exists."); + } + if (((Number) acct.get("status_enum")).intValue() != SAVINGS_STATUS_ACTIVE) { + throw new PlatformDataIntegrityException("error.msg.share.tranche.savings.not.active", + "The linked savings account is not active."); + } + final BigDecimal balance = acct.get("account_balance_derived") != null ? (BigDecimal) acct.get("account_balance_derived") + : BigDecimal.ZERO; + if (balance.compareTo(amount) < 0) { + throw new PlatformDataIntegrityException("error.msg.share.tranche.savings.insufficient", + "The linked savings account holds " + balance + ", which is less than the " + amount + " these shares cost."); + } + + final java.sql.Date sqlDate = java.sql.Date.valueOf(entryDate); + final OffsetDateTime now = OffsetDateTime.now(DateUtils.getDateTimeZoneOfTenant()); + final String refNo = UUID.randomUUID().toString().replace("-", "").substring(0, 20); + final long userId = this.context.authenticatedUser().getId(); + + final KeyHolder kh = new GeneratedKeyHolder(); + this.jdbcTemplate.update(con -> { + // transaction_type_enum 2 = withdrawal. + final PreparedStatement ps = con.prepareStatement("INSERT INTO m_savings_account_transaction " + + "(savings_account_id, office_id, payment_detail_id, transaction_type_enum, " + + "is_reversed, transaction_date, amount, submitted_on_date, ref_no, " + + "running_balance_derived, is_lien_transaction, " + + "created_by, last_modified_by, created_on_utc, last_modified_on_utc) " + + "VALUES (?, ?, NULL, 2, false, ?, ?, ?, ?, ?, false, ?, ?, ?, ?)", new String[] { "id" }); + ps.setLong(1, savingsAccountId); + ps.setLong(2, officeId); + ps.setDate(3, sqlDate); + ps.setBigDecimal(4, amount); + ps.setDate(5, sqlDate); + ps.setString(6, refNo); + ps.setBigDecimal(7, balance.subtract(amount)); + ps.setLong(8, userId); + ps.setLong(9, userId); + ps.setObject(10, now); + ps.setObject(11, now); + return ps; + }, kh); + + this.jdbcTemplate.update("UPDATE m_savings_account SET account_balance_derived = account_balance_derived - ?, " + + "total_withdrawals_derived = total_withdrawals_derived + ? WHERE id = ?", amount, amount, savingsAccountId); + return Objects.requireNonNull(kh.getKey()).longValue(); } - private Long executeSavingsDeposit(final Long savingsAccountId, final BigDecimal amount, final LocalDate txnDate, final String note) { - final JsonObject payload = new JsonObject(); - payload.addProperty("locale", "en"); - payload.addProperty("dateFormat", "yyyy-MM-dd"); - payload.addProperty("transactionDate", txnDate.toString()); - payload.addProperty("transactionAmount", amount); - payload.addProperty("paymentTypeId", resolveSettlementPaymentTypeId()); - payload.addProperty("note", note); + /** + * Credits the member's savings account as a book transfer, with no journal entry of its own — the + * counterpart to {@link #debitSavingsAccount} for the one case that runs the other way: a cash-funded + * tranche whose receipts added up to more than it cost, where the difference has nowhere else to go + * but back to the member. + * + *

+ * Written directly for the same reason as the debit side: Fineract's deposit command would post its + * own Dr Cash / Cr Savings Control pair, and no cash is arriving at the counter here — the money was + * already in Share Sundry (or wherever the receipt's own GL was), and this is only moving the + * member's own balance to reflect it. No balance/status checks beyond existence: crediting an + * account can never leave it short of anything, unlike a debit. + */ + private Long creditSavingsAccount(final Long savingsAccountId, final Long officeId, final BigDecimal amount, + final LocalDate entryDate) { + final Map acct = this.jdbcTemplate + .queryForList("SELECT account_balance_derived FROM m_savings_account WHERE id = ?", savingsAccountId).stream() + .findFirst().orElse(null); + if (acct == null) { + throw new PlatformDataIntegrityException("error.msg.share.tranche.savings.missing", + "The linked savings account " + savingsAccountId + " no longer exists."); + } + final BigDecimal balance = acct.get("account_balance_derived") != null ? (BigDecimal) acct.get("account_balance_derived") + : BigDecimal.ZERO; - final CommandWrapper cmd = new CommandWrapperBuilder().savingsAccountDeposit(savingsAccountId).withJson(payload.toString()).build(); - final CommandProcessingResult result = this.commandsSourceWritePlatformService.logCommandSource(cmd); - return result.getResourceId(); + final java.sql.Date sqlDate = java.sql.Date.valueOf(entryDate); + final OffsetDateTime now = OffsetDateTime.now(DateUtils.getDateTimeZoneOfTenant()); + final String refNo = UUID.randomUUID().toString().replace("-", "").substring(0, 20); + final long userId = this.context.authenticatedUser().getId(); + + final KeyHolder kh = new GeneratedKeyHolder(); + this.jdbcTemplate.update(con -> { + // transaction_type_enum 1 = deposit. + final PreparedStatement ps = con.prepareStatement("INSERT INTO m_savings_account_transaction " + + "(savings_account_id, office_id, payment_detail_id, transaction_type_enum, " + + "is_reversed, transaction_date, amount, submitted_on_date, ref_no, " + + "running_balance_derived, is_lien_transaction, " + + "created_by, last_modified_by, created_on_utc, last_modified_on_utc) " + + "VALUES (?, ?, NULL, 1, false, ?, ?, ?, ?, ?, false, ?, ?, ?, ?)", new String[] { "id" }); + ps.setLong(1, savingsAccountId); + ps.setLong(2, officeId); + ps.setDate(3, sqlDate); + ps.setBigDecimal(4, amount); + ps.setDate(5, sqlDate); + ps.setString(6, refNo); + ps.setBigDecimal(7, balance.add(amount)); + ps.setLong(8, userId); + ps.setLong(9, userId); + ps.setObject(10, now); + ps.setObject(11, now); + return ps; + }, kh); + + this.jdbcTemplate.update("UPDATE m_savings_account SET account_balance_derived = account_balance_derived + ?, " + + "total_deposits_derived = total_deposits_derived + ? WHERE id = ?", amount, amount, savingsAccountId); + return Objects.requireNonNull(kh.getKey()).longValue(); } /** @@ -293,12 +484,10 @@ public class AdditionalShareTrancheFundingService { "A cash-funded share request must name the voucher the money was received on."); } - // Fineract's stock APPLIED posting raises Dr Cash at Vault / Cr Share Sundry for a new - // purchase. On this route the cashier already posted exactly that pair when the voucher was - // issued, so leaving it would count the member's money twice — Sundry credited 200 against a - // 100 receipt, and the till debited twice for cash that came in once. Both legs are reversed: - // submitting a cash-funded tranche moves no money, it only earmarks the receipt. - reverseStockAppliedPair(transaction.getId(), account.getShareProduct().getId(), account.getOfficeId()); + // No accounting at all on this route. The cashier already posted the money when the voucher + // was issued, and submitting a cash-funded tranche moves none of it -- it only earmarks that + // receipt for this account. Fineract's own APPLIED posting, which would have booked the same + // money a second time, is not made for a funded tranche (see applyAddtionalShares). for (final Long collectionId : collectionIds) { this.jdbcTemplate.update("UPDATE m_share_advance_collection SET applied_share_account_id = ? WHERE id = ?", account.getId(), @@ -309,7 +498,7 @@ public class AdditionalShareTrancheFundingService { /** * Savings route: debits the member and credits Share Sundry. */ - private void debitSavingsIntoSundry(final ShareAccount account, final ShareAccountTransaction transaction) { + private void validateSavingsFunding(final ShareAccount account, final ShareAccountTransaction transaction) { if (account.getSavingsAccount() == null) { throw new PlatformDataIntegrityException("error.msg.share.tranche.no.savings", "This share account has no linked savings account to debit."); @@ -332,90 +521,10 @@ public class AdditionalShareTrancheFundingService { throw new PlatformDataIntegrityException("error.msg.share.tranche.savings.insufficient", "Savings account " + savings.get("account_no") + " does not hold enough to cover this share request."); } - - final Long productId = account.getShareProduct().getId(); - final Long officeId = account.getOfficeId(); - final String currencyCode = account.getCurrency().getCode(); - final Long glSundry = requireShareGl(productId, FA_SHARES_SUSPENSE, "Share Sundry (Suspense)"); - final Long glSavingsControl = savingsControlGlOf(savingsAccountId); - if (glSavingsControl == null) { + if (savingsControlGlOf(savingsAccountId) == null) { throw new PlatformDataIntegrityException("error.msg.share.tranche.savings.gl.not.mapped", "The Savings Control GL is not mapped for this savings account's product."); } - - final LocalDate entryDate = DateUtils.getBusinessLocalDate(); - final String note = "Additional shares funded from savings | share account " + account.getAccountNumber(); - - // Fineract's own withdrawal command, so the member's passbook and statement show it. Its GL - // pair (Dr Savings Control / Cr Cash) is reversed below: no cash left the branch. - final Long savingsTxnId = executeSavingsWithdrawal(savingsAccountId, amount, entryDate, note); - // Guarded by reversed = false: if the "S"+id transaction_id text is ever reused (e.g. - // after a sequence reset), this can only flip rows that are currently live, never - // re-touch an already-reversed unrelated transaction's rows. - this.jdbcTemplate.update( - "UPDATE acc_gl_journal_entry SET reversed = true WHERE transaction_id = ? AND reversed = false", - "S" + savingsTxnId); - - // Fineract's stock APPLIED leg for a purchase posts Dr Shares Reference / Cr Share Sundry, - // assuming cash over the counter. BOTH legs are reversed, not just the reference debit: - // leaving its Sundry credit standing and then posting the savings pair below would credit - // Sundry twice — 200 held against a 100 debit — and the surplus would never clear. - reverseStockAppliedPair(transaction.getId(), productId, officeId); - - final AppUser currentUser = this.context.authenticatedUser(); - final long userId = currentUser.getId(); - final LocalDateTime now = LocalDateTime.now(DateUtils.getDateTimeZoneOfTenant()); - final String txnId = "SHTRSAV-" + transaction.getId(); - - insertEntry(officeId, glSavingsControl, DEBIT, amount, txnId, entryDate, account.getId(), userId, now, currencyCode, - "Additional shares debited from savings " + savings.get("account_no") + " | " + account.getAccountNumber()); - insertEntry(officeId, glSundry, CREDIT, amount, txnId, entryDate, account.getId(), userId, now, currencyCode, - "Additional shares held in Share Sundry pending approval | " + account.getAccountNumber()); - - transaction.updateSavingsDebitRequestId(savingsTxnId); - } - - /** - * Reverses the Shares Reference debit that Fineract's stock APPLIED posting raised for this - * tranche, leaving its Sundry credit standing. - * - * Scoped to this tranche's own entries and to those two GL accounts and sides specifically, so - * the Sundry-to-Share-Capital pair raised later at approval is never caught by it. - */ - private void reverseStockAppliedPair(final Long shareTransactionId, final Long productId, final Long officeId) { - reverseTrancheEntries(shareTransactionId, officeId, requireShareGl(productId, FA_SHARES_REFERENCE, "Shares Reference"), DEBIT); - reverseTrancheEntries(shareTransactionId, officeId, requireShareGl(productId, FA_SHARES_SUSPENSE, "Share Sundry (Suspense)"), - CREDIT); - } - - private Long executeSavingsWithdrawal(final Long savingsAccountId, final BigDecimal amount, final LocalDate txnDate, - final String note) { - final JsonObject payload = new JsonObject(); - payload.addProperty("locale", "en"); - payload.addProperty("dateFormat", "yyyy-MM-dd"); - payload.addProperty("transactionDate", txnDate.toString()); - payload.addProperty("transactionAmount", amount); - payload.addProperty("paymentTypeId", resolveSettlementPaymentTypeId()); - payload.addProperty("note", note); - - final CommandWrapper cmd = new CommandWrapperBuilder().savingsAccountWithdrawal(savingsAccountId).withJson(payload.toString()) - .build(); - final CommandProcessingResult result = this.commandsSourceWritePlatformService.logCommandSource(cmd); - return result.getResourceId(); - } - - private Long resolveSettlementPaymentTypeId() { - final List> rows = this.jdbcTemplate - .queryForList("SELECT id FROM m_payment_type WHERE value = ? ORDER BY id LIMIT 1", SETTLEMENT_PAYMENT_TYPE); - if (!rows.isEmpty()) { - return toLong(rows.get(0).get("id")); - } - final List> fallback = this.jdbcTemplate.queryForList("SELECT id FROM m_payment_type ORDER BY id LIMIT 1"); - if (fallback.isEmpty()) { - throw new PlatformDataIntegrityException("error.msg.share.tranche.no.payment.type", - "No payment types are configured. Add one before funding shares from savings."); - } - return toLong(fallback.get(0).get("id")); } private Map loadSavingsAccount(final Long savingsAccountId) { @@ -450,19 +559,28 @@ public class AdditionalShareTrancheFundingService { } } + /** + * {@code share_transaction_id} and {@code entity_type_enum} are written, not left null as a manual + * entry normally would: these rows stand in for the ones the platform would have posted under the + * same {@code SH} transaction id, so they have to be indistinguishable from them to everything + * that goes looking. Fineract's own {@code revertShareAccountJournalEntries} matches on the + * transaction id and {@code entity_type_enum = SHARES}, and would silently skip a row + * missing that column, leaving the accounting standing after the tranche behind it was reverted. + */ private void insertEntry(final Long officeId, final Long glAccountId, final int typeEnum, final BigDecimal amount, final String txnId, - final LocalDate entryDate, final Long shareAccountId, final long userId, final LocalDateTime now, final String currencyCode, - final String description) { + final LocalDate entryDate, final Long shareAccountId, final Long shareTransactionId, final long userId, + final LocalDateTime now, final String currencyCode, final String description) { GLAccountTransactionGuard.assertPostable(this.jdbcTemplate, glAccountId); this.jdbcTemplate.update( "INSERT INTO acc_gl_journal_entry " + "(account_id, office_id, currency_code, transaction_id, reversed, manual_entry, " - + " entry_date, type_enum, amount, description, entity_id, " + + " entry_date, type_enum, amount, description, entity_id, entity_type_enum, share_transaction_id, " + " created_by, last_modified_by, created_date, lastmodified_date, " + " is_running_balance_calculated, office_running_balance, " + " organization_running_balance, submitted_on_date, transaction_date, " + " created_on_utc, last_modified_on_utc) " - + "VALUES (?,?,?,?,false,true,?,?,?,?,?,?,?,?,?,false,0,0,?,?,?,?)", + + "VALUES (?,?,?,?,false,true,?,?,?,?,?,?,?,?,?,?,?,false,0,0,?,?,?,?)", glAccountId, officeId, currencyCode, txnId, java.sql.Date.valueOf(entryDate), typeEnum, amount, description, shareAccountId, - userId, userId, now, now, java.sql.Date.valueOf(entryDate), java.sql.Date.valueOf(entryDate), toUtc(now), toUtc(now)); + PRODUCT_TYPE_SHARES, shareTransactionId, userId, userId, now, now, java.sql.Date.valueOf(entryDate), + java.sql.Date.valueOf(entryDate), toUtc(now), toUtc(now)); } /** diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountSchedularServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountSchedularServiceImpl.java index e2ff9bf38..d0fcd2409 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountSchedularServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountSchedularServiceImpl.java @@ -24,6 +24,8 @@ import java.util.Map; import lombok.RequiredArgsConstructor; import org.apache.fineract.accounting.common.DividendGlAccountResolver; import org.apache.fineract.accounting.glaccount.validation.GLAccountTransactionGuard; +import org.apache.fineract.accounting.journalentry.service.LedgerTransactionId; +import org.apache.fineract.accounting.journalentry.service.LedgerTxnPrefix; import org.apache.fineract.infrastructure.core.service.DateUtils; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; import org.apache.fineract.portfolio.savings.domain.SavingsAccount; @@ -134,7 +136,7 @@ public class ShareAccountSchedularServiceImpl implements ShareAccountSchedularSe final AppUser currentUser = this.context.authenticatedUser(); final LocalDate entryDate = DateUtils.getBusinessLocalDate(); final java.sql.Date sqlEntryDate = java.sql.Date.valueOf(entryDate); - final String transactionId = "DIVFORFEIT" + lockId; + final String transactionId = LedgerTransactionId.next(this.jdbcTemplate, LedgerTxnPrefix.DIVIDEND_FORFEIT); // Tagged with the year the dividend was DECLARED for, not the year it is being forfeited in — // a 2026-27 dividend forfeits in 2029-30. That is what lets the ledger say how much of each // year's declaration went unclaimed and came back to profit. diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountWritePlatformServiceJpaRepositoryImpl.java index 37410ae78..b6d4c3b73 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareaccounts/service/ShareAccountWritePlatformServiceJpaRepositoryImpl.java @@ -178,13 +178,20 @@ public class ShareAccountWritePlatformServiceJpaRepositoryImpl implements ShareA if (!changes.isEmpty()) { this.shareAccountRepository.save(account); } - // since we are reverting all journal entries we need to add journal - // entries for application request + // Modifying a pending application replaces its purchase transaction(s) with new ones of + // the same shape, and only ever runs before the account has been approved -- at which + // point nothing has been posted for it in the first place (see createShareAccount: no + // cash has been collected yet, so there is nothing to post until + // ShareAdvanceCollectionApiResource settles the real receipt, which requires the account + // to already be Active). revertShareAccountJournalEntries is still called as a safety net + // for any legacy account that predates that change and does carry a native posting for + // its original transaction; it is a no-op where there is nothing to find. What it must + // NOT do is post a fresh entry for the replacement transaction, which this used to do + // unconditionally -- booking Dr Shares Reference (i.e. "cash received") for an + // application nobody has paid for yet. if (changes.containsKey("reversalIds")) { ArrayList reversalIds = (ArrayList) changes.get("reversalIds"); this.journalEntryWritePlatformService.revertShareAccountJournalEntries(reversalIds, transactionDate); - journalEntryWritePlatformService.createJournalEntriesForShares( - populateJournalEntries(account, account.getPendingForApprovalSharePurchaseTransactions())); changes.remove("reversalIds"); } return new CommandProcessingResultBuilder() // @@ -217,9 +224,17 @@ public class ShareAccountWritePlatformServiceJpaRepositoryImpl implements ShareA changes.put(ShareAccountApiConstants.additionalshares_paramname, transaction.getId()); Set transactions = new HashSet<>(); transactions.add(transaction); - this.journalEntryWritePlatformService.createJournalEntriesForShares(populateJournalEntries(account, transactions)); - // After the stock APPLIED posting, not before: the savings route reverses that - // posting's Shares Reference debit, which has to exist to be reversed. + // The stock APPLIED posting assumes cash came over the counter for these shares, + // and books Dr Shares Reference / Cr Share Sundry. A tranche funded through one of + // this fork's own routes did not arrive that way: the cash route's money was + // already taken at the till against a voucher, and the savings route debits the + // member's own account. Both used to let this post and then reverse it a moment + // later, leaving two dead rows behind on every tranche. Not posting it is the same + // outcome without the wreckage -- fund() below books what actually happened. + if (transaction.getFundingSource() == null) { + this.journalEntryWritePlatformService + .createJournalEntriesForShares(populateJournalEntries(account, transactions)); + } this.trancheFundingService.fund(account, transaction); } } @@ -297,15 +312,12 @@ public class ShareAccountWritePlatformServiceJpaRepositoryImpl implements ShareA this.noteRepository.save(note); } } - Set transactions = account.getShareAccountTransactions(); - Set journalTransactions = new HashSet<>(); - for (ShareAccountTransaction transaction : transactions) { - if (transaction.isActive() && !transaction.isChargeTransaction()) { - journalTransactions.add(transaction); - } - } - - this.journalEntryWritePlatformService.createJournalEntriesForShares(populateJournalEntries(account, journalTransactions)); + // Rejecting an application only ever happens before the account has been approved, at + // which point nothing has been posted for it -- createShareAccount posts nothing, and the + // real receipt is not booked until ShareAdvanceCollectionApiResource settles it, which + // requires the account to already be Active. Posting a REJECTED entry here used to credit + // Shares Reference (i.e. "cash returned") for cash that was never collected in the first + // place, because this application never reached the stage where any was. return new CommandProcessingResultBuilder() // .withCommandId(jsonCommand.commandId()) // .withEntityId(accountId) // @@ -412,9 +424,20 @@ public class ShareAccountWritePlatformServiceJpaRepositoryImpl implements ShareA transactions.add(transaction); totalSubscribedShares += transaction.getTotalShares(); } - this.journalEntryWritePlatformService.createJournalEntriesForShares(populateJournalEntries(account, transactions)); - // The APPROVED posting above has released the money from Share Sundry into - // share capital, so the funding behind it can now be closed off. + // The stock APPROVED posting releases the money into share capital out of the + // product's mapped Share Sundry account, whatever the money actually did. For a + // tranche funded through this fork's own routes that account is not always where + // it is: a voucher-funded one is sitting wherever the cashier banked the receipt. + // settle() books those itself, against the account the money is really in. + final Set stockPosted = new HashSet<>(); + for (ShareAccountTransaction transaction : transactions) { + if (transaction.getFundingSource() == null) { + stockPosted.add(transaction); + } + } + if (!stockPosted.isEmpty()) { + this.journalEntryWritePlatformService.createJournalEntriesForShares(populateJournalEntries(account, stockPosted)); + } for (ShareAccountTransaction transaction : transactions) { this.trancheFundingService.settle(account, transaction); } @@ -451,9 +474,20 @@ public class ShareAccountWritePlatformServiceJpaRepositoryImpl implements ShareA ShareAccountTransaction transaction = account.retrievePurchasedShares(id); transactions.add(transaction); } - this.journalEntryWritePlatformService.createJournalEntriesForShares(populateJournalEntries(account, transactions)); - // After the stock REJECTED posting, whose Shares Reference credit the savings - // route reverses in favour of refunding the member. + // The stock REJECTED posting unwinds the stock APPLIED one. A tranche funded + // through this fork's own routes never had that APPLIED posting made (see + // applyAddtionalShares), so posting its reversal here would book the undoing of + // something that was never booked. unwind() below returns the money the way it + // actually came in. + final Set stockPosted = new HashSet<>(); + for (ShareAccountTransaction transaction : transactions) { + if (transaction.getFundingSource() == null) { + stockPosted.add(transaction); + } + } + if (!stockPosted.isEmpty()) { + this.journalEntryWritePlatformService.createJournalEntriesForShares(populateJournalEntries(account, stockPosted)); + } for (ShareAccountTransaction transaction : transactions) { this.trancheFundingService.unwind(account, transaction); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareproducts/service/ShareProductWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareproducts/service/ShareProductWritePlatformServiceJpaRepositoryImpl.java index 7a9ac3d05..f35f10d21 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareproducts/service/ShareProductWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/shareproducts/service/ShareProductWritePlatformServiceJpaRepositoryImpl.java @@ -31,6 +31,8 @@ import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.fineract.accounting.common.DividendGlAccountResolver; import org.apache.fineract.accounting.glaccount.validation.GLAccountTransactionGuard; +import org.apache.fineract.accounting.journalentry.service.LedgerTransactionId; +import org.apache.fineract.accounting.journalentry.service.LedgerTxnPrefix; import org.apache.fineract.accounting.producttoaccountmapping.service.ProductToGLAccountMappingWritePlatformService; import org.apache.fineract.commands.domain.CommandProcessingResultType; import org.apache.fineract.commands.domain.CommandSource; @@ -304,7 +306,8 @@ public class ShareProductWritePlatformServiceJpaRepositoryImpl implements ShareP // back-dating it to 31 March files it months before it happened, where nobody reviewing the // approval would find it. The financial year it belongs to is carried in the description. postJournalEntryPair(distributableAccountId, settledAccountId, remainder, currencyCodeOfProduct(period.get("product_id")), - "DIVCLOSE" + declaration.getId(), java.sql.Date.valueOf(DateUtils.getBusinessLocalDate()), + LedgerTransactionId.next(this.jdbcTemplate, LedgerTxnPrefix.DIVIDEND_CLOSED), + java.sql.Date.valueOf(DateUtils.getBusinessLocalDate()), "FY " + financialYear + " undistributed profit closed off after dividend approval — declaration #" + declaration.getId()); } @@ -406,7 +409,7 @@ public class ShareProductWritePlatformServiceJpaRepositoryImpl implements ShareP final LocalDate periodEndDate = ((java.sql.Date) row.get("period_end_date")).toLocalDate(); final java.sql.Date sweptDate = java.sql.Date.valueOf(DateUtils.getBusinessLocalDate()); - final String transactionId = "DIVROUND" + dividendPayOutId; + final String transactionId = LedgerTransactionId.next(this.jdbcTemplate, LedgerTxnPrefix.DIVIDEND_ROUNDING); // Approval credited the whole declared amount to Dividend Payable; this takes back the slice // that will never be paid to a member, because no member's share divided into it evenly. @@ -555,7 +558,7 @@ public class ShareProductWritePlatformServiceJpaRepositoryImpl implements ShareP final AppUser currentUser = this.context.authenticatedUser(); final java.sql.Date sqlEntryDate = java.sql.Date.valueOf(DateUtils.getBusinessLocalDate()); - final String transactionId = "DIVDECL" + dividendPayOutId; + final String transactionId = LedgerTransactionId.next(this.jdbcTemplate, LedgerTxnPrefix.DIVIDEND_DECLARED); // The financial year is stamped into every dividend journal entry so the ledger itself can // answer "how much of FY 2026-27's dividend is still unclaimed" — the GL accounts are shared // across years, and without the year on the line the balances only ever give a running total. @@ -602,7 +605,8 @@ public class ShareProductWritePlatformServiceJpaRepositoryImpl implements ShareP + " (Reserve released to members)"; postJournalEntryPair(reserveAccountId, payableDividendsAccountId, amount, product.getCurrency().getCode(), - "DIVAPPR" + declaration.getId(), java.sql.Date.valueOf(DateUtils.getBusinessLocalDate()), description); + LedgerTransactionId.next(this.jdbcTemplate, LedgerTxnPrefix.DIVIDEND_APPROVED), + java.sql.Date.valueOf(DateUtils.getBusinessLocalDate()), description); } /** @@ -712,7 +716,7 @@ public class ShareProductWritePlatformServiceJpaRepositoryImpl implements ShareP final String forfeitedYear = financialYearOf(((java.sql.Date) lock.get("dividend_period_end_date")).toLocalDate()); postJournalEntryPair(payableDividendsAccountId, bankProfitAccountId, amount, (String) lock.get("currency_code"), - "DIVFORF" + lockId, entryDate, + LedgerTransactionId.next(this.jdbcTemplate, LedgerTxnPrefix.DIVIDEND_FORFEIT), entryDate, "FY " + forfeitedYear + " dividend unclaimed for " + CLAIMABLE_FINANCIAL_YEARS + " years — forfeited to profit on approval of FY " + financialYearOf(periodEndDate)); diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/voucher/api/VoucherVerificationApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/voucher/api/VoucherVerificationApiResource.java index a52042032..75aa18bf7 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/voucher/api/VoucherVerificationApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/voucher/api/VoucherVerificationApiResource.java @@ -940,88 +940,20 @@ public class VoucherVerificationApiResource { * The other side of a split transaction — GL entries a native platform command posted on its own, * outside this voucher, for the same underlying event. * - * Most vouchers already ARE the whole transaction: {@code raiseLoanVoucher()}, - * {@code raiseClosureVoucher()} and the rest each build every line themselves from the amount they - * are posting, so there is nothing else to find. Share redemption is the one place today where - * that is not true: approval runs the native {@code redeemshares} command, which posts its own - * Dr Share Capital / Cr Share Sundry pair directly (no voucher raised for it, since it is not this - * class that posts it). Only the later savings-settlement leg (Dr Sundry / Cr Savings Control) - * becomes a voucher. Showing just that half leaves a reader unable to see where the money - * actually came from. + * Nothing splits that way today, so this returns empty for every voucher. Share redemption used to: + * approval posted Dr Share Capital / Cr Share Sundry through the native {@code redeemshares} + * command, and only the settlement that followed (Dr Sundry / Cr Savings Control) became a + * voucher, so a reader saw half an entry and no sign of where the money came from. A redemption + * now books as the single entry the clerk actually keyed — Dr Share Capital straight to its + * destination — and the voucher carries both of its sides, so there is no longer a second half to + * show. Returning those same rows here would print this voucher's own lines a second time under a + * banner saying they are not part of it. * - * Extending this for a future split transaction is one more {@code if} branch keyed on - * referenceType, in the same shape as the share-withdrawal one below — not a new endpoint or a new - * field on the response. + * Kept rather than deleted because it is the seam a genuinely split transaction would hook into: + * one {@code if} branch keyed on referenceType, not a new endpoint or a new field on the response. */ private List> relatedGlEntries(final String referenceType, final Object referenceId) { - if (!VoucherService.REF_CLERK_SHARE_WITHDRAWAL_REQUEST.equals(referenceType) || referenceId == null) { - return List.of(); - } - final List> requestRows = jdbcTemplate.queryForList( - "SELECT share_transaction_id, settlement_mode, actioned_at FROM m_clerk_share_withdrawal_request " - + "WHERE id = ? AND share_transaction_id IS NOT NULL", - referenceId); - if (requestRows.isEmpty()) { - return List.of(); - } - // A settlement_mode 'NONE' request's own voucher (raiseSundryOnlyVoucher()) already IS leg 1 - // in full -- Dr Share Capital, Cr Share Sundry -- so showing it again here as "related" would - // duplicate this voucher's own lines under a banner claiming they are not part of it. - if ("NONE".equals(requestRows.get(0).get("settlement_mode"))) { - return List.of(); - } - final Long shareTxnId = Long.parseLong(String.valueOf(requestRows.get(0).get("share_transaction_id"))); - final java.sql.Timestamp actionedAt = (java.sql.Timestamp) requestRows.get(0).get("actioned_at"); - // Matched by transaction_id, not share_transaction_id: routeRedeemIntoSundry() reroutes the - // native posting's Cr Shares Reference leg by reversing it and inserting a fresh Cr Sundry row - // that carries the same transaction_id but no share_transaction_id of its own (only the native - // rows get that column populated) -- filtering on share_transaction_id found the Dr Link Share - // leg but silently dropped its Cr Sundry counterpart, showing only half of a balanced entry. - // transaction_id is what groups a balanced entry in this codebase (see routeRedeemIntoSundry's - // own comment), and the savings-settlement leg raiseShareWithdrawalVoucher() itself posts - // carries its own distinct transaction_id, so this still cleanly excludes it — that leg is - // already covered by this voucher's own lines. - // - // "SH" + shareTransactionId is only unique so long as m_share_account_transactions.id never - // repeats, which a reset/reseeded id sequence in a dev/test environment can violate -- two - // unrelated redemptions landing on the same id would otherwise share one transaction_id, and - // this query would return both pairs instead of the one this specific request actually - // posted. Disambiguated by picking whichever candidate transaction_id's own entry_date sits - // closest to actioned_at (when this request was approved), rather than assuming there is - // only ever one. - final List> nativeTxnCandidates = actionedAt == null - ? jdbcTemplate.queryForList("SELECT DISTINCT transaction_id FROM acc_gl_journal_entry WHERE share_transaction_id = ?", - shareTxnId) - : jdbcTemplate.queryForList(""" - SELECT transaction_id FROM acc_gl_journal_entry - WHERE share_transaction_id = ? - GROUP BY transaction_id - ORDER BY MIN(ABS(EXTRACT(EPOCH FROM (created_date - ?::timestamp)))) ASC - LIMIT 1 - """, shareTxnId, actionedAt); - if (nativeTxnCandidates.isEmpty()) { - return List.of(); - } - final String nativeTxnId = String.valueOf(nativeTxnCandidates.get(0).get("transaction_id")); - final List> rows = jdbcTemplate.queryForList(""" - SELECT je.entry_date, je.type_enum, je.amount, je.description, ga.name AS gl_name, ga.gl_code - FROM acc_gl_journal_entry je JOIN acc_gl_account ga ON ga.id = je.account_id - WHERE je.transaction_id = ? AND je.reversed = false - ORDER BY je.id - """, nativeTxnId); - - final List> out = new java.util.ArrayList<>(); - for (final Map r : rows) { - final Map m = new LinkedHashMap<>(); - m.put("stage", "Posted at manager approval"); - m.put("entryType", ((Number) r.get("type_enum")).intValue() == 2 ? "DR" : "CR"); - m.put("partyName", r.get("gl_name")); - m.put("accountNo", r.get("gl_code")); - m.put("amount", r.get("amount")); - m.put("description", r.get("description")); - out.add(m); - } - return out; + return List.of(); } // ── GET /v1/vouchers/detail?voucherNo=CR/2708/0001 ──────────────────────── 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 2f0be1dc1..0e6164682 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 @@ -108,4 +108,5 @@ + diff --git a/fineract-provider/src/main/resources/db/changelog/tenant/parts/0563_ledger_txn_sequence.xml b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0563_ledger_txn_sequence.xml new file mode 100644 index 000000000..e89250971 --- /dev/null +++ b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0563_ledger_txn_sequence.xml @@ -0,0 +1,48 @@ + + + + + + + + + + + + + + + + + + + + + + + + + +