alberto-art3ch commented on PR #6586:
URL: https://github.com/apache/fineract/pull/6586#issuecomment-6157845964
> @alberto-art3ch Please check my below concerns:
>
> 1. **C106711 is not un-skipped.** The description says C106711 is no
longer skipped, but the diff doesn't touch it. On `develop`, C106711 is an
unrelated, non-skipped scenario ("delinquency Id is overridable… on modify loan
account – UC2" in `WorkingCapitalDelinquency.feature`), and no skip tag is
removed anywhere in the PR. The ID is probably wrong; the new scenarios are
C111070–C111080.
> 2. **C111077's rate-change check doesn't test undo approval.** In C111077
the rate change is reversed by the undo disbursal step. Undo approval can't
have a rate change to reverse, because rate changes are only allowed on open
loans. So C111077 doesn't cover any rate-change behaviour that belongs to undo
approval, despite what the description says.
> 3. **Title doesn't match the change.** "Working Capital undo disbursal
clears schedules and action" describes behaviour that, per the description, was
already on `develop`. This PR adds rate-change reversal on undo disbursal and
projection deletion on undo approval.
> 4. **The description leaves out about half the diff.** Not mentioned:
about nine more E2E scenarios (batch API undo, breach/near-breach/delinquency
overrides on modify after undo disbursement), renamed and split step
definitions, and the adoc updates.
Thanks @adamsaghy — all four addressed: dropped the wrong C106711 reference,
removed the rate change from C111077 (C111076 already covers the reversal) and
retitled it to what undo approval does, and rewrote the title and description
to cover the full diff.
--
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]
