marta-jankovics commented on code in PR #3321:
URL: https://github.com/apache/fineract/pull/3321#discussion_r1275191605


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java:
##########
@@ -173,9 +173,8 @@ public LoanTransaction makeRepayment(final 
LoanTransactionType repaymentTransact
         // TODO: Is it required to validate transaction date with meeting dates
         // if repayments is synced with meeting?
         /*
-         * if(loan.isSyncDisbursementWithMeeting()){ // validate actual 
disbursement date against meeting date
-         * CalendarInstance calendarInstance = 
this.calendarInstanceRepository.findCalendarInstaneByLoanId
-         * (loan.getId(), CalendarEntityType.LOANS.getValue()); 
this.loanEventApiJsonValidator
+         * if(loan.isSyncDisbursementWithMeeting()){ // validate actual 
disbursement date against meeting date CalendarInstance calendarInstance =

Review Comment:
   Spotless



##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/database/DatabaseSpecificSQLGenerator.java:
##########
@@ -36,14 +42,26 @@ public DatabaseSpecificSQLGenerator(DatabaseTypeResolver 
databaseTypeResolver) {
     }
 
     public String escape(String arg) {
-        if (databaseTypeResolver.isMySQL()) {
+        return escape(databaseTypeResolver.databaseType(), arg);
+    }
+
+    public static String escape(DatabaseType dialect, String arg) {

Review Comment:
   I do not agree with your opinion.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/search/SavingsTransactionSearch.java:
##########
@@ -43,7 +43,7 @@ public static class Filters {
     @Data
     public static class RangeFilter<T> {
 
-        private RangeOperator operator;
+        private SqlOperator operator;

Review Comment:
   I like this. Why can't sql operators be exposed to higher levels??? 
(Actually it was exposed before but as simple string or RangeOparator)
   I was thinking on even using graphQL, but this was not complex enough for 
that.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/api/SavingsAccountTransactionsApiResource.java:
##########
@@ -137,6 +141,20 @@ public String searchTransactions(@PathParam("savingsId") 
@Parameter(description
         return toApiJsonSerializer.serialize(transactionsData);
     }
 
+    @POST
+    @Path("query")
+    @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 = List.class))) })
+    public String advancedQuery(@PathParam("savingsId") @Parameter(description 
= "savingsId") final Long savingsId,
+            PagedRequest<AdvancedQueryRequest> queryRequest, @Context final 
UriInfo uriInfo) {
+        final org.springframework.data.domain.Page<JsonObject> result = 
transactionsSearchServiceImpl.queryAdvanced(savingsId,
+                queryRequest);
+        return 
this.toApiJsonSerializer.serializePretty(ApiParameterHelper.prettyPrint(uriInfo.getQueryParameters()),
 result);

Review Comment:
   I do not know where you said this already, but this is a v1 API, and I 
learned that only v2 APIs can go with Object response. Additionally the 
response here is dynamic.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/AbstractLoanRepaymentScheduleTransactionProcessor.java:
##########
@@ -48,8 +48,7 @@
 import 
org.apache.fineract.portfolio.loanaccount.domain.transactionprocessor.impl.InterestPrincipalPenaltyFeesOrderLoanRepaymentScheduleTransactionProcessor;
 
 /**
- * Abstract implementation of {@link 
LoanRepaymentScheduleTransactionProcessor} which is more convenient for concrete
- * implementations to extend.
+ * Abstract implementation of {@link 
LoanRepaymentScheduleTransactionProcessor} which is more convenient for 
concrete implementations to extend.

Review Comment:
   spotless



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchUtil.java:
##########
@@ -0,0 +1,266 @@
+package org.apache.fineract.portfolio.search.service;
+
+import static java.util.Locale.ENGLISH;
+import static 
org.apache.fineract.infrastructure.core.data.ApiParameterError.parameterErrorWithValue;
+import static 
org.apache.fineract.infrastructure.dataqueries.api.DataTableApiConstant.API_FIELD_MANDATORY;
+import static 
org.apache.fineract.portfolio.search.SearchConstants.API_PARAM_COLUMN;
+
+import com.google.common.base.Predicate;
+import com.google.gson.JsonObject;
+import jakarta.validation.constraints.NotNull;
+import java.math.BigDecimal;
+import java.sql.Date;
+import java.sql.Timestamp;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.*;
+import java.util.stream.Collectors;
+import org.apache.commons.lang3.BooleanUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.fineract.infrastructure.core.data.ApiParameterError;
+import org.apache.fineract.infrastructure.core.data.SqlOperator;
+import 
org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
+import 
org.apache.fineract.infrastructure.core.exception.PlatformDataIntegrityException;
+import org.apache.fineract.infrastructure.core.serialization.JsonParserHelper;
+import org.apache.fineract.infrastructure.core.service.database.DatabaseType;
+import org.apache.fineract.infrastructure.core.service.database.JdbcJavaType;
+import 
org.apache.fineract.infrastructure.dataqueries.data.ResultsetColumnHeaderData;
+import org.apache.fineract.infrastructure.security.utils.SQLInjectionValidator;
+import org.apache.fineract.portfolio.search.data.AdvancedQueryData;
+import org.apache.fineract.portfolio.search.data.ColumnFilterData;
+import org.apache.fineract.portfolio.search.data.FilterData;
+import org.springframework.jdbc.support.rowset.SqlRowSet;
+
+public class SearchUtil {

Review Comment:
   What do you mean? A util class with 10 static methods, all search related is 
not complex.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/data/EntityTables.java:
##########
@@ -18,88 +18,114 @@
  */
 package org.apache.fineract.infrastructure.dataqueries.data;
 
+import static 
org.apache.fineract.infrastructure.dataqueries.data.StatusEnum.ACTIVATE;
+import static 
org.apache.fineract.infrastructure.dataqueries.data.StatusEnum.APPROVE;
+import static 
org.apache.fineract.infrastructure.dataqueries.data.StatusEnum.CLOSE;
+import static 
org.apache.fineract.infrastructure.dataqueries.data.StatusEnum.CREATE;
+import static 
org.apache.fineract.infrastructure.dataqueries.data.StatusEnum.DISBURSE;
+import static 
org.apache.fineract.infrastructure.dataqueries.data.StatusEnum.REJECTED;
+import static 
org.apache.fineract.infrastructure.dataqueries.data.StatusEnum.WITHDRAWN;
+import static 
org.apache.fineract.infrastructure.dataqueries.data.StatusEnum.WRITE_OFF;
+
 import com.google.common.collect.ImmutableList;
-import java.util.ArrayList;
-import java.util.HashMap;
+import jakarta.validation.constraints.NotNull;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 public enum EntityTables {
 
-    CLIENT("m_client", ImmutableList.of(StatusEnum.CREATE.getCode(), 
StatusEnum.ACTIVATE.getCode(), StatusEnum.CLOSE.getCode()),
-            "client_id"), LOAN("m_loan",
-                    ImmutableList.of(StatusEnum.CREATE.getCode(), 
StatusEnum.APPROVE.getCode(), StatusEnum.DISBURSE.getCode(),
-                            StatusEnum.WITHDRAWN.getCode(), 
StatusEnum.REJECTED.getCode(), StatusEnum.WRITE_OFF.getCode()),
-                    "loan_id"), GROUP("m_group",
-                            ImmutableList.of(StatusEnum.CREATE.getCode(), 
StatusEnum.ACTIVATE.getCode(), StatusEnum.CLOSE.getCode()),
-                            "group_id"), SAVING("m_savings_account",
-                                    
ImmutableList.of(StatusEnum.CREATE.getCode(), StatusEnum.APPROVE.getCode(),
-                                            StatusEnum.ACTIVATE.getCode(), 
StatusEnum.WITHDRAWN.getCode(), StatusEnum.REJECTED.getCode(),
-                                            StatusEnum.CLOSE.getCode()),
-                                    "savings_account_id"), 
SAVINGS_TRANSACTION("m_savings_account_transaction", ImmutableList.of(),
-                                            "savings_account_transcation_id"), 
OFFICE("m_office", ImmutableList.of(),
-                                                    "office_id"), 
PRODUCT_LOAN("m_product_loan", ImmutableList.of(),
-                                                            
"product_loan_id"), SAVINGS_PRODUCT("m_savings_product", ImmutableList.of(),
-                                                                    
"savings_product_id"), SHARE_PRODUCT("m_share_product",
-                                                                            
ImmutableList.of(), "share_product_id");
-
-    private static final Map<String, EntityTables> lookup = new 
HashMap<String, EntityTables>();
-
-    static {
-        for (EntityTables d : EntityTables.values()) {
-            lookup.put(d.getName(), d);
-        }
-    }
+    CLIENT("m_client", "client_id", "id", CREATE, ACTIVATE, CLOSE), //
+    GROUP("m_group", "group_id", "id", CREATE, ACTIVATE, CLOSE), //
+    CENTER("m_center", "m_group", "center_id", "id"), //
+    OFFICE("m_office", "office_id", "id"), //
+    LOAN_PRODUCT("m_product_loan", "product_loan_id", "id"), //
+    LOAN("m_loan", "loan_id", "id", CREATE, APPROVE, DISBURSE, WITHDRAWN, 
REJECTED, WRITE_OFF), //
+    SAVING_PRODUCT("m_savings_product", "savings_product_id", "id"), //

Review Comment:
   It was SAVING and SAVING_PRODUCT, but Muthu just renamed them with the PR 
which was merged lately. Anyway I'll check how many classes are involved, 
probably not too much if the other PR renamed them already.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to