alberto-art3ch commented on code in PR #6546:
URL: https://github.com/apache/fineract/pull/6546#discussion_r4388897262


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanApplicationValidator.java:
##########
@@ -1080,7 +1096,21 @@ public void validateForModify(final JsonCommand command, 
final Loan loan) {
                 final Integer repaymentEveryType = this.fromApiJsonHelper
                         
.extractIntegerWithLocaleNamed(LoanApiConstants.repaymentFrequencyTypeParameterName,
 element);
                 
baseDataValidator.reset().parameter(LoanApiConstants.repaymentFrequencyTypeParameterName).value(repaymentEveryType)
-                        .notNull().inMinMaxRange(0, 3);
+                        
.notNull().isOneOfTheseValues(SUPPORTED_PERIOD_FREQUENCY_TYPES);
+                // A loan being modified keeps the days it already has before 
falling back to the product's.
+                final LoanProductRelatedDetail dueDaysFallback = 
loan.getLoanRepaymentScheduleDetail().getFirstRepaymentDayOfMonth() != null
+                        ? loan.getLoanRepaymentScheduleDetail()
+                        : loanProduct.getLoanProductRelatedDetail();
+                validateSemiMonthlyRepayment(element, baseDataValidator, 
repaymentEveryType, repaymentEvery,
+                        dueDaysFallback.getFirstRepaymentDayOfMonth(), 
dueDaysFallback.getSecondRepaymentDayOfMonth());
+            } else if 
(this.fromApiJsonHelper.parameterExists(LoanApiConstants.FIRST_REPAYMENT_DAY_OF_MONTH,
 element)
+                    || 
this.fromApiJsonHelper.parameterExists(LoanApiConstants.SECOND_REPAYMENT_DAY_OF_MONTH,
 element)) {
+                // Only the days are changing, so they are checked against the 
frequency the loan already has.

Review Comment:
   Good catch, thanks. The two branches are gone: `validateForModify(command, 
loan)` now resolves the effective frequency first (the request's 
`repaymentFrequencyType`, or the loan's stored one when the request doesn't 
carry it) and calls `validateSemiMonthlyRepayment` once, unconditionally, with 
the same day fallback as before (loan's days, then the product's). So a `PUT` 
with only `repaymentEvery: 2`, or only a `calendarId`, is rejected on a 
semi-monthly loan. The rules are a no-op for any other frequency when the 
request carries no days, so non-semi-monthly modifications are unchanged. This 
mirrors what `LoanProductDataValidator#validateForUpdate` already did.
   
   IT: `aModificationWithRepaymentEveryOtherThanOneIsRejected` (400, 
`repaymentEvery.must.be.one.for.semi.monthly.repayment.frequency`).



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