galovics commented on code in PR #4554:
URL: https://github.com/apache/fineract/pull/4554#discussion_r2040597498
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/importhandler/ImportHandlerUtils.java:
##########
@@ -53,7 +53,15 @@ 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) {
+ while (true) {
+ Row row = sheet.getRow(noOfEntries + 1);
+ if (row == null) {
+ break;
+ }
+ Cell cell = row.getCell(primaryColumn);
+ if (cell == null || cell.getCellType() == CellType.BLANK) {
+ break;
+ }
Review Comment:
Well, let's not do an infinite loop, it's always fishy.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/populator/recurringdeposit/RecurringDepositTransactionWorkbookPopulator.java:
##########
@@ -204,7 +205,12 @@ private void populateSavingsTable(Sheet
savingsTransactionSheet, String dateForm
row = savingsTransactionSheet.createRow(rowIndex++);
writeString(TransactionConstants.LOOKUP_CLIENT_NAME_COL, row,
savingsAccount.getClientName() + "(" +
savingsAccount.getClientId() + ")");
- writeString(TransactionConstants.LOOKUP_ACCOUNT_NO_COL, row,
savingsAccount.getAccountNo());
+ try {
+ BigDecimal accountNoAsBigDecimal = new
BigDecimal(savingsAccount.getAccountNo());
Review Comment:
Why would you need a BigDecimal for the accountNo? Can you pls explain?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/populator/AbstractWorkbookPopulator.java:
##########
@@ -116,6 +116,7 @@ protected void setClientAndGroupDateLookupTable(Sheet
sheet, List<ClientData> cl
dateCellStyle.setDataFormat(df);
int rowIndex = 0;
DateTimeFormatter outputFormat = new
DateTimeFormatterBuilder().appendPattern(dateFormat).toFormatter();
+ DateTimeFormatter formatter =
DateTimeFormatter.ofPattern("dd/MM/yyyy");
Review Comment:
I'm not that familiar with the import process but the pattern shouldn't be
hardcoded, that's for sure.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/populator/chartofaccounts/ChartOfAccountsWorkbook.java:
##########
@@ -132,24 +125,18 @@ private void setRules(Sheet chartOfAccountsSheet) {
DataValidationConstraint parentConstraint = validationHelper
.createFormulaListConstraint("INDIRECT(CONCATENATE(\"AccountName_\",$A1))");
DataValidationConstraint tagConstraint =
validationHelper.createFormulaListConstraint("INDIRECT(CONCATENATE(\"Tags_\",$A1))");
- DataValidationConstraint officeNameConstraint =
validationHelper.createFormulaListConstraint("Office");
- DataValidationConstraint currencyCodeConstraint =
validationHelper.createExplicitListConstraint(getCurrency());
Review Comment:
Why were these removed?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/populator/chartofaccounts/ChartOfAccountsWorkbook.java:
##########
@@ -132,24 +125,18 @@ private void setRules(Sheet chartOfAccountsSheet) {
DataValidationConstraint parentConstraint = validationHelper
.createFormulaListConstraint("INDIRECT(CONCATENATE(\"AccountName_\",$A1))");
DataValidationConstraint tagConstraint =
validationHelper.createFormulaListConstraint("INDIRECT(CONCATENATE(\"Tags_\",$A1))");
- DataValidationConstraint officeNameConstraint =
validationHelper.createFormulaListConstraint("Office");
- DataValidationConstraint currencyCodeConstraint =
validationHelper.createExplicitListConstraint(getCurrency());
DataValidation accountTypeValidation =
validationHelper.createValidation(accountTypeConstraint, accountTypeRange);
DataValidation accountUsageValidation =
validationHelper.createValidation(accountUsageConstraint, accountUsageRange);
DataValidation manualEntriesValidation =
validationHelper.createValidation(booleanConstraint, manualEntriesAllowedRange);
DataValidation parentValidation =
validationHelper.createValidation(parentConstraint, parentRange);
DataValidation tagValidation =
validationHelper.createValidation(tagConstraint, tagRange);
- DataValidation officeNameValidation =
validationHelper.createValidation(officeNameConstraint, officeNameRange);
- DataValidation currencyCodeValidation =
validationHelper.createValidation(currencyCodeConstraint, currencyCodeRange);
Review Comment:
Why were these removed?
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java:
##########
@@ -212,9 +212,8 @@ public Page<SavingsAccountData> retrieveAll(final
SearchParameters searchParamet
arrayPos = arrayPos + 1;
}
if (searchParameters.getOfficeId() != null) {
- sqlBuilder.append("and c.office_id =?");
- objectArray[arrayPos] = searchParameters.getOfficeId();
- arrayPos = arrayPos + 1;
+ sqlBuilder.append(" and c.office_id = ?");
+ objectArray[arrayPos++] = searchParameters.getOfficeId();
Review Comment:
I don't get what this change does exactly other than making the code less
readable.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/populator/chartofaccounts/ChartOfAccountsWorkbook.java:
##########
@@ -112,13 +112,6 @@ private void setRules(Sheet chartOfAccountsSheet) {
ChartOfAcountsConstants.PARENT_COL,
ChartOfAcountsConstants.PARENT_COL);
CellRangeAddressList tagRange = new CellRangeAddressList(1,
SpreadsheetVersion.EXCEL97.getLastRowIndex(),
ChartOfAcountsConstants.TAG_COL,
ChartOfAcountsConstants.TAG_COL);
- CellRangeAddressList officeNameRange = new CellRangeAddressList(1,
SpreadsheetVersion.EXCEL97.getLastRowIndex(),
- ChartOfAcountsConstants.OFFICE_COL,
ChartOfAcountsConstants.OFFICE_COL); // validation for opening bal
-
// office column
- CellRangeAddressList currencyCodeRange = new CellRangeAddressList(1,
SpreadsheetVersion.EXCEL97.getLastRowIndex(),
- ChartOfAcountsConstants.CURRENCY_CODE,
ChartOfAcountsConstants.CURRENCY_CODE);// validation for currency
-
// code for opening
-
// balance
Review Comment:
Why were these removed?
--
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]
