adamsaghy commented on PR #6209:
URL: https://github.com/apache/fineract/pull/6209#issuecomment-6152649240
@aya-abdallah-FOO Please review below concerns:
- **Bug 2 is not fixed:** the existing `GET /loanproducts` is unchanged.
Only a new `/v2/loanproducts` endpoint uses the new rule.
- **Bug 1 is not fixed:** the mapping dropdown is only fixed if the web-app
switches to v2, and nothing in this PR does that.
- **The description is out of date:**
- It mentions a self-service endpoint that doesn't exist here.
- It names a method,
`getSQLQueryInClauseIDList_ForLoanProductsVisibleToOffice`, that has since been
replaced.
- It doesn't mention the FINERACT-2728 charge filtering that was merged
into this PR.
- **Issues (all confirmed):**
1. **Regression:** `ChargeReadPlatformServiceImpl.java:420` dropped
`c.is_active=true` when `feeChargesOnly=false`. Savings and deposit templates
now offer deactivated charges, whether or not the flag is on.
2. **Two visibility rules now exist side by side.**
- `GET /loans/{id}?template=true` (`LoansApiResource.java:1224`) and
several `LoanProductReadPlatformServiceImpl` queries still use the old
allow-list.
- Example: a head-office user with no mappings sees products in
`/loans/template` but an empty list in `/loans/{id}?template=true`.
- Validation uses the new rule, so it accepts products those screens
don't offer.
3. `GET /charges/{id}` lost its office filter with nothing replacing it.
No claim in the description covers this change.
4. With no office given, sub-offices are included, so products mapped to a
child office become visible. That goes beyond the stated "office or its
parents" rule.
- **Tests:** A Feign integration test covers bug 3. Nothing covers bugs 1
and 2, the v2 endpoint, or the charge filtering.
--
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]
