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-2148: Process monetary transaction after zero interest charge off #4236

Conversation

mariiaKraievska
Copy link
Contributor

@mariiaKraievska mariiaKraievska commented Dec 24, 2024

Description

Continue work made in the PR - #4227

"In case a repayment or any other monetary activity happens, after the charge-off, we dont need to reprocess all the transactions and fetch the ProgressiveLoanInterestScheduleModel and use the EmiCalculator, we can simply use the existing repayment periods and process the transaction

In the AdvancedPaymentScheduleTransactionProcessor should handle like the loan does not do interest recalculation!"

And fix Reverse-replayed issue.

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.

@@ -3569,4 +3569,8 @@ public LoanRepaymentScheduleTransactionProcessor getTransactionProcessor() {
public boolean isProgressiveSchedule() {
return getLoanProductRelatedDetail().getLoanScheduleType() == PROGRESSIVE;
}

public boolean isTransactionBeforeChargeOff(final boolean isChargedOff) {
return !isChargedOff || !LoanChargeOffBehaviour.ZERO_INTEREST.equals(this.getLoanProductRelatedDetail().getChargeOffBehaviour());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this !LoanChargeOffBehaviour.ZERO_INTEREST.equals(this.getLoanProductRelatedDetail().getChargeOffBehaviour() condition?

I think this whole method can be switched with !ctx.isChargedOff().

Am I missing something?

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 condition !LoanChargeOffBehaviour.ZERO_INTEREST.equals(this.getLoanProductRelatedDetail().getChargeOffBehaviour()) was added as a safeguard for future scenarios where someone might set isChargedOff to true when the chargeOffBehaviour is not ZERO_INTEREST.

By including this specific check, we avoid potential issues where the logic could break or produce unexpected results due to changes or incorrect assumptions about the relationship between chargeOffBehaviour and the isChargedOff flag. Replacing this with !ctx.isChargedOff() could simplify the code but would remove this safeguard, potentially introducing bugs if this edge case arises.

Copy link
Contributor

Choose a reason for hiding this comment

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

When the loan got charged-off from that moment we dont wanna recalculate interest for any charge-off behaviours, so i would say we are safe to go without the 2nd condition ;)

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

@mariiaKraievska mariiaKraievska force-pushed the FINERACT-2148/process-monetary-transaction-after-zero-interest-charge-off branch from 4b9e03b to d1fd60a Compare December 30, 2024 16:01
@adamsaghy adamsaghy merged commit faaf44d into apache:develop Jan 1, 2025
10 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
Development

Successfully merging this pull request may close these issues.

3 participants