mariiaKraievska commented on PR #6625:
URL: https://github.com/apache/fineract/pull/6625#issuecomment-6215836298
> ## Review
> Hi @mariiaKraievska, thanks for the PR. The fix itself makes sense to me,
I just think it only fixes half of the problem.
>
>
`fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanApplicationReadPlatformServiceImpl.java`
(`enrichWithStartDates`, ~L250):
>
> > `findTopByLoanIdAndMinPaymentCriteriaMetFalseOrderByFromDateAsc` is
still used here for `delinquencyStartDate`, `delinquencyEffectiveStartDate` and
`summary.overdueSinceDate`. It has the same root cause: when instant
classification runs before COB, `minPaymentCriteriaMet` is still NULL, so on
the loan details in your first scenario we'll report `pastDueDays = 1` while
`delinquencyStartDate` and `overdueSinceDate` stay empty. I think this should
go through the same query (or the same "open past due period" definition) so
the two can't disagree. Can we also assert these fields in the new scenarios?
Thoughts?
>
> Also:
>
> * `WorkingCapitalDelinquency.feature:1357, 1382, 1406`: the 3 new
scenarios have no `@TestRailId`, they're the only untagged ones in the file.
> * `WorkingCapitalLoanDelinquencyReadPlatformServiceImplTest` already
covers `getCollectionData` but nothing covers `pastDueDays`, a short test for
the NULL-criteria case would back up the e2e.
> * Scenario 2 pins `delinquent.delinquentDays = 1` together with
`pastDueDays = 58`. Loan-level `delinquentDays` comes from the tag's
`addedOnDate`, not the period due date. That's pre-existing, but is 1 vs 58
really the contract we want to lock in?
> * The query accepts `delinquentDays = 0`. A fully paid period normally
gets `minPaymentCriteriaMet = true` so it's probably unreachable, but
`s.delinquentDays > 0` would make that explicit.
>
> Please also fill in the PR description before taking it out of draft.
>
> Recommendation: COMMENT
Thanks — addressed in the latest push:
- enrichWithStartDates now uses the same open-past-due query as pastDueDays;
- e2e asserts the start-date fields
- unit test for the NULL criteria case, query uses delinquentDays > 0
--
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]
