ptuomola commented on a change in pull request #1985:
URL: https://github.com/apache/fineract/pull/1985#discussion_r774480901
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/infrastructure/accountnumberformat/data/AccountNumberFormatDataValidator.java
##########
@@ -84,6 +86,19 @@ public void validateForCreate(final String json) {
DataValidatorBuilder dataValidatorForValidatingPrefixType =
baseDataValidator.reset()
.parameter(AccountNumberFormatConstants.prefixTypeParamName).value(prefixType).notNull().integerGreaterThanZero();
+ if (prefixType == 401) {
Review comment:
What's 401? Instead of using a "magic number" can we use the correct
constant for this?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/infrastructure/accountnumberformat/domain/AccountNumberFormat.java
##########
@@ -37,21 +37,40 @@
@Column(name = AccountNumberFormatConstants.PREFIX_TYPE_ENUM_COLUMN_NAME,
nullable = true)
private Integer prefixEnum;
+ @Column(name = AccountNumberFormatConstants.DIGITS_COLUMN_NAME, nullable =
true)
+ private Integer digits;
+
+ @Column(name = AccountNumberFormatConstants.PREFIX_CHARACTER_COLUMN_NAME,
nullable = true)
+ private String prefixCharacter;
+
protected AccountNumberFormat() {
//
}
- public AccountNumberFormat(EntityAccountType entityAccountType,
AccountNumberPrefixType prefixType) {
+ public AccountNumberFormat(EntityAccountType entityAccountType,
AccountNumberPrefixType prefixType, Integer digits,
+ String prefixCharacter) {
this.accountTypeEnum = entityAccountType.getValue();
if (prefixType != null) {
this.prefixEnum = prefixType.getValue();
}
+ if (prefixType.getValue() == 401) {
Review comment:
Same as above
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/infrastructure/accountnumberformat/service/AccountNumberFormatWritePlatformServiceJpaRepositoryImpl.java
##########
@@ -68,7 +68,15 @@ public CommandProcessingResult
createAccountNumberFormat(JsonCommand command) {
accountNumberPrefixType =
AccountNumberPrefixType.fromInt(prefixTypeId);
}
- AccountNumberFormat accountNumberFormat = new
AccountNumberFormat(entityAccountType, accountNumberPrefixType);
+ Integer digits = 0;
+ String prefixCharacter = null;
+ if (prefixTypeId == 401) {
Review comment:
As above
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/domain/AccountNumberGenerator.java
##########
@@ -134,11 +147,50 @@ private String generateAccountNumber(Map<String, String>
propertyMap, AccountNum
prefix = prefix.substring(0, Math.min(prefix.length(), 10));
}
+ if (accountNumberPrefixType.getValue() == 401) {
+
+ AccountNumberFormat accountNumberByPrefix =
this.accountNumberFormatRepository
+
.findOneByPrefixEnumAccountTypeEnum(accountNumberFormat.getAccountTypeEnum(),
401);
+ Integer digits = accountNumberByPrefix.getDigits();
+ Integer numberLength = accountMaxLength - digits;
+ accountNumber = StringUtils.leftPad(propertyMap.get(ID),
numberLength, '0');
+
+ } else {
+ accountNumber = StringUtils.leftPad(accountNumber,
Integer.valueOf(propertyMap.get(ID).length()), '0');
+ }
+
accountNumber = StringUtils.overlay(accountNumber, prefix, 0, 0);
}
return accountNumber;
}
+ private Map<String, String> prefixEight(Map<String, String> propertyMap,
String accountNumber, Integer accountMaxLength,
+ AccountNumberFormat accountNumberFormat) {
+ AccountNumberFormat accountNumberByPrefix =
this.accountNumberFormatRepository
+
.findOneByPrefixEnumAccountTypeEnum(accountNumberFormat.getAccountTypeEnum(),
401);
+ Integer digits = accountNumberByPrefix.getDigits();
Review comment:
Do we need to pass in digits as a separate argument and store it
separately in the database? Could we not infer the length of the prefix from
the length of the prefix string? Ie if we want the prefix to be 3 characters of
"123" we just store "123" as the prefix string.
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/infrastructure/accountnumberformat/domain/AccountNumberFormat.java
##########
@@ -37,21 +37,40 @@
@Column(name = AccountNumberFormatConstants.PREFIX_TYPE_ENUM_COLUMN_NAME,
nullable = true)
private Integer prefixEnum;
+ @Column(name = AccountNumberFormatConstants.DIGITS_COLUMN_NAME, nullable =
true)
+ private Integer digits;
+
+ @Column(name = AccountNumberFormatConstants.PREFIX_CHARACTER_COLUMN_NAME,
nullable = true)
+ private String prefixCharacter;
+
protected AccountNumberFormat() {
//
}
- public AccountNumberFormat(EntityAccountType entityAccountType,
AccountNumberPrefixType prefixType) {
+ public AccountNumberFormat(EntityAccountType entityAccountType,
AccountNumberPrefixType prefixType, Integer digits,
+ String prefixCharacter) {
this.accountTypeEnum = entityAccountType.getValue();
if (prefixType != null) {
this.prefixEnum = prefixType.getValue();
}
+ if (prefixType.getValue() == 401) {
Review comment:
Why would we only set these variables for a specific type? What people
expect is that the constructor will set the parameters that are passed in...
what's the harm of just setting these every time?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/domain/AccountNumberGenerator.java
##########
@@ -134,11 +147,50 @@ private String generateAccountNumber(Map<String, String>
propertyMap, AccountNum
prefix = prefix.substring(0, Math.min(prefix.length(), 10));
}
+ if (accountNumberPrefixType.getValue() == 401) {
Review comment:
Why do we need this block? We've already determined the prefix above....
can't we just use the length of the prefix to determine how much we need to
pad the account number? Why do we need to retrieve the length again?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/domain/AccountNumberGenerator.java
##########
@@ -134,11 +147,50 @@ private String generateAccountNumber(Map<String, String>
propertyMap, AccountNum
prefix = prefix.substring(0, Math.min(prefix.length(), 10));
}
+ if (accountNumberPrefixType.getValue() == 401) {
+
+ AccountNumberFormat accountNumberByPrefix =
this.accountNumberFormatRepository
+
.findOneByPrefixEnumAccountTypeEnum(accountNumberFormat.getAccountTypeEnum(),
401);
+ Integer digits = accountNumberByPrefix.getDigits();
+ Integer numberLength = accountMaxLength - digits;
+ accountNumber = StringUtils.leftPad(propertyMap.get(ID),
numberLength, '0');
+
+ } else {
+ accountNumber = StringUtils.leftPad(accountNumber,
Integer.valueOf(propertyMap.get(ID).length()), '0');
+ }
+
accountNumber = StringUtils.overlay(accountNumber, prefix, 0, 0);
}
return accountNumber;
}
+ private Map<String, String> prefixEight(Map<String, String> propertyMap,
String accountNumber, Integer accountMaxLength,
+ AccountNumberFormat accountNumberFormat) {
+ AccountNumberFormat accountNumberByPrefix =
this.accountNumberFormatRepository
+
.findOneByPrefixEnumAccountTypeEnum(accountNumberFormat.getAccountTypeEnum(),
401);
+ Integer digits = accountNumberByPrefix.getDigits();
+ String prefix = accountNumberByPrefix.getPrefixCharacter();
+
+ Integer numberLength = accountMaxLength - digits;
+
+ if (Integer.valueOf(propertyMap.get(ID).length()) == numberLength + 1)
{ // 888-999999
+ prefix = prefix.substring(0, prefix.length() - 1);
Review comment:
This logic seems to be a) hardcoded to expect a specific length for
prefix and digits b) only handle specific cases of length/prefix length. Can
this not be made generic so that we automatically work out the length of the
number / prefix based on the lengths of the inputs?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/domain/AccountNumberGenerator.java
##########
@@ -134,11 +147,50 @@ private String generateAccountNumber(Map<String, String>
propertyMap, AccountNum
prefix = prefix.substring(0, Math.min(prefix.length(), 10));
}
+ if (accountNumberPrefixType.getValue() == 401) {
+
+ AccountNumberFormat accountNumberByPrefix =
this.accountNumberFormatRepository
+
.findOneByPrefixEnumAccountTypeEnum(accountNumberFormat.getAccountTypeEnum(),
401);
+ Integer digits = accountNumberByPrefix.getDigits();
+ Integer numberLength = accountMaxLength - digits;
+ accountNumber = StringUtils.leftPad(propertyMap.get(ID),
numberLength, '0');
+
+ } else {
+ accountNumber = StringUtils.leftPad(accountNumber,
Integer.valueOf(propertyMap.get(ID).length()), '0');
+ }
+
accountNumber = StringUtils.overlay(accountNumber, prefix, 0, 0);
}
return accountNumber;
}
+ private Map<String, String> prefixEight(Map<String, String> propertyMap,
String accountNumber, Integer accountMaxLength,
+ AccountNumberFormat accountNumberFormat) {
+ AccountNumberFormat accountNumberByPrefix =
this.accountNumberFormatRepository
+
.findOneByPrefixEnumAccountTypeEnum(accountNumberFormat.getAccountTypeEnum(),
401);
Review comment:
Why do we need to look up again the AccountNumberFormat? Should we not
use the one that we've been passed in as an argument, rather than looking up a
new one?
--
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]
