You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2021/01/05 11:17:33 UTC

[GitHub] [fineract] eitanfr opened a new pull request #1562: Draft: Fix - use expected disbursement for ideal disbursement to fix wrong interest

eitanfr opened a new pull request #1562:
URL: https://github.com/apache/fineract/pull/1562


   **ISSUE**
   My issue/bug happens when I am setting "First repayment on" field I get a repayment schedule with a wrong interest rate for the first installment.
   Example: When I set "First repayment on" to 01/02/21 and expected disbursement is 27/1/21(Frist image) then I get the wrong interest.The first installment interest is 21,232.88$ (second Image) - calculated for the entire month (31 days) instead of 5 days (the time from the disbursement to the first repayment), the right interest should be 3424.75$.
   
   **After debugging:**
   In AbstractLoanScheduleGenerator there is a function `calculateInterestStartDateForPeriod` - it uses the `idealDisbursment` for setting the interest start date for the first installment, The `idealDisbursment` is calculated by `idealDisbursementDateBasedOnFirstRepaymentDate` which does [`firstRepaymentDate` - 1 repayment Period Frequency] aka, for 01/02/21 with months as repyment frequency it will return 01/01/21.
   
   So when we dont set the "First repayment on" the `idealDisbursment` will be the expected disbursement date because the first repayment is always `expectedDisbursment`+1. But if we set the  "First repayment on" we get the wrong value for the `idealDisbursment` - in my example we got 01/0/21 and this is why the first installment interest was calculated for an entire month instead of  5 days.
   
   **Is it a bug or not?**
   I am not sure why `idealDisbursementDateBasedOnFirstRepaymentDate` exists, the simple solution is to do  idealDisbursment=expectedDisbursmentDate.I tried it locally and looks like it works, I will be happy to make a PR, but want to make sure indeed that the `idealDisbursementDateBasedOnFirstRepaymentDate` is redundant.
   
   @vorburger 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] awasum commented on pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-774006991


   @eitanfr  Travis fails this PR...Do you want to take and look and see if you can fix the issue?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] eitanfr commented on pull request #1562: Draft: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
eitanfr commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-761808359


   > https://lists.apache.org/thread.html/raf85df08b65ced14ae6294ea5af0695c5124d196389c9cccf388b848%40%3Cdev.fineract.apache.org%3E thread seems to related to this PR? Given @bharathc27 (?) reply there, would you still like this to be merged?
   
   As I understood Bharath mentioned a way for a workaround around the issue, but the issue still exists... 
   I think its need to be merged, and it works fine when I tested it, With that said I want to mention that I am still a Fineract beginner :)
   
   
   I will rebase soon,
   
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] bharathcgowda commented on pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
bharathcgowda commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-775876714


   @eitanfr
   Just wanted to mention the functional scenarios again here
   
   Scenario 1
   when "**interest calculation period**" is set to "**daily**" then interest has to calculate from the **Disbursement date**
   
   Scenario 2
   when "**interest calculation period**" is set to "**same as repayment**" and  "Calculate interest for exact days in partial period" checkbox is **selected** then interest has to calculate from the **Disbursement date**
   
   Scenario 3
   when "**interest calculation period**" is set to "**same as repayment**" and  "Calculate interest for exact days in partial period" checkbox is **not selected** then interest has to calculate for that month.
   
   As long as this fix doesn't break these 3 scenarios, it should be fine. 
   
   Also, I would suggest a thorough test with different configurations before making any changes to the interest calculation functionality.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] github-actions[bot] commented on pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-839346449


   This pull request seems to be stale.  Are you still planning to work on it?  We will automatically close it in 30 days.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger commented on pull request #1562: Draft: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-761778854


   https://lists.apache.org/thread.html/raf85df08b65ced14ae6294ea5af0695c5124d196389c9cccf388b848%40%3Cdev.fineract.apache.org%3E thread seems to related to this PR? Given @bharathc27 (?) reply there, would you still like this to be merged?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger commented on pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-766088206


   @eitanfr I would merge this PR if you could please create a bug about it with the description above on https://issues.apache.org/jira/ ? Only so that we have Release Notes for it in the upcoming 1.5.0 (we derive those from JIRA, not the commits).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] edcable commented on pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
