FINERACT-1992-FINERACT-Installment-level-delinquency-calculation#3515
Conversation
There was a problem hiding this comment.
Since we would not keep this entries, rather regenerate them, please remove the "History" from everywhere
There was a problem hiding this comment.
Can you help me understand more what is "firstOverdueDate"?
There was a problem hiding this comment.
As i understood it, firstOverdueDate is the date when installment became delinquent for the first time. It can be installment due date or chargeback transaction date ,which we get after delinquency calculation. I am storing it when the record is getting created for the first time and if delinquency range changes i copy it from previous record and save it in new one.
This column was defined as requirement in db ticket and this ticket as well.
ruchiD
force-pushed
the
FINERACT-1992-FINERACT-Installment-level-delinquency-calculation
branch
from
a9ead0d to
c8f0f8e
Compare
...va/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
When the loan delinquency gets calculated, the installment level delinquency can be also calculated. Would you consider to do merge them together?
There was a problem hiding this comment.
When Loan level delinquency gets calculated it calculates overdueSince date till oldest overdue installment or chargeback date for first non due installment if exists and does not calculates for rest of them. so we need to calculate for all installments separately.
There was a problem hiding this comment.
i was just wondering when we are calculating the loan delinquency and we are going through on the installments one by one, we can calculate the delinquency for installment as well. What do you think?
There was a problem hiding this comment.
Loan and installment delinquency calculation logic merged.
There was a problem hiding this comment.
We dont need to fetch the delinquency tags to be deleted. Please write a query which deletes all tags based on the loan.
ruchiD
force-pushed
the
FINERACT-1992-FINERACT-Installment-level-delinquency-calculation
branch
from
c8f0f8e to
ae321e3
Compare
There was a problem hiding this comment.
We dont keep any history, please "m_loan_installment_delinquency_tag_history" rename this table!
There was a problem hiding this comment.
Why to drop the FK for the loan? and for the range?
There was a problem hiding this comment.
done. dropped FK for installment only
There was a problem hiding this comment.
Still loading the loan for no reason (by fetching LoanInstallmentDelinquencyTag you are fetching the loan as well hence the ManyToOne relation is by default using eager fetching).
Please fetch the required loanInstallmentsDelinquencyData (LoanInstallmentDelinquencyTagData) via the repository using JPA projections:
There was a problem hiding this comment.
done. Fetching only Delinquency Range and outstanding amount for installments
There was a problem hiding this comment.
This can be optimised a little bit:
Iterate through the installments of the loan and get by id from the installmentCollectionData the entry. If there is none, go next.
This way you only iterate through on the installments only once.
There was a problem hiding this comment.
currentInstallmentDelinquencyTag is kind of meaningless... please review this logic again
...a/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java
Outdated
Show resolved
Hide resolved
...a/org/apache/fineract/portfolio/delinquency/service/DelinquencyWritePlatformServiceImpl.java
Outdated
Show resolved
Hide resolved
...java/org/apache/fineract/portfolio/delinquency/service/LoanDelinquencyDomainServiceImpl.java
Outdated
Show resolved
Hide resolved
...java/org/apache/fineract/portfolio/delinquency/service/LoanDelinquencyDomainServiceImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
i am not fully sure why we need this:
- Either you are deleting all of the tags and recalculate all of them and store
or - You are removing which is not delinquent anymore / not existing (when you calculated the new entries, you have all the information to remove the "old" ones)
There was a problem hiding this comment.
Moved this logic to delinquency calculation service.
There was a problem hiding this comment.
Please undo this. There must be a different place and way to handle it!
adamsaghy
left a comment
adamsaghy
left a comment
There was a problem hiding this comment.
Please kindly see my comments!
ruchiD
force-pushed
the
FINERACT-1992-FINERACT-Installment-level-delinquency-calculation
branch
from
ae321e3 to
0f6c22f
Compare
2 participants
Description
Changes for calculating and persisting installment level delinquency data.
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.