Skip to content

Conversation

@alberto-art3ch
Copy link
Contributor

Description

When user tries a Contract termination on disbursement day it results an 500-Integral server error because there were not an Installment to be updated

FINERACT-2326

Screenshot 2025-10-03 at 12 55 18 p m

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.

loanTransactionHelper.moveLoanState(loanId,
new PostLoansLoanIdRequest().note("Contract Termination Test").externalId(Utils.randomStringGenerator("", 20)),
"contractTermination");
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All tests should have some kind of assertions, if we execute these operations what is the expected result?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alberto-art3ch Have you addressed this?

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2326/loan-contract-termination-in-disbursement-date branch 2 times, most recently from 09b64cc to d7add12 Compare October 21, 2025 13:36
@adamsaghy
Copy link
Contributor

@alberto-art3ch Please rebase and take a look on @budaidev's comment

@adamsaghy adamsaghy force-pushed the FINERACT-2326/loan-contract-termination-in-disbursement-date branch 2 times, most recently from 9a56a87 to 193d270 Compare October 28, 2025 08:27
Comment on lines 133 to 151
/*
PostLoanProductsResponse loanProductsResponse = loanProductHelper.createLoanProduct(create4IProgressive().interestRecognitionOnDisbursementDate(false));
Long loanId = applyAndApproveProgressiveLoan(client.getClientId(), loanProductsResponse.getResourceId(), "1 January 2024",
500.0, 7.0, 6, (request) -> request.interestRecognitionOnDisbursementDate(false));
disburseLoan(loanId, BigDecimal.valueOf(100), "1 January 2024");
loanTransactionHelper.moveLoanState(loanId,
new PostLoansLoanIdRequest().note("Contract Termination Test").externalId(Utils.randomStringGenerator("", 20)),
"contractTermination");
verifyTransactions(loanId, //
transaction(100.0, "Disbursement", "01 January 2024"), //
transaction(100.0, "Contract Termination", "01 January 2024"));
GetLoansLoanIdResponse loanDetails = loanTransactionHelper.getLoanDetails(loanId);
assertEquals(BigDecimal.ZERO.stripTrailingZeros(),
loanDetails.getSummary().getInterestCharged().stripTrailingZeros());
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alberto-art3ch Remove if not needed!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ITC updated, It is required

@adamsaghy adamsaghy force-pushed the FINERACT-2326/loan-contract-termination-in-disbursement-date branch 3 times, most recently from daf9a80 to ccc36cf Compare October 28, 2025 11:23
@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2326/loan-contract-termination-in-disbursement-date branch from ccc36cf to d193a88 Compare October 28, 2025 23:21
@adamsaghy adamsaghy force-pushed the FINERACT-2326/loan-contract-termination-in-disbursement-date branch from d193a88 to fb90c01 Compare October 29, 2025 09:23
@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2326/loan-contract-termination-in-disbursement-date branch 4 times, most recently from dc07f34 to 4923088 Compare October 30, 2025 02:40
@alberto-art3ch
Copy link
Contributor Author

@alberto-art3ch Please rebase and take a look on @budaidev's comment

PR updated and rebased with develop branch

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2326/loan-contract-termination-in-disbursement-date branch 6 times, most recently from c332517 to 149b2ba Compare October 30, 2025 21:06
transactionDate);
ProgressiveLoanInterestScheduleModel model = savedModel
.orElseGet(() -> processor.calculateInterestScheduleModel(loan.getId(), transactionDate));
// final LocalDate tillDate = loanApplicationTerms.isInterestRecognitionOnDisbursementDate() ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Commented lines removed those are not needed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

transactionProcessingStrategyCode);
if (loanRepaymentScheduleTransactionProcessor instanceof AdvancedPaymentScheduleTransactionProcessor advancedProcessor) {
LocalDate currentDate = DateUtils.getBusinessLocalDate();
if (interestRecognitionOnDisbursementDate && currentDate.equals(disbursementDate)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks not right... we cannot go into the future...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Changed

Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check my comments and let me know if we should connect. I see many changes which concerns me.

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2326/loan-contract-termination-in-disbursement-date branch from 149b2ba to 8cef0fd Compare November 4, 2025 00:41
public boolean isFirstNormalInstallment() {
return loan.getRepaymentScheduleInstallments().stream().filter(rp -> !rp.isDownPayment()).findFirst().stream()
.anyMatch(rp -> rp.equals(this));
public boolean isFirstNormalInstallment(List<LoanRepaymentScheduleInstallment> installments) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why to change this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is required to find the first normal installment because we are talking about the contract termination in the disbursement date and we were not able to identify the first normal installment @adamsaghy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was talking about why to provide installments as argument? What was the problem with the original?

if (loanRepaymentScheduleTransactionProcessor instanceof AdvancedPaymentScheduleTransactionProcessor advancedProcessor) {
LocalDate currentDate = DateUtils.getBusinessLocalDate();
LocalDate interestForDate = DateUtils.getBusinessLocalDate();
if (interestRecognitionOnDisbursementDate && interestForDate.equals(disbursementDate)) {
Copy link
Contributor

@adamsaghy adamsaghy Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think its correct to use current date + 1 which effectively a future date. We cannot work against future date for transaction processing. Also interest is payable on the current date anyway, we dont need this. interestRecognitionOnDisbursementDate is ONLY for accruals, nothing else!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter is used only to calculate the interest till that date, but I've restored to the original variable name @adamsaghy

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2326/loan-contract-termination-in-disbursement-date branch from 8cef0fd to 31c717d Compare November 6, 2025 14:56
default -> throw new UnsupportedOperationException();
};
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont need this on the model, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not now, I've removed from that part

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2326/loan-contract-termination-in-disbursement-date branch from 31c717d to fa7d2fb Compare November 7, 2025 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants