adamsaghy commented on code in PR #3366:
URL: https://github.com/apache/fineract/pull/3366#discussion_r1301319485


##########
fineract-loan/src/main/java/org/apache/fineract/accounting/producttoaccountmapping/serialization/ProductToGLAccountMappingFromApiJsonDeserializer.java:
##########
@@ -84,7 +84,7 @@ public void validateForLoanProductCreate(final String json) {
         final Integer accountingRuleType = 
this.fromApiJsonHelper.extractIntegerNamed("accountingRule", element, 
Locale.getDefault());
         
baseDataValidator.reset().parameter("accountingRule").value(accountingRuleType).notNull().inMinMaxRange(1,
 4);
 
-        if (isCashBasedAccounting(accountingRuleType) || 
isAccrualBasedAccounting(accountingRuleType)) {
+        if (AccountingValidations.isCashBasedAccounting(accountingRuleType)) {

Review Comment:
   I dont mind to put this logic into a new class (however a not static one 
would be better, because it can be overridden if needed), but the original 
condition said: IF it is cash based on accrual based, but your validator is 
checkint only if it is cash based. Are you sure this change is correct?



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