adamsaghy commented on PR #6399: URL: https://github.com/apache/fineract/pull/6399#issuecomment-5727653574
@oleksii-novikov-onix Can you please review the below Findings / concerns? 1. fineract-working-capital-loan/.../WorkingCapitalLoanChargeEnricher.java:57 — amountUnrecognized is still amount − amountAccrued, but WorkingCapitalLoanChargeAccrualService.java:144 now caps the accrual target at amount − amountWaived. A fully waived, never-accrued fee of 100 reports amountUnrecognized = 100 on every balance-changed event forever, though nothing more will ever be recognized. MathUtil.subtractToZero(charge.getAmount(), amountAccrued, amountWaived) would match the new target. It's a published, e2e-asserted event field (WorkingCapitalLoanAccountBusinessEvents.feature:161), so it's worth getting right. Seems some fields are missing. Can we add them to the WC Loan charge event data? The avro WorkingCapitalLoanChargeDataV1 gets no amountWaived field while its amountOutstanding now nets the waiver off. I checked before flagging it: the mapper already has @Mapping(target = "amountWaived"... )'s sibling amountWrittenOff set to ignore = true, so the same reconciliation gap already exists for write-offs — pre-existing, not introduced here. Consumers can still reconcile via the new waiver transaction event. The comment at WorkingCapitalLoanChargeWritePlatformServiceImpl.java:592 got mangled by the formatter — "an adjustment on a / // fully waived charge", "The paid part stays / // adjustable". Cosmetic, but it reads badly. -- 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]
