fix(closure): post interest for real at submission, not as a dry-run estimate

Ticking "Close this account" used to only preview the payout: assemble the
account, run the same postInterest domain call close() will, read the
resulting balance, then roll the whole thing back (GET /preview/{id}).
Nothing was ever credited until the manager approved, and the account's own
Interest Posted / Interest Earned figures stayed stale in the meantime.

Changed to actually post the interest at that moment (POST
/post-interest/{id} and again in submit()), via
SavingsAccountWritePlatformService#postInterest(SavingsAccount, boolean,
LocalDate, boolean) -- the exact service method close() itself calls, not a
new calculation. Not routed through the POSTINTEREST_SAVINGSACCOUNT command:
that command ships maker-checker-enabled in base Fineract, and
CommandSourceService#processCommand runs the handler first, then throws
RollbackTransactionNotApprovedException from inside the @Transactional
method -- which would silently roll back the very posting just made.

Depends on the sibling SavingsAccount fix (double-posting across runs) --
without it, ticking the checkbox on one business day and having the manager
approve on a later one re-slices posting periods and credits the overlapping
days twice.

Also:
- Locks the closure date at submission (stored on the request row) and
  replays that same date at approve() instead of re-reading today's business
  date, so close()'s own postInterest(account, true, closedOnDate, false)
  slices identical periods, matches the amounts already posted at submission,
  and adds nothing -- the figure quoted to the member at submission is what
  gets paid out however long the request sits in the manager's queue.
- Populates closing_amount/rounding_amount/cashier_disbursed_amount at
  submission time instead of leaving them null until approval, since the
  interest posting above already makes the real payout figure known then --
  the manager's review screen no longer shows a blank amount for a PENDING
  request.

fineract-provider/src/main/java/org/apache/fineract/portfolio/clerkwithdrawal/api/ClerkClosureRequestApiResource.java

@@ -37,7 +37,6 @@ import jakarta.ws.rs.WebApplicationException;
 import jakarta.ws.rs.core.MediaType;
 import jakarta.ws.rs.core.Response;
 import java.math.BigDecimal;
-import java.math.MathContext;
 import java.math.RoundingMode;
 import java.sql.Date;
 import java.sql.Timestamp;
@@ -45,6 +44,7 @@ import java.time.Instant;
 import java.time.LocalDate;
 import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -53,13 +53,12 @@ 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.configuration.domain.ConfigurationDomainService;
 import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper;
 import org.apache.fineract.infrastructure.core.service.DateUtils;
 import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
-import org.apache.fineract.organisation.monetary.domain.MoneyHelper;
 import org.apache.fineract.portfolio.savings.domain.SavingsAccount;
 import org.apache.fineract.portfolio.savings.domain.SavingsAccountAssembler;
+import org.apache.fineract.portfolio.savings.service.SavingsAccountWritePlatformService;
 import org.apache.fineract.portfolio.voucher.service.VoucherLine;
 import org.apache.fineract.portfolio.voucher.service.VoucherService;
 import org.apache.fineract.useradministration.domain.AppUser;
@@ -67,10 +66,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.stereotype.Component;
-import org.springframework.transaction.PlatformTransactionManager;
-import org.springframework.transaction.TransactionDefinition;
 import org.springframework.transaction.annotation.Transactional;
