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]