fineract icon indicating copy to clipboard operation
fineract copied to clipboard
verified publisher icon apache

Fineract-2408: Update search api resource to support SAVING & SHARE entities

Open Akshat-Soni02 opened this issue 3 months ago • 4 comments

Description

Implemented missing status mappings for savings and share entities in the SearchMapper to ensure accurate and complete search results.

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
  • [ ] 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 coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/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
  • [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)

FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.

Akshat-Soni02 avatar Nov 19 '25 06:11 Akshat-Soni02

Make sure to do a squash and commit. Are you proposing to look up by the internal ID?

I’m not directly using the internal status ID. I’m following the same pattern used for loan and client: using SavingsEnumerations , SharesEnumerations

Akshat-Soni02 avatar Nov 20 '25 06:11 Akshat-Soni02

Implementation looks good to me. I would recommend adding a test in SearchResourcesTest.java, something like this:

    @Test
    public void searchSavingsAndSharesHaveEntityStatus() {
        // Create client
        String jsonPayload = ClientHelper.getBasicClientAsJSON(ClientHelper.DEFAULT_OFFICE_ID, ClientHelper.LEGALFORM_ID_PERSON, null);
        final PostClientsResponse clientResponse = ClientHelper.addClientAsPerson(requestSpec, responseSpec, jsonPayload);
        final Long clientId = clientResponse.getClientId();
        final GetClientsClientIdResponse getClientResponse = ClientHelper.getClient(requestSpec, responseSpec, clientId.intValue());

        // Search for savings accounts - should return results with proper entityStatus
        final List<String> savingsResources = Arrays.asList("savings");
        final ArrayList<GetSearchResponse> savingsSearchResponse = SearchHelper.getSearch(requestSpec, responseSpec,
                getClientResponse.getAccountNo(), Boolean.FALSE, getResources(savingsResources));
        
        // Verify that if any SAVING entities are returned, they have valid entityStatus
        savingsSearchResponse.stream()
                .filter(result -> "SAVING".equalsIgnoreCase(result.getEntityType()))
                .forEach(result -> {
                    assertNotNull(result.getEntityStatus(), "SAVING entity should have entityStatus");
                    assertNotNull(result.getEntityStatus().getId(), "SAVING entityStatus should have id");
                    assertNotNull(result.getEntityStatus().getCode(), "SAVING entityStatus should have code");
                });

        // Search for share accounts - should return results with proper entityStatus
        final List<String> shareResources = Arrays.asList("shares");
        final ArrayList<GetSearchResponse> shareSearchResponse = SearchHelper.getSearch(requestSpec, responseSpec,
                getClientResponse.getAccountNo(), Boolean.FALSE, getResources(shareResources));
        
        // Verify that if any SHARE entities are returned, they have valid entityStatus
        shareSearchResponse.stream()
                .filter(result -> "SHARE".equalsIgnoreCase(result.getEntityType()))
                .forEach(result -> {
                    assertNotNull(result.getEntityStatus(), "SHARE entity should have entityStatus");
                    assertNotNull(result.getEntityStatus().getId(), "SHARE entityStatus should have id");
                    assertNotNull(result.getEntityStatus().getCode(), "SHARE entityStatus should have code");
                });
    }

rhopman avatar Nov 23 '25 15:11 rhopman

@Akshat-Soni02 Could you please add the appropriate test cases to this PR to verify the correct behavior?

adamsaghy avatar Nov 25 '25 10:11 adamsaghy

@Akshat-Soni02 Please kindly check the failing test cases.

adamsaghy avatar Dec 01 '25 13:12 adamsaghy

This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days.

github-actions[bot] avatar Jan 01 '26 00:01 github-actions[bot]

@Akshat-Soni02 I think it's a simple Spotless issue. Here's a properly formatted version of the test (I hope):

    @Test
    public void searchSavingsAndSharesHaveEntityStatus() {

        // Create client
        String jsonPayload = ClientHelper.getBasicClientAsJSON(ClientHelper.DEFAULT_OFFICE_ID, ClientHelper.LEGALFORM_ID_PERSON, null);
        final PostClientsResponse clientResponse = ClientHelper.addClientAsPerson(requestSpec, responseSpec, jsonPayload);
        final Long clientId = clientResponse.getClientId();
        final GetClientsClientIdResponse getClientResponse = ClientHelper.getClient(requestSpec, responseSpec, clientId.intValue());

        final List<String> savingsResources = Arrays.asList("savings");
        final ArrayList<GetSearchResponse> savingsSearchResponse = SearchHelper.getSearch(requestSpec, responseSpec,
                getClientResponse.getAccountNo(), Boolean.FALSE, getResources(savingsResources));

        // Verify valid entityStatus
        savingsSearchResponse.stream().filter(result -> "SAVING".equalsIgnoreCase(result.getEntityType())).forEach(result -> {
            assertNotNull(result.getEntityStatus(), "SAVING entity should have entityStatus");
            assertNotNull(result.getEntityStatus().getId(), "SAVING entityStatus should have id");
            assertNotNull(result.getEntityStatus().getCode(), "SAVING entityStatus should have code");
        });

        final List<String> shareResources = Arrays.asList("shares");
        final ArrayList<GetSearchResponse> shareSearchResponse = SearchHelper.getSearch(requestSpec, responseSpec,
                getClientResponse.getAccountNo(), Boolean.FALSE, getResources(shareResources));

        // Verify valid entityStatus
        shareSearchResponse.stream().filter(result -> "SHARE".equalsIgnoreCase(result.getEntityType())).forEach(result -> {
            assertNotNull(result.getEntityStatus(), "SHARE entity should have entityStatus");
            assertNotNull(result.getEntityStatus().getId(), "SHARE entityStatus should have id");
            assertNotNull(result.getEntityStatus().getCode(), "SHARE entityStatus should have code");
        });
    }

