You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "jmtdev0 (via GitHub)" <gi...@apache.org> on 2026/07/07 16:46:07 UTC
[PR] FINERACT-2676: Fix named PPI survey score overview [fineract]
jmtdev0 opened a new pull request, #6102:
URL: https://github.com/apache/fineract/pull/6102
## Description
Fixes [FINERACT-2676](https://issues.apache.org/jira/browse/FINERACT-2676).
The named PPI survey score overview endpoint accepted a `surveyName` path parameter, but the resource method delegated to the all-surveys service overload and the named-survey service path attempted to use a JDBC bind parameter as a SQL table identifier (`FROM ? tz`). JDBC parameters can safely bind values, but not SQL identifiers, so the named-survey query could not work correctly.
This PR updates the endpoint to delegate to the named-survey overload and updates `ReadSurveyServiceImpl.retrieveClientSurveyScoreOverview(String surveyName, Long clientId)` to:
- validate the requested `surveyName`;
- resolve it through the existing registered/permitted PPI datatable query, narrowed by `registered_table_name = ?`;
- return an empty result when the requested survey is not registered/permitted for the current user;
- build the `FROM` clause with the resolved registered survey table name;
- keep runtime values (`ppi_name`, enabled status, client id) bound as JDBC parameters.
The identifier is not additionally quoted in this change. Instead, the table name is first looked up from the existing PPI datatable registration/permission query and then validated again with `SqlValidator` before it is used as an identifier. This mirrors the existing all-surveys implementation in the same service, while narrowing it to the requested registered survey. The values that remain user/runtime data continue to use JDBC bind parameters.
Test coverage was added for both the API delegation and the SQL construction/permission behavior in the service.
## Testing
Targeted unit tests passed:
```bash
./gradlew --no-daemon -Dorg.gradle.jvmargs="-Xmx3g -Dfile.encoding=UTF-8" :fineract-provider:test --tests "org.apache.fineract.infrastructure.survey.service.ReadSurveyServiceImplTest" --tests "org.apache.fineract.infrastructure.survey.api.SurveyApiResourceTest" --stacktrace
```
I also performed a deeper local smoke test on Windows with Docker Desktop:
- built a local Fineract image from this branch with `jibBuildTar` and loaded it into Docker;
- started the PostgreSQL test Docker Compose stack and confirmed Fineract reached `{"status":"UP","groups":["liveness","readiness"]}`;
- seeded two minimal registered PPI survey datatables, `ppi_test_alpha` and `ppi_test_beta`, plus the required PPI likelihood fixtures;
- verified `GET /fineract-provider/api/v1/survey/ppi_test_alpha/1` returned only `surveyName: ppi_test_alpha`;
- verified `GET /fineract-provider/api/v1/survey/ppi_test_beta/1` returned only `surveyName: ppi_test_beta`;
- verified an unknown survey returned `[]`.
That smoke test specifically checks that the named endpoint respects the requested `surveyName` instead of falling back to all registered PPI surveys.
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [x] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests)
- [x] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [x] Create/update [unit or integration tests](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made.
- [x] Follow our [coding conventions](https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions).
- [x] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes. N/A: this fixes behavior of an existing endpoint and does not change the public API contract.
- [x] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute).
Your assigned reviewer(s) will follow our [guidelines for code reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide).
--
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-2676: Fix named PPI survey score overview [fineract]
Posted by "jmtdev0 (via GitHub)" <gi...@apache.org>.
jmtdev0 commented on PR #6102:
URL: https://github.com/apache/fineract/pull/6102#issuecomment-4916738554
@adamsaghy
Thank you for the review and for pointing this out. I have now signed the commit and force-pushed the updated branch. GitHub marks the new commit as verified on my side.
Please let me know if anything else is needed from me.
--
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-2676: Fix named PPI survey score overview [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #6102:
URL: https://github.com/apache/fineract/pull/6102
--
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-2676: Fix named PPI survey score overview [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6102:
URL: https://github.com/apache/fineract/pull/6102#issuecomment-4913342817
@jmtdev0
```
Run chmod +x scripts/verify-signed-commits.sh
Verifying commit signatures in range: d8fe7bdede2b7f66a5982ba3771bff45e4657245..d5d5053f42e521dd43d75365e892c3618c8539e3
Error: Commit d5d5053 by jmtdev0 is not signed.
Summary: 1 unsigned commit(s) found.
Error: 1 unsigned commit(s). See CONTRIBUTING.md#signing-your-commits
Error: Process completed with exit code 1.
```
Kindly asking you to set up GPG signature and sign your commit.
--
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