Skip to content

FINERACT-2676: Fix named PPI survey score overview#6102

Open
jmtdev0 wants to merge 1 commit into
apache:developfrom
jmtdev0-open-source:fix/FINERACT-2676-named-ppi-score-overview
Open

FINERACT-2676: Fix named PPI survey score overview#6102
jmtdev0 wants to merge 1 commit into
apache:developfrom
jmtdev0-open-source:fix/FINERACT-2676-named-ppi-score-overview

Conversation

@jmtdev0

@jmtdev0 jmtdev0 commented Jul 7, 2026

Copy link
Copy Markdown

Description

Fixes 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:

./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!

  • Write the commit message as per our guidelines
  • 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.
  • Create/update unit or integration tests for verifying the changes made.
  • Follow our coding conventions.
  • 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.
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.

Your assigned reviewer(s) will follow our guidelines for code reviews.

This PR was developed by Codex with supervision from jmtdev0.

@adamsaghy adamsaghy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adamsaghy

Copy link
Copy Markdown
Contributor

@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.

@jmtdev0 jmtdev0 force-pushed the fix/FINERACT-2676-named-ppi-score-overview branch from d5d5053 to edf568a Compare July 8, 2026 15:52
@jmtdev0

jmtdev0 commented Jul 8, 2026

Copy link
Copy Markdown
Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants