Semi-Monthly Feature (FINERACT-1322)#1884
Conversation
FINERACT-1322: Semi-Monthly Feature
ptuomola
left a comment
ptuomola
left a comment
There was a problem hiding this comment.
Also have we tested what happens if one of the semi dates falls on a date which does not exist in every month - i.e. 31st? Does the logic work sensibly in that case as well?
| productRelatedDetail.getRepayEvery(), productRelatedDetail.getRepaymentPeriodFrequencyType().getValue(), | ||
| newLoanApplication); | ||
| // this.fromApiJsonDeserializer.validateLoanTermAndRepaidEveryValues(newLoanApplication.getTermFrequency(), | ||
| // newLoanApplication.getTermPeriodFrequencyType(), productRelatedDetail.getNumberOfRepayments(), |
There was a problem hiding this comment.
Why is this validation commented out?
| assertEquals(Float.parseFloat(period.get("totalOriginalDueForPeriod").toString()), | ||
| expectedRepaymentSchedule.getTotalOriginalDueForPeriod().floatValue(), 0.0f); | ||
| expectedRepaymentSchedule.getTotalOriginalDueForPeriod().floatValue(), 0.1f); | ||
|
|
There was a problem hiding this comment.
Why have these test results changed? I would not expect the existing integration tests to have to change when adding new products...
| final Collection<CalendarData> calendarOptions = null; | ||
| final Collection<StaffData> loanOfficerOptions = null; | ||
|
|
||
| final EnumOptionData termPeriodFrequencyType = product.getRepaymentFrequencyType(); |
| final Integer minimumGapBetweenInstallments, final Integer maximumGapBetweenInstallments, | ||
| final boolean syncExpectedWithDisbursementDate, final boolean canUseForTopup, final boolean isEqualAmortization, | ||
| Collection<RateData> rateOptions, Collection<RateData> rates, final boolean isRatesEnabled) { | ||
| Collection<RateData> rateOptions, Collection<RateData> rates, final boolean isRatesEnabled, final LocalDate firstDateForSemi, |
There was a problem hiding this comment.
Why rateOptions and rates are not final? Any reassignment?
|
|
||
| assertEquals(period.get("dueDate").toString(), expectedRepaymentSchedule.getDueDate()); | ||
| assertEquals(period.get("principalLoanBalanceOutstanding"), expectedRepaymentSchedule.getPrincipalLoanBalanceOutstanding()); | ||
| assertEquals(Float.parseFloat(period.get("principalLoanBalanceOutstanding").toString()), |
There was a problem hiding this comment.
Why converted to a Float comparison?
|
@Cadreia , Please resolve merge conflicts here and address review comments above. |
|
This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days. |
|
@Cadreia Could you please address the review comments? |
|
This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days. |
|
@Cadreia Please fix review comments and merge conflicts so this can be reviewed and merged. Important feature. |
|
This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days. |
4 participants
Description
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/api-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.