ptuomola commented on a change in pull request #2002:
URL: https://github.com/apache/fineract/pull/2002#discussion_r775185926
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepository.java
##########
@@ -67,10 +67,12 @@
String DOES_PRODUCT_HAVE_NON_CLOSED_LOANS = "select case when (count
(loan) > 0) then 'true' else 'false' end from Loan loan where
loan.loanProduct.id = :productId and loan.loanStatus in
(100,200,300,303,304,700)";
- String FIND_BY_ACCOUNT_NUMBER = "select loan from Loan loan where
loan.accountNumber = :accountNumber and loan.loanStatus in
(100,200,300,303,304)";
+ String FIND_NON_CLOSED_BY_ACCOUNT_NUMBER = "select loan from Loan loan
where loan.accountNumber = :accountNumber and loan.loanStatus in
(100,200,300,303,304)";
Review comment:
So this allows reusing an account number from a closed loan. Is that
really OK? Would it not make more sense to ensure loan account numbers are
unique as well?
##########
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:
Not sure I understand this code... why do we make the random account
number the same length as the prefix? And since we are truncating the random
number here, how do we ensure it is still unique after this truncation?
If the problem is that we don't know the length of the random number we are
creating, maybe one solution would be to store that as a configuration
parameter as part of the AccountNumberFormat?
##########
File path:
fineract-provider/src/main/resources/sql/migrations/core_db/V379__configuration_for_random_account_number.sql
##########
@@ -0,0 +1,20 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one
+-- or more contributor license agreements. See the NOTICE file
+-- distributed with this work for additional information
+-- regarding copyright ownership. The ASF licenses this file
+-- to you under the Apache License, Version 2.0 (the
+-- "License"); you may not use this file except in compliance
+-- with the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing,
+-- software distributed under the License is distributed on an
+-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+-- KIND, either express or implied. See the License for the
+-- specific language governing permissions and limitations
+-- under the License.
+--
+
+INSERT INTO c_configuration ( name, description) VALUES (
'random-account-number', 'if enabled, the savings account will be created with
Random Savings Account Number');
Review comment:
This only talks about savings accounts but the code seems to impact
clients and loans as well?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/domain/AccountNumberGenerator.java
##########
@@ -108,7 +124,14 @@ private String generateAccountNumber(Map<String, String>
propertyMap, AccountNum
}
}
- String accountNumber = StringUtils.leftPad(propertyMap.get(ID),
accountMaxLength, '0');
+ final GlobalConfigurationPropertyData randomAccountNumber =
this.configurationReadPlatformService
+ .retrieveGlobalConfiguration("random-account-number");
Review comment:
Does it make sense to have this as a single global configuration
property so that then all numbers are random (client, loan, savings account)?
Or would it make more sense to store this as part of the AccountNumberFormat as
a parameter so that people can enable this for different entities if they'd
like to? Then we could also store the length of the random number to be created
as part of the AccountNumberFormat.
--
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]
