Skip to content

FINERACT-1992: Multi Delinquency event for installment range change#3610

Merged
adamsaghy merged 1 commit intoapache:developfrom
ruchiD:FINERACT-1992-Multi-Deliquency-event-for-installment-range-change
Nov 30, 2023
Merged

FINERACT-1992: Multi Delinquency event for installment range change#3610
adamsaghy merged 1 commit intoapache:developfrom
ruchiD:FINERACT-1992-Multi-Deliquency-event-for-installment-range-change

Conversation

@ruchiD
Copy link
Contributor

@ruchiD ruchiD commented Nov 28, 2023

Description

Change made to emit range change event for installment level range changes.

Describe the changes made and why they were made.

Ignore if these details are present on the associated Apache Fineract JIRA ticket.

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we raise an event also if the ranges are not changed just the amount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adamsaghy Please comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

No need.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have tests for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, will add one.

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.

@ruchiD ruchiD force-pushed the FINERACT-1992-Multi-Deliquency-event-for-installment-range-change branch from fd43679 to a7befb0 Compare November 29, 2023 12:14
}
}
// remove tags for non existing installments that got deleted due to re-schedule
removeDelinquencyTagsForNonExistingInstallments(loan.getId());
Copy link
Contributor

Choose a reason for hiding this comment

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

If an installment is deleted and we delete the related delinquency installment, then we might also want to raise an event, don't we?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These installments are non existing for the loan. Reschedule/Disbursal flow creates new installments for loan and deletes previous ones for loan, but tags for old non existing installments exits and needs to be cleaned up. No need to raise event.

Copy link
Contributor

@reluxa reluxa 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 570b67b into apache:develop Nov 30, 2023
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