adamsaghy commented on code in PR #3321:
URL: https://github.com/apache/fineract/pull/3321#discussion_r1280484662
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java:
##########
@@ -86,4 +89,106 @@ public static LocalDate getBusinessLocalDate() {
public static long getDifferenceInDays(final LocalDate localDateBefore,
final LocalDate localDateAfter) {
return DAYS.between(localDateBefore, localDateAfter);
}
+
+ public static int compareToBusinessDate(LocalDate date) {
+ return compare(date, getBusinessLocalDate());
+ }
+
+ public static boolean isEqualBusinessDate(LocalDate date) {
+ return isEqual(date, getBusinessLocalDate());
+ }
+
+ public static boolean isBeforeBusinessDate(LocalDate date) {
Review Comment:
unused
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java:
##########
@@ -86,4 +89,106 @@ public static LocalDate getBusinessLocalDate() {
public static long getDifferenceInDays(final LocalDate localDateBefore,
final LocalDate localDateAfter) {
return DAYS.between(localDateBefore, localDateAfter);
}
+
+ public static int compareToBusinessDate(LocalDate date) {
+ return compare(date, getBusinessLocalDate());
+ }
+
+ public static boolean isEqualBusinessDate(LocalDate date) {
+ return isEqual(date, getBusinessLocalDate());
+ }
+
+ public static boolean isBeforeBusinessDate(LocalDate date) {
+ return isBefore(date, getBusinessLocalDate());
+ }
+
+ public static boolean isAfterBusinessDate(LocalDate date) {
Review Comment:
unused
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java:
##########
@@ -86,4 +89,106 @@ public static LocalDate getBusinessLocalDate() {
public static long getDifferenceInDays(final LocalDate localDateBefore,
final LocalDate localDateAfter) {
return DAYS.between(localDateBefore, localDateAfter);
}
+
+ public static int compareToBusinessDate(LocalDate date) {
+ return compare(date, getBusinessLocalDate());
+ }
+
+ public static boolean isEqualBusinessDate(LocalDate date) {
+ return isEqual(date, getBusinessLocalDate());
+ }
+
+ public static boolean isBeforeBusinessDate(LocalDate date) {
+ return isBefore(date, getBusinessLocalDate());
+ }
+
+ public static boolean isAfterBusinessDate(LocalDate date) {
+ return isAfter(date, getBusinessLocalDate());
+ }
+
+ public static int compare(LocalDate first, LocalDate second) {
+ return first == null ? -1 : (second == null ? 1 :
first.compareTo(second));
+ }
+
+ public static boolean isEqual(LocalDate first, LocalDate second) {
+ return first == null ? second == null : (second != null &&
first.isEqual(second));
+ }
+
+ public static boolean isBefore(LocalDate first, LocalDate second) {
Review Comment:
unused
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/database/JdbcJavaType.java:
##########
@@ -30,14 +30,14 @@ public enum JdbcJavaType {
@Override
public Object toJdbcValueImpl(@NotNull DatabaseType dialect, Object
value) {
- return Boolean.TRUE.equals(value) ? 1 : 0;
+ return value == null ? null : (Boolean.TRUE.equals(value) ? 1 : 0);
Review Comment:
Previously the default was false. Are you sure it wise to change?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java:
##########
@@ -1890,7 +1997,7 @@ public boolean
isDatatableAttachedToEntityDatatableCheck(final String datatableN
+ " JOIN m_entity_datatable_check edc ON
edc.x_registered_table_name = xrt.registered_table_name"
+ " WHERE edc.x_registered_table_name = '" + datatableName +
"'";
final Long count = this.jdbcTemplate.queryForObject(sql, Long.class);
- return count > 0;
+ return count != null && count > 0;
Review Comment:
count function can never return null. using `int` is more than fine and no
need for null check
##########
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:
The rule is
for V2 it is mandatory to be object response
for V1 new API it is okay to be object response
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java:
##########
@@ -86,4 +89,106 @@ public static LocalDate getBusinessLocalDate() {
public static long getDifferenceInDays(final LocalDate localDateBefore,
final LocalDate localDateAfter) {
return DAYS.between(localDateBefore, localDateAfter);
}
+
+ public static int compareToBusinessDate(LocalDate date) {
+ return compare(date, getBusinessLocalDate());
+ }
+
+ public static boolean isEqualBusinessDate(LocalDate date) {
Review Comment:
unused
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/api/SavingsAccountTransactionsApiResourceSwagger.java:
##########
@@ -182,8 +182,8 @@ private GetSavingsAccountChargesPaidByData() {}
}
@Schema(example = "2")
- public Long totalFilteredRecords;
Review Comment:
This is breaking change.
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java:
##########
@@ -86,4 +89,106 @@ public static LocalDate getBusinessLocalDate() {
public static long getDifferenceInDays(final LocalDate localDateBefore,
final LocalDate localDateAfter) {
return DAYS.between(localDateBefore, localDateAfter);
}
+
+ public static int compareToBusinessDate(LocalDate date) {
+ return compare(date, getBusinessLocalDate());
+ }
+
+ public static boolean isEqualBusinessDate(LocalDate date) {
+ return isEqual(date, getBusinessLocalDate());
+ }
+
+ public static boolean isBeforeBusinessDate(LocalDate date) {
+ return isBefore(date, getBusinessLocalDate());
+ }
+
+ public static boolean isAfterBusinessDate(LocalDate date) {
+ return isAfter(date, getBusinessLocalDate());
+ }
+
+ public static int compare(LocalDate first, LocalDate second) {
+ return first == null ? -1 : (second == null ? 1 :
first.compareTo(second));
+ }
+
+ public static boolean isEqual(LocalDate first, LocalDate second) {
+ return first == null ? second == null : (second != null &&
first.isEqual(second));
+ }
+
+ public static boolean isBefore(LocalDate first, LocalDate second) {
+ return first == null ? second != null : (second != null &&
first.isBefore(second));
+ }
+
+ public static boolean isAfter(LocalDate first, LocalDate second) {
Review Comment:
unused
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java:
##########
@@ -86,4 +89,106 @@ public static LocalDate getBusinessLocalDate() {
public static long getDifferenceInDays(final LocalDate localDateBefore,
final LocalDate localDateAfter) {
return DAYS.between(localDateBefore, localDateAfter);
}
+
+ public static int compareToBusinessDate(LocalDate date) {
+ return compare(date, getBusinessLocalDate());
+ }
+
+ public static boolean isEqualBusinessDate(LocalDate date) {
+ return isEqual(date, getBusinessLocalDate());
+ }
+
+ public static boolean isBeforeBusinessDate(LocalDate date) {
+ return isBefore(date, getBusinessLocalDate());
+ }
+
+ public static boolean isAfterBusinessDate(LocalDate date) {
+ return isAfter(date, getBusinessLocalDate());
+ }
+
+ public static int compare(LocalDate first, LocalDate second) {
Review Comment:
unused, also it is wrong: if we are comparing two null LocalDate it will
says the first is earlier which is not true.
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/database/JdbcJavaType.java:
##########
@@ -30,14 +30,14 @@ public enum JdbcJavaType {
@Override
public Object toJdbcValueImpl(@NotNull DatabaseType dialect, Object
value) {
- return Boolean.TRUE.equals(value) ? 1 : 0;
+ return value == null ? null : (Boolean.TRUE.equals(value) ? 1 : 0);
}
},
BOOLEAN(JavaType.BOOLEAN, new DialectType(JDBCType.BIT), new
DialectType(JDBCType.BOOLEAN, null, "BOOL")) { //
@Override
public Object toJdbcValueImpl(@NotNull DatabaseType dialect, Object
value) {
- return dialect.isMySql() ? (Boolean.TRUE.equals(value) ? 1 : 0) :
super.toJdbcValueImpl(dialect, value);
+ return (value != null && dialect.isMySql()) ?
(Boolean.TRUE.equals(value) ? 1 : 0) : super.toJdbcValueImpl(dialect, value);
Review Comment:
same here
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java:
##########
@@ -86,4 +89,106 @@ public static LocalDate getBusinessLocalDate() {
public static long getDifferenceInDays(final LocalDate localDateBefore,
final LocalDate localDateAfter) {
return DAYS.between(localDateBefore, localDateAfter);
}
+
+ public static int compareToBusinessDate(LocalDate date) {
Review Comment:
unused
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java:
##########
@@ -86,4 +89,106 @@ public static LocalDate getBusinessLocalDate() {
public static long getDifferenceInDays(final LocalDate localDateBefore,
final LocalDate localDateAfter) {
return DAYS.between(localDateBefore, localDateAfter);
}
+
+ public static int compareToBusinessDate(LocalDate date) {
+ return compare(date, getBusinessLocalDate());
+ }
+
+ public static boolean isEqualBusinessDate(LocalDate date) {
+ return isEqual(date, getBusinessLocalDate());
+ }
+
+ public static boolean isBeforeBusinessDate(LocalDate date) {
+ return isBefore(date, getBusinessLocalDate());
+ }
+
+ public static boolean isAfterBusinessDate(LocalDate date) {
+ return isAfter(date, getBusinessLocalDate());
+ }
+
+ public static int compare(LocalDate first, LocalDate second) {
+ return first == null ? -1 : (second == null ? 1 :
first.compareTo(second));
+ }
+
+ public static boolean isEqual(LocalDate first, LocalDate second) {
Review Comment:
unused
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/DatatablesApiResource.java:
##########
@@ -225,6 +228,18 @@ public String queryValues(@PathParam("datatable")
@Parameter(description = "data
return
this.toApiJsonSerializer.serializePretty(ApiParameterHelper.prettyPrint(uriInfo.getQueryParameters()),
result);
}
+ @POST
+ @Path("{datatable}/query")
+ @Produces({ MediaType.APPLICATION_JSON })
+ @Operation(summary = "Query Data Table values", description = "Query
values from a registered data table.")
+ @ApiResponses({
+ @ApiResponse(responseCode = "200", description = "OK", content =
@Content(schema = @Schema(implementation = List.class))) })
+ public String advancedQuery(@PathParam("datatable") @Parameter(description
= "datatable") final String datatable,
+ PagedRequest<AdvancedQueryData> queryRequest, @Context final
UriInfo uriInfo) {
+ final Page<JsonObject> result =
this.readWriteNonCoreDataService.queryDataTableAdvanced(datatable,
queryRequest);
+ return
this.toApiJsonSerializer.serializePretty(ApiParameterHelper.prettyPrint(uriInfo.getQueryParameters()),
result);
Review Comment:
The rule is
- for V2 it is mandatory to be object response
- for V1 **new** API it is okay to be object response
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchUtil.java:
##########
@@ -116,47 +116,75 @@ public static void extractJsonResult(@NotNull SqlRowSet
rowSet, @NotNull List<St
}
@NotNull
- public static List<String> validateToJdbcColumns(List<String> columns,
Map<String, ResultsetColumnHeaderData> columnHeaders,
- List<ApiParameterError> errors, boolean allowEmpty) {
- List<String> result = new ArrayList<>();
+ public static List<String> validateToJdbcColumnNames(List<String> columns,
Map<String, ResultsetColumnHeaderData> headersByName,
+ boolean allowEmpty) {
+ List<ResultsetColumnHeaderData> columnHeaders =
validateToJdbcColumns(columns, headersByName, allowEmpty);
+ return columnHeaders.stream().map(e -> e == null ? null :
e.getColumnName()).toList();
Review Comment:
in what situation can the `columnHeaders` element be null?
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/datatable/DatatableIntegrationTest.java:
##########
@@ -499,37 +499,40 @@ public void validateInsertNullValues() {
assertNotNull(items);
assertEquals(2, ((List) items.get("data")).size());
- assertEquals("id", ((Map) ((List)
items.get("columnHeaders")).get(0)).get("columnName"));
- assertEquals(1, ((List) ((Map) ((List)
items.get("data")).get(0)).get("row")).get(0));
- assertEquals("loan_id", ((Map) ((List)
items.get("columnHeaders")).get(1)).get("columnName"));
- assertEquals(loanID, ((List) ((Map) ((List)
items.get("data")).get(0)).get("row")).get(1));
- assertEquals("itsABoolean", ((Map) ((List)
items.get("columnHeaders")).get(2)).get("columnName"));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(0)).get("row")).get(2));
- assertEquals("itsADate", ((Map) ((List)
items.get("columnHeaders")).get(3)).get("columnName"));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(0)).get("row")).get(3));
- assertEquals("itsADatetime", ((Map) ((List)
items.get("columnHeaders")).get(4)).get("columnName"));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(0)).get("row")).get(4));
- assertEquals("itsADecimal", ((Map) ((List)
items.get("columnHeaders")).get(5)).get("columnName"));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(0)).get("row")).get(5));
- assertEquals("TST_TST_TST_cd_itsADropdown", ((Map) ((List)
items.get("columnHeaders")).get(6)).get("columnName"));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(0)).get("row")).get(6));
- assertEquals("itsANumber", ((Map) ((List)
items.get("columnHeaders")).get(7)).get("columnName"));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(0)).get("row")).get(7));
- assertEquals("itsAString", ((Map) ((List)
items.get("columnHeaders")).get(8)).get("columnName"));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(0)).get("row")).get(8));
- assertEquals("itsAText", ((Map) ((List)
items.get("columnHeaders")).get(9)).get("columnName"));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(0)).get("row")).get(9));
-
- assertEquals(2, ((List) ((Map) ((List)
items.get("data")).get(1)).get("row")).get(0));
- assertEquals(loanID, ((List) ((Map) ((List)
items.get("data")).get(1)).get("row")).get(1));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(1)).get("row")).get(2));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(1)).get("row")).get(3));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(1)).get("row")).get(4));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(1)).get("row")).get(5));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(1)).get("row")).get(6));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(1)).get("row")).get(7));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(1)).get("row")).get(8));
- assertNull(((List) ((Map) ((List)
items.get("data")).get(1)).get("row")).get(9));
+ List headers = (List) items.get("columnHeaders");
+ List values0 = (List) ((Map) ((List)
items.get("data")).get(0)).get("row");
Review Comment:
Can you please rename this to `values`?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java:
##########
@@ -1940,8 +2047,8 @@ private String validateDatatableRegistered(String
datatable) {
private boolean isRegisteredDataTable(final String datatable) {
final String sql = "SELECT COUNT(application_table_name) FROM " +
TABLE_REGISTERED_TABLE + " WHERE registered_table_name = ?";
- final int count = jdbcTemplate.queryForObject(sql, Integer.class,
datatable);
- return count > 0;
+ final Integer count = jdbcTemplate.queryForObject(sql, Integer.class,
datatable);
Review Comment:
count function can never return null. using `int` is more than fine and no
need for null check
--
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]
