Aman-Mittal opened a new issue, #283:
URL: https://github.com/apache/fineract-backoffice-ui/issues/283

   ## Business value
   
   An interest pause suspends interest accrual on a loan for a date range — 
used when a borrower is granted relief after a flood, a harvest failure, or an 
illness. The screen supports creating one and deleting one, and nothing else.
   
   
`src/app/features/loans/interest-pauses/interest-pauses-list.component.ts:113` 
deletes:
   
   ```ts
   this.pauseService.deleteLoansLoanIdInterestPausesVariationId(this.loanId, 
row.id)
   ```
   
   There is no edit affordance anywhere in the component. The API has had the 
operation all along — `src/app/api/api/loanInterestPause.service.ts`:
   
   ```
   postLoansLoanIdInterestPauses(...)                    used
   deleteLoansLoanIdInterestPausesVariationId(...)       used
   putLoansLoanIdInterestPausesVariationId(...)          ← never called
   getLoansLoanIdInterestPauses(...)                     used
   ```
   
   So correcting a date typed wrong means deleting the pause and creating a new 
one. That is not equivalent, for two reasons:
   
   - **It reprocesses the loan twice.** Removing a pause makes the platform 
recalculate the schedule as if interest had accrued throughout; adding it back 
recalculates again. On a loan with interest recalculation enabled these are 
real, journaled recalculations, not a UI refresh. Two are strictly worse than 
one, and if the second call fails the borrower is left with a loan that has 
*no* relief applied and a schedule that has silently changed.
   - **It breaks the audit trail.** The delete and the create are separate 
commands with separate ids. Under four-eyes approval they are two separate 
items for a checker to approve, and the record no longer shows "this pause was 
corrected" — it shows a relief period that was withdrawn and a different one 
that was granted.
   
   Neither is acceptable for an operation that changes what a borrower owes.
   
   ## Reproducing it
   
   ```
   grep -n "put\|edit" 
src/app/features/loans/interest-pauses/interest-pauses-list.component.ts   # 
nothing
   grep -c "putLoansLoanIdInterestPausesVariationId" 
src/app/api/api/loanInterestPause.service.ts # present
   ```
   
   ## Describing the change
   
   An edit control on each row, opening the same form the create flow uses, 
pre-filled, and submitting through `putLoansLoanIdInterestPausesVariationId`.
   
   `interest-pause-form.component.ts` already builds and posts the payload 
(`:177`); it needs to learn a second mode. The established pattern in this 
codebase is a route parameter distinguishing create from edit — 
`src/app/features/system/data-tables/datatables-form.component.ts:310-315` 
reads a route param and sets `isEditMode`, and `client-form.component.ts` does 
the same. Follow whichever fits better; do not invent a third.
   
   The row action should sit next to the delete button and use 
`create-outline`, matching the convention in 
`group-notes-list.component.ts:92-98`.
   
   **Two things to check before you assume the payload is symmetrical.**
   
   1. **Confirm the PUT body against a live platform, do not copy the POST body 
and hope.** Fineract commands are frequently asymmetric — in this repo, 
`acceptTransfer` rejects the `locale`, `dateFormat` and `transferDate` that 
`proposeTransfer` requires. Bring up a backend with `npm run e2e:stack` and try 
it. If a field is refused the error names it.
   2. **The list currently swallows errors** — `:115` is `error: (err) => 
console.error(...)`, so a rejected delete looks to the user like it worked. Do 
not copy that into the edit path. The error interceptor already raises a toast 
carrying the platform's message; `error: () => undefined` is the right handler 
here. Fixing the existing delete handler while you are in the file is welcome.
   
   ## Testing
   
   The platform's own acceptance tests name both operations. From 
`fineract-e2e-tests-runner/src/test/resources/features/LoanInterestPause.feature`
 in `apache/fineract` (`develop`), 30 scenarios, ending with:
   
   ```gherkin
   Scenario: Verify interest pause deletion
   Scenario: Verify interest pause update
   ```
   
   and, importantly for what an edit has to preserve:
   
   ```gherkin
   Scenario: Multiple interest pauses
   Scenario: Backdated interest pause after the repayment - UC3
   Scenario: Interest pause that overlaps a few installments - UC8
   Scenario: Verify interest pause period is forbidden for loan with zero 
interest rate - UC1
   ```
   
   Those last two are the ones worth designing tests around. **Overlapping and 
forbidden cases are where an edit differs most from a create** — a pause edited 
to overlap an existing one, or edited onto a zero-interest loan, must be 
refused by the platform, and the UI has to surface that refusal rather than 
appear to succeed.
   
   What to write:
   
   - **A unit spec** asserting the edit path calls 
`putLoansLoanIdInterestPausesVariationId` with the variation id and the 
formatted dates, and that the create path still calls `post`. Both, in the same 
spec — the risk in adding a mode is breaking the mode that worked.
   - **A unit spec** asserting the form pre-fills from the loaded pause.
   - **A mocked e2e** driving list → edit → save, asserting the intercepted 
request method and URL. The `Probe` pattern in 
`e2e/client-servicing-gaps.spec.ts` records method, command and body.
   
   A backend e2e (registered in `BACKEND_SPECS` in `playwright.config.ts`) 
would be a strong addition, because it is the only test that proves the PUT 
body is actually accepted: create a loan, approve, disburse, add a pause, edit 
its end date, and assert the list shows the new date. Drive everything through 
the UI — no API seeding.
   
   ## Scope
   
   In scope: editing an existing pause, and the error handling in this 
component.
   
   Out of scope: validation of overlapping ranges in the client. The platform 
enforces it and returns a message; surfacing that message is enough. Do not 
reimplement the rules.
   
   ## Getting started
   
   - List: 
`src/app/features/loans/interest-pauses/interest-pauses-list.component.ts`
   - Form: 
`src/app/features/loans/interest-pauses/interest-pause-form.component.ts`
   - Routes: `src/app/features/loans/loans.routes.ts`
   - Both components already have specs — extend them rather than starting new 
files.
   - `npm test`, `npm run lint:prune` and `npm run build` must pass.
   


-- 
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]

Reply via email to