Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FINERACT-1971: RepaymentOverDueBusinessEvent should not be sent when … #3856

Conversation

alberto-art3ch
Copy link
Contributor

@alberto-art3ch alberto-art3ch commented Apr 10, 2024

…balance is 0

Description

Approved (orphaned) accounts with no disbursement are triggering RepaymentOverdueDueBusinessEvent reminder event after their maturity date.

FINERACT-1971

Screenshot 2024-04-12 at 7 22 04 a 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.

@@ -933,6 +936,35 @@ public void saleExceptionHandling() {
}
}

@Test
public void buybackOnATobeDeclinedPendingSale() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this test case as it is part of an another PR.

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! Test case removed from this class

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 kindly review my comments!

@@ -175,4 +184,27 @@ public void givenLoanWithInstallmentOverdueAfterConfiguredDaysInLoanProductWhenS
assertEquals(repaymentInstallment, loanPayloadForEvent);
assertEquals(processedLoan, loanForProcessing);
}

@Test
public void givenNonDisbursedLoanWhenStepExecutionThenNoBusinessEventIsRaised() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not fully understand this test case, but please be sure to cover the following:
GIVEN Loan is SUBMITTED or APPROVED and outstanding amount is greater than ZERO and one of the instalment is OVERDUE
WHEN job got executed
THEN no overdue event

GIVEN Loan is ACTIVE, outstanding amount is ZERO, one of the installment is due date is matching with the actual date
WHEN job got executed
THEN no overdue event triggered

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! Test cases created:

givenActiveLoanWithZeroOutstandingWhenStepExecutionThenNoBusinessEventIsRaised
givenActiveLoanWithNonZeroOutstandingWhenStepExecutionThenBusinessEventIsRaised

@alberto-art3ch alberto-art3ch force-pushed the fix/loan_repayment_overdue_event_non_zero_outstanding branch from 8a49bd5 to fdd362e Compare April 10, 2024 21:56
}

@Test
public void givenActiveLoanWithNonZeroOutstandingWhenStepExecutionThenBusinessEventIsRaised() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont see test case to check whether the event got not raised if the loan status is SUBMITTED or APPROVED

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! Unit Test created:

givenSubmittedLoanWhenStepExecutionThenNoBusinessEventIsRaised
givenApprovedLoanWhenStepExecutionThenNoBusinessEventIsRaised

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 kindly see my comments!

@alberto-art3ch alberto-art3ch force-pushed the fix/loan_repayment_overdue_event_non_zero_outstanding branch from fdd362e to fe1c916 Compare April 12, 2024 07:16
private static boolean isOverDueEventNeededToBeSent(Loan loan, Long numberOfDaysBeforeDueDateToRaiseEvent, LocalDate currentDate,
LoanRepaymentScheduleInstallment repaymentScheduleInstallment, LocalDate repaymentDate) {
return repaymentDate.plusDays(numberOfDaysBeforeDueDateToRaiseEvent).equals(currentDate)
&& loan.getLoanSummary().getTotalOutstanding().compareTo(BigDecimal.ZERO) > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind to move the loan.getLoanSummary().getTotalOutstanding().compareTo(BigDecimal.ZERO) > 0 check to the top, next to the loan status check?

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!

@alberto-art3ch alberto-art3ch force-pushed the fix/loan_repayment_overdue_event_non_zero_outstanding branch from fe1c916 to dfe5483 Compare April 12, 2024 13:24
@alberto-art3ch alberto-art3ch force-pushed the fix/loan_repayment_overdue_event_non_zero_outstanding branch from dfe5483 to b68840a Compare April 12, 2024 16:54
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.

LGTM

@adamsaghy adamsaghy merged commit 99ba463 into apache:develop Apr 15, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants