You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2022/07/26 08:05:18 UTC

[GitHub] [fineract] adamsaghy commented on a diff in pull request #2446: Datatables datetime fields management

adamsaghy commented on code in PR #2446:
URL: https://github.com/apache/fineract/pull/2446#discussion_r929309238


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java:
##########
@@ -1471,18 +1472,43 @@ private String mapToActualAppTable(final String appTable) {
         return appTable;
     }
 
-    private List<ResultsetRowData> fillDatatableResultSetDataRows(final String sql) {
+    private List<ResultsetRowData> fillDatatableResultSetDataRows(final String sql, final List<ResultsetColumnHeaderData> columnHeaders) {
         final List<ResultsetRowData> resultsetDataRows = new ArrayList<>();
+        final GenericResultsetData genericResultsetData = new GenericResultsetData(columnHeaders, null);
 
         final SqlRowSet rowSet = jdbcTemplate.queryForRowSet(sql); // NOSONAR
         final SqlRowSetMetaData rsmd = rowSet.getMetaData();
 
         while (rowSet.next()) {
-            final List<String> columnValues = new ArrayList<>();
+            final List<Object> columnValues = new ArrayList<>();
+
             for (int i = 0; i < rsmd.getColumnCount(); i++) {
                 final String columnName = rsmd.getColumnName(i + 1);
-                final String columnValue = rowSet.getString(columnName);
-                columnValues.add(columnValue);
+                if (columnHeaders.isEmpty()) {
+                    columnValues.add(rowSet.getString(columnName));
+                } else {
+                    final String colType = genericResultsetData.getColTypeOfColumnNamed(columnName);
+                    if ("DECIMAL".equalsIgnoreCase(colType)) {
+                        columnValues.add(rowSet.getBigDecimal(columnName));
+                    } else if ("DATE".equalsIgnoreCase(colType)) {
+                        columnValues.add(rowSet.getDate(columnName));
+                    } else if ("timestamp without time zone".equalsIgnoreCase(colType) // PostgreSQL

Review Comment:
   This is not needed!
   Please use this:
   
   else if ("DATE".equalsIgnoreCase(colType) || ("TIMESTAMP WITHOUT TIME ZONE".equalsIgnoreCase(colType) // PostgreSQL
                           || "DATETIME".equalsIgnoreCase(colType) || "TIMESTAMP".equalsIgnoreCase(colType))) {
                       columnValues.add(rowSet.getObject(columnName));
                   }



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/GenericDataServiceImpl.java:
##########
@@ -160,23 +162,28 @@ public String generateJsonFromGenericResultsetData(final GenericResultsetData gr
                 if (currColType == null && colType.equalsIgnoreCase("DATE")) {
                     currColType = "DATE";
                 }
+                if (currColType == null && colType.equalsIgnoreCase("DATETIME")) {
+                    currColType = "DATETIME";
+                }
                 currVal = row.get(j);
                 if (currVal != null && currColType != null) {
                     if (currColType.equals("DECIMAL") || currColType.equals("INTEGER")) {
                         writer.append(currVal);
                     } else {
                         if (currColType.equals("DATE")) {
-                            final LocalDate localDate = LocalDate.parse(currVal);
-                            writer.append(
-                                    "[" + localDate.getYear() + ", " + localDate.getMonthValue() + ", " + localDate.getDayOfMonth() + "]");
+                            final LocalDate localDate = LocalDate.parse(currVal.toString());
+                            writer.append(format("[%d,%d,%d]", localDate.getYear(), localDate.getMonthValue(), localDate.getDayOfMonth()));
                         } else if (currColType.equals("DATETIME")) {
-                            final LocalDateTime localDateTime = LocalDateTime.parse(formatDateTimeValue(currVal),
-                                    DateUtils.DEFAULT_DATETIME_FORMATER);
-                            writer.append("[" + localDateTime.getYear() + ", " + localDateTime.getMonthValue() + ", "
-                                    + localDateTime.getDayOfMonth() + ", " + localDateTime.getHour() + ", " + localDateTime.getMinute()
-                                    + ", " + localDateTime.getSecond() + ", " + localDateTime.get(ChronoField.MILLI_OF_SECOND) + "]");
+                            LocalDateTime ldt = null;

Review Comment:
   This is not needed!
   Please use this:
                final LocalDateTime localDateTime = currVal instanceof Timestamp ?
                                       ((Timestamp) currVal).toLocalDateTime() :
                                       (LocalDateTime) currVal;



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/GenericDataServiceImpl.java:
##########
@@ -160,23 +162,28 @@ public String generateJsonFromGenericResultsetData(final GenericResultsetData gr
                 if (currColType == null && colType.equalsIgnoreCase("DATE")) {
                     currColType = "DATE";
                 }
+                if (currColType == null && colType.equalsIgnoreCase("DATETIME")) {
+                    currColType = "DATETIME";
+                }
                 currVal = row.get(j);
                 if (currVal != null && currColType != null) {
                     if (currColType.equals("DECIMAL") || currColType.equals("INTEGER")) {
                         writer.append(currVal);
                     } else {
                         if (currColType.equals("DATE")) {
-                            final LocalDate localDate = LocalDate.parse(currVal);
-                            writer.append(
-                                    "[" + localDate.getYear() + ", " + localDate.getMonthValue() + ", " + localDate.getDayOfMonth() + "]");
+                            final LocalDate localDate = LocalDate.parse(currVal.toString());

Review Comment:
   Please rather use this:
   final LocalDate localDate = new java.sql.Date(((Date)currVal).getTime()).toLocalDate();



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/GenericDataServiceImpl.java:
##########
@@ -160,23 +161,32 @@ public String generateJsonFromGenericResultsetData(final GenericResultsetData gr
                 if (currColType == null && colType.equalsIgnoreCase("DATE")) {
                     currColType = "DATE";
                 }
+                if (currColType == null && colType.equalsIgnoreCase("DATETIME")) {
+                    currColType = "DATETIME";
+                }
                 currVal = row.get(j);
                 if (currVal != null && currColType != null) {
                     if (currColType.equals("DECIMAL") || currColType.equals("INTEGER")) {
                         writer.append(currVal);
                     } else {
                         if (currColType.equals("DATE")) {
-                            final LocalDate localDate = LocalDate.parse(currVal);
+                            final LocalDate localDate = LocalDate.parse(currVal.toString());
                             writer.append(
                                     "[" + localDate.getYear() + ", " + localDate.getMonthValue() + ", " + localDate.getDayOfMonth() + "]");
                         } else if (currColType.equals("DATETIME")) {
-                            final LocalDateTime localDateTime = LocalDateTime.parse(formatDateTimeValue(currVal),
-                                    DateUtils.DEFAULT_DATETIME_FORMATER);
-                            writer.append("[" + localDateTime.getYear() + ", " + localDateTime.getMonthValue() + ", "
-                                    + localDateTime.getDayOfMonth() + ", " + localDateTime.getHour() + ", " + localDateTime.getMinute()
-                                    + ", " + localDateTime.getSecond() + ", " + localDateTime.get(ChronoField.MILLI_OF_SECOND) + "]");
+                            Calendar dateVal = Calendar.getInstance();
+                            if (currVal instanceof Date) {
+                                dateVal.setTime((Date) currVal);
+                            } else if (currVal instanceof LocalDateTime) {
+                                dateVal.setTime(
+                                        Date.from(((LocalDateTime) currVal).atZone(DateUtils.getDateTimeZoneOfTenant()).toInstant()));

Review Comment:
   I am a little bit lost, but @josehernandezfintecheandomx We should avoid the following types: Calendar, Date, converting back and forth, etc...
   
   We should only support LocalDate and LocalDateTime... 



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java:
##########
@@ -102,6 +104,11 @@ public static LocalDate parseLocalDate(final String stringDate, final String pat
         return parseLocalDate(stringDate, pattern, getDateTimeZoneOfTenant());
     }
 
+    public static LocalDateTime convertDateToLocalDateTime(final Date date) {
+        Instant instant = Instant.ofEpochMilli(date.getTime());
+        return LocalDateTime.ofInstant(instant, getDateTimeZoneOfTenant());
+    }
+

Review Comment:
   This is not needed!



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