You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "janez89 (via GitHub)" <gi...@apache.org> on 2024/09/05 19:32:28 UTC
[PR] FINERACT-2081: Fix backdated multi disbursement on Progressive Loan [fineract]
janez89 opened a new pull request, #4048:
URL: https://github.com/apache/fineract/pull/4048
This P.R. fixes that case when there is a multi-disbursement progressive loan and the second disbursement date is earlier than the first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2081: Fix backdated multi disbursement on Progressive Loan [fineract]
Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #4048:
URL: https://github.com/apache/fineract/pull/4048#discussion_r1754269878
##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/ProgressiveLoanScheduleGenerator.java:
##########
@@ -103,11 +104,15 @@ public LoanScheduleModel generate(final MathContext mc, final LoanApplicationTer
prepareDisbursementsOnLoanApplicationTerms(loanApplicationTerms);
+ final ArrayList<DisbursementData> disbursementDataList = new ArrayList<>(loanApplicationTerms.getDisbursementDatas());
+ disbursementDataList.sort(Comparator.comparing(DisbursementData::disbursementDate));
Review Comment:
noted, do you think we could create a test-case to cover the "same disbursement day" behavior?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2081: Fix backdated multi disbursement on Progressive Loan [fineract]
Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #4048:
URL: https://github.com/apache/fineract/pull/4048#discussion_r1746661832
##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/ProgressiveLoanScheduleGenerator.java:
##########
@@ -103,11 +104,15 @@ public LoanScheduleModel generate(final MathContext mc, final LoanApplicationTer
prepareDisbursementsOnLoanApplicationTerms(loanApplicationTerms);
+ final ArrayList<DisbursementData> disbursementDataList = new ArrayList<>(loanApplicationTerms.getDisbursementDatas());
+ disbursementDataList.sort(Comparator.comparing(DisbursementData::disbursementDate));
Review Comment:
Is disbursementDate a Date or a Timestamp? Is it possible that they get same "disbursementDate"s? If so, we should fall back to a secondary sort attribute like created date/etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2081: Fix backdated multi disbursement on Progressive Loan [fineract]
Posted by "janez89 (via GitHub)" <gi...@apache.org>.
janez89 commented on code in PR #4048:
URL: https://github.com/apache/fineract/pull/4048#discussion_r1754637471
##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/ProgressiveLoanScheduleGenerator.java:
##########
@@ -103,11 +104,15 @@ public LoanScheduleModel generate(final MathContext mc, final LoanApplicationTer
prepareDisbursementsOnLoanApplicationTerms(loanApplicationTerms);
+ final ArrayList<DisbursementData> disbursementDataList = new ArrayList<>(loanApplicationTerms.getDisbursementDatas());
+ disbursementDataList.sort(Comparator.comparing(DisbursementData::disbursementDate));
Review Comment:
yes, I added an extra case to the integration test and the EMI calculator to cover the same date scenario.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2081: Fix backdated multi disbursement on Progressive Loan [fineract]
Posted by "janez89 (via GitHub)" <gi...@apache.org>.
janez89 commented on code in PR #4048:
URL: https://github.com/apache/fineract/pull/4048#discussion_r1754144705
##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/ProgressiveLoanScheduleGenerator.java:
##########
@@ -103,11 +104,15 @@ public LoanScheduleModel generate(final MathContext mc, final LoanApplicationTer
prepareDisbursementsOnLoanApplicationTerms(loanApplicationTerms);
+ final ArrayList<DisbursementData> disbursementDataList = new ArrayList<>(loanApplicationTerms.getDisbursementDatas());
+ disbursementDataList.sort(Comparator.comparing(DisbursementData::disbursementDate));
Review Comment:
Okay, I understand.
The secondary sorting occurs in the original order in the submitted JSON array, just like before.
If the date matches, we will retrieve the identical order as previously submitted by the client.
From a calculation perspective, if we have multiple disbursements on the same date, the amounts of those disbursements are accumulated. The "addition" operation is associative, so it doesn't matter which disbursement came first on the same day.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: FINERACT-2081: Fix backdated multi disbursement on Progressive Loan [fineract]
Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #4048:
URL: https://github.com/apache/fineract/pull/4048#discussion_r1747090180
##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/ProgressiveLoanScheduleGenerator.java:
##########
@@ -103,11 +104,15 @@ public LoanScheduleModel generate(final MathContext mc, final LoanApplicationTer
prepareDisbursementsOnLoanApplicationTerms(loanApplicationTerms);
+ final ArrayList<DisbursementData> disbursementDataList = new ArrayList<>(loanApplicationTerms.getDisbursementDatas());
+ disbursementDataList.sort(Comparator.comparing(DisbursementData::disbursementDate));
Review Comment:
I mean what if there are two disbursements on the same date? Which one comes first, do we know?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: FINERACT-2081: Fix backdated multi disbursement on Progressive Loan [fineract]
Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #4048:
URL: https://github.com/apache/fineract/pull/4048#discussion_r1746699383
##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/ProgressiveLoanScheduleGenerator.java:
##########
@@ -103,11 +104,15 @@ public LoanScheduleModel generate(final MathContext mc, final LoanApplicationTer
prepareDisbursementsOnLoanApplicationTerms(loanApplicationTerms);
+ final ArrayList<DisbursementData> disbursementDataList = new ArrayList<>(loanApplicationTerms.getDisbursementDatas());
+ disbursementDataList.sort(Comparator.comparing(DisbursementData::disbursementDate));
Review Comment:
Can you pls explain how it would handle a secondary sorting in case there's a collision in date?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: FINERACT-2081: Fix backdated multi disbursement on Progressive Loan [fineract]
Posted by "janez89 (via GitHub)" <gi...@apache.org>.
janez89 commented on code in PR #4048:
URL: https://github.com/apache/fineract/pull/4048#discussion_r1746715679
##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/ProgressiveLoanScheduleGenerator.java:
##########
@@ -103,11 +104,15 @@ public LoanScheduleModel generate(final MathContext mc, final LoanApplicationTer
prepareDisbursementsOnLoanApplicationTerms(loanApplicationTerms);
+ final ArrayList<DisbursementData> disbursementDataList = new ArrayList<>(loanApplicationTerms.getDisbursementDatas());
+ disbursementDataList.sort(Comparator.comparing(DisbursementData::disbursementDate));
Review Comment:
It could be easier to talk about the purpose of that code.
It's sorting the disbursement based on their effect date. Until there is no "actual date" it will use the expected date, and if disbursed it will use the actual date. The goal is just to apply disbursements in chronological order
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: FINERACT-2081: Fix backdated multi disbursement on Progressive Loan [fineract]
Posted by "janez89 (via GitHub)" <gi...@apache.org>.
janez89 commented on code in PR #4048:
URL: https://github.com/apache/fineract/pull/4048#discussion_r1746703233
##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/ProgressiveLoanScheduleGenerator.java:
##########
@@ -103,11 +104,15 @@ public LoanScheduleModel generate(final MathContext mc, final LoanApplicationTer
prepareDisbursementsOnLoanApplicationTerms(loanApplicationTerms);
+ final ArrayList<DisbursementData> disbursementDataList = new ArrayList<>(loanApplicationTerms.getDisbursementDatas());
+ disbursementDataList.sort(Comparator.comparing(DisbursementData::disbursementDate));
Review Comment:
I'm not sure what you mean by "secondary sorting" ? If this logic runs again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: FINERACT-2081: Fix backdated multi disbursement on Progressive Loan [fineract]
Posted by "janez89 (via GitHub)" <gi...@apache.org>.
janez89 commented on code in PR #4048:
URL: https://github.com/apache/fineract/pull/4048#discussion_r1746697918
##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/ProgressiveLoanScheduleGenerator.java:
##########
@@ -103,11 +104,15 @@ public LoanScheduleModel generate(final MathContext mc, final LoanApplicationTer
prepareDisbursementsOnLoanApplicationTerms(loanApplicationTerms);
+ final ArrayList<DisbursementData> disbursementDataList = new ArrayList<>(loanApplicationTerms.getDisbursementDatas());
+ disbursementDataList.sort(Comparator.comparing(DisbursementData::disbursementDate));
Review Comment:
The `DisbursementData::disbursementDate` is a LocalDate. This method will handle the fallback situation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2081: Fix backdated multi disbursement on Progressive Loan [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #4048:
URL: https://github.com/apache/fineract/pull/4048
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org