FINERACT-1971: Down payment (auto) transaction using installment mult… #3873
Conversation
There was a problem hiding this comment.
Something does not add up here...
First installment is 3
2nd and 3rd are 2
4th is 3 again.
For 1st one 2.5 is the calculated but it does using force HALF_UP rounding (i guess it is hardcoded somewhere), however for 2nd and 3rd it is does not using HALF_UP, i guess it comes from a configuration which is not HALF_UP..
Please make sure all calculation is using the some rounding to avoid these situations.
There was a problem hiding this comment.
Are there clear tests for this? I think we cannot have ambiguous rounding results so tests need to capture what is expected @alberto-art3ch .
There was a problem hiding this comment.
@jdailey There are few places using the Rounding Mode fixed, fortunately almost all are related to DownPayment and now are being fixed in this PR. There are other two that I raised a ticket and we can fix these two too. Thanks
aab2062 to
14c5606
Compare
There was a problem hiding this comment.
I dont think profile do anything here..l its a static method...part of the class... :/
Also MoneyHelper has not much to do with configuration rounding mode.
Would you mind moving this logic into the ConfigurationDomainService without the profile annotation? The profile annotation do absolutely nothing here...
There was a problem hiding this comment.
Money helper init the rounding variable the first time with X value, so If you change the GC with the API or the database directly, this new value is not used until you restart Fineract
There was a problem hiding this comment.
Ok, but still the annotation can be removed, it is meaningless here
There was a problem hiding this comment.
also please either change the method name, as it is not reset the rounding mode, its fetching it via configuraiton service again, or change the implementation to set the roundingMode to NULL
There was a problem hiding this comment.
Done! Annotation and method renamed
There was a problem hiding this comment.
How can the down payment installment be 3 with HALF_DOWN rounding where 10 / 4 = 2.5 ?
There was a problem hiding this comment.
Done! This is fixed and now we are testing Half Down and Up to have different values
4dafdd6 to
53e8128
Compare
53e8128 to
227164a
Compare
There was a problem hiding this comment.
This will fail! the disbursement installment cannot be 0!
Please make sure it contains the correct amounts! (1250 in this example)
There was a problem hiding this comment.
Done! file updated and tests passed
0fc5836 to
8ea1b66
Compare
8ea1b66 to
739c4ce
Compare
…iples of setting
Description
When a Loan with Down payment (auto) transaction was ignoring installment multiples of configuration, so this was not paying the full amount
FINERACT-1971
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.