adamsaghy commented on PR #6223:
URL: https://github.com/apache/fineract/pull/6223#issuecomment-5205579768
@alberto-art3ch Please kindly see my concerns:
1. Migration 0066 never inserts the permissions. Charge-off's
parts/0063_wc_loan_charge_off.xml:70-88 inserts CHARGEOFF_WORKINGCAPITALLOAN /
UNDOCHARGEOFF_WORKINGCAPITALLOAN into m_permission; 0066_wc_loan_write_off.xml
inserts nothing.
**Two consequences:**
With the maker-checker global config enabled,
ConfigurationDomainServiceJpa.java:54 does
permissionRepository.findOneByCode("WRITEOFF_WORKINGCAPITALLOAN") and throws
PermissionNotFoundException — every write-off call fails.
No role other than a super-user (ALL_FUNCTIONS) can ever be granted the
permission.
The new doc chapter documents both permissions as if they exist. Add the two
m_permission changesets mirroring 0063.
2. writeOffReasonId vs progressive's writeoffReasonId. The AC says incoming
parameters follow progressive loans. Progressive/cumulative use lower-case
writeoffReasonId — LoanTransactionValidatorImpl.java:437 and
LoanWritePlatformServiceJpaRepositoryImpl.java:1500. The PR introduces
writeOffReasonId. Everything else in the set (transactionDate, note, locale,
dateFormat, externalId) matches.
Also the field was not added to PostWorkingCapitalLoanTransactionsRequest in
WorkingCapitalLoanTransactionsApiResourceSwagger.java:192-215, even though
chargeOffReasonId is there (line 209). So the generated client can't send a
write-off reason at all, which is also why no test exercises it. Same for
reversalExternalId on undo — accepted by the validator, absent from the request
model.
3. Write-off of an already charged-off loan double-credits the receivables.
Charge-off keeps the loan ACTIVE and validateWriteOff only requires ACTIVE, so
write-off is reachable straight after a charge-off. chargeOffPostings already
credits LOAN_PORTFOLIO / FEES_RECEIVABLE / PENALTIES_RECEIVABLE for the full
outstanding; the new writeOffPostings credits them again — the service
hardcodes postJournalEntries(..., false) and the WRITEOFF case ignores
isChargedOff entirely. Regular loans branch explicitly at
AccrualBasedAccountingProcessorForLoan.java:1379 into
createJournalEntriesForWriteOffsWhenLoanIsChargedOff (Dr CHARGE_OFF_EXPENSE /
INCOME_FROM_CHARGE_OFF_FEES / _PENALTY, Cr FUND_SOURCE, with
CHARGE_OFF_FRAUD_EXPENSE when the loan is flagged fraud — and
m_wc_loan.is_fraud exists here too).
Let's replicate the implementation from the progressive loan handling.
4. The "post-write-off lock" NOTE in the doc is wrong. The chapter says the
lock is "pending confirmation and not yet enforced". It's actually already
enforced by pre-existing status gates: repayment/goodwill/payout-refund via
REPAYMENT_LIKE_TXN_ALLOWED_LOAN_STATUSES
(WorkingCapitalLoanDataValidator.java:136), transaction undo via
validateUndoTransaction (line 958), charges via
WorkingCapitalLoanChargeWritePlatformServiceImpl.java:398, charge-off and
undo-disbursal via their own ACTIVE checks. So the AC is met — but nothing pins
it. Drop the NOTE and add a test or two asserting a repayment and a
transaction-undo are rejected on a written-off loan.
5. No status-changed / balance-changed events, no delinquency reset. Regular
write-off emits LoanBalanceChangedBusinessEvent and calls
setLoanDelinquencyTag; the WC repayment path emits
WorkingCapitalLoanBalanceChangedBusinessEvent and calls
delinquencyRangeScheduleService.reprocessDelinquencySchedule. The write-off
service emits only the two new transaction events. Since CLOSED_WRITTEN_OFF is
excluded from COB (WorkingCapitalLoanRetrieveIdServiceImpl
NON_CLOSED_LOAN_STATUSES), a delinquent loan stays tagged delinquent forever
after write-off, and undo never re-derives it.
6. Null balance is silently tolerated. zeroOutstanding returns early when
balance == null and the service defaults all portions to zero — the loan closes
as written-off with a zero-amount transaction. Charge-off deliberately throws
error.msg.wc.loan.balance.not.found for exactly this case. Make it consistent.
--
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]
