alberto-art3ch commented on PR #6482: URL: https://github.com/apache/fineract/pull/6482#issuecomment-5902673522
> Thanks @alberto-art3ch, this addresses everything I raised. > > The reset case now returns null when `fromDate + grace` falls after the period's `toDate`, and you added tests for the reset and its undo. On the reschedule you're right. After the rebase a RESCHEDULE goes through `replayForBreachAction` -> `naturalToDate`, so period 1 keeps its grace days. Moving the rule into `calculateNaturalToDate` so the validator uses it too is a nice cleanup. The docs, Swagger and test Javadoc are fixed and the mapper test asserts the Avro field. I'm fine leaving the pause-over-grace question open as a product decision. > > One thing before merging: this PR and #6476 still conflict in 5 files (the adoc, `WorkingCapitalLoanApplicationReadPlatformServiceImpl`, `EventCheckHelper`, `WorkingCapitalLoanAccountDataMapperTest`, `WorkingCapitalLoanStartDatesTest`). This PR adds the generic `resolveEffectiveStartDate(periodNumber, fromDate, toDate, graceDays)`, but #6476 still has its own `resolveDelinquencyEffectiveStartDate` without the `toDate` check. So the two dates would follow different rules. Whichever PR merges second, let's have the delinquency side use the same helper so the contract is the same for both. Thoughts? > > Recommendation: APPROVE Thanks — closing the last open item. Shared helper: #6476 merged first, so this PR adopts it. resolveDelinquencyEffectiveStartDate and resolveBreachEffectiveStartDate both delegate to the same resolveEffectiveStartDate(periodNumber, fromDate, toDate, graceDays), so one contract covers both dates and the delinquency side gains the toDate check. Existing expectations hold: WorkingCapitalDelinquency.feature UC5 still ends period 1 on Jan 5, so 2026-01-04 stays inside it. Conflicts: resolved in the rebase by @adamsaghy Thanks!, PR is mergeable again. Reschedule: added UC9 to WorkingCapitalBreachStartDateType.feature to pin the argument you accepted — the first period is re-dated to [Jan 1, Jan 7] with the grace kept, and breachEffectiveStartDate = 2026-01-06 on the API and in the event payload. Business date Jan 4 is deliberate: without the grace the reschedule would be rejected by validateFrequencyDoesNotEndBeforeBusinessDate, so the scenario fails loudly if the rule drifts. -- 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]
