[GitHub] [fineract] galovics commented on a diff in pull request #3192: FINERACT-1912 Support Pagination, sorting and filtering for Savings account transactions

["galovics (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org