rymghosn commented on PR #6284: URL: https://github.com/apache/fineract/pull/6284#issuecomment-5992512403
@adamsaghy Fair point. I've reworked the PR along the lines you suggested: - **Entity GET responses are untouched.** The `pendingMakerCheckerApprovals` field and all its wiring are gone from this PR; it no longer changes the client, loan, savings or deposit data or read services. - **The checker API is the way to find pending entries.** I checked what `GET /makercheckers` already supports (`clientId`, `loanid`, `savingsAccountId`, `groupId`, `resourceId`, `entityName`, …). It covers "what is pending for this entity", so no new endpoint is needed, except for one gap: the wrappers for update, delete, (un)assign officer and withhold-tax on savings, fixed deposit and recurring deposit accounts never set `savingsId`. Their pending entries were therefore missing from `GET /makercheckers?savingsAccountId=…`. The PR now only sets `savingsId` on those wrappers, the same way the loan wrappers already set `loanId`. - **Tests:** a unit test for the nine wrappers, and a `MakercheckerTest` integration test where a pending savings-account update must be found by `savingsAccountId`. It fails on current `develop` (0 entries) and passes with the change. The description now includes the gap analysis. I'll still raise the broader question (exposing pending maker-checker state outside the checker API) on the dev mailing list, as you suggested, before proposing anything more. Could you please re-review? Thanks! -- 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]
