ptuomola commented on a change in pull request #2002:
URL: https://github.com/apache/fineract/pull/2002#discussion_r775188140



##########
File path: 
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/domain/AccountNumberGenerator.java
##########
@@ -143,8 +166,13 @@ private String generateAccountNumber(Map<String, String> 
propertyMap, AccountNum
             }
             if 
(accountNumberPrefixType.getValue().equals(AccountNumberPrefixType.PREFIX_SHORT_NAME.getValue()))
 {
                 Integer prefixLength = prefix.length();
-                Integer numberLength = accountMaxLength - prefixLength;
-                accountNumber = StringUtils.leftPad(propertyMap.get(ID), 
numberLength, '0');
+
+                if (randomAccountNumber.isEnabled()) {
+                    accountNumber = accountNumber.substring(prefixLength);

Review comment:
       How do we ensure the substring is unique? What if we had generated a 
string that only differs from existing ones by the positions that are now being 
removed? Would it not make more sense to do the full generation of account ID 
(including any prefix etc) and only after that check for duplicates? Or some 
other similar way to ensure uniqueness...

##########
File path: 
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/domain/AccountNumberGenerator.java
##########
@@ -154,6 +182,36 @@ private String generateAccountNumber(Map<String, String> 
propertyMap, AccountNum
         return accountNumber;
     }
 
+    private String randomNumberGenerator(int accountMaxLength, Map<String, 
String> propertyMap) {
+        String randomNumber = RandomStringUtils.random(accountMaxLength, 
false, true);
+        String accountNumber = randomNumber.substring(0, accountMaxLength);
+        BigInteger accNumber = new BigInteger(accountNumber);
+
+        if (!accNumber.equals(BigInteger.ZERO)) { // to avoid account no. 00 
in randomisation
+            String entityType = propertyMap.get(ENTITY_TYPE);
+            if (entityType.equals("client")) { // avoid duplication it will 
loop until it finds new random account no.
+
+                Client client = 
this.clientRepository.getClientByAccountNumber(accountNumber);

Review comment:
       What if the account numbers have prefixes / shortnames added to them? 
Then this will never return a match even though there may be matching ones 
after we append the prefix...




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