galovics commented on a change in pull request #2215:
URL: https://github.com/apache/fineract/pull/2215#discussion_r839296376
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountTransactionDataValidator.java
##########
@@ -232,25 +232,68 @@ public SavingsAccountTransaction
validateHoldAndAssembleForm(final String json,
baseDataValidator.reset().parameter(SavingsApiConstants.statusParamName)
.failWithCodeNoParameterAddedToErrorCode(SavingsApiConstants.ERROR_MSG_SAVINGS_ACCOUNT_NOT_ACTIVE);
}
- account.holdAmount(amount);
- if (account.getEnforceMinRequiredBalance()) {
- if (account.getWithdrawableBalance().compareTo(BigDecimal.ZERO) <
0) {
-
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient
balance", account.getId());
+ Boolean isEnforceMinRequiredBalanceEnabled =
account.getEnforceMinRequiredBalance();
+ Boolean isAccountLienEnabled = account.isLienAllowed();
+ Boolean isOverdraftEnabled = account.isAllowOverdraft();
+
+ Boolean lienAllowed = false;
Review comment:
This could be simplified into
`BooleanUtils.isTrue(fromApiJsonHelper.extractBooleanNamed(lienAllowedParamName,
element))`
instead of the multi-layered if statements.
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountTransactionDataValidator.java
##########
@@ -232,25 +232,68 @@ public SavingsAccountTransaction
validateHoldAndAssembleForm(final String json,
baseDataValidator.reset().parameter(SavingsApiConstants.statusParamName)
.failWithCodeNoParameterAddedToErrorCode(SavingsApiConstants.ERROR_MSG_SAVINGS_ACCOUNT_NOT_ACTIVE);
}
- account.holdAmount(amount);
- if (account.getEnforceMinRequiredBalance()) {
- if (account.getWithdrawableBalance().compareTo(BigDecimal.ZERO) <
0) {
-
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient
balance", account.getId());
+ Boolean isEnforceMinRequiredBalanceEnabled =
account.getEnforceMinRequiredBalance();
+ Boolean isAccountLienEnabled = account.isLienAllowed();
+ Boolean isOverdraftEnabled = account.isAllowOverdraft();
+
+ Boolean lienAllowed = false;
+ if (this.fromApiJsonHelper.parameterExists(lienAllowedParamName,
element)) {
+ lienAllowed =
this.fromApiJsonHelper.extractBooleanNamed(lienAllowedParamName, element);
+ if (lienAllowed) {
+ if (isAccountLienEnabled) {
+ if (isOverdraftEnabled) {
+ if
(account.getOverdraftLimit().compareTo(BigDecimal.ZERO) > 0
+ &&
account.getMaxAllowedLienLimit().compareTo(BigDecimal.ZERO) > 0) {
+ if
(account.getOverdraftLimit().compareTo(account.getMaxAllowedLienLimit()) > 0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode(
+
"Overdraft.limit.can.not.be.greater.than.lien.limit", account.getId());
+ }
+ }
+ }
+
+ if (amount.compareTo(account.getMaxAllowedLienLimit()) >
0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("lien.limit.exceeded",
account.getId());
+ }
+ } else {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("lien.is.not.allowed.in.product.level",
+ account.getId());
+ }
+ } else {
+ if (isOverdraftEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) > 0
&& amount.compareTo(account.getOverdraftLimit()) > 0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
+ }
+ }
+ if (isEnforceMinRequiredBalanceEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) >
0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
+ }
+ }
+ if (!isOverdraftEnabled &&
!isEnforceMinRequiredBalanceEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) >
0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
+ }
+ }
}
- }
-
- Boolean lien = false;
-
- if (this.fromApiJsonHelper.parameterExists(lienParamName, element)) {
- lien = this.fromApiJsonHelper.extractBooleanNamed(lienParamName,
element);
- if (!lien) {
- if
(account.getWithdrawableBalanceWithoutMinimumBalance().compareTo(BigDecimal.ZERO)
< 0) {
-
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient
balance", account.getId());
+ } else {
+ if (isOverdraftEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) > 0 &&
amount.compareTo(account.getOverdraftLimit()) > 0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
+ }
+ }
+ if (isEnforceMinRequiredBalanceEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) > 0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
+ }
+ }
+ if (!isOverdraftEnabled && !isEnforceMinRequiredBalanceEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) > 0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
}
}
}
-
+ account.holdAmount(amount);
Review comment:
I commented this on another PR but I really think we should avoid things
like this. A validator should not touch a domain object's state especially when
it's a JPA entity.
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsProductDataValidator.java
##########
@@ -340,6 +343,22 @@ public void validateForCreate(final String json) {
baseDataValidator.reset().parameter(minBalanceForInterestCalculationParamName).value(minBalanceForInterestCalculation)
.ignoreIfNull().zeroOrPositiveAmount();
}
+ Boolean isLienAllowed =
this.fromApiJsonHelper.parameterExists(lienAllowedParamName, element);
+ Boolean isOverdraftAllowed =
this.fromApiJsonHelper.parameterExists(allowOverdraftParamName, element);
Review comment:
Same as above.
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -290,6 +292,12 @@
@Column(name = "min_required_balance", scale = 6, precision = 19, nullable
= true)
private BigDecimal minRequiredBalance;
+ @Column(name = "is_lien_allowed")
+ private boolean lienAllowed;
Review comment:
No need to. If `lienAllowed` is a binary switch then let's stick with
primitive boolean. Less headache when dealing with it's value. Just make sure
the database column is not nullable.
##########
File path:
integration-tests/src/test/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -2017,6 +2017,68 @@ private Integer createSavingsProduct(final
RequestSpecification requestSpec, fin
enforceMinRequiredBalance, allowOverdraft, taxGroupId, false);
}
+ // LienAtProductLevel
+ private Integer createSavingsProduct(final RequestSpecification
requestSpec, final ResponseSpecification responseSpec,
+ final String minOpenningBalance, String
minBalanceForInterestCalculation, final boolean enforceMinRequiredBalance,
+ final boolean allowOverDraft, final boolean lienAllowed) {
+
+ LOG.info("------------------------------CREATING NEW SAVINGS PRODUCT
WITH LIEN---------------------------------------");
+ SavingsProductHelper savingsProductHelper = new SavingsProductHelper();
+ if (lienAllowed) {
+ final String maxAllowedLienLimit = "2000.0";
+ savingsProductHelper.withLienAllowed(maxAllowedLienLimit);
+ }
+ if (enforceMinRequiredBalance) {
+ final String minRequiredBalance = "100.0";
+ savingsProductHelper.withMinRequiredBalance(minRequiredBalance);
+ savingsProductHelper.withEnforceMinRequiredBalance("true");
+ }
+ if (allowOverDraft) {
+ final String overDraftLimit = "1000.0";
+ savingsProductHelper.withOverDraft(overDraftLimit);
+ }
+ final String savingsProductJSON =
savingsProductHelper.withInterestCompoundingPeriodTypeAsDaily()
+ //
Review comment:
Why the slashes?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -290,6 +292,12 @@
@Column(name = "min_required_balance", scale = 6, precision = 19, nullable
= true)
private BigDecimal minRequiredBalance;
+ @Column(name = "is_lien_allowed")
+ private boolean lienAllowed;
+
+ @Column(name = "max_Allowed_Lien_Limit", scale = 6, precision = 19,
nullable = true)
Review comment:
Use only lowercase characters in the colum name.
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java
##########
@@ -1504,7 +1517,9 @@ public SavingsAccountTransactionData mapRow(final
ResultSet rs, @SuppressWarning
// ");
// sqlBuilder.append("sp.annual_fee_on_day as annualFeeOnDay ");
sqlBuilder.append("sp.min_required_balance as minRequiredBalance,
");
- sqlBuilder.append("sp.enforce_min_required_balance as
enforceMinRequiredBalance ");
+ sqlBuilder.append("sp.enforce_min_required_balance as
enforceMinRequiredBalance, ");
+ sqlBuilder.append("sp.max_Allowed_Lien_Limit as
maxAllowedLienLimit, ");
Review comment:
Lowercase column name here as well.
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsProductDataValidator.java
##########
@@ -340,6 +343,22 @@ public void validateForCreate(final String json) {
baseDataValidator.reset().parameter(minBalanceForInterestCalculationParamName).value(minBalanceForInterestCalculation)
.ignoreIfNull().zeroOrPositiveAmount();
}
+ Boolean isLienAllowed =
this.fromApiJsonHelper.parameterExists(lienAllowedParamName, element);
Review comment:
I don't think this is correct because you're only checking whether the
parameter exists instead of the parameter's value. Am I missing something?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsProductDataValidator.java
##########
@@ -340,6 +343,22 @@ public void validateForCreate(final String json) {
baseDataValidator.reset().parameter(minBalanceForInterestCalculationParamName).value(minBalanceForInterestCalculation)
.ignoreIfNull().zeroOrPositiveAmount();
}
+ Boolean isLienAllowed =
this.fromApiJsonHelper.parameterExists(lienAllowedParamName, element);
Review comment:
I don't think this is correct because you're only checking whether the
parameter exists instead of the parameter's value. Am I missing something?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java
##########
@@ -332,6 +332,8 @@ public SavingsAccountData retrieveOne(final Long accountId)
{
sqlBuilder.append("sa.min_balance_for_interest_calculation as
minBalanceForInterestCalculation,");
sqlBuilder.append("sa.min_required_balance as minRequiredBalance,
");
sqlBuilder.append("sa.enforce_min_required_balance as
enforceMinRequiredBalance, ");
+ sqlBuilder.append("sa.max_Allowed_Lien_Limit as
maxAllowedLienLimit, ");
Review comment:
Lowercase colum names pls.
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountTransactionDataValidator.java
##########
@@ -232,25 +232,68 @@ public SavingsAccountTransaction
validateHoldAndAssembleForm(final String json,
baseDataValidator.reset().parameter(SavingsApiConstants.statusParamName)
.failWithCodeNoParameterAddedToErrorCode(SavingsApiConstants.ERROR_MSG_SAVINGS_ACCOUNT_NOT_ACTIVE);
}
- account.holdAmount(amount);
- if (account.getEnforceMinRequiredBalance()) {
- if (account.getWithdrawableBalance().compareTo(BigDecimal.ZERO) <
0) {
-
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient
balance", account.getId());
+ Boolean isEnforceMinRequiredBalanceEnabled =
account.getEnforceMinRequiredBalance();
+ Boolean isAccountLienEnabled = account.isLienAllowed();
+ Boolean isOverdraftEnabled = account.isAllowOverdraft();
+
+ Boolean lienAllowed = false;
+ if (this.fromApiJsonHelper.parameterExists(lienAllowedParamName,
element)) {
+ lienAllowed =
this.fromApiJsonHelper.extractBooleanNamed(lienAllowedParamName, element);
+ if (lienAllowed) {
+ if (isAccountLienEnabled) {
+ if (isOverdraftEnabled) {
+ if
(account.getOverdraftLimit().compareTo(BigDecimal.ZERO) > 0
+ &&
account.getMaxAllowedLienLimit().compareTo(BigDecimal.ZERO) > 0) {
+ if
(account.getOverdraftLimit().compareTo(account.getMaxAllowedLienLimit()) > 0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode(
+
"Overdraft.limit.can.not.be.greater.than.lien.limit", account.getId());
+ }
+ }
+ }
+
+ if (amount.compareTo(account.getMaxAllowedLienLimit()) >
0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("lien.limit.exceeded",
account.getId());
+ }
+ } else {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("lien.is.not.allowed.in.product.level",
+ account.getId());
+ }
+ } else {
+ if (isOverdraftEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) > 0
&& amount.compareTo(account.getOverdraftLimit()) > 0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
+ }
+ }
+ if (isEnforceMinRequiredBalanceEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) >
0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
+ }
+ }
+ if (!isOverdraftEnabled &&
!isEnforceMinRequiredBalanceEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) >
0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
+ }
+ }
}
- }
-
- Boolean lien = false;
-
- if (this.fromApiJsonHelper.parameterExists(lienParamName, element)) {
- lien = this.fromApiJsonHelper.extractBooleanNamed(lienParamName,
element);
- if (!lien) {
- if
(account.getWithdrawableBalanceWithoutMinimumBalance().compareTo(BigDecimal.ZERO)
< 0) {
-
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient
balance", account.getId());
+ } else {
+ if (isOverdraftEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) > 0 &&
amount.compareTo(account.getOverdraftLimit()) > 0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
+ }
+ }
+ if (isEnforceMinRequiredBalanceEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) > 0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
+ }
+ }
+ if (!isOverdraftEnabled && !isEnforceMinRequiredBalanceEnabled) {
+ if (amount.compareTo(account.getWithdrawableBalance()) > 0) {
+
baseDataValidator.reset().failWithCodeNoParameterAddedToErrorCode("insufficient.balance",
account.getId());
}
}
}
-
+ account.holdAmount(amount);
Review comment:
I commented this on another PR but I really think we should avoid things
like this. A validator should not touch a domain object's state especially when
it's a JPA entity.
##########
File path:
integration-tests/src/test/java/org/apache/fineract/integrationtests/ClientSavingsIntegrationTest.java
##########
@@ -2017,6 +2017,68 @@ private Integer createSavingsProduct(final
RequestSpecification requestSpec, fin
enforceMinRequiredBalance, allowOverdraft, taxGroupId, false);
}
+ // LienAtProductLevel
+ private Integer createSavingsProduct(final RequestSpecification
requestSpec, final ResponseSpecification responseSpec,
+ final String minOpenningBalance, String
minBalanceForInterestCalculation, final boolean enforceMinRequiredBalance,
+ final boolean allowOverDraft, final boolean lienAllowed) {
+
+ LOG.info("------------------------------CREATING NEW SAVINGS PRODUCT
WITH LIEN---------------------------------------");
+ SavingsProductHelper savingsProductHelper = new SavingsProductHelper();
+ if (lienAllowed) {
+ final String maxAllowedLienLimit = "2000.0";
+ savingsProductHelper.withLienAllowed(maxAllowedLienLimit);
+ }
+ if (enforceMinRequiredBalance) {
+ final String minRequiredBalance = "100.0";
+ savingsProductHelper.withMinRequiredBalance(minRequiredBalance);
+ savingsProductHelper.withEnforceMinRequiredBalance("true");
+ }
+ if (allowOverDraft) {
+ final String overDraftLimit = "1000.0";
+ savingsProductHelper.withOverDraft(overDraftLimit);
+ }
+ final String savingsProductJSON =
savingsProductHelper.withInterestCompoundingPeriodTypeAsDaily()
+ //
+ .withInterestPostingPeriodTypeAsMonthly()
+ //
+ .withInterestCalculationPeriodTypeAsDailyBalance()
+ //
+
.withMinBalanceForInterestCalculation(minBalanceForInterestCalculation)
+ //
+ .withMinimumOpenningBalance(minOpenningBalance).build();
+
+ return SavingsProductHelper.createSavingsProduct(savingsProductJSON,
requestSpec, responseSpec);
+ }
+
+ // LienAtProductlevel with Overdraft Limit > Lien Limit
+ private Integer createSavingsProduct(final RequestSpecification
requestSpec, final ResponseSpecification responseSpec,
+ final String minOpenningBalance, String
minBalanceForInterestCalculation, final boolean enforceMinRequiredBalance,
+ final boolean allowOverDraft, final String overDraftLimit, final
boolean lienAllowed, final String lienAllowedLimit) {
+ LOG.info("------------------------------CREATING NEW SAVINGS PRODUCT
WITH LIEN---------------------------------------");
+ SavingsProductHelper savingsProductHelper = new SavingsProductHelper();
+ if (lienAllowed) {
+ savingsProductHelper.withLienAllowed(lienAllowedLimit);
+ }
+ if (enforceMinRequiredBalance) {
+ final String minRequiredBalance = "100.0";
+ savingsProductHelper.withMinRequiredBalance(minRequiredBalance);
+ savingsProductHelper.withEnforceMinRequiredBalance("true");
+ }
+ if (allowOverDraft) {
+ savingsProductHelper.withOverDraft(overDraftLimit);
+ }
+ final String savingsProductJSON =
savingsProductHelper.withInterestCompoundingPeriodTypeAsDaily()
+ //
Review comment:
Slashes here too.
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsProductDataValidator.java
##########
@@ -340,6 +343,22 @@ public void validateForCreate(final String json) {
baseDataValidator.reset().parameter(minBalanceForInterestCalculationParamName).value(minBalanceForInterestCalculation)
.ignoreIfNull().zeroOrPositiveAmount();
}
+ Boolean isLienAllowed =
this.fromApiJsonHelper.parameterExists(lienAllowedParamName, element);
+ Boolean isOverdraftAllowed =
this.fromApiJsonHelper.parameterExists(allowOverdraftParamName, element);
Review comment:
Same as above.
--
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]