Run ./gradlew spotlessApply spotbugsMain spotbugsTest checkstyleMain checkstyleTest to verify.

rhopman avatar Jan 01 '26 20:01 rhopman

@rhopman apart from the formatting issue, the test case is failing. and i am getting error like this

Test searchSavingsAndSharesHaveEntityStatus() FAILED
  com.google.gson.JsonSyntaxException: java.lang.NumberFormatException: For input string: "Head Offic000000208"
    at org.apache.fineract.integrationtests.SearchResourcesTest.searchSavingsAndSharesHaveEntityStatus(SearchResourcesTest.java:116)
  Caused by: java.lang.NumberFormatException: For input string: "Head Offic000000208"
    at org.apache.fineract.integrationtests.SearchResourcesTest.searchSavingsAndSharesHaveEntityStatus(SearchResourcesTest.java:116)

This error is new to me, can you spot where exactly i should look for it ?

Akshat-Soni02 avatar Jan 05 '26 14:01 Akshat-Soni02

@Akshat-Soni02 I am not getting that error. Could you fix the formatting and see if it still fails?

rhopman avatar Jan 06 '26 10:01 rhopman 

* What went wrong:
Execution failed for task ':integration-tests:spotlessJavaCheck'.
> The following files had format violations:
      src/test/java/org/apache/fineract/integrationtests/SearchResourcesTest.java
          @@ -111,32 +111,27 @@
           ········final·Long·clientId·=·clientResponse.getClientId();
           ········final·GetClientsClientIdResponse·getClientResponse·=·ClientHelper.getClient(requestSpec,·responseSpec,·clientId.intValue());
           
          -
           ········final·List<String>·savingsResources·=·Arrays.asList("savings");
           ········final·ArrayList<GetSearchResponse>·savingsSearchResponse·=·SearchHelper.getSearch(requestSpec,·responseSpec,
           ················getClientResponse.getAccountNo(),·Boolean.FALSE,·getResources(savingsResources));
          -········
          +
           ········//·Verify·valid·entityStatus
          -········savingsSearchResponse.stream()
          -················.filter(result·->·"SAVING".equalsIgnoreCase(result.getEntityType()))
          -················.forEach(result·->·{
          -····················assertNotNull(result.getEntityStatus(),·"SAVING·entity·should·have·entityStatus");
          -····················assertNotNull(result.getEntityStatus().getId(),·"SAVING·entityStatus·should·have·id");
          -····················assertNotNull(result.getEntityStatus().getCode(),·"SAVING·entityStatus·should·have·code");
          -················});
          +········savingsSearchResponse.stream().filter(result·->·"SAVING".equalsIgnoreCase(result.getEntityType())).forEach(result·->·{
          +············assertNotNull(result.getEntityStatus(),·"SAVING·entity·should·have·entityStatus");
          +············assertNotNull(result.getEntityStatus().getId(),·"SAVING·entityStatus·should·have·id");
          +············assertNotNull(result.getEntityStatus().getCode(),·"SAVING·entityStatus·should·have·code");
          +········});
           
           ········final·List<String>·shareResources·=·Arrays.asList("shares");
           ········final·ArrayList<GetSearchResponse>·shareSearchResponse·=·SearchHelper.getSearch(requestSpec,·responseSpec,
           ················getClientResponse.getAccountNo(),·Boolean.FALSE,·getResources(shareResources));
          -········
          +
           ········//·Verify·valid·entityStatus
          -········shareSearchResponse.stream()
          -················.filter(result·->·"SHARE".equalsIgnoreCase(result.getEntityType()))
          -················.forEach(result·->·{
          -····················assertNotNull(result.getEntityStatus(),·"SHARE·entity·should·have·entityStatus");
          -····················assertNotNull(result.getEntityStatus().getId(),·"SHARE·entityStatus·should·have·id");
          -····················assertNotNull(result.getEntityStatus().getCode(),·"SHARE·entityStatus·should·have·code");
          -················});
          +········shareSearchResponse.stream().filter(result·->·"SHARE".equalsIgnoreCase(result.getEntityType())).forEach(result·->·{
          +············assertNotNull(result.getEntityStatus(),·"SHARE·entity·should·have·entityStatus");
          +············assertNotNull(result.getEntityStatus().getId(),·"SHARE·entityStatus·should·have·id");
          +············assertNotNull(result.getEntityStatus().getCode(),·"SHARE·entityStatus·should·have·code");
          +········});
           ····}
           
           ····private·String·getResources(final·List<String>·resources)·{
  Run './gradlew :integration-tests:spotlessApply' to fix these violations.

adamsaghy avatar Jan 15 '26 08:01 adamsaghy

@adamsaghy I provided the fix for the Spotless issue in https://github.com/apache/fineract/pull/5166#issuecomment-3704094026. @Akshat-Soni02 can you please apply this in the PR? Or just run ./gradlew :integration-tests:spotlessApply; that should also fix it.

rhopman avatar Jan 15 '26 08:01 rhopman

@Akshat-Soni02 Can you please rebase this PR with latest develop branch and run ./gradlew spotlessApply spotbugsMain spotbugsTest checkstyleMain checkstyleTest before commit. (Dont forget to squash the commits if multiple would have been raised).

adamsaghy avatar Jan 20 '26 08:01 adamsaghy