adamsaghy commented on code in PR #2797: URL: https://github.com/apache/fineract/pull/2797#discussion_r1046294626
########## fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepaymentScheduleProcessingWrapper.java: ########## @@ -213,6 +213,10 @@ private Money cumulativePenaltyChargesDueWithin(final LocalDate periodStart, fin cumulative = cumulative.plus(loanChargeAmt); } else if (loanCharge.isDueForCollectionFromAndUpToAndIncluding(periodStart, periodEnd)) { cumulative = cumulative.plus(loanCharge.amount()); + // Special case for Loan Charges (Due Date) added the same disbursement date + } else if (period.isFirstPeriod() Review Comment: I would rather do this: - If this is the first period AND - first period startdate == loanCharge due date I dont think your approach is wrong, but its harder to understand. `loanCharge.isDueForCollectionFromIncludingAndUpToAndIncluding` is hard to understand what it is doing. Also this special case is only when the charge due date is equals with first instalment start date. @josehernandezfintecheandomx what do you think? -- 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: commits-unsubscr...@fineract.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org