FINERACT-2455: working capital breach reset split period undo

[budaidev]

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.

[galovics]

The undo path is well covered - unit tests on both the resolver and the schedule methods, plus six integration tests including the pause-extended and payment-reassignment cases. That's usually the part I have to ask for, so nice work there.

My concern is with how the undo figures out what to revert. restoreSplitPeriod reverse-engineers the pre-split state from the schedule as it stands now plus the current breach config, using three guard clauses to decide whether the split it's looking at is actually the one this undo owns. Two of those guards fall through to a log.warn and a false, and the caller then just recalculates past due and returns 200 - so the user gets a success response for an undo that only lifted a flag and left the split in place.

Concretely: a RESCHEDULE recorded between the reset and the undo changes resolveEffectiveRescheduleParams, so the restore recomputes a boundary that never existed. And any PAUSE/RESUME between reset and undo rewrites every period contiguously from the schedule's first fromDate without knowing about the split, so the split disappears from the schedule entirely while the RESET row still says restartPeriodFromResetDate = true - the undo then silently falls into the flag-only branch.

Wouldn't it be simpler and deterministic to record on the RESET action row what the reset actually changed (the pre-split toDate, maybe the period id), and have the undo just write that back? Then there's nothing to guess and nothing that can silently degrade. The same reconstruction problem shows up in findLatestFlaggedPeriod's "last flagged period by period number" fallback in the reset service - it assumes period-number order tracks reset-date order, which is exactly the assumption the boundary rewrites can break.

Two smaller things while I was in there:

• splitPeriodAtReset only guards resetDate == fromDate; resetDate < fromDate falls through and can persist a period with toDate < fromDate / negative numberOfDays.
• The RESET-undo replay in WorkingCapitalLoanActiveBreachResetResolver is ordered by startDate, id - since reset/undo dates are always "the current business date," if the business date is ever moved backwards between two actions the replay order diverges from creation order, and the break-on-id can stop before the reset it's meant to cancel.

This is a draft so nothing's blocking yet, but I'd like to settle the design question (store vs. reconstruct) before it comes out of draft - it affects several of the methods above at once.

Recommendation: COMMENT