You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@fineract.apache.org by "rymghosn (via GitHub)" <gi...@apache.org> on 2026/08/18 13:22:03 UTC

[PR] FINERACT-2763: Checker Users Unable to Approve Pending Maker-Checker Tasks Without Direct Operational Permissions and Missing Tasks in Checker Inbox [fineract]

rymghosn opened a new pull request, #6284:
URL: https://github.com/apache/fineract/pull/6284

   Currently, when a client, loan, or savings account has one or more maker-checker actions awaiting checker approval, there is no way to discover this from the corresponding GET API response. A caller has to separately query the checker inbox/pending-audit endpoints and correlate entries by entity, which is easy to miss and leaves API consumers unaware that the record they just fetched has a pending change awaiting approval.
   
   This PR adds a pendingMakerCheckerApprovals field to the ClientData, LoanAccountData, SavingsAccountData, and DepositAccountData (fixed/recurring deposit) response objects. When a GET request for one of these entities is served, the field is populated with any CommandSource entries still AWAITING_APPROVAL for that entity (id, action name, entity name, permission code, maker username, and submission date), or omitted (null) when there is nothing pending.
   Changes:                                                                                                                                         
     - New PendingMakerCheckerData DTO and MakerCheckerReadService/MakerCheckerReadServiceImpl to look up pending CommandSource rows by               
     loan/client/savings id.                                                                                                                          
     - New CommandSourceRepository.findPendingBy{Loan,Client,Savings}Id queries (join fetch c.maker to avoid N+1 lookups for the maker username).     
     - Wired into ClientReadPlatformServiceImpl#retrieveOne, LoanReadPlatformServiceImpl#retrieveOne,                                                 
     SavingsAccountReadPlatformServiceImpl#retrieveOne, and DepositAccountReadPlatformServiceImpl#retrieveOne.                                        
                                                                                                                                                      
     This is a read-only, additive change — it does not alter maker-checker approval/permission logic itself, only exposes existing pending-approval  
     state that was previously only visible via the checker inbox. 
     PR:(https://issues.apache.org/jira/browse/FINERACT-2763)


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] FINERACT-2763: Checker Users Unable to Approve Pending Maker-Checker Tasks Without Direct Operational Permissions and Missing Tasks in Checker Inbox [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.

adamsaghy commented on PR #6284:
URL: https://github.com/apache/fineract/pull/6284#issuecomment-6128687081

   @rymghosn Can you please review the below?
   
   Scope gap: JIRA scenario 1 (a checker can’t approve without the operational permission) and scenario 2 (actions bypass the pending queue) are not addressed. 


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] FINERACT-2763: Checker Users Unable to Approve Pending Maker-Checker Tasks Without Direct Operational Permissions and Missing Tasks in Checker Inbox [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.

adamsaghy commented on PR #6284:
URL: https://github.com/apache/fineract/pull/6284#issuecomment-6128664162

   @rymghosn Please rebase


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] FINERACT-2763: Checker Users Unable to Approve Pending Maker-Checker Tasks Without Direct Operational Permissions and Missing Tasks in Checker Inbox [fineract]

Posted by "rymghosn (via GitHub)" <gi...@apache.org>.

rymghosn commented on PR #6284:
URL: https://github.com/apache/fineract/pull/6284#issuecomment-6336274564

@adamsaghy Rebased onto the latest `develop` (4725d3ebba). `MakercheckerTest` had moved to Feign, so I rewrote the savings-account test on the Feign helpers. On the scope gap, I reproduced both ticket scenarios on current `develop` against PostgreSQL, with maker-checker enabled globally and for `DISBURSE_LOAN`:

- **Scenario 1 (a checker can't approve without the operational permission):** it doesn't reproduce. A user holding only `DISBURSE_LOAN_CHECKER` approves a maker's queued disbursement through `POST /makercheckers/{id}?command=approve`, and the loan becomes active. `approveEntry` checks the `_CHECKER` permission, and the base permission is only checked when a command is first submitted.
- **Scenario 2 (actions bypass the pending queue):** a regular maker's disbursement is queued, and `GET /makercheckers?loanid=<id>` lists it. It's applied immediately only for a checker super user, and `AppUser#hasPermissionTo` gives every code, including `CHECKER_SUPER_USER`, to anyone holding `ALL_FUNCTIONS`. So an admin's direct disbursement is the designed super-user auto-approval, not a missing check. Changing what `ALL_FUNCTIONS` implies would affect every deployment, so I haven't touched it here. I'll put it on the dev list together with the broader question you raised.

I added `MakerCheckerLoanDisbursementTest` to pin both behaviours: queued, listed by loan and approved by a checker-only user, and a super user's disbursement applied immediately. The ticket's remaining symptom, "the request may not appear in the Checker Inbox", is the savings-account filter gap this PR fixes. `testPendingSavingsAccountUpdateIsFoundBySavingsAccountId` fails on `develop` (0 entries) and passes with the change. The whole `MakercheckerTest` class and the new test pass. I've updated the description to match. 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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] FINERACT-2763: Checker Users Unable to Approve Pending Maker-Checker Tasks Without Direct Operational Permissions and Missing Tasks in Checker Inbox [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.

adamsaghy commented on PR #6284:
URL: https://github.com/apache/fineract/pull/6284#issuecomment-5527904897

   @rymghosn I dont think i agree with this approach, neither attach "irrelevant" details into the target APIs.
   
   Would you mind to raise this question to the Fineract DEV mail list to hear other's opinions as well?
   
   My 2 cents:
   - Checker API is there for this purpose, i dont see why we should not use that...
   
   If you want something global, i dont mind to introduce endpoint for that purpose.


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] FINERACT-2763: Checker Users Unable to Approve Pending Maker-Checker Tasks Without Direct Operational Permissions and Missing Tasks in Checker Inbox [fineract]

Posted by "rymghosn (via GitHub)" <gi...@apache.org>.

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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org