oleksii-novikov-onix commented on PR #6496: URL: https://github.com/apache/fineract/pull/6496#issuecomment-5830757631
> I couldn't find a blocking bug, and I checked the risky parts. The discount fee is always dated on the related disbursement's date (a supplied `transactionDate` must equal it, so it can never be dated after the business date), charged-off loans are still rejected before anything is written, and `isOpen()` only returns true for ACTIVE, so closed/overpaid loans are rejected too - which is why not calling reprocessing here is safe (a higher principal on an ACTIVE loan with no overpayment doesn't change how later repayments were allocated). `applyDiscountFeeAdjustment` rebuilds the schedule from the payments already on it, so a repayment after disbursement is kept, the backdated `DISCOUNT_FEE` and the catch-up amortization on the next COB have the right journal entries in UC3, and undo still only goes through undo disbursal. > > Things to settle before it leaves draft: > > * **GL closure.** This is the first PR that backdates a discount fee journal entry (before it could only land on today's business date). `postDiscountFeeDeferralEntries` does call `checkForBranchClosures` for accrual products, so backdating into a closed period is blocked there - but nothing in the WC module checks closures for other paths, so please confirm the check covers this entry and add an e2e case for a disbursement on or before the latest branch closure. > * **No unit tests for the new lookup.** `WorkingCapitalLoanWritePlatformServiceImpl` ~L553-562 has four outcomes (id only, id plus matching date, id plus a different date, date only with no active disbursement on that date) and the PR only deletes the validator test. The `Optional.map().or().filter()` chain also hides which check failed - "no disbursement on that date" and "date mismatch" share one error code. > * **The negative e2e cases skip the likeliest client mistake.** UC2 only tries 31 December 2025 (before disbursement); there's no case sending the later business date (08 January), with or without `relatedResourceId`. > * `transactionDate` was already an allowed parameter but silently ignored; now a mismatching date is rejected with a 400. That's arguably right, but call it out in the ticket/release notes. > * `findActiveByTypeAndTransactionDate` returns an `Optional`, so two active disbursements on one date would 500. Can't happen while a loan is disbursed once, but it's a latent bug if multi-tranche WC lands. > * Question: `makeDiscountFeeAdjustment` reprocesses when there's an overpayment above zero but `makeDiscountFee` never does - I couldn't find a path that gives an ACTIVE loan a leftover overpayment (a reopen, maybe), just flagging it. The charge-off rule also differs (a discount fee before the charge-off is rejected, an adjustment before it is allowed) - a sentence in the doc would explain why. > > Recommendation: COMMENT 1. GL closure: Covered. The discount fee journal entry is checked against the branch closure on its backdated date, and without accounting nothing is posted. New e2e: branch closed on the disbursement date gives 403 and nothing is saved. 2. Unit tests: Added for your four cases plus two more, with the lookup moved into its own method. "No disbursement on that date" and "date mismatch" now return separate errors. 3. Negative e2e: Added to UC2: 08 January on business date 08 January, with and without relatedResourceId, each with its own error. 4. Release notes: In the PR description. transactionDate used to be ignored; now a wrong or unparsable date gets 400. Requests without it work as before, just no longer limited to the disbursement day. 5. Two disbursements on one date: Agreed, though it would be a generic 403 data.integrity.issue rather than a 500. Today a loan has only one active disbursement and a per-loan discount, so we'll revisit the lookup with multi-tranche rather than silently pick a tranche now. 6. Overpayment: No such path: the status check after every money movement moves any loan with an overpayment to OVERPAID, and discountFee is ACTIVE-only. Undo of a discount adjustment skips reprocessing for the same reason. Caveat: with a not-yet-due charge, a bigger principal can change an earlier repayment's split, but develop already does the same on that undo, so it's out of scope. 7. Charge-off: Added to the doc. The discount fee path doesn't replay the charge-off, so it would leave principal the charge-off never wrote off, and it stays rejected as before this PR. A backdated adjustment only lowers the discount, and the loan is reprocessed so the charge-off is restated. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