edcable commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-790151654


   @Nayan could Pramod Sharma from your team help in doing the functional review requested above by Bharath. 
   
   @eitanfr have you had a chance to test against these scenarios yet? 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger commented on pull request #1562: Draft: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-761779172


   /rebase


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger edited a comment on pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
vorburger edited a comment on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-774773557


   > @eitanfr Travis fails this PR...Do you want to take and look and see if you can fix the issue?
   
   @eitanfr it's just because of a Spotless violation. Do you know how to find the log? It says what you have to do...


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger commented on pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-766088309


   And you should amend the git commit to refer to that JIRA - see the convention documented on https://github.com/apache/fineract#pull-requests.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] github-actions[bot] commented on pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-812762031


   This pull request seems to be stale.  Are you still planning to work on it?  We will automatically close it in 30 days.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] eitanfr commented on pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
eitanfr commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-770395590


   Hi, 
   done and done: https://issues.apache.org/jira/browse/FINERACT-1307


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] github-actions[bot] closed pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #1562:
URL: https://github.com/apache/fineract/pull/1562


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger commented on pull request #1562: Draft: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-761150450


   @eitanfr my functional understanding is actually not deep enough to be able to easily review this. Wondering if perhaps @ptuomola or @vidakovic or @awasum would like to chime in here? Otherwise this probably a good kind of topic to take the mailing list. Having said all that, you appear to have a good grasp of what's going here, so I would merge this PR once it passes the build, and if you could please create a bug about it with the description above on https://issues.apache.org/jira/ - we also operate on "trust" in this project, so I'm pretty open to merging a PR like this.
   
   PS: All PR builds are currently broken due to FINERACT-924, just FYI.
   
   PPS: Sorry it took me a while to respond here; been busy at my day job work.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] eitanfr commented on pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
eitanfr commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-817269245


   @bharathcgowda @edcable 
   Hi, as you guys requested :)
   
   **First Scenario:**
   when "interest calculation period" is set to "daily" then interest has to calculate from the Disbursement date.
   Expected:
   ![image](https://user-images.githubusercontent.com/9163916/114296763-b9174480-9ab5-11eb-80fe-58c0deab4b45.png)
   
   Works as Expected:
   ![image](https://user-images.githubusercontent.com/9163916/114296639-0e9f2180-9ab5-11eb-8089-007b4f36406c.png)
   
   **Second Scenario:**
   when "interest calculation period" is set to "same as repayment" and "Calculate interest for exact days in partial period" checkbox is selected then interest has to calculate from the Disbursement date
   Expected:
   ![image](https://user-images.githubusercontent.com/9163916/114296972-bd902d00-9ab6-11eb-9614-9163eeccdab9.png)
   
   Works as Expected:
   ![image](https://user-images.githubusercontent.com/9163916/114296977-ca148580-9ab6-11eb-966f-931a90409878.png)
   
   **Third Scenario**
   when "interest calculation period" is set to "same as repayment" and "Calculate interest for exact days in partial period" checkbox is not selected then interest has to calculate for that month.
   Expected:
   ![image](https://user-images.githubusercontent.com/9163916/114297072-44450a00-9ab7-11eb-808d-554899431ddc.png)
   
   Works as Expected:
   ![image](https://user-images.githubusercontent.com/9163916/114297083-4c9d4500-9ab7-11eb-945f-80c3712990cd.png)
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger commented on pull request #1562: Fix - use expected disbursement for ideal disbursement to fix wrong interest

Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1562:
URL: https://github.com/apache/fineract/pull/1562#issuecomment-774773557


   > @eitanfr Travis fails this PR...Do you want to take and look and see if you can fix the issue?
   
   @eitanfr it's because of a Spotless violation? Do you know how to find the log? It says what you have to do...


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org