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]