RamsesCamacho1171 commented on code in PR #4510:
URL: https://github.com/apache/fineract/pull/4510#discussion_r2027696870


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/populator/AbstractWorkbookPopulator.java:
##########
@@ -78,8 +80,7 @@ protected void writeDate(int colIndex, Row row, String value, 
CellStyle dateCell
                 throw new IllegalArgumentException("Unrecognised format of 
date value: " + value);
             }
             LocalDate date1 = LocalDate.parse(value, formatinDB);
-            DateTimeFormatter expectedFormat = new 
DateTimeFormatterBuilder().appendPattern(dateFormat).toFormatter();
-            
row.createCell(colIndex).setCellValue(expectedFormat.format(date1));
+            
row.createCell(colIndex).setCellValue(Date.from(date1.atStartOfDay(ZoneId.systemDefault()).toInstant()));

Review Comment:
   This change was made because it generated the date with an apostrophe at the 
beginning "'24/02/2025" and when trying to fill out the Excel it showed an 
error when evaluating the dates.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/populator/AbstractWorkbookPopulator.java:
##########
@@ -78,8 +80,7 @@ protected void writeDate(int colIndex, Row row, String value, 
CellStyle dateCell
                 throw new IllegalArgumentException("Unrecognised format of 
date value: " + value);
             }
             LocalDate date1 = LocalDate.parse(value, formatinDB);
-            DateTimeFormatter expectedFormat = new 
DateTimeFormatterBuilder().appendPattern(dateFormat).toFormatter();
-            
row.createCell(colIndex).setCellValue(expectedFormat.format(date1));
+            
row.createCell(colIndex).setCellValue(Date.from(date1.atStartOfDay(ZoneId.systemDefault()).toInstant()));

Review Comment:
   Hey @adamsaghy We switched to the second version because the first one 
writes the date as text, which makes Excel not recognize it as a real date and 
causes validation to fail. The second option stores the date as a Date object, 
allowing Excel to properly apply date validations and related functions.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanApplicationValidator.java:
##########
@@ -338,13 +338,6 @@ private void validateForCreate(final JsonElement element) {
                 
baseDataValidator.reset().parameter(LoanApiConstants.externalIdParameterName).value(externalId).ignoreIfNull()
                         .notExceedingLengthOf(100);
             }
-
-            if 
(this.fromApiJsonHelper.parameterExists(LoanApiConstants.fundIdParameterName, 
element)) {

Review Comment:
   It was removed because that field was not mandatory to generate the loan.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanApplicationValidator.java:
##########
@@ -1034,6 +1019,13 @@ public void validateForModify(final JsonCommand command, 
final Loan loan) {
                 
baseDataValidator.reset().parameter(LoanApiConstants.interestTypeParameterName).value(interestType).notNull()
                         .inMinMaxRange(0, 1);
             }
+            if 
(this.fromApiJsonHelper.parameterExists(LoanApiConstants.repaymentFrequencyTypeParameterName,
 element)) {

Review Comment:
   You're right, I already removed one.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanApplicationValidator.java:
##########
@@ -1034,6 +1019,13 @@ public void validateForModify(final JsonCommand command, 
final Loan loan) {
                 
baseDataValidator.reset().parameter(LoanApiConstants.interestTypeParameterName).value(interestType).notNull()
                         .inMinMaxRange(0, 1);
             }
+            if 
(this.fromApiJsonHelper.parameterExists(LoanApiConstants.repaymentFrequencyTypeParameterName,
 element)) {

Review Comment:
   I have already removed the duplicate validation and moved it to the position 
it had originally.
   



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanApplicationValidator.java:
##########
@@ -934,13 +926,6 @@ public void validateForModify(final JsonCommand command, 
final Loan loan) {
                         .notExceedingLengthOf(100);
             }
 
-            if 
(this.fromApiJsonHelper.parameterExists(LoanApiConstants.fundIdParameterName, 
element)) {

Review Comment:
   This validation was returned
   



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/importhandler/ImportHandlerUtils.java:
##########
@@ -53,8 +53,14 @@ public static Integer getNumberOfRows(Sheet sheet, int 
primaryColumn) {
         Integer noOfEntries = 0;
         // getLastRowNum and getPhysicalNumberOfRows showing false values
         // sometimes
-        while (sheet.getRow(noOfEntries + 1) != null && 
sheet.getRow(noOfEntries + 1).getCell(primaryColumn) != null) {
-            noOfEntries++;
+        while (sheet.getRow(noOfEntries + 1) != null) {

Review Comment:
   In this case I had to add the {} in the if because the tests marked it as an 
error
   



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanApplicationValidator.java:
##########
@@ -934,13 +926,6 @@ public void validateForModify(final JsonCommand command, 
final Loan loan) {
                         .notExceedingLengthOf(100);
             }
 
-            if 
(this.fromApiJsonHelper.parameterExists(LoanApiConstants.fundIdParameterName, 
element)) {

Review Comment:
   It was removed because that field was not mandatory to generate the loan.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/importhandler/loan/LoanImportHandler.java:
##########
@@ -124,17 +124,20 @@ private DisbursementData readDisbursalData(final Row row, 
final String locale, f
         if (ImportHandlerUtils.readAsLong(LoanConstants.LINK_ACCOUNT_ID, row) 
!= null) {
             linkAccountId = 
Objects.requireNonNull(ImportHandlerUtils.readAsLong(LoanConstants.LINK_ACCOUNT_ID,
 row)).toString();
         }
-
         if (disbursedDate != null) {
+

Review Comment:
   This is already eliminated



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