You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@fineract.apache.org by jroweboy <gi...@git.apache.org> on 2016/05/10 22:21:36 UTC
[GitHub] incubator-fineract pull request: FINERACT-37 Reschedule payments t...
GitHub user jroweboy opened a pull request:
https://github.com/apache/incubator-fineract/pull/110
FINERACT-37 Reschedule payments to the next payment date on holidays
A simple patch that adds a field to the database to enable rescheduling repayments that land on holidays to the next repayment date instead of a set date.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jroweboy/incubator-fineract holiday_reschedule
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-fineract/pull/110.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #110
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: FINERACT-37 Reschedule payments t...
Posted by rajuan <gi...@git.apache.org>.
Github user rajuan commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/110#discussion_r63000003
--- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/DefaultScheduledDateGenerator.java ---
@@ -96,10 +96,37 @@ public LocalDate generateNextRepaymentDate(final LocalDate lastRepaymentDate, fi
loanApplicationTerms.getNumberOfdays());
}
}
-
+ if (currentCalendar == null && holidayDetailDTO.getWorkingDays().getExtendTermForRepaymentsOnHolidays()) {
--- End diff --
What if calendar is already present and a holiday is newly added?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: FINERACT-37 Reschedule payments t...
Posted by jroweboy <gi...@git.apache.org>.
Github user jroweboy commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/110#discussion_r63062104
--- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/DefaultScheduledDateGenerator.java ---
@@ -96,10 +96,37 @@ public LocalDate generateNextRepaymentDate(final LocalDate lastRepaymentDate, fi
loanApplicationTerms.getNumberOfdays());
}
}
-
+ if (currentCalendar == null && holidayDetailDTO.getWorkingDays().getExtendTermForRepaymentsOnHolidays()) {
+ boolean repaymentDateIsOnHoliday = false;
+ // compile the list of holidays into Intervals to see if this date lands on a holiday
+ final List<Interval> holidayInterval = new ArrayList<>();
+ for (Holiday holiday : holidayDetailDTO.getHolidays()) {
+ holidayInterval.add(new Interval(
+ holiday.getFromDateLocalDate().toDateTimeAtStartOfDay(),
+ holiday.getToDateLocalDate().toDateTime(LocalTime.parse("23:59:59"))
+ ));
+ }
+ while (intervalListContainsDate(holidayInterval, dueRepaymentPeriodDate)) {
+ LocalDate previousDate = dueRepaymentPeriodDate;
+ dueRepaymentPeriodDate = getRepaymentPeriodDate(loanApplicationTerms.getRepaymentPeriodFrequencyType(),
+ loanApplicationTerms.getRepaymentEvery(), dueRepaymentPeriodDate, loanApplicationTerms.getNthDay(),
+ loanApplicationTerms.getWeekDayType());
+ if (previousDate == dueRepaymentPeriodDate) {
--- End diff --
Hmmm... I wrote this patch 6 months ago and looking at it again, I can't remember why I added this either. I will remove this then.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: FINERACT-37 Reschedule payments t...
Posted by rajuan <gi...@git.apache.org>.
Github user rajuan commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/110#discussion_r62999595
--- Diff: fineract-provider/src/main/java/org/apache/fineract/organisation/workingdays/domain/WorkingDays.java ---
@@ -74,6 +78,8 @@ public Boolean getExtendTermForDailyRepayments(){
return this.extendTermForDailyRepayments;
}
+ public Boolean getExtendTermForRepaymentsOnHolidays() { return this.extendTermForRepaymentsOnHolidays; }
+
public Map<String, Object> update(final JsonCommand command) {
--- End diff --
No code changes that would set this variable is written..... no one can change this attribute
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: FINERACT-37 Reschedule payments t...
Posted by rajuan <gi...@git.apache.org>.
Github user rajuan commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/110#discussion_r63000180
--- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/DefaultScheduledDateGenerator.java ---
@@ -18,9 +18,10 @@
*/
--- End diff --
I have generally given the comments.... I will let other code experts to comment on sufficiency of the solution and also if there are any more problems with usecases
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: FINERACT-37 Reschedule payments t...
Posted by rajuan <gi...@git.apache.org>.
Github user rajuan commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/110#discussion_r62999640
--- Diff: fineract-provider/src/main/java/org/apache/fineract/organisation/workingdays/domain/WorkingDays.java ---
@@ -41,15 +41,19 @@
@Column(name = "extend_term_daily_repayments", nullable = false)
private Boolean extendTermForDailyRepayments;
+
+ @Column(name = "extend_term_holiday_repayment", nullable = false)
+ private Boolean extendTermForRepaymentsOnHolidays;
--- End diff --
Read platform changes to be able to read this variable is required
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request #110: FINERACT-37 Reschedule payments to the...
Posted by jroweboy <gi...@git.apache.org>.
Github user jroweboy closed the pull request at:
https://github.com/apache/incubator-fineract/pull/110
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: FINERACT-37 Reschedule payments t...
Posted by rajuan <gi...@git.apache.org>.
Github user rajuan commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/110#discussion_r62999753
--- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/DefaultScheduledDateGenerator.java ---
@@ -96,10 +96,37 @@ public LocalDate generateNextRepaymentDate(final LocalDate lastRepaymentDate, fi
loanApplicationTerms.getNumberOfdays());
}
}
-
+ if (currentCalendar == null && holidayDetailDTO.getWorkingDays().getExtendTermForRepaymentsOnHolidays()) {
+ boolean repaymentDateIsOnHoliday = false;
+ // compile the list of holidays into Intervals to see if this date lands on a holiday
+ final List<Interval> holidayInterval = new ArrayList<>();
+ for (Holiday holiday : holidayDetailDTO.getHolidays()) {
+ holidayInterval.add(new Interval(
+ holiday.getFromDateLocalDate().toDateTimeAtStartOfDay(),
+ holiday.getToDateLocalDate().toDateTime(LocalTime.parse("23:59:59"))
+ ));
+ }
+ while (intervalListContainsDate(holidayInterval, dueRepaymentPeriodDate)) {
+ LocalDate previousDate = dueRepaymentPeriodDate;
+ dueRepaymentPeriodDate = getRepaymentPeriodDate(loanApplicationTerms.getRepaymentPeriodFrequencyType(),
+ loanApplicationTerms.getRepaymentEvery(), dueRepaymentPeriodDate, loanApplicationTerms.getNthDay(),
+ loanApplicationTerms.getWeekDayType());
+ if (previousDate == dueRepaymentPeriodDate) {
--- End diff --
As long as repayment period frequency type is valid, this condition will never satisfy.... I do not understand why if condition is added
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: FINERACT-37 Reschedule payments t...
Posted by jroweboy <gi...@git.apache.org>.
Github user jroweboy commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/110#discussion_r63060689
--- Diff: fineract-provider/src/main/java/org/apache/fineract/organisation/workingdays/domain/WorkingDays.java ---
@@ -74,6 +78,8 @@ public Boolean getExtendTermForDailyRepayments(){
return this.extendTermForDailyRepayments;
}
+ public Boolean getExtendTermForRepaymentsOnHolidays() { return this.extendTermForRepaymentsOnHolidays; }
+
public Map<String, Object> update(final JsonCommand command) {
--- End diff --
I apologize that I didn't mention the fact that I was asking for feedback on how I added the changes. That said, I will add in the rest of the backend changes in order to make this an acceptable PR when I get a little extra time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fineract pull request: FINERACT-37 Reschedule payments t...
Posted by jroweboy <gi...@git.apache.org>.
Github user jroweboy commented on a diff in the pull request:
https://github.com/apache/incubator-fineract/pull/110#discussion_r63061348
--- Diff: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/DefaultScheduledDateGenerator.java ---
@@ -96,10 +96,37 @@ public LocalDate generateNextRepaymentDate(final LocalDate lastRepaymentDate, fi
loanApplicationTerms.getNumberOfdays());
}
}
-
+ if (currentCalendar == null && holidayDetailDTO.getWorkingDays().getExtendTermForRepaymentsOnHolidays()) {
--- End diff --
This sounds like a design decision that I will need to ask about. But I agree that something needs to change. Imagine the case where a loan has been dispersed, and then a holiday is added that affects a future repayment. This patch does not account for this case, and I'm guessing that it should. Good catch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---