oleksii-novikov-onix commented on PR #6399: URL: https://github.com/apache/fineract/pull/6399#issuecomment-5729220773
> @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? > > 2. 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, but still it would worth to fix it > 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. 1. amountUnrecognized ignored the waived amount. Fixed: it is now amount - amountAccrued - amountWaived, not below zero, which matches the accrual target. 3. The charge event data had no amountWaived, and amountWrittenOff was always null. Fixed: added amountWaived to WorkingCapitalLoanChargeDataV1 and removed the ignore on amountWrittenOff, so the event now fills both. Removed the outdated "write-off is not implemented" note on amountWrittenOff. 4. The formatter broke the comment at WorkingCapitalLoanChargeWritePlatformServiceImpl.java:592. Fixed: the lines were over the formatter's 120 character limit, so it split them. I rewrapped them. -- 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]
