You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "Cocoa-Puffs (via GitHub)" <gi...@apache.org> on 2025/05/08 08:17:15 UTC
[PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Cocoa-Puffs opened a new pull request, #4651:
URL: https://github.com/apache/fineract/pull/4651
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2081480527
##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java:
##########
@@ -1133,7 +1122,73 @@ private void handleDisbursementWithoutEMICalculator(LoanTransaction disbursement
Money amortizableAmount = disbursementTransaction.getAmount(currency).minus(downPaymentAmount);
+ recalculateRepaymentInstallmentsWithoutEMICalculation(disbursementTransaction, amortizableAmount, candidateRepaymentInstallments,
+ currency, installmentAmountInMultiplesOf);
+ allocateOverpayment(disbursementTransaction, transactionCtx);
+ }
+
+ private void handleCapitalizedIncome(LoanTransaction capitalizedIncomeTransaction, TransactionCtx transactionCtx) {
+ if (capitalizedIncomeTransaction.getLoan().isInterestBearing()) {
+ handleCapitalizedIncomeWithEMICalculator(capitalizedIncomeTransaction, transactionCtx);
+ } else {
+ handleCapitalizedIncomeWithoutEMICalculator(capitalizedIncomeTransaction, transactionCtx);
+ }
+ }
+
+ private void handleCapitalizedIncomeWithEMICalculator(LoanTransaction capitalizedIncomeTransaction, TransactionCtx transactionCtx) {
+ ProgressiveLoanInterestScheduleModel model;
+ if (!(transactionCtx instanceof ProgressiveTransactionCtx)
+ || (model = ((ProgressiveTransactionCtx) transactionCtx).getModel()) == null) {
+ throw new IllegalStateException("TransactionCtx has no model");
+ }
+ List<LoanRepaymentScheduleInstallment> installments = transactionCtx.getInstallments();
+ LocalDate transactionDate = capitalizedIncomeTransaction.getTransactionDate();
+ MonetaryCurrency currency = transactionCtx.getCurrency();
+
+ Money amortizableAmount = capitalizedIncomeTransaction.getAmount(currency);
+ emiCalculator.addDisbursement(model, transactionDate, amortizableAmount);
Review Comment:
Can we please distinct disbursement and capitalized income? I know it is behaving like disbursement, but it was not that... :/
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2101997435
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanSummary.java:
##########
@@ -38,6 +38,12 @@
public class LoanSummary {
// derived totals fields
+ @Column(name = "total_principal_derived", scale = 6, precision = 19)
+ private BigDecimal totalPrincipal;
Review Comment:
We might wanna add these fields to the `fineract-avro-schemas/src/main/avro/loan/v1/LoanAccountDataV1.avsc` so they goes in the business events as well
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "Cocoa-Puffs (via GitHub)" <gi...@apache.org>.
Cocoa-Puffs commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2102323765
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/importhandler/loan/LoanImportHandler.java:
##########
@@ -464,6 +464,7 @@ private Integer importLoanRepayment(final List<LoanTransactionData> loanRepaymen
JsonObject loanRepaymentJsonob = gsonBuilder.create().toJsonTree(loanRepayments.get(rowIndex)).getAsJsonObject();
loanRepaymentJsonob.remove("manuallyReversed");
loanRepaymentJsonob.remove("numberOfRepayments");
+ loanRepaymentJsonob.remove("reversed");
Review Comment:
During this part of the loan import we create a json object made from LoanTransactionData objects, however there are certain fields that the loan import validates for, that cannot exist in the json object during import. Reversed is one of these fields and since I added it to the LoanTransactionData at one point the tests related to loan imports started failing. I found this part, where we do the same with the "manuallyReversed" and "numberOfRepayments" fields. However since I removed the reversed field from LoanTransactionData we don't need this. I'll remove it.
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2081477747
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanDownPaymentHandlerServiceImpl.java:
##########
@@ -172,8 +172,14 @@ public void handleRepaymentOrRecoveryOrWaiverTransaction(final Loan loan, final
loan.updateLoanSummaryDerivedFields();
}
+ BigDecimal disbursedThroughCapitalizedIncome = BigDecimal.ZERO;
+ if (loan.getLoanProductRelatedDetail().isEnableIncomeCapitalization()) {
+ disbursedThroughCapitalizedIncome = loan.getLoanTransactions().stream().filter(LoanTransaction::isCapitalizedIncome)
Review Comment:
`disbursedThroughCapitalizedIncome` is incorrect. it was not disbursed...
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "Cocoa-Puffs (via GitHub)" <gi...@apache.org>.
Cocoa-Puffs commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2097177691
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanSummary.java:
##########
@@ -38,6 +38,12 @@
public class LoanSummary {
// derived totals fields
+ @Column(name = "total_principal_derived", scale = 6, precision = 19)
Review Comment:
I use it on the ui summary page. Previously the ui used the totalPrincipalDisbursed field as the total principal for the loan. From now on we need to take into account the capitalized income transactions as well. I store it because I thought it would be useful in the future to have the total principal already available.
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2095858040
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/data/LoanSchedulePeriodDataHolder.java:
##########
@@ -0,0 +1,32 @@
+/**
+ * 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.
+ */
+package org.apache.fineract.portfolio.loanaccount.data;
+
+import java.time.LocalDate;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+
+@Getter
+@AllArgsConstructor
+public class LoanSchedulePeriodDataHolder {
+
+ private final Object data;
Review Comment:
Should we have som common interface? Object is way too common.
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2081428392
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java:
##########
@@ -306,6 +306,12 @@ public static boolean isDateInThePast(final LocalDate localDate) {
return isBeforeBusinessDate(localDate);
}
+ public static boolean occursOnDayFromAndIncludingAndUpTo(final LocalDate fromInclusive, final LocalDate upToNotInclusive,
Review Comment:
We already have this: `isDateInRangeFromExclusiveToInclusive`, and i like the naming strategy of this more.
Can you please rename to `isDateInRangeFromInclusiveToExclusive` ?
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2095864157
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java:
##########
@@ -635,6 +640,25 @@ public org.springframework.data.domain.Page<LoanTransactionData> retrieveLoanTra
return transactionPage.map(loanTransactionMapper::mapLoanTransaction);
}
+ @Override
+ public List<LoanTransactionData> retrieveLoanTransactionsByTypes(Long loanId, Collection<LoanTransactionType> transactionTypes) {
Review Comment:
A simple JPQL query in Repository would have been better... I dont like the idea to fetch LoanTransaction entities and then map to LoanTransactionData.
Lets do DTO projection instead!
Example: org.apache.fineract.organisation.monetary.domain.ApplicationCurrencyRepository#FIND_CURRENCY_DETAILS
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy closed pull request #4651: FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans
URL: https://github.com/apache/fineract/pull/4651
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2081477172
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanDownPaymentHandlerServiceImpl.java:
##########
@@ -172,8 +172,14 @@ public void handleRepaymentOrRecoveryOrWaiverTransaction(final Loan loan, final
loan.updateLoanSummaryDerivedFields();
}
+ BigDecimal disbursedThroughCapitalizedIncome = BigDecimal.ZERO;
+ if (loan.getLoanProductRelatedDetail().isEnableIncomeCapitalization()) {
+ disbursedThroughCapitalizedIncome = loan.getLoanTransactions().stream().filter(LoanTransaction::isCapitalizedIncome)
+ .map(LoanTransaction::getPrincipalPortion).reduce(BigDecimal.ZERO, BigDecimal::add);
+ }
+
if (loan.getLoanProduct().isMultiDisburseLoan()) {
- final BigDecimal totalDisbursed = loan.getDisbursedAmount();
+ final BigDecimal totalDisbursed = loan.getDisbursedAmount().add(disbursedThroughCapitalizedIncome);
Review Comment:
Please dont mix disbursement amounts and capitalized income amounts! Keep them separate please!
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2102005385
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/bulkimport/importhandler/loan/LoanImportHandler.java:
##########
@@ -464,6 +464,7 @@ private Integer importLoanRepayment(final List<LoanTransactionData> loanRepaymen
JsonObject loanRepaymentJsonob = gsonBuilder.create().toJsonTree(loanRepayments.get(rowIndex)).getAsJsonObject();
loanRepaymentJsonob.remove("manuallyReversed");
loanRepaymentJsonob.remove("numberOfRepayments");
+ loanRepaymentJsonob.remove("reversed");
Review Comment:
what is this?
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #4651:
URL: https://github.com/apache/fineract/pull/4651#issuecomment-2908028053
> Is it possible to split this into smaller portions? 6k lines changed, 86 files, this is impossible to review at this point.. @adamsaghy @Cocoa-Puffs
Its kind on me.. I have told @Cocoa-Puffs if he touches the swagger, do not add one more Double field, rather change them to BigDecimal... which caused most of the integration tests to be also changed... :/
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2081481738
##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/CapitalizedIncomeWritePlatformServiceImpl.java:
##########
@@ -74,9 +76,13 @@ public CommandProcessingResult addCapitalizedIncome(final Long loanId, final Jso
final Money capitalizedIncomeAmount = calculateCapitalizedIncomeAmount(loan, transactionAmount);
final LoanTransaction capitalizedIncomeTransaction = LoanTransaction.capitalizedIncome(loan, capitalizedIncomeAmount, paymentDetail,
transactionDate, txnExternalId);
- // TODO: Transaction reverse-replay + loan repayment schedule update (follow-up story)
+ // TODO: Transaction reverse-replay (follow-up story)
// Update loan with capitalized income
loan.addLoanTransaction(capitalizedIncomeTransaction);
+ loan.getLoanProductRelatedDetail()
Review Comment:
We dont need this. During allocation, we should set the portions!
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2095846889
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanSummary.java:
##########
@@ -38,6 +38,12 @@
public class LoanSummary {
// derived totals fields
+ @Column(name = "total_principal_derived", scale = 6, precision = 19)
+ private BigDecimal totalPrincipal;
+
+ @Column(name = "principal_through_capitalized_income_derived", scale = 6, precision = 19)
Review Comment:
`capitalized_income_derived` is enough... capitalizing income means turn it to principal, so implicitly we know that already...
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "Cocoa-Puffs (via GitHub)" <gi...@apache.org>.
Cocoa-Puffs commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2096022708
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -1550,6 +1552,13 @@ public BigDecimal getPrincipalAmountForRepaymentSchedule() {
} else {
principalAmount = getPrincipal().getAmount();
}
+ if (isDisbursed()) {
Review Comment:
We talked about that the "principal_amount" attribute of the loans is used elsewhere as principal that was disbursed. That means that I should not add the capitalized income portions to this value. This change was unfortunately left in. I'll remove it.
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2095855699
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/api/LoansApiResource.java:
##########
@@ -498,12 +499,14 @@ public String retrieveAll(@Context final UriInfo uriInfo,
loanBasicDetails.getPageItems().forEach(i -> {
if (i.getSummary() != null) {
Collection<DisbursementData> disbursementData = this.loanReadPlatformService.retrieveLoanDisbursementDetails(i.getId());
+ List<LoanTransactionData> capitalizedIncomeTransactions = this.loanReadPlatformService
Review Comment:
We dont need the transactions. all the balance can be found in `m_loan_capitalized_income_balance`
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2095857155
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/api/LoansApiResourceSwagger.java:
##########
@@ -517,7 +517,11 @@ private GetLoansLoanIdFeeFrequency() {}
public GetLoansLoanIdCurrency currency;
@Schema(example = "1000000.000000")
+ public Double totalPrincipal;
+ @Schema(example = "1000000.000000")
Review Comment:
We should not work with Double, use BigDecimal. Since you touch it, fix it too Double -> BigDecimal ;)
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2081485163
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanReadPlatformServiceImpl.java:
##########
@@ -1399,23 +1426,70 @@ public LoanScheduleData extractData(@NotNull final ResultSet rs) throws SQLExcep
totalPaidLate.getAmount(), totalOutstanding.getAmount(), totalCredits.getAmount());
}
- private BigDecimal processDisbursementData(LoanScheduleType loanScheduleType, Collection<DisbursementData> disbursementData,
- LocalDate fromDate, LocalDate dueDate, Set<Long> disbursementPeriodIds, BigDecimal disbursementChargeAmount,
- BigDecimal waivedChargeAmount, List<LoanSchedulePeriodData> periods) {
+ private BigDecimal processDisbursementAndCapitalizedIncomeData(LoanScheduleType loanScheduleType,
Review Comment:
Please make distinction between disbursement and capitalized income! We should not merge them together!
Disbursement amount shall NOT include capitalized income!
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2095842484
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -1550,6 +1552,13 @@ public BigDecimal getPrincipalAmountForRepaymentSchedule() {
} else {
principalAmount = getPrincipal().getAmount();
}
+ if (isDisbursed()) {
Review Comment:
Why dont we update the principal on the loan when capitalized income occurred?
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2095848056
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanSummary.java:
##########
@@ -38,6 +38,12 @@
public class LoanSummary {
// derived totals fields
+ @Column(name = "total_principal_derived", scale = 6, precision = 19)
Review Comment:
Do we need to store this? what it is used for?
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2095849740
##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanSummary.java:
##########
@@ -225,7 +238,7 @@ public void updateSummary(final MonetaryCurrency currency, final Money principal
this.totalPenaltyChargesOutstanding = totalPenaltyChargesCharged.minus(this.totalPenaltyChargesRepaid)
.minus(this.totalPenaltyChargesWaived).minus(this.totalPenaltyChargesWrittenOff).getAmount();
- final Money totalExpectedRepayment = Money.of(currency, this.totalPrincipalDisbursed).plus(this.totalInterestCharged)
+ final Money totalExpectedRepayment = Money.of(currency, this.totalPrincipalOutstanding).plus(this.totalInterestCharged)
Review Comment:
This looks incorrect... outstanding always changing, but total expected repayment does not....
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #4651:
URL: https://github.com/apache/fineract/pull/4651
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2232: add income capitalization principal handling and repayment schedule calculations on loans [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4651:
URL: https://github.com/apache/fineract/pull/4651#discussion_r2101988944
##########
fineract-e2e-tests-core/src/test/java/org/apache/fineract/test/stepdef/loan/LoanRepaymentStepDef.java:
##########
@@ -550,7 +550,7 @@ public void amountsEquallyDistributedInSchedule(double totalAmount) throws IOExc
BigDecimal lastExpectedAmount = new BigDecimal(totalAmount).setScale(0, RoundingMode.HALF_DOWN);
for (int i = 1; i < periods.size(); i++) {
- BigDecimal actualAmount = new BigDecimal(periods.get(i).getPrincipalOriginalDue()).setScale(0, RoundingMode.HALF_DOWN);
+ BigDecimal actualAmount = periods.get(i).getPrincipalOriginalDue().setScale(0, RoundingMode.HALF_DOWN);
Review Comment:
I think this is a bug. 0 scale means we can only have whole numbers... go with 2 at least please!
--
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-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org