Skip to content

FINERACT-2455: WC breach/delinquency reschedule, re-date current period on frequency change + endDate validation - #6205

Open
oleksii-novikov-onix wants to merge 1 commit into
apache:developfrom
openMF:FINERACT-2455/breach-delinquency-reschedule-redate-validation
Open

FINERACT-2455: WC breach/delinquency reschedule, re-date current period on frequency change + endDate validation#6205
oleksii-novikov-onix wants to merge 1 commit into
apache:developfrom
openMF:FINERACT-2455/breach-delinquency-reschedule-redate-validation

Conversation

@oleksii-novikov-onix

Copy link
Copy Markdown
Contributor

Description

Describe the changes made and why they were made. (Ignore if these details are present on the associated Apache Fineract JIRA ticket.)

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Write the commit message as per our guidelines
  • Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
  • Create/update unit or integration tests for verifying the changes made.
  • Follow our coding conventions.
  • Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

@oleksii-novikov-onix
oleksii-novikov-onix force-pushed the FINERACT-2455/breach-delinquency-reschedule-redate-validation branch from 476d15b to 9233fed Compare July 31, 2026 07:29
@adamsaghy
adamsaghy marked this pull request as ready for review July 31, 2026 07:40
@oleksii-novikov-onix
oleksii-novikov-onix force-pushed the FINERACT-2455/breach-delinquency-reschedule-redate-validation branch 2 times, most recently from 08ffad5 to b3b45bc Compare July 31, 2026 10:57
@adamsaghy

Copy link
Copy Markdown
Contributor

AC3 is not met — 8 existing TestRail scenarios had their inputs changed to dodge the new validation, no?

Breach C85273, C85274, C85275, C85280, C85588: business date 01 June 2026 → 15 May 2026
Breach C85282, C85283: business date 10 March 2019 → 15 January 2019
Delinquency C85583: reschedule frequency 15 DAYS → 30 DAYS, with a new comment stating outright it's so the recalculated end date doesn't fall before the business date

Each of those originally exercised "reschedule deep inside a long period", which is now precisely the AC2 error case. Rather than moving the date, keep the original inputs and flip the assertion to the new validation error, then add a separate happy-path scenario at the earlier date. That both satisfies AC3 honestly and gives AC2 coverage across several product configs instead of one. Scenarios where only the expected tables changed (e.g. C74496, C74497, C85488, C85501) are legitimate consequences of AC1.

The pause-drop isn't stable. It's action.getFrequency() != null "a frequency was supplied", not "the frequency changed". Re-sending the same frequency still re-dates the period and discards any pause extension, which the new "ignoring an overlapping pause" scenario locks in. It isn't durable: a later PAUSE/RESUME calls recalculatePeriodsForPauses, which rebuilds every period from period 1 using the effective frequency and re-applies all recorded pauses: resurrecting the extension that was just dropped. Same loan, different boundaries depending on action order.

The validator and the service compute the same date two different ways. The validator uses action.getFrequency()/getFrequencyType(); the service uses params.frequency() from resolveEffectiveRescheduleParams. They agree only because the action is saved before the service runs and is the latest with a frequency group. Both also do their own findCurrentOpenPeriod lookup. Extract one helper both call, so the check can't drift from the mutation it's guarding.

findCurrentOpenPeriod returning Optional is stricter than the loop it replaces. The old code tolerated multiple matches (last one won); the derived query throws IncorrectResultSizeDataAccessException if two open periods ever cover the business date. findTop…OrderByPeriodNumberDesc would preserve the old tolerance.

The delinquency queries add AND s.reset = false, which the replaced loops did not have. A behaviour change bundled into a refactor. Unreachable today (resetPeriods only touches periods with toDate < resetDate), but if a reset period ever landed in the future window, updateFuturePeriods renumbers sequentially and would skip it → duplicate period numbers. Also note the breach queries have no equivalent filter, so the two schedules now diverge.

Three different "period expired" predicates coexist in WorkingCapitalLoanBreachScheduleServiceImpl: evaluateBreachOnDate uses businessDate >= toDate, evaluateExpiredPeriods uses toDate < businessDate, recomputeBreach uses businessDate > toDate. Pre-existing, but this PR makes the boundary reachable by design, since the validation deliberately permits a re-dated period ending exactly on the business date. The "equals the current business date" scenario closes the loan immediately and never runs another COB, so what that period does at the next evaluation is unasserted. Add a COB step and pin it.

Minor

The breach validator now mixes two error mechanisms (failWithCodeNoParameterAddedToErrorCode and the new failGeneralValidation) and hardcodes "workingCapitalLoanBreachAction" in two places. The delinquency validator has a VALIDATION_RESOURCE constant — mirror it.

No unit tests. calculateToDate and both validateFrequencyDoesNotEndBeforeBusinessDate methods are cheap to cover directly, especially the == businessDate / -1 day boundary that currently only has slow e2e coverage.

@oleksii-novikov-onix
oleksii-novikov-onix force-pushed the FINERACT-2455/breach-delinquency-reschedule-redate-validation branch from b3b45bc to bfc6254 Compare August 4, 2026 06:29
@oleksii-novikov-onix
oleksii-novikov-onix force-pushed the FINERACT-2455/breach-delinquency-reschedule-redate-validation branch from bfc6254 to f7125f3 Compare August 4, 2026 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants