galovics commented on code in PR #2314:
URL: https://github.com/apache/fineract/pull/2314#discussion_r873622402
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/GenericDataServiceImpl.java:
##########
@@ -262,4 +261,14 @@ private SqlRowSet getDatatableMetaData(final String
datatable) {
throw new DatatableNotFoundException(datatable);
}
}
+
+ private String formatDateTimeValue(String dateTimeValue) {
Review Comment:
Even though this solves the problem for now, this logic shouldn't be here.
On one note, we should definitely not decide based on the string
representation of the datetime value, neither based on the length and whether
it contains a `T`.
I looked at it a tiny bit deeper and we're completely missing the
standardization on the datatable reading side.
If you look at
`ReadWriteNonCoreDataServiceImpl#fillDatatableResultSetDataRows`, we're always
reading the column value as string even though it's not necessarily a string.
In case of Date values, we could simply read them as Date type and then we
don't have to deal with the non-sense DateTime parsing here.
It'd simplify a lot of things and set up proper boundaries on reading and
processing the data.
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/common/system/DatatableIntegrationTest.java:
##########
@@ -0,0 +1,83 @@
+/**
+ * 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.common.system;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+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 org.apache.fineract.integrationtests.common.ClientHelper;
+import org.apache.fineract.integrationtests.common.Utils;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class DatatableIntegrationTest {
+
+ private RequestSpecification requestSpec;
+ private ResponseSpecification responseSpec;
+ private DatatableHelper datatableHelper;
+
+ private static final String CLIENT_APP_TABLE_NAME = "m_client";
+
+ public static final String DATE_TIME_FORMAT = "yyyy-MM-dd";
+
+ @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.datatableHelper = new DatatableHelper(this.requestSpec,
this.responseSpec);
+ }
+
+ @Test
+ public void validateCreateReadDeleteDatatableCheck() {
+ // creating datatable for client entity
+ String datatableName =
this.datatableHelper.createDatatable(CLIENT_APP_TABLE_NAME, false);
+ DatatableHelper.verifyDatatableCreatedOnServer(this.requestSpec,
this.responseSpec, datatableName);
+
+ // creating client with datatables
+ final Integer clientID =
ClientHelper.createClientAsPerson(requestSpec, responseSpec);
+
+ // creating new entity datatable check
+ final boolean genericResultSet = true;
+ Integer datatableResourceID =
this.datatableHelper.createDatatableEntry(CLIENT_APP_TABLE_NAME, datatableName,
clientID,
+ genericResultSet, DATE_TIME_FORMAT);
+ assertNotNull(datatableResourceID, "ERROR IN CREATING THE ENTITY
DATATABLE RECORD");
+
+ // Read the Datatable entry generated with genericResultSet in true
(default)
+ this.datatableHelper.readDatatableEntry(datatableName, clientID,
genericResultSet);
Review Comment:
I understand the test case. My point is that the result of the
`datatableHelper.readDatatableEntry()` method call is never checked.
For example we should check the amount of rows in the result along with
their value.
--
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]
