galovics commented on code in PR #3192:
URL: https://github.com/apache/fineract/pull/3192#discussion_r1219410650


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/search/SavingsTransactionSearch.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.domain.search;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.util.List;
+import lombok.Data;
+import org.apache.fineract.portfolio.savings.SavingsAccountTransactionType;
+
+@Data
+public class SavingsTransactionSearch {

Review Comment:
   I don't agree with this structure at all.
   If I translate this to JSON, looks like this:
   
   ```
   {
     "filters": {
        "transactionDate": {
           "gte": "2022-01-01"
        }
     }
   }
   ```
   While it should be something like:
   ```
   {
     "filters": {
        "transactionDate": {
           "operator": "gte",
           "value": "2022-01-01"
        }
     }
   }
   ```
   
   Much more readable, maintainable and extendable this way.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/search/SavingsTransactionsSearchRepositoryImpl.java:
##########
@@ -0,0 +1,176 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.domain.search;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import javax.persistence.EntityManager;
+import javax.persistence.TypedQuery;
+import lombok.RequiredArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.fineract.portfolio.savings.SavingsAccountTransactionType;
+import 
org.apache.fineract.portfolio.savings.data.SavingsTransactionSearchResult;
+import 
org.apache.fineract.portfolio.savings.domain.search.SavingsTransactionSearch.Filters;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
+import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.support.PageableExecutionUtils;
+import org.springframework.stereotype.Repository;
+
+@Repository
+@RequiredArgsConstructor
+public class SavingsTransactionsSearchRepositoryImpl implements 
SavingsTransactionsSearchRepository {
+
+    private final EntityManager entityManager;
+
+    @Override
+    public Page<SavingsTransactionSearchResult> searchTransactions(Long 
savingsId, Integer depositAccountType, Filters filters,

Review Comment:
   Let's work with types, why don't we use a DepositAccountType enum here?



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/search/SavingsTransactionsSearchRepositoryImpl.java:
##########
@@ -0,0 +1,176 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.domain.search;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import javax.persistence.EntityManager;
+import javax.persistence.TypedQuery;
+import lombok.RequiredArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.fineract.portfolio.savings.SavingsAccountTransactionType;
+import 
org.apache.fineract.portfolio.savings.data.SavingsTransactionSearchResult;
+import 
org.apache.fineract.portfolio.savings.domain.search.SavingsTransactionSearch.Filters;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
+import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.support.PageableExecutionUtils;
+import org.springframework.stereotype.Repository;
+
+@Repository
+@RequiredArgsConstructor
+public class SavingsTransactionsSearchRepositoryImpl implements 
SavingsTransactionsSearchRepository {
+
+    private final EntityManager entityManager;
+
+    @Override
+    public Page<SavingsTransactionSearchResult> searchTransactions(Long 
savingsId, Integer depositAccountType, Filters filters,
+            Pageable pageable) {
+        String jpql = buildJpqlQuery();

Review Comment:
   The whole implementation lacks structure.
   
   I'd imagine on a high level this:
   
   1. Build base query with filters but without the selection
   2. Attach the selection
   3. Attach the ordering
   4. Execute the query
   5. Build base query with filters
   6. Attach the COUNT selection
   7. Execute the query (with the PageExecutionUtils..)
   8. Construct the page object



##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/SavingsAccountTransactionsSearchIntegrationTest.java:
##########
@@ -0,0 +1,386 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.integrationtests;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import io.restassured.builder.RequestSpecBuilder;
+import io.restassured.builder.ResponseSpecBuilder;
+import io.restassured.http.ContentType;
+import io.restassured.specification.RequestSpecification;
+import io.restassured.specification.ResponseSpecification;
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.time.format.DateTimeFormatter;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.fineract.client.models.AmountFilters;
+import org.apache.fineract.client.models.DateFilters;
+import org.apache.fineract.client.models.Filters;
+import org.apache.fineract.client.models.GetSavingsAccountTransactionsPageItem;
+import 
org.apache.fineract.client.models.SavingsAccountTransactionsSearchResponse;
+import org.apache.fineract.client.models.SortOrder;
+import org.apache.fineract.integrationtests.common.ClientHelper;
+import org.apache.fineract.integrationtests.common.CommonConstants;
+import org.apache.fineract.integrationtests.common.GlobalConfigurationHelper;
+import org.apache.fineract.integrationtests.common.Utils;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsAccountHelper;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsProductHelper;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsStatusChecker;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+@SuppressWarnings({ "rawtypes" })
+public class SavingsAccountTransactionsSearchIntegrationTest {
+
+    public static final String ACCOUNT_TYPE_INDIVIDUAL = "INDIVIDUAL";
+    final String startDate = "01 May 2023";
+    final String firstDepositDate = "05 May 2023";
+    final String secondDepositDate = "09 May 2023";
+    final String withdrawDate = "10 May 2023";
+
+    private ResponseSpecification responseSpec;
+    private RequestSpecification requestSpec;
+    private SavingsProductHelper savingsProductHelper;
+    private SavingsAccountHelper savingsAccountHelper;
+
+    @BeforeEach
+    public void setup() {
+        Utils.initializeRESTAssured();
+        this.requestSpec = new 
RequestSpecBuilder().setContentType(ContentType.JSON).build();
+        this.requestSpec.header("Authorization", "Basic " + 
Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey());
+        this.responseSpec = new 
ResponseSpecBuilder().expectStatusCode(200).build();
+        this.savingsAccountHelper = new SavingsAccountHelper(this.requestSpec, 
this.responseSpec);
+        this.savingsProductHelper = new SavingsProductHelper();
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithAmountFilterLteGte() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        AmountFilters amountFilters = new AmountFilters();
+        amountFilters.setGte(BigDecimal.valueOf(100));
+        amountFilters.setLte(BigDecimal.valueOf(200));
+        filters.setAmount(amountFilters);
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(1, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            BigDecimal expectedAmount = BigDecimal.valueOf(100).setScale(6);

Review Comment:
   BigDecimals shouldn't be compared with equals but rather compareTo.



##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/SavingsAccountTransactionsSearchIntegrationTest.java:
##########
@@ -0,0 +1,386 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.integrationtests;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import io.restassured.builder.RequestSpecBuilder;
+import io.restassured.builder.ResponseSpecBuilder;
+import io.restassured.http.ContentType;
+import io.restassured.specification.RequestSpecification;
+import io.restassured.specification.ResponseSpecification;
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.time.format.DateTimeFormatter;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.fineract.client.models.AmountFilters;
+import org.apache.fineract.client.models.DateFilters;
+import org.apache.fineract.client.models.Filters;
+import org.apache.fineract.client.models.GetSavingsAccountTransactionsPageItem;
+import 
org.apache.fineract.client.models.SavingsAccountTransactionsSearchResponse;
+import org.apache.fineract.client.models.SortOrder;
+import org.apache.fineract.integrationtests.common.ClientHelper;
+import org.apache.fineract.integrationtests.common.CommonConstants;
+import org.apache.fineract.integrationtests.common.GlobalConfigurationHelper;
+import org.apache.fineract.integrationtests.common.Utils;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsAccountHelper;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsProductHelper;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsStatusChecker;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+@SuppressWarnings({ "rawtypes" })
+public class SavingsAccountTransactionsSearchIntegrationTest {
+
+    public static final String ACCOUNT_TYPE_INDIVIDUAL = "INDIVIDUAL";
+    final String startDate = "01 May 2023";
+    final String firstDepositDate = "05 May 2023";
+    final String secondDepositDate = "09 May 2023";
+    final String withdrawDate = "10 May 2023";
+
+    private ResponseSpecification responseSpec;
+    private RequestSpecification requestSpec;
+    private SavingsProductHelper savingsProductHelper;
+    private SavingsAccountHelper savingsAccountHelper;
+
+    @BeforeEach
+    public void setup() {
+        Utils.initializeRESTAssured();
+        this.requestSpec = new 
RequestSpecBuilder().setContentType(ContentType.JSON).build();
+        this.requestSpec.header("Authorization", "Basic " + 
Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey());
+        this.responseSpec = new 
ResponseSpecBuilder().expectStatusCode(200).build();
+        this.savingsAccountHelper = new SavingsAccountHelper(this.requestSpec, 
this.responseSpec);
+        this.savingsProductHelper = new SavingsProductHelper();
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithAmountFilterLteGte() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        AmountFilters amountFilters = new AmountFilters();
+        amountFilters.setGte(BigDecimal.valueOf(100));
+        amountFilters.setLte(BigDecimal.valueOf(200));
+        filters.setAmount(amountFilters);
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(1, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            BigDecimal expectedAmount = BigDecimal.valueOf(100).setScale(6);
+            assertEquals(expectedAmount, data.getAmount());
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithAmountFilterLtGt() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        AmountFilters amountFilters = new AmountFilters();
+        amountFilters.setGt(BigDecimal.valueOf(100));
+        amountFilters.setLt(BigDecimal.valueOf(400));
+        filters.setAmount(amountFilters);
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(1, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            BigDecimal expectedAmount = BigDecimal.valueOf(300).setScale(6);
+            assertEquals(expectedAmount, data.getAmount());
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithDateFilterLteGte() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        DateFilters dateFilters = new DateFilters();
+        dateFilters.setGte(LocalDate.of(2023, 05, 06));
+        dateFilters.setLte(LocalDate.of(2023, 05, 10));
+        filters.setTransactionDate(dateFilters);
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(2, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(2, transactionsResponse.getPageItems().size());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        List<LocalDate> expectedDates = 
List.of(LocalDate.parse(secondDepositDate, DateTimeFormatter.ofPattern("dd MMM 
yyyy")),
+                LocalDate.parse(withdrawDate, DateTimeFormatter.ofPattern("dd 
MMM yyyy")));
+        pageItems.forEach(data -> {
+            assertTrue(expectedDates.contains(data.getDate()));
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithTransactionTypeDeposit() 
throws JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        
filters.setTransactionType(List.of(Filters.TransactionTypeEnum.DEPOSIT));
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(2, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(2, transactionsResponse.getPageItems().size());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            assertEquals(true, data.getTransactionType().getDeposit());
+        });
+    }
+
+    @Test
+    public void 
testSavingsTransactionsSearchWithTransactionTypeWithdrawAndDeposit() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        
filters.setTransactionType(List.of(Filters.TransactionTypeEnum.DEPOSIT, 
Filters.TransactionTypeEnum.WITHDRAWAL));
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(3, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(3, transactionsResponse.getPageItems().size());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            List<String> expectedTransactionTypes = 
List.of(Filters.TransactionTypeEnum.DEPOSIT.getValue(),
+                    Filters.TransactionTypeEnum.WITHDRAWAL.getValue());
+            
assertTrue(expectedTransactionTypes.contains(data.getTransactionType().getValue().toUpperCase()));
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithPaginationAndNoFilter() {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.postInterestForSavings(savingsId);
+        Filters filters = new Filters();
+        int page = 0;
+        int size = 10;
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters,
+                null, page, size);
+        Assertions.assertNotNull(transactionsResponse);
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(10, transactionsResponse.getPageItems().size());
+    }
+
+    @Test
+    public void 
testSavingsTransactionsSearchWithTransactionTypeDepositAndDefaultSort() {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        Filters filters = new Filters();
+        
filters.setTransactionType(List.of(Filters.TransactionTypeEnum.DEPOSIT));
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(2, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(2, transactionsResponse.getPageItems().size());
+        List<LocalDate> transactionDates = 
transactionsResponse.getPageItems().stream().map(data -> data.getDate())
+                .collect(Collectors.toList());
+        assertTrue(isDateListOrdered(transactionDates, "desc"));

Review Comment:
   Why don't you check the individual items one by one in the proper order?



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/search/SavingsTransactionsSearchRepositoryImpl.java:
##########
@@ -0,0 +1,176 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.domain.search;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import javax.persistence.EntityManager;
+import javax.persistence.TypedQuery;
+import lombok.RequiredArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.fineract.portfolio.savings.SavingsAccountTransactionType;
+import 
org.apache.fineract.portfolio.savings.data.SavingsTransactionSearchResult;
+import 
org.apache.fineract.portfolio.savings.domain.search.SavingsTransactionSearch.Filters;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
+import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.support.PageableExecutionUtils;
+import org.springframework.stereotype.Repository;
+
+@Repository
+@RequiredArgsConstructor
+public class SavingsTransactionsSearchRepositoryImpl implements 
SavingsTransactionsSearchRepository {
+
+    private final EntityManager entityManager;
+
+    @Override
+    public Page<SavingsTransactionSearchResult> searchTransactions(Long 
savingsId, Integer depositAccountType, Filters filters,
+            Pageable pageable) {
+        String jpql = buildJpqlQuery();
+        StringBuilder queryBuilder = new StringBuilder(jpql);
+        queryBuilder.append(" WHERE ");
+        queryBuilder.append(" tr.savingsAccount.id = :savingsId ");
+        queryBuilder.append(" AND tr.savingsAccount.depositType = :depositType 
");
+        Map<String, Object> parameterMap = new HashMap<>();
+        parameterMap.put("savingsId", savingsId);
+        parameterMap.put("depositType", depositAccountType);
+
+        if (Objects.nonNull(filters)) {
+            SavingsTransactionSearch.DateFilters dateFilters = 
filters.getDateFilters();
+            if (Objects.nonNull(dateFilters)) {
+                if (Objects.nonNull(dateFilters.getLte())) {
+                    queryBuilder.append(" AND tr.dateOf <= :dateLte ");
+                    parameterMap.put("dateLte", dateFilters.getLte());
+                }
+                if (Objects.nonNull(dateFilters.getGte())) {
+                    queryBuilder.append(" AND tr.dateOf >= :dateGte ");
+                    parameterMap.put("dateGte", dateFilters.getGte());
+                }
+                // We prioritize the lte/gte filter by applying it first. If 
lte/gte is not provided, only then we

Review Comment:
   All this can be removed if you do the structure I mentioned above.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/search/SavingsTransactionSearch.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.domain.search;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.util.List;
+import lombok.Data;
+import org.apache.fineract.portfolio.savings.SavingsAccountTransactionType;
+
+@Data
+public class SavingsTransactionSearch {

Review Comment:
   I don't get why you need the JsonProperty annotations everywhere. Why don't 
you just name the attributes appropriately?



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/search/SavingsTransactionsSearchRepositoryImpl.java:
##########
@@ -0,0 +1,176 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.domain.search;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import javax.persistence.EntityManager;
+import javax.persistence.TypedQuery;
+import lombok.RequiredArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.fineract.portfolio.savings.SavingsAccountTransactionType;
+import 
org.apache.fineract.portfolio.savings.data.SavingsTransactionSearchResult;
+import 
org.apache.fineract.portfolio.savings.domain.search.SavingsTransactionSearch.Filters;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
+import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.support.PageableExecutionUtils;
+import org.springframework.stereotype.Repository;
+
+@Repository
+@RequiredArgsConstructor
+public class SavingsTransactionsSearchRepositoryImpl implements 
SavingsTransactionsSearchRepository {
+
+    private final EntityManager entityManager;
+
+    @Override
+    public Page<SavingsTransactionSearchResult> searchTransactions(Long 
savingsId, Integer depositAccountType, Filters filters,

Review Comment:
   Why don't we use a Parameter Object here instead of the individual params?



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/search/SavingsTransactionsSearchRepositoryImpl.java:
##########
@@ -0,0 +1,176 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.domain.search;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import javax.persistence.EntityManager;
+import javax.persistence.TypedQuery;
+import lombok.RequiredArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.fineract.portfolio.savings.SavingsAccountTransactionType;
+import 
org.apache.fineract.portfolio.savings.data.SavingsTransactionSearchResult;
+import 
org.apache.fineract.portfolio.savings.domain.search.SavingsTransactionSearch.Filters;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
+import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.support.PageableExecutionUtils;
+import org.springframework.stereotype.Repository;
+
+@Repository
+@RequiredArgsConstructor
+public class SavingsTransactionsSearchRepositoryImpl implements 
SavingsTransactionsSearchRepository {
+
+    private final EntityManager entityManager;
+
+    @Override
+    public Page<SavingsTransactionSearchResult> searchTransactions(Long 
savingsId, Integer depositAccountType, Filters filters,
+            Pageable pageable) {
+        String jpql = buildJpqlQuery();
+        StringBuilder queryBuilder = new StringBuilder(jpql);
+        queryBuilder.append(" WHERE ");
+        queryBuilder.append(" tr.savingsAccount.id = :savingsId ");
+        queryBuilder.append(" AND tr.savingsAccount.depositType = :depositType 
");
+        Map<String, Object> parameterMap = new HashMap<>();
+        parameterMap.put("savingsId", savingsId);
+        parameterMap.put("depositType", depositAccountType);
+
+        if (Objects.nonNull(filters)) {
+            SavingsTransactionSearch.DateFilters dateFilters = 
filters.getDateFilters();
+            if (Objects.nonNull(dateFilters)) {
+                if (Objects.nonNull(dateFilters.getLte())) {
+                    queryBuilder.append(" AND tr.dateOf <= :dateLte ");
+                    parameterMap.put("dateLte", dateFilters.getLte());
+                }
+                if (Objects.nonNull(dateFilters.getGte())) {
+                    queryBuilder.append(" AND tr.dateOf >= :dateGte ");
+                    parameterMap.put("dateGte", dateFilters.getGte());
+                }
+                // We prioritize the lte/gte filter by applying it first. If 
lte/gte is not provided, only then we
+                // apply the lt/gt filter.
+                // By enforcing this order of conditions, we ensure that if 
both lte/gte and lt/gt are provided, the
+                // lte/gte condition takes precedence and the lt/gt condition 
is ignored. This helps avoid
+                // conflicting or redundant filter conditions and provides a 
consistent behavior for the users.
+                if (Objects.isNull(dateFilters.getLte()) && 
Objects.nonNull(dateFilters.getLt())) {
+                    queryBuilder.append(" AND tr.dateOf < :dateLt ");
+                    parameterMap.put("dateLt", dateFilters.getLt());
+                }
+                if (Objects.isNull(dateFilters.getGte()) && 
Objects.nonNull(dateFilters.getGt())) {
+                    queryBuilder.append(" AND tr.dateOf > :dateGt ");
+                    parameterMap.put("dateGt", dateFilters.getGt());
+                }
+            }
+
+            SavingsTransactionSearch.AmountFilters amountFilters = 
filters.getAmountFilters();
+            if (Objects.nonNull(amountFilters)) {
+                if (Objects.nonNull(amountFilters.getLte())) {
+                    queryBuilder.append(" AND tr.amount <= :amountLte ");
+                    parameterMap.put("amountLte", amountFilters.getLte());
+                }
+                if (Objects.nonNull(amountFilters.getGte())) {
+                    queryBuilder.append(" AND tr.amount >= :amountGte ");
+                    parameterMap.put("amountGte", amountFilters.getGte());
+                }
+                // We prioritize the lte/gte filter by applying it first. If 
lte/gte is not provided, only then we
+                // apply the lt/gt filter.
+                // By enforcing this order of conditions, we ensure that if 
both lte/gte and lt/gt are provided, the
+                // lte/gte condition takes precedence and the lt/gt condition 
is ignored. This helps avoid
+                // conflicting or redundant filter conditions and provides a 
consistent behavior for the users.
+                if (Objects.isNull(amountFilters.getLte()) && 
Objects.nonNull(amountFilters.getLt())) {
+                    queryBuilder.append(" AND tr.amount < :amountLt ");
+                    parameterMap.put("amountLt", amountFilters.getLt());
+                }
+                if (Objects.isNull(amountFilters.getGte()) && 
Objects.nonNull(amountFilters.getGt())) {
+                    queryBuilder.append(" AND tr.amount > :amountGt ");
+                    parameterMap.put("amountGt", amountFilters.getGt());
+                }
+            }
+            List<SavingsAccountTransactionType> transactionTypes = 
filters.getTransactionType();
+            if (CollectionUtils.isNotEmpty(transactionTypes)) {
+                List<Integer> transactionTypeValues = 
transactionTypes.stream().map(SavingsAccountTransactionType::getValue)
+                        .collect(Collectors.toList());
+                queryBuilder.append(" AND tr.typeOf IN :transactionTypes ");
+                parameterMap.put("transactionTypes", transactionTypeValues);
+            }
+        }
+        Sort sortFromPageable = pageable.getSort();
+        if (Objects.nonNull(pageable.getSort()) && 
pageable.getSort().isSorted()) {
+            queryBuilder.append(getSortOrders(sortFromPageable));
+        }
+
+        TypedQuery<SavingsTransactionSearchResult> queryToExecute = 
entityManager.createQuery(queryBuilder.toString(),
+                SavingsTransactionSearchResult.class);
+
+        for (Map.Entry<String, Object> entry : parameterMap.entrySet()) {
+            queryToExecute.setParameter(entry.getKey(), entry.getValue());
+        }
+        if (pageable.isPaged()) {
+            queryToExecute.setFirstResult((int) pageable.getOffset());
+            queryToExecute.setMaxResults(pageable.getPageSize());
+        }
+        List<SavingsTransactionSearchResult> resultList = 
queryToExecute.getResultList();
+        return PageableExecutionUtils.getPage(resultList, pageable, () -> 
getTotalElements(queryBuilder, parameterMap));
+    }
+
+    private String buildJpqlQuery() {
+        return """
+                        SELECT NEW 
org.apache.fineract.portfolio.savings.data.SavingsTransactionSearchResult(tr.id,
+                    tr.typeOf, tr.dateOf, tr.amount, 
tr.releaseIdOfHoldAmountTransaction, tr.reasonForBlock,
+                    tr.createdDate, tr.appUser, nt.note, tr.runningBalance, 
tr.reversed,
+                    tr.reversalTransaction, tr.originalTxnId, 
tr.lienTransaction, tr.isManualTransaction,
+                    fromTran, toTran, tr.savingsAccount, tr.paymentDetail, 
currency
+                    )
+                        FROM SavingsAccountTransaction tr
+                        JOIN ApplicationCurrency currency ON (currency.code = 
tr.savingsAccount.currency.code)
+                        LEFT JOIN AccountTransferTransaction fromtran ON 
(fromtran.fromSavingsTransaction = tr)
+                        LEFT JOIN AccountTransferTransaction totran ON 
(totran.toSavingsTransaction = tr)
+                        LEFT JOIN tr.notes nt ON (nt.savingsTransaction = tr)
+                """;
+    }
+
+    private Long getTotalElements(StringBuilder queryBuilder, Map<String, 
Object> parameterMap) {
+        // ORDER BY Clause not required for Count Query
+        int orderByIndex = queryBuilder.indexOf("ORDER BY");
+        if (orderByIndex != -1) {
+            queryBuilder.replace(orderByIndex, queryBuilder.length(), "");
+        }
+        String countJPQL = "SELECT COUNT(tr) " + 
queryBuilder.substring(queryBuilder.indexOf("FROM"), queryBuilder.length());

Review Comment:
   No, please no. No string manipulation. As soon as you start doing it, 
there's never a stop to it. Really difficult to refactor and maintain + as I 
said above, extremely fragile.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/search/SavingsTransactionsSearchRepositoryImpl.java:
##########
@@ -0,0 +1,176 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.domain.search;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import javax.persistence.EntityManager;
+import javax.persistence.TypedQuery;
+import lombok.RequiredArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.fineract.portfolio.savings.SavingsAccountTransactionType;
+import 
org.apache.fineract.portfolio.savings.data.SavingsTransactionSearchResult;
+import 
org.apache.fineract.portfolio.savings.domain.search.SavingsTransactionSearch.Filters;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
+import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.support.PageableExecutionUtils;
+import org.springframework.stereotype.Repository;
+
+@Repository
+@RequiredArgsConstructor
+public class SavingsTransactionsSearchRepositoryImpl implements 
SavingsTransactionsSearchRepository {
+
+    private final EntityManager entityManager;
+
+    @Override
+    public Page<SavingsTransactionSearchResult> searchTransactions(Long 
savingsId, Integer depositAccountType, Filters filters,
+            Pageable pageable) {
+        String jpql = buildJpqlQuery();
+        StringBuilder queryBuilder = new StringBuilder(jpql);
+        queryBuilder.append(" WHERE ");
+        queryBuilder.append(" tr.savingsAccount.id = :savingsId ");
+        queryBuilder.append(" AND tr.savingsAccount.depositType = :depositType 
");
+        Map<String, Object> parameterMap = new HashMap<>();
+        parameterMap.put("savingsId", savingsId);
+        parameterMap.put("depositType", depositAccountType);
+
+        if (Objects.nonNull(filters)) {
+            SavingsTransactionSearch.DateFilters dateFilters = 
filters.getDateFilters();
+            if (Objects.nonNull(dateFilters)) {
+                if (Objects.nonNull(dateFilters.getLte())) {
+                    queryBuilder.append(" AND tr.dateOf <= :dateLte ");
+                    parameterMap.put("dateLte", dateFilters.getLte());
+                }
+                if (Objects.nonNull(dateFilters.getGte())) {
+                    queryBuilder.append(" AND tr.dateOf >= :dateGte ");
+                    parameterMap.put("dateGte", dateFilters.getGte());
+                }
+                // We prioritize the lte/gte filter by applying it first. If 
lte/gte is not provided, only then we
+                // apply the lt/gt filter.
+                // By enforcing this order of conditions, we ensure that if 
both lte/gte and lt/gt are provided, the
+                // lte/gte condition takes precedence and the lt/gt condition 
is ignored. This helps avoid
+                // conflicting or redundant filter conditions and provides a 
consistent behavior for the users.
+                if (Objects.isNull(dateFilters.getLte()) && 
Objects.nonNull(dateFilters.getLt())) {
+                    queryBuilder.append(" AND tr.dateOf < :dateLt ");
+                    parameterMap.put("dateLt", dateFilters.getLt());
+                }
+                if (Objects.isNull(dateFilters.getGte()) && 
Objects.nonNull(dateFilters.getGt())) {
+                    queryBuilder.append(" AND tr.dateOf > :dateGt ");
+                    parameterMap.put("dateGt", dateFilters.getGt());
+                }
+            }
+
+            SavingsTransactionSearch.AmountFilters amountFilters = 
filters.getAmountFilters();
+            if (Objects.nonNull(amountFilters)) {
+                if (Objects.nonNull(amountFilters.getLte())) {
+                    queryBuilder.append(" AND tr.amount <= :amountLte ");
+                    parameterMap.put("amountLte", amountFilters.getLte());
+                }
+                if (Objects.nonNull(amountFilters.getGte())) {
+                    queryBuilder.append(" AND tr.amount >= :amountGte ");
+                    parameterMap.put("amountGte", amountFilters.getGte());
+                }
+                // We prioritize the lte/gte filter by applying it first. If 
lte/gte is not provided, only then we
+                // apply the lt/gt filter.
+                // By enforcing this order of conditions, we ensure that if 
both lte/gte and lt/gt are provided, the
+                // lte/gte condition takes precedence and the lt/gt condition 
is ignored. This helps avoid
+                // conflicting or redundant filter conditions and provides a 
consistent behavior for the users.
+                if (Objects.isNull(amountFilters.getLte()) && 
Objects.nonNull(amountFilters.getLt())) {
+                    queryBuilder.append(" AND tr.amount < :amountLt ");
+                    parameterMap.put("amountLt", amountFilters.getLt());
+                }
+                if (Objects.isNull(amountFilters.getGte()) && 
Objects.nonNull(amountFilters.getGt())) {
+                    queryBuilder.append(" AND tr.amount > :amountGt ");
+                    parameterMap.put("amountGt", amountFilters.getGt());
+                }
+            }
+            List<SavingsAccountTransactionType> transactionTypes = 
filters.getTransactionType();
+            if (CollectionUtils.isNotEmpty(transactionTypes)) {
+                List<Integer> transactionTypeValues = 
transactionTypes.stream().map(SavingsAccountTransactionType::getValue)
+                        .collect(Collectors.toList());
+                queryBuilder.append(" AND tr.typeOf IN :transactionTypes ");
+                parameterMap.put("transactionTypes", transactionTypeValues);
+            }
+        }
+        Sort sortFromPageable = pageable.getSort();
+        if (Objects.nonNull(pageable.getSort()) && 
pageable.getSort().isSorted()) {
+            queryBuilder.append(getSortOrders(sortFromPageable));
+        }
+
+        TypedQuery<SavingsTransactionSearchResult> queryToExecute = 
entityManager.createQuery(queryBuilder.toString(),
+                SavingsTransactionSearchResult.class);
+
+        for (Map.Entry<String, Object> entry : parameterMap.entrySet()) {
+            queryToExecute.setParameter(entry.getKey(), entry.getValue());
+        }
+        if (pageable.isPaged()) {
+            queryToExecute.setFirstResult((int) pageable.getOffset());
+            queryToExecute.setMaxResults(pageable.getPageSize());
+        }
+        List<SavingsTransactionSearchResult> resultList = 
queryToExecute.getResultList();
+        return PageableExecutionUtils.getPage(resultList, pageable, () -> 
getTotalElements(queryBuilder, parameterMap));
+    }
+
+    private String buildJpqlQuery() {
+        return """
+                        SELECT NEW 
org.apache.fineract.portfolio.savings.data.SavingsTransactionSearchResult(tr.id,
+                    tr.typeOf, tr.dateOf, tr.amount, 
tr.releaseIdOfHoldAmountTransaction, tr.reasonForBlock,
+                    tr.createdDate, tr.appUser, nt.note, tr.runningBalance, 
tr.reversed,
+                    tr.reversalTransaction, tr.originalTxnId, 
tr.lienTransaction, tr.isManualTransaction,
+                    fromTran, toTran, tr.savingsAccount, tr.paymentDetail, 
currency
+                    )
+                        FROM SavingsAccountTransaction tr
+                        JOIN ApplicationCurrency currency ON (currency.code = 
tr.savingsAccount.currency.code)
+                        LEFT JOIN AccountTransferTransaction fromtran ON 
(fromtran.fromSavingsTransaction = tr)
+                        LEFT JOIN AccountTransferTransaction totran ON 
(totran.toSavingsTransaction = tr)
+                        LEFT JOIN tr.notes nt ON (nt.savingsTransaction = tr)
+                """;
+    }
+
+    private Long getTotalElements(StringBuilder queryBuilder, Map<String, 
Object> parameterMap) {
+        // ORDER BY Clause not required for Count Query
+        int orderByIndex = queryBuilder.indexOf("ORDER BY");

Review Comment:
   This is a super no-go. Why construct the order by in there in the first 
place? What if the ORDER BY clause is all lowercase? This is extremely fragile.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/api/SavingsAccountTransactionsApiResource.java:
##########
@@ -118,6 +124,19 @@ public String retrieveOne(@PathParam("savingsId") final 
Long savingsId, @PathPar
                 
SavingsApiSetConstants.SAVINGS_TRANSACTION_RESPONSE_DATA_PARAMETERS);
     }
 
+    @POST
+    @Path("search")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(summary = "Search Savings Account Transactions")
+    @ApiResponses({
+            @ApiResponse(responseCode = "200", description = "OK", content = 
@Content(schema = @Schema(implementation = 
SavingsAccountTransactionsApiResourceSwagger.SavingsAccountTransactionsSearchResponse.class)))
 })
+    public String searchTransactions(@PathParam("savingsId") 
@Parameter(description = "savingsId") final Long savingsId,

Review Comment:
   Is there any reason we must return a String from this instead of the Page 
itself? 



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/search/SavingsAccountTransactionsSearchServiceImpl.java:
##########
@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.service.search;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.infrastructure.core.service.Page;
+import org.apache.fineract.infrastructure.core.service.PagedRequest;
+import 
org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
+import org.apache.fineract.portfolio.savings.DepositAccountType;
+import 
org.apache.fineract.portfolio.savings.data.SavingsAccountTransactionData;
+import 
org.apache.fineract.portfolio.savings.data.SavingsTransactionSearchResult;
+import 
org.apache.fineract.portfolio.savings.domain.SavingsAccountTransactionRepository;
+import 
org.apache.fineract.portfolio.savings.domain.search.SavingsTransactionSearch;
+import 
org.apache.fineract.portfolio.savings.domain.search.SavingsTransactionSearch.Filters;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort.Order;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+
+@Service
+@Transactional(readOnly = true)
+@RequiredArgsConstructor
+public class SavingsAccountTransactionsSearchServiceImpl {

Review Comment:
   Class shouldn't be named as Impl since there's no interface behind it.



##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/PagedRequest.java:
##########
@@ -53,6 +55,18 @@ public Pageable toPageable() {
         }
     }
 
+    public Pageable toPageableWithDefaultSortOrders(List<Order> 
defaultSortOrders) {

Review Comment:
   This is mostly copy paste and as I said on the service class, the default 
ordering shouldn't be on this level but rather on the repo.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/search/SavingsAccountTransactionSearchService.java:
##########
@@ -0,0 +1,28 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.service.search;
+
+import org.apache.fineract.infrastructure.core.service.PagedRequest;
+import 
org.apache.fineract.portfolio.savings.domain.search.SavingsTransactionSearch;
+
+public interface SavingsAccountTransactionSearchService {

Review Comment:
   Interface is not used.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/search/SavingsAccountTransactionsSearchServiceImpl.java:
##########
@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.service.search;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.infrastructure.core.service.Page;
+import org.apache.fineract.infrastructure.core.service.PagedRequest;
+import 
org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
+import org.apache.fineract.portfolio.savings.DepositAccountType;
+import 
org.apache.fineract.portfolio.savings.data.SavingsAccountTransactionData;
+import 
org.apache.fineract.portfolio.savings.data.SavingsTransactionSearchResult;
+import 
org.apache.fineract.portfolio.savings.domain.SavingsAccountTransactionRepository;
+import 
org.apache.fineract.portfolio.savings.domain.search.SavingsTransactionSearch;
+import 
org.apache.fineract.portfolio.savings.domain.search.SavingsTransactionSearch.Filters;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort.Order;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+
+@Service
+@Transactional(readOnly = true)
+@RequiredArgsConstructor
+public class SavingsAccountTransactionsSearchServiceImpl {
+
+    private final PlatformSecurityContext context;
+
+    private final SavingsAccountTransactionRepository 
savingsTransactionRepository;
+
+    public Page<SavingsAccountTransactionData> searchTransactions(Long 
savingsId, PagedRequest<SavingsTransactionSearch> searchRequest) {
+        validateSearchRequest(searchRequest);
+        return executeSearch(savingsId, DepositAccountType.SAVINGS_DEPOSIT, 
searchRequest);
+    }
+
+    private void validateSearchRequest(PagedRequest<SavingsTransactionSearch> 
searchRequest) {
+        Objects.requireNonNull(searchRequest, "searchRequest must not be 
null");
+
+        context.isAuthenticated();
+    }
+
+    private Page<SavingsAccountTransactionData> executeSearch(Long savingsId, 
DepositAccountType depositType,
+            PagedRequest<SavingsTransactionSearch> searchRequest) {
+        Optional<SavingsTransactionSearch> request = 
searchRequest.getRequest();
+        Pageable pageable = 
searchRequest.toPageableWithDefaultSortOrders(getDefaultOrders());
+        Filters searchFilters = 
request.map(SavingsTransactionSearch::getFilters).orElse(null);
+        org.springframework.data.domain.Page<SavingsAccountTransactionData> 
pageResult = savingsTransactionRepository
+                .searchTransactions(savingsId, depositType.getValue(), 
searchFilters, pageable)
+                
.map(SavingsTransactionSearchResult::toSavingsAccountTransactionData);
+        return new Page<>(pageResult.getContent(), 
Long.valueOf(pageResult.getTotalElements()).intValue());
+    }
+
+    private List<Order> getDefaultOrders() {

Review Comment:
   I don't think putting the default ordering on is the responsibility of the 
service but rather the repository layer since it has to make sure for a paged 
query, there must be a consistent ordering in place.



##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/SavingsAccountTransactionsSearchIntegrationTest.java:
##########
@@ -0,0 +1,386 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.integrationtests;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import io.restassured.builder.RequestSpecBuilder;
+import io.restassured.builder.ResponseSpecBuilder;
+import io.restassured.http.ContentType;
+import io.restassured.specification.RequestSpecification;
+import io.restassured.specification.ResponseSpecification;
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.time.format.DateTimeFormatter;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.fineract.client.models.AmountFilters;
+import org.apache.fineract.client.models.DateFilters;
+import org.apache.fineract.client.models.Filters;
+import org.apache.fineract.client.models.GetSavingsAccountTransactionsPageItem;
+import 
org.apache.fineract.client.models.SavingsAccountTransactionsSearchResponse;
+import org.apache.fineract.client.models.SortOrder;
+import org.apache.fineract.integrationtests.common.ClientHelper;
+import org.apache.fineract.integrationtests.common.CommonConstants;
+import org.apache.fineract.integrationtests.common.GlobalConfigurationHelper;
+import org.apache.fineract.integrationtests.common.Utils;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsAccountHelper;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsProductHelper;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsStatusChecker;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+@SuppressWarnings({ "rawtypes" })
+public class SavingsAccountTransactionsSearchIntegrationTest {
+
+    public static final String ACCOUNT_TYPE_INDIVIDUAL = "INDIVIDUAL";
+    final String startDate = "01 May 2023";
+    final String firstDepositDate = "05 May 2023";
+    final String secondDepositDate = "09 May 2023";
+    final String withdrawDate = "10 May 2023";
+
+    private ResponseSpecification responseSpec;
+    private RequestSpecification requestSpec;
+    private SavingsProductHelper savingsProductHelper;
+    private SavingsAccountHelper savingsAccountHelper;
+
+    @BeforeEach
+    public void setup() {
+        Utils.initializeRESTAssured();
+        this.requestSpec = new 
RequestSpecBuilder().setContentType(ContentType.JSON).build();
+        this.requestSpec.header("Authorization", "Basic " + 
Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey());
+        this.responseSpec = new 
ResponseSpecBuilder().expectStatusCode(200).build();
+        this.savingsAccountHelper = new SavingsAccountHelper(this.requestSpec, 
this.responseSpec);
+        this.savingsProductHelper = new SavingsProductHelper();
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithAmountFilterLteGte() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        AmountFilters amountFilters = new AmountFilters();
+        amountFilters.setGte(BigDecimal.valueOf(100));
+        amountFilters.setLte(BigDecimal.valueOf(200));
+        filters.setAmount(amountFilters);
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(1, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            BigDecimal expectedAmount = BigDecimal.valueOf(100).setScale(6);
+            assertEquals(expectedAmount, data.getAmount());
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithAmountFilterLtGt() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        AmountFilters amountFilters = new AmountFilters();
+        amountFilters.setGt(BigDecimal.valueOf(100));
+        amountFilters.setLt(BigDecimal.valueOf(400));
+        filters.setAmount(amountFilters);
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(1, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            BigDecimal expectedAmount = BigDecimal.valueOf(300).setScale(6);
+            assertEquals(expectedAmount, data.getAmount());
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithDateFilterLteGte() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        DateFilters dateFilters = new DateFilters();
+        dateFilters.setGte(LocalDate.of(2023, 05, 06));
+        dateFilters.setLte(LocalDate.of(2023, 05, 10));
+        filters.setTransactionDate(dateFilters);
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(2, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(2, transactionsResponse.getPageItems().size());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        List<LocalDate> expectedDates = 
List.of(LocalDate.parse(secondDepositDate, DateTimeFormatter.ofPattern("dd MMM 
yyyy")),
+                LocalDate.parse(withdrawDate, DateTimeFormatter.ofPattern("dd 
MMM yyyy")));
+        pageItems.forEach(data -> {
+            assertTrue(expectedDates.contains(data.getDate()));
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithTransactionTypeDeposit() 
throws JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        
filters.setTransactionType(List.of(Filters.TransactionTypeEnum.DEPOSIT));
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(2, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(2, transactionsResponse.getPageItems().size());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            assertEquals(true, data.getTransactionType().getDeposit());
+        });
+    }
+
+    @Test
+    public void 
testSavingsTransactionsSearchWithTransactionTypeWithdrawAndDeposit() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        
filters.setTransactionType(List.of(Filters.TransactionTypeEnum.DEPOSIT, 
Filters.TransactionTypeEnum.WITHDRAWAL));
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(3, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(3, transactionsResponse.getPageItems().size());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            List<String> expectedTransactionTypes = 
List.of(Filters.TransactionTypeEnum.DEPOSIT.getValue(),
+                    Filters.TransactionTypeEnum.WITHDRAWAL.getValue());
+            
assertTrue(expectedTransactionTypes.contains(data.getTransactionType().getValue().toUpperCase()));
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithPaginationAndNoFilter() {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.postInterestForSavings(savingsId);
+        Filters filters = new Filters();
+        int page = 0;
+        int size = 10;
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters,
+                null, page, size);
+        Assertions.assertNotNull(transactionsResponse);
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(10, transactionsResponse.getPageItems().size());

Review Comment:
   This test is not following the FIRST principles, specifically the I and R 
are violated since you're expecting 10 savings transactions but you're only 
saving 3 transactions + the interest posting which I assume is only 1 
transaction.
   So if this test is being ran by itself, this will fail.
   
   Note: unless the post savings interest thingy creates 7+ transactions, then 
just disregard my comment.



##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/common/savings/SavingsAccountHelper.java:
##########
@@ -646,6 +653,47 @@ public HashMap getSavingsTransaction(final Integer 
savingsID, final Integer savi
         return Utils.performServerGet(requestSpec, responseSpec, URL, "");
     }
 
+    public SavingsAccountTransactionsSearchResponse searchTransactions(Integer 
savingsId, Filters filters) {

Review Comment:
   These methods are copy paste. Why don't you reuse them properly?



##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/SavingsAccountTransactionsSearchIntegrationTest.java:
##########
@@ -0,0 +1,386 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.integrationtests;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import io.restassured.builder.RequestSpecBuilder;
+import io.restassured.builder.ResponseSpecBuilder;
+import io.restassured.http.ContentType;
+import io.restassured.specification.RequestSpecification;
+import io.restassured.specification.ResponseSpecification;
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.time.format.DateTimeFormatter;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.fineract.client.models.AmountFilters;
+import org.apache.fineract.client.models.DateFilters;
+import org.apache.fineract.client.models.Filters;
+import org.apache.fineract.client.models.GetSavingsAccountTransactionsPageItem;
+import 
org.apache.fineract.client.models.SavingsAccountTransactionsSearchResponse;
+import org.apache.fineract.client.models.SortOrder;
+import org.apache.fineract.integrationtests.common.ClientHelper;
+import org.apache.fineract.integrationtests.common.CommonConstants;
+import org.apache.fineract.integrationtests.common.GlobalConfigurationHelper;
+import org.apache.fineract.integrationtests.common.Utils;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsAccountHelper;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsProductHelper;
+import 
org.apache.fineract.integrationtests.common.savings.SavingsStatusChecker;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+@SuppressWarnings({ "rawtypes" })
+public class SavingsAccountTransactionsSearchIntegrationTest {
+
+    public static final String ACCOUNT_TYPE_INDIVIDUAL = "INDIVIDUAL";
+    final String startDate = "01 May 2023";
+    final String firstDepositDate = "05 May 2023";
+    final String secondDepositDate = "09 May 2023";
+    final String withdrawDate = "10 May 2023";
+
+    private ResponseSpecification responseSpec;
+    private RequestSpecification requestSpec;
+    private SavingsProductHelper savingsProductHelper;
+    private SavingsAccountHelper savingsAccountHelper;
+
+    @BeforeEach
+    public void setup() {
+        Utils.initializeRESTAssured();
+        this.requestSpec = new 
RequestSpecBuilder().setContentType(ContentType.JSON).build();
+        this.requestSpec.header("Authorization", "Basic " + 
Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey());
+        this.responseSpec = new 
ResponseSpecBuilder().expectStatusCode(200).build();
+        this.savingsAccountHelper = new SavingsAccountHelper(this.requestSpec, 
this.responseSpec);
+        this.savingsProductHelper = new SavingsProductHelper();
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithAmountFilterLteGte() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        AmountFilters amountFilters = new AmountFilters();
+        amountFilters.setGte(BigDecimal.valueOf(100));
+        amountFilters.setLte(BigDecimal.valueOf(200));
+        filters.setAmount(amountFilters);
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(1, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            BigDecimal expectedAmount = BigDecimal.valueOf(100).setScale(6);
+            assertEquals(expectedAmount, data.getAmount());
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithAmountFilterLtGt() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
startDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        AmountFilters amountFilters = new AmountFilters();
+        amountFilters.setGt(BigDecimal.valueOf(100));
+        amountFilters.setLt(BigDecimal.valueOf(400));
+        filters.setAmount(amountFilters);
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(1, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            BigDecimal expectedAmount = BigDecimal.valueOf(300).setScale(6);
+            assertEquals(expectedAmount, data.getAmount());
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithDateFilterLteGte() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        DateFilters dateFilters = new DateFilters();
+        dateFilters.setGte(LocalDate.of(2023, 05, 06));
+        dateFilters.setLte(LocalDate.of(2023, 05, 10));
+        filters.setTransactionDate(dateFilters);
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(2, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(2, transactionsResponse.getPageItems().size());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        List<LocalDate> expectedDates = 
List.of(LocalDate.parse(secondDepositDate, DateTimeFormatter.ofPattern("dd MMM 
yyyy")),
+                LocalDate.parse(withdrawDate, DateTimeFormatter.ofPattern("dd 
MMM yyyy")));
+        pageItems.forEach(data -> {
+            assertTrue(expectedDates.contains(data.getDate()));
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithTransactionTypeDeposit() 
throws JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        
filters.setTransactionType(List.of(Filters.TransactionTypeEnum.DEPOSIT));
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(2, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(2, transactionsResponse.getPageItems().size());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            assertEquals(true, data.getTransactionType().getDeposit());
+        });
+    }
+
+    @Test
+    public void 
testSavingsTransactionsSearchWithTransactionTypeWithdrawAndDeposit() throws 
JsonProcessingException {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+
+        Filters filters = new Filters();
+        
filters.setTransactionType(List.of(Filters.TransactionTypeEnum.DEPOSIT, 
Filters.TransactionTypeEnum.WITHDRAWAL));
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(3, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(3, transactionsResponse.getPageItems().size());
+        Set<GetSavingsAccountTransactionsPageItem> pageItems = 
transactionsResponse.getPageItems();
+        pageItems.forEach(data -> {
+            List<String> expectedTransactionTypes = 
List.of(Filters.TransactionTypeEnum.DEPOSIT.getValue(),
+                    Filters.TransactionTypeEnum.WITHDRAWAL.getValue());
+            
assertTrue(expectedTransactionTypes.contains(data.getTransactionType().getValue().toUpperCase()));
+        });
+    }
+
+    @Test
+    public void testSavingsTransactionsSearchWithPaginationAndNoFilter() {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.postInterestForSavings(savingsId);
+        Filters filters = new Filters();
+        int page = 0;
+        int size = 10;
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters,
+                null, page, size);
+        Assertions.assertNotNull(transactionsResponse);
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(10, transactionsResponse.getPageItems().size());
+    }
+
+    @Test
+    public void 
testSavingsTransactionsSearchWithTransactionTypeDepositAndDefaultSort() {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        Filters filters = new Filters();
+        
filters.setTransactionType(List.of(Filters.TransactionTypeEnum.DEPOSIT));
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(2, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(2, transactionsResponse.getPageItems().size());
+        List<LocalDate> transactionDates = 
transactionsResponse.getPageItems().stream().map(data -> data.getDate())
+                .collect(Collectors.toList());
+        assertTrue(isDateListOrdered(transactionDates, "desc"));
+    }
+
+    @Test
+    public void 
testSavingsTransactionsSearchWithTransactionTypeDepositAndSortByAmountAsc() {
+        final Integer clientID = ClientHelper.createClient(this.requestSpec, 
this.responseSpec, startDate);
+        Assertions.assertNotNull(clientID);
+        final Integer savingsId = createSavingsAccountDailyPosting(clientID, 
startDate);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "100", 
firstDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.depositToSavingsAccount(savingsId, "300", 
secondDepositDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        this.savingsAccountHelper.withdrawalFromSavingsAccount(savingsId, 
"100", withdrawDate, CommonConstants.RESPONSE_RESOURCE_ID);
+        Filters filters = new Filters();
+        
filters.setTransactionType(List.of(Filters.TransactionTypeEnum.DEPOSIT));
+        SortOrder sortOrder = new SortOrder();
+        sortOrder.setProperty("amount");
+        sortOrder.setDirection(SortOrder.DirectionEnum.ASC);
+        List<SortOrder> sortOrders = List.of(sortOrder);
+        SavingsAccountTransactionsSearchResponse transactionsResponse = 
this.savingsAccountHelper.searchTransactions(savingsId, filters,
+                sortOrders);
+        Assertions.assertNotNull(transactionsResponse);
+        assertEquals(2, transactionsResponse.getTotalFilteredRecords());
+        Assertions.assertNotNull(transactionsResponse.getPageItems());
+        assertEquals(2, transactionsResponse.getPageItems().size());
+        List<BigDecimal> amountList = 
transactionsResponse.getPageItems().stream().map(data -> data.getAmount())
+                .collect(Collectors.toList());
+        assertTrue(isListOrdered(amountList, "asc"));

Review Comment:
   Same as above.



-- 
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]


Reply via email to