-import org.springframework.transaction.support.TransactionTemplate;
 
 /**
  * Maker-checker Cash-only savings account closure. Mirrors {@link ClerkWithdrawalRequestApiResource}'s
@@ -99,11 +95,15 @@ public class ClerkClosureRequestApiResource {
     private final FromJsonHelper fromJsonHelper;
     private final PlatformSecurityContext context;
     private final PortfolioCommandSourceWritePlatformService commandsSourceWritePlatformService;
-    // Used only by preview(), to compute the closing figure the way close() will actually compute it.
+    // Used by postClosureInterest(), invoked from both submit() and postInterestForClosure(), to
+    // assemble the account before posting real interest onto it.
     private final SavingsAccountAssembler savingsAccountAssembler;
-    private final ConfigurationDomainService configurationDomainService;
-    private final PlatformTransactionManager transactionManager;
     private final VoucherService voucherService;
+    // The interest posting that has always backed "Post Interest As On" and close() alike. Called as
+    // the service method rather than through the POSTINTEREST_SAVINGSACCOUNT command, whose
+    // maker-checker flag would roll the posting straight back out again (CommandSourceService runs
+    // the handler first, then throws RollbackTransactionNotApprovedException).
+    private final SavingsAccountWritePlatformService savingsAccountWritePlatformService;
     private final Gson gson = new GsonBuilder().serializeNulls().create();
 
     // ── POST /v1/clerk/closure-requests ── Clerk submits new request ────────────
@@ -154,64 +154,97 @@ public class ClerkClosureRequestApiResource {
             throw apiError(409, "This account already has an open closure request (pending or awaiting cashier).");
         }
 
+        // The closure date is fixed HERE, at submission, and every later step reads it back off the row
+        // rather than re-reading the business date. That is what keeps the figure the clerk quoted the
+        // member identical to the figure the cashier hands over: closedOnDate drives which posting
+        // periods close() slices, so letting it float to the manager's approval day would recompute the
+        // interest against different period boundaries and pay out a different amount.
+        final LocalDate closureDate = DateUtils.getBusinessLocalDate();
+
+        // Post the interest for real, now -- the account's balance must already include everything the
+        // member has earned the moment the clerk raises the closure, not merely be estimated. This is
+        // the same service method "Post Interest As On" and close() both go through, invoked with the
+        // same arguments close() will use (postInterestAs = true, transactionDate = closureDate), so
+        // when the manager approves, close()'s own postInterest(account, true, closureDate, false) finds
+        // these very transactions at the very same period ends, matches their amounts, and adds nothing.
+        final BigDecimal closingAmount = postClosureInterest(savingsAccountId, closureDate);
+        // Rounded the same way approve() rounds it -- so the manager sees, while it is still PENDING,
+        // exactly what cash the cashier will end up handing over, not just the exact-to-the-paisa figure.
+        final BigDecimal cashierDisbursedAmount = closingAmount.setScale(0, RoundingMode.HALF_UP);
+        final BigDecimal roundingAmount = cashierDisbursedAmount.subtract(closingAmount);
+
         final String activityLog = buildActivityLog(null, "SUBMITTED", currentUser.getUsername(), null);
 
         final Long newId = jdbcTemplate.queryForObject("""
                 INSERT INTO m_clerk_closure_request
                 (savings_account_id, account_no, client_id, client_name, closure_date,
                  payment_type_id, payment_type_name, description, tok_no,
-                 status, clerk_id, clerk_name, clerk_username, activity_log, submitted_at)
-                VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
+                 status, closing_amount, rounding_amount, cashier_disbursed_amount,
+                 clerk_id, clerk_name, clerk_username, activity_log, submitted_at)
+                VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
                 RETURNING id
                 """,
             Long.class,
             savingsAccountId, account.get("account_no"), account.get("client_id"), account.get("client_name"),
-            Date.valueOf(DateUtils.getBusinessLocalDate()),
+            Date.valueOf(closureDate),
             paymentTypeId, paymentTypeName, description, tokNo,
-            "PENDING", currentUser.getId(), currentUser.getDisplayName(), currentUser.getUsername(),
+            "PENDING", closingAmount, roundingAmount, cashierDisbursedAmount,
+            currentUser.getId(), currentUser.getDisplayName(), currentUser.getUsername(),
             activityLog, Timestamp.from(Instant.now())
         );
 
-        return gson.toJson(Map.of("resourceId", newId, "status", "PENDING"));
+        final Map<String, Object> response = new LinkedHashMap<>();
+        response.put("resourceId", newId);
+        response.put("status", "PENDING");
+        response.put("closureDate", closureDate.toString());
+        response.put("payableAmount", closingAmount);
+        response.put("roundedPayableAmount", cashierDisbursedAmount);
+        return gson.toJson(response);
     }
 
-    // ── GET /v1/clerk/closure-requests/preview/{savingsAccountId} ──────────────
+    /**
+     * Posts everything the member has earned up to the closure date, and returns the resulting real
+     * account balance -- the figure the closure will pay out.
+     *
+     * Deliberately not the CALCULATEINTEREST command and not a bespoke calculation: it is the very
+     * {@code postInterest(account, true, closureDate, false)} call that
+     * {@code SavingsAccountWritePlatformServiceJpaRepositoryImpl#close} makes, so the two agree by
+     * construction rather than by coincidence. Going through the service (not the entity directly)
+     * keeps the journal entries and the interest voucher that a posting is supposed to raise.
+     */
+    private BigDecimal postClosureInterest(final Long savingsAccountId, final LocalDate closureDate) {
+        final SavingsAccount account = savingsAccountAssembler.assembleFrom(savingsAccountId, false);
+        savingsAccountWritePlatformService.postInterest(account, true, closureDate, false);
+        return zeroIfNull(account.getSummary().getAccountBalance());
+    }
+
+    // ── POST /v1/clerk/closure-requests/post-interest/{savingsAccountId} ───────
 
     /**
-     * "What would this member actually receive" preview, so a clerk sees the real closing figure
-     * before submitting.
+     * Posts everything the member has earned up to the closure date and reports the account's real
+     * resulting balance -- the figure the closure will pay out.
      *
-     * This must reproduce what close() will pay out, to the paisa, and reading the stored
-     * {@code total_interest_earned_derived} does not: that column is only as fresh as whatever last
-     * touched the account, so it reads days stale. Nor does the CALCULATEINTEREST command, which is
-     * close but not identical -- it uses {@code MathContext(15)} and passes
-     * {@code postInterestOnDate = null}, whereas close() calls
-     * {@code postInterest(account, true, closedDate, false)}, which uses {@code MathContext(10)} and
-     * feeds {@code closedDate} in as an extra posting-period boundary
-     * (SavingsAccount#calculateInterestUsing adds it to postedAsOnDates). Splitting the final period
-     * changes how many per-period roundings happen, so the two can disagree by paise.
+     * A POST, not a GET, because it genuinely mutates: ticking "Close this account" is meant to put
+     * the interest INTO the account, not to estimate it. A dry run used to stand here, and it read
+     * correctly to the paisa, but it left the account's own balance behind the figure on screen and
+     * the clerk had no way to see that the member had actually been credited.
      *
-     * So this runs the real thing as a dry run: assemble the account, invoke the same DOMAIN method
-     * close() ends up in with the same arguments, read the resulting balance -- which is exactly the
-     * figure close() withdraws (it takes {@code account.getSummary().getAccountBalance()} right
-     * after posting interest) -- and then roll the whole thing back.
+     * The posting itself is {@link #postClosureInterest}, i.e. the very
+     * {@code postInterest(account, true, closureDate, false)} that
+     * {@code SavingsAccountWritePlatformServiceJpaRepositoryImpl#close} performs -- so approval later
+     * finds these transactions at the identical period ends and adds nothing.
      *
-     * Why this is safe to run per keystroke:
-     * <ul>
-     * <li>The domain {@code postInterest} touches nothing outside the entity graph: no repository
-     * save, no journal entry, no voucher, no event. All of that lives in the service layer, which is
-     * deliberately bypassed here.</li>
-     * <li>REQUIRES_NEW, so marking this rollback-only cannot poison a caller's transaction, and the
-     * dirtied entities live in a separate persistence context that is discarded.</li>
-     * <li>Config is read before the account is assembled: a cache-miss JPA query afterwards could
-     * auto-flush the in-memory interest transactions, and while the rows would roll back, the
-     * identity-sequence values they consumed would not.</li>
-     * </ul>
+     * Safe to tick and untick, and safe across days: SavingsAccount#postInterest reverses any manual
+     * posting this run's periods have orphaned before applying them, so a second run can no longer
+     * double-credit the days an earlier one already paid. Cancelling the popup or a manager rejecting
+     * the request deliberately leaves the posting in place -- it is interest the member had genuinely
+     * earned either way, and the next posting run reconciles it correctly.
      */
-    @GET
-    @Path("/preview/{savingsAccountId}")
+    @POST
+    @Path("/post-interest/{savingsAccountId}")
     @Produces(MediaType.APPLICATION_JSON)
-    public String preview(@PathParam("savingsAccountId") final Long savingsAccountId) {
+    @Transactional
+    public String postInterestForClosure(@PathParam("savingsAccountId") final Long savingsAccountId) {
         final List<Map<String, Object>> rows = jdbcTemplate.queryForList("""
                 SELECT sa.account_no, sa.status_enum, sa.deposit_type_enum,
                        COALESCE(c.display_name, g.display_name) AS client_name,
@@ -227,63 +260,39 @@ public class ClerkClosureRequestApiResource {
         final Map<String, Object> account = rows.get(0);
         // Savings only (deposit_type_enum 100). An FD/RD assembles as a FixedDepositAccount /
         // RecurringDepositAccount, whose interest and tax on closure run through
-        // postMaturityInterest / applyWithholdTaxForDepositAccounts instead -- the dry run below
-        // would quietly compute the wrong number for them.
+        // postMaturityInterest / applyWithholdTaxForDepositAccounts instead -- posting through the
+        // plain savings path would quietly compute the wrong number for them.
         final int depositType = ((Number) account.get("deposit_type_enum")).intValue();
         if (depositType != 100) {
             throw apiError(400, "This closure flow supports savings accounts only (deposit type " + depositType + ").");
         }
-
-        final BigDecimal storedBalance = new BigDecimal(account.get("balance").toString());
-        // The CBS business date, not wall-clock: this is the date the closure will actually post on
-        // (executeClose sends it as closedOnDate) and the date close() itself calculates interest up
-        // to. Using LocalDate.now() here would silently disagree with approval around a day roll.
-        final LocalDate closureDate = DateUtils.getBusinessLocalDate();
-
-        // Read configuration first -- see the flush note in the javadoc.
-        final boolean isSavingsInterestPostingAtCurrentPeriodEnd = configurationDomainService
-            .isSavingsInterestPostingAtCurrentPeriodEnd();
-        final Integer financialYearBeginningMonth = configurationDomainService.retrieveFinancialYearBeginningMonth();
-        final MathContext mc = new MathContext(10, MoneyHelper.getRoundingMode());
-
-        final TransactionTemplate dryRun = new TransactionTemplate(transactionManager);
-        dryRun.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW);
-
-        BigDecimal[] figures = null;
-        try {
-            figures = dryRun.execute(status -> {
-                final SavingsAccount acct = savingsAccountAssembler.assembleFrom(savingsAccountId, false);
-                final BigDecimal before = zeroIfNull(acct.getSummary().getAccountBalance());
-                // The exact call close() makes, via postInterest(account, true, closedDate, false).
-                acct.postInterest(mc, closureDate, false, isSavingsInterestPostingAtCurrentPeriodEnd,
-                        financialYearBeginningMonth, closureDate, false, false);
-                final BigDecimal after = zeroIfNull(acct.getSummary().getAccountBalance());
-                status.setRollbackOnly();
-                return new BigDecimal[] { before, after };
-            });
-        } catch (final RuntimeException e) {
-            // Never cost the clerk the preview entirely -- fall through to the stored balance and
-            // flag that the interest figure could not be computed.
-            figures = null;
+        final int statusEnum = ((Number) account.get("status_enum")).intValue();
+        if (statusEnum != 300) {
+            throw apiError(409, "Only an ACTIVE account can be closed. Current status: " + statusEnum);
         }
 
-        final boolean computed = figures != null;
-        final BigDecimal balance = computed ? figures[0] : storedBalance;
-        final BigDecimal total = computed ? figures[1] : storedBalance;
-        final BigDecimal interest = total.subtract(balance);
+        final BigDecimal balanceBefore = new BigDecimal(account.get("balance").toString());
+        // The CBS business date, not wall-clock: submit() stamps this same date onto the request as
+        // closure_date, and approve() replays it as closedOnDate -- so it is the date the interest is
+        // calculated up to at every step. Using LocalDate.now() here would silently disagree around a
+        // day roll.
+        final LocalDate closureDate = DateUtils.getBusinessLocalDate();
+
+        final BigDecimal total = postClosureInterest(savingsAccountId, closureDate);
+        final BigDecimal interest = total.subtract(balanceBefore);
 
         final JsonObject res = new JsonObject();
         res.addProperty("savingsAccountId", savingsAccountId);
         res.addProperty("accountNo", String.valueOf(account.get("account_no")));
         res.addProperty("clientName", account.get("client_name") == null ? "" : String.valueOf(account.get("client_name")));
-        res.addProperty("statusEnum", ((Number) account.get("status_enum")).intValue());
-        res.addProperty("balance", balance);
+        res.addProperty("statusEnum", statusEnum);
+        res.addProperty("balance", balanceBefore);
         res.addProperty("accruedInterest", interest);
         res.addProperty("total", total);
         // What a cashier could physically count out: no paisa coins circulate, so the hand-over is
         // always whole rupees (the difference lands on the Round Off Account at approval).
         res.addProperty("roundedTotal", total.setScale(0, RoundingMode.HALF_UP));
-        res.addProperty("interestUpToDate", computed);
+        res.addProperty("interestUpToDate", true);
         res.addProperty("asOfDate", closureDate.toString());
         return gson.toJson(res);
     }
@@ -475,9 +484,14 @@ public class ClerkClosureRequestApiResource {
             throw apiError(409, "Account is no longer ACTIVE (status " + statusEnum + ") — cannot close.");
         }
 
-        // Fineract's own close command: pro-rata interest, TDS and the real GL withdrawal all post
-        // atomically here -- exactly as they would from the ordinary Close Savings Account screen.
-        executeClose(savingsAccountId);
+        // Fineract's own close command, run against the date the closure was RAISED on, not today's.
+        // submit() already posted the interest up to that date; replaying the same closedOnDate here
+        // makes close()'s postInterest(account, true, closedOnDate, false) slice the identical posting
+        // periods, find those very transactions at the identical period ends, match their amounts and
+        // add nothing -- so the balance it withdraws is exactly the figure the clerk quoted, however
+        // many days the request sat in the manager's queue.
+        final LocalDate closureDate = toLocalDate(record.get("closure_date"));
+        executeClose(savingsAccountId, closureDate);
 
         // Read back the withdrawal transaction close() just posted -- its CommandProcessingResult
         // doesn't return the transaction id/amount directly. transaction_type_enum 2 = WITHDRAWAL.
@@ -672,14 +686,34 @@ public class ClerkClosureRequestApiResource {
 
     // ── Helpers ───────────────────────────────────────────────────────────────
 
-    private void executeClose(final Long savingsAccountId) {
-        // Business date, matching what preview() computed the figure against.
-        final String today = DateUtils.getBusinessLocalDate().format(DateTimeFormatter.ofPattern("dd MMMM yyyy"));
+    /**
+     * Reads a DATE column back out of a JDBC result map. The driver hands back {@link java.sql.Date}
+     * today, but a {@link LocalDate} under a different driver or mapper configuration would otherwise
+     * blow up the whole approval on a ClassCastException, so both are accepted.
+     */
+    private LocalDate toLocalDate(final Object value) {
+        if (value == null) {
+            throw apiError(500, "Closure request is missing its closure date — cannot close the account.");
+        }
+        if (value instanceof LocalDate localDate) {
+            return localDate;
+        }
+        if (value instanceof Date sqlDate) {
+            return sqlDate.toLocalDate();
+        }
+        return LocalDate.parse(value.toString().substring(0, 10));
+    }
+
+    private void executeClose(final Long savingsAccountId, final LocalDate closureDate) {
+        // The date the closure was RAISED on, read off the request row -- never today's business date.
+        // See the call site: it is what keeps the amount quoted to the member and the amount paid out
+        // identical no matter how long approval takes.
+        final String closedOn = closureDate.format(DateTimeFormatter.ofPattern("dd MMMM yyyy"));
 
         final JsonObject payload = new JsonObject();
         payload.addProperty("locale", "en");
         payload.addProperty("dateFormat", "dd MMMM yyyy");
-        payload.addProperty("closedOnDate", today);
+        payload.addProperty("closedOnDate", closedOn);
         // Unconditional and never skippable -- closure always withdraws the full balance.
         payload.addProperty("withdrawBalance", true);
 

fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java

@@ -595,6 +595,62 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom<Long>
             withholdTransactions.addAll(findWithHoldTransactions());
         }
 
+        // Reverse manual interest postings that no longer line up with any period this run computed,
+        // BEFORE the periods below are applied.
+        //
+        // A user posting ("post interest as on D") lands its transaction on D-1, because
+        // SavingsHelper#determineInterestPostingPeriods closes the period at D-1 when it meets D in
+        // postedAsOnDates. But getManualPostingDates() then feeds that transaction's own date -- D-1
+        // -- back in as a boundary on every later run, which closes a period at D-2 instead. So the
+        // posting's date matches no period end the second time around: findInterestPostingTransactionFor
+        // below misses it, it is never reversed or corrected, and it survives alongside a new posting
+        // covering the very same days. The member is credited twice, and total_interest_posted_derived
+        // drifts above what was actually earned.
+        //
+        // A posting is only treated as orphaned, and only reversed, when BOTH hold:
+        // - its date is not any period's posting date, so the loop below will never find and correct it;
+        // - some period that WILL be posted in this run (posting date on or before
+        // interestPostingUpToDate) covers that date, so the days it credited are being credited again.
+        // The second condition is what keeps this from ever destroying interest. A run whose remaining
+        // period only matures in the future posts nothing for those days -- the recalculation that
+        // follows a withdrawal is exactly that case -- and reversing there would silently take earned
+        // interest back off the member instead of de-duplicating it.
+        // Automatic postings are left alone: they sit on natural period ends, which always recur.
+        final Set<LocalDate> periodPostingDates = new HashSet<>();
+        final List<LocalDateInterval> repostedIntervals = new ArrayList<>();
+        for (final PostingPeriod period : postingPeriods) {
+            periodPostingDates.add(period.dateOfPostingTransaction());
+            if (!DateUtils.isAfter(period.dateOfPostingTransaction(), interestPostingUpToDate)) {
+                repostedIntervals.add(period.getPeriodInterval());
+            }
+        }
+        final List<SavingsAccountTransaction> postingsInScope = backdatedTxnsAllowedTill
+                ? getSavingsAccountTransactionsWithPivotConfig()
+                : getTransactions();
+        for (final SavingsAccountTransaction transaction : new ArrayList<>(postingsInScope)) {
+            final LocalDate transactionDate = transaction.getTransactionDate();
+            if (!transaction.isInterestPosting() || !transaction.isNotReversed() || transaction.isReversalTransaction()
+                    || !transaction.isManualTransaction() || periodPostingDates.contains(transactionDate)) {
+                continue;
+            }
+            boolean coveredByAPostedPeriod = false;
+            for (final LocalDateInterval interval : repostedIntervals) {
+                if (interval.contains(transactionDate)) {
+                    coveredByAPostedPeriod = true;
+                    break;
+                }
+            }
+            if (!coveredByAPostedPeriod) {
+                continue;
+            }
+            transaction.reverse();
+            final SavingsAccountTransaction strandedWithholdTransaction = findTransactionFor(transactionDate, withholdTransactions);
+            if (strandedWithholdTransaction != null) {
+                strandedWithholdTransaction.reverse();
+            }
+            recalucateDailyBalanceDetails = true;
+        }
+
         for (final PostingPeriod interestPostingPeriod : postingPeriods) {
             final LocalDate interestPostingTransactionDate = interestPostingPeriod.dateOfPostingTransaction();
             final Money interestEarnedToBePostedForPeriod = interestPostingPeriod.getInterestEarned();