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/11/04 20:54:20 UTC

[GitHub] [fineract] rrpawar96 opened a new pull request #1940: FINERACT-1374: Withdraw-charge-discount

rrpawar96 opened a new pull request #1940:
URL: https://github.com/apache/fineract/pull/1940


   ## Description
   
   Describe the changes made and why they were made.
   
   Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
   
   
   ## 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.
   


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



[GitHub] [fineract] francisguchie commented on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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


   @rrpawar96 @BLasan  
   I would like to say that i have already done a funtional (user) test and its working fine. 
   What we need to add in the next PR is Deposit charges to be added on the list of charge time type 


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



[GitHub] [fineract] awasum commented on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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


   @rrpawar96  Please address comment by @ptuomola above...
   
   Trying to reopen to trigger Travis.


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



[GitHub] [fineract] ptuomola closed pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
ptuomola closed pull request #1940:
URL: https://github.com/apache/fineract/pull/1940


   


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



[GitHub] [fineract] rrpawar96 commented on a change in pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
rrpawar96 commented on a change in pull request #1940:
URL: https://github.com/apache/fineract/pull/1940#discussion_r743399090



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1066,15 +1070,98 @@ public BigDecimal calculateWithdrawalFee(final BigDecimal transactionAmount) {
         return result;
     }
 
-    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user) {
+    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user,
+            final PaymentDetail paymentDetail) {
+
         for (SavingsAccountCharge charge : this.charges()) {
+
             if (charge.isWithdrawalFee() && charge.isActive()) {
-                charge.updateWithdralFeeAmount(transactionAmount);
-                this.payCharge(charge, charge.getAmountOutstanding(this.getCurrency()), transactionDate, user);
+                if (charge.getFreeWithdrawalCount() == null) {
+                    charge.setFreeWithdrawalCount(0);
+                }
+
+                if (charge.isEnableFreeWithdrawal()) {
+                    resetFreeChargeDaysCount(charge, transactionAmount, transactionDate, user);

Review comment:
       @ptuomola, Yes, Thanks for pointing this out, I am testing it again with different conditions, resolving at soonest, sorry for getting back late.




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



[GitHub] [fineract] awasum closed pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
awasum closed pull request #1940:
URL: https://github.com/apache/fineract/pull/1940


   


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



[GitHub] [fineract] ptuomola commented on a change in pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #1940:
URL: https://github.com/apache/fineract/pull/1940#discussion_r741547343



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1066,15 +1070,98 @@ public BigDecimal calculateWithdrawalFee(final BigDecimal transactionAmount) {
         return result;
     }
 
-    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user) {
+    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user,
+            final PaymentDetail paymentDetail) {
+
         for (SavingsAccountCharge charge : this.charges()) {
+
             if (charge.isWithdrawalFee() && charge.isActive()) {
-                charge.updateWithdralFeeAmount(transactionAmount);
-                this.payCharge(charge, charge.getAmountOutstanding(this.getCurrency()), transactionDate, user);
+                if (charge.getFreeWithdrawalCount() == null) {
+                    charge.setFreeWithdrawalCount(0);
+                }
+
+                if (charge.isEnableFreeWithdrawal()) {
+                    resetFreeChargeDaysCount(charge, transactionAmount, transactionDate, user);

Review comment:
       But what about the case where free withdrawal is _not_ enabled - ie current behaviour?
   
   Currently this function would do the charge by calling this.payCharge(). But this is now removed. So where does the charging now happen?
   
   In other words, do you need an else statement here with:
   
   else  {
   charge.updateWithdralFeeAmount(transactionAmount);
   this.payCharge(charge, charge.getAmountOutstanding(this.getCurrency()), transactionDate, user);
   }




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



[GitHub] [fineract] ptuomola commented on a change in pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #1940:
URL: https://github.com/apache/fineract/pull/1940#discussion_r741547343



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1066,15 +1070,98 @@ public BigDecimal calculateWithdrawalFee(final BigDecimal transactionAmount) {
         return result;
     }
 
-    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user) {
+    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user,
+            final PaymentDetail paymentDetail) {
+
         for (SavingsAccountCharge charge : this.charges()) {
+
             if (charge.isWithdrawalFee() && charge.isActive()) {
-                charge.updateWithdralFeeAmount(transactionAmount);
-                this.payCharge(charge, charge.getAmountOutstanding(this.getCurrency()), transactionDate, user);
+                if (charge.getFreeWithdrawalCount() == null) {
+                    charge.setFreeWithdrawalCount(0);
+                }
+
+                if (charge.isEnableFreeWithdrawal()) {
+                    resetFreeChargeDaysCount(charge, transactionAmount, transactionDate, user);

Review comment:
       But what about the case where free withdrawal is _not_ enabled - ie current behaviour?
   
   Currently this function would do the charge by calling this.payCharge(). But this is now removed. So where does the charging now happen?
   
   In other words, do you need an else statement here with:
   
   else  {
   charge.updateWithdralFeeAmount(transactionAmount);
   this.payCharge(charge, charge.getAmountOutstanding(this.getCurrency()), transactionDate, user);
   }




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



[GitHub] [fineract] ptuomola merged pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
ptuomola merged pull request #1940:
URL: https://github.com/apache/fineract/pull/1940


   


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



[GitHub] [fineract] francisguchie edited a comment on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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


   @rrpawar96 @BLasan  
   I would like to say that i have already done a funtional (user) test and its working fine. 
   What we need to add in the next PR is Deposit charges to be added on the list of charge time type 
   
   ![image](https://user-images.githubusercontent.com/22683654/139670623-e611157a-ce3f-4421-8ebb-f57628861e6b.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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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



[GitHub] [fineract] rrpawar96 commented on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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


   @ptuomola , @awasum Yes I was testing with different scenarios which will also be sync with issue [FINERACT-1390](https://issues.apache.org/jira/browse/FINERACT-1390) debug it as soonest. 


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



[GitHub] [fineract] rrpawar96 commented on a change in pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
rrpawar96 commented on a change in pull request #1940:
URL: https://github.com/apache/fineract/pull/1940#discussion_r739614585



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1066,15 +1070,98 @@ public BigDecimal calculateWithdrawalFee(final BigDecimal transactionAmount) {
         return result;
     }
 
-    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user) {
+    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user,
+            final PaymentDetail paymentDetail) {
+
         for (SavingsAccountCharge charge : this.charges()) {
+
             if (charge.isWithdrawalFee() && charge.isActive()) {
-                charge.updateWithdralFeeAmount(transactionAmount);
-                this.payCharge(charge, charge.getAmountOutstanding(this.getCurrency()), transactionDate, user);
+                if (charge.getFreeWithdrawalCount() == null) {
+                    charge.setFreeWithdrawalCount(0);
+                }
+
+                if (charge.isEnableFreeWithdrawal()) {
+                    resetFreeChargeDaysCount(charge, transactionAmount, transactionDate, user);
+
+                }
+            }
+
+        }
+    }
+
+    private void resetFreeChargeDaysCount(SavingsAccountCharge charge, final BigDecimal transactionAmount, final LocalDate transactionDate,
+            final AppUser user) {
+        Date resetDate = charge.getResetChargeDate();
+
+        Integer restartPeriod = charge.getRestartFrequency();

Review comment:
       The logic of the PR is to compare the interval between the current period and the lastdate or Month when it was got reset.
   (but if the lastdate is null such that it is first-time is getting reset then it will set the current date as resetdate)
   then it will do the addition of monthvalue of lastresetdate and restartPeriod(which will be taken from the user, like after which month it should reset) 
   
   if that calculated period exceeds the gap between the currentMonth and lastResetMonth then it will reset it.
   
   till than period it will also make sure that it will keep a count (+1) how many times the withdrawal transaction was done free, if that count exceeds with the freeWithdrawalCount which is given by user then it will start taking charges until the restartPeriod gets reset as explained above. 
   
   Once it gets reset then it will start the count from 0 until it reaches to the frequency of freeWithdrawalCount given by user.
   




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



[GitHub] [fineract] rrpawar96 commented on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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






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



[GitHub] [fineract] awasum commented on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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


   @rrpawar96  Please address comment by @ptuomola above...
   
   Trying to reopen to trigger Travis.


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



[GitHub] [fineract] rrpawar96 commented on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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


   @ptuomola I have kept a separate commit of fix test-case to ease review once approved will merge into it.


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



[GitHub] [fineract] awasum closed pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
awasum closed pull request #1940:
URL: https://github.com/apache/fineract/pull/1940


   


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



[GitHub] [fineract] rrpawar96 commented on a change in pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
rrpawar96 commented on a change in pull request #1940:
URL: https://github.com/apache/fineract/pull/1940#discussion_r743399090



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1066,15 +1070,98 @@ public BigDecimal calculateWithdrawalFee(final BigDecimal transactionAmount) {
         return result;
     }
 
-    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user) {
+    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user,
+            final PaymentDetail paymentDetail) {
+
         for (SavingsAccountCharge charge : this.charges()) {
+
             if (charge.isWithdrawalFee() && charge.isActive()) {
-                charge.updateWithdralFeeAmount(transactionAmount);
-                this.payCharge(charge, charge.getAmountOutstanding(this.getCurrency()), transactionDate, user);
+                if (charge.getFreeWithdrawalCount() == null) {
+                    charge.setFreeWithdrawalCount(0);
+                }
+
+                if (charge.isEnableFreeWithdrawal()) {
+                    resetFreeChargeDaysCount(charge, transactionAmount, transactionDate, user);

Review comment:
       @ptuomola, Yes, Thanks for pointing this out, I am testing it again with different conditions, resolving at soonest, sorry for getting back late.




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



[GitHub] [fineract] awasum closed pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
awasum closed pull request #1940:
URL: https://github.com/apache/fineract/pull/1940


   


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



[GitHub] [fineract] ptuomola commented on a change in pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #1940:
URL: https://github.com/apache/fineract/pull/1940#discussion_r741547343



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1066,15 +1070,98 @@ public BigDecimal calculateWithdrawalFee(final BigDecimal transactionAmount) {
         return result;
     }
 
-    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user) {
+    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user,
+            final PaymentDetail paymentDetail) {
+
         for (SavingsAccountCharge charge : this.charges()) {
+
             if (charge.isWithdrawalFee() && charge.isActive()) {
-                charge.updateWithdralFeeAmount(transactionAmount);
-                this.payCharge(charge, charge.getAmountOutstanding(this.getCurrency()), transactionDate, user);
+                if (charge.getFreeWithdrawalCount() == null) {
+                    charge.setFreeWithdrawalCount(0);
+                }
+
+                if (charge.isEnableFreeWithdrawal()) {
+                    resetFreeChargeDaysCount(charge, transactionAmount, transactionDate, user);

Review comment:
       But what about the case where free withdrawal is _not_ enabled - ie current behaviour?
   
   Currently this function would do the charge by calling this.payCharge(). But this is now removed. So where does the charging now happen?
   
   In other words, do you need an else statement here with:
   
   else  {
   charge.updateWithdralFeeAmount(transactionAmount);
   this.payCharge(charge, charge.getAmountOutstanding(this.getCurrency()), transactionDate, user);
   }




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



[GitHub] [fineract] BLasan commented on a change in pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1940:
URL: https://github.com/apache/fineract/pull/1940#discussion_r739632375



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccountCharge.java
##########
@@ -94,6 +94,13 @@
     @Column(name = "charge_calculation_enum")
     private Integer chargeCalculation;
 
+    @Column(name = "free_withdrawal_count", nullable = true)
+    private Integer freeWithdrawalCount;
+
+    @Temporal(TemporalType.DATE)
+    @Column(name = "charge_reset_date", nullable = true)
+    protected Date chargeResetDate;

Review comment:
       Why `protected` here?




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



[GitHub] [fineract] rrpawar96 commented on a change in pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
rrpawar96 commented on a change in pull request #1940:
URL: https://github.com/apache/fineract/pull/1940#discussion_r739659131



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccountCharge.java
##########
@@ -94,6 +94,13 @@
     @Column(name = "charge_calculation_enum")
     private Integer chargeCalculation;
 
+    @Column(name = "free_withdrawal_count", nullable = true)
+    private Integer freeWithdrawalCount;
+
+    @Temporal(TemporalType.DATE)
+    @Column(name = "charge_reset_date", nullable = true)
+    protected Date chargeResetDate;

Review comment:
       comment addressed. changed to private.




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



[GitHub] [fineract] ptuomola commented on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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


   @rrpawar96 looks good and all the tests pass. If you can squash the commits, I can then merge the PR.


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



[GitHub] [fineract] rrpawar96 commented on a change in pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
rrpawar96 commented on a change in pull request #1940:
URL: https://github.com/apache/fineract/pull/1940#discussion_r743399090



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java
##########
@@ -1066,15 +1070,98 @@ public BigDecimal calculateWithdrawalFee(final BigDecimal transactionAmount) {
         return result;
     }
 
-    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user) {
+    private void payWithdrawalFee(final BigDecimal transactionAmount, final LocalDate transactionDate, final AppUser user,
+            final PaymentDetail paymentDetail) {
+
         for (SavingsAccountCharge charge : this.charges()) {
+
             if (charge.isWithdrawalFee() && charge.isActive()) {
-                charge.updateWithdralFeeAmount(transactionAmount);
-                this.payCharge(charge, charge.getAmountOutstanding(this.getCurrency()), transactionDate, user);
+                if (charge.getFreeWithdrawalCount() == null) {
+                    charge.setFreeWithdrawalCount(0);
+                }
+
+                if (charge.isEnableFreeWithdrawal()) {
+                    resetFreeChargeDaysCount(charge, transactionAmount, transactionDate, user);

Review comment:
       @ptuomola, Yes, Thanks for pointing this out, I am testing it again with different conditions, resolving at soonest, sorry for getting back late.




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



[GitHub] [fineract] rrpawar96 commented on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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


   > All the runs are failing on the same test ie testApplyAnnualFeeForSavingsJobOutcome()
   >  
   > Can you reproduce the failure locally and see what's going on? As this change is about fees, I'm wondering if it has broken something...
   
   Sure, I will check on it locally 


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



[GitHub] [fineract] awasum commented on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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


   @rrpawar96  Please address comment by @ptuomola above...
   
   Trying to reopen to trigger Travis.


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



[GitHub] [fineract] rrpawar96 commented on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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


   @ptuomola, Requesting you to re-review this PR.


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



[GitHub] [fineract] ptuomola closed pull request #1940: FINERACT-1374: Withdraw-charge-discount

Posted by GitBox <gi...@apache.org>.
ptuomola closed pull request #1940:
URL: https://github.com/apache/fineract/pull/1940


   


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



[GitHub] [fineract] ptuomola commented on pull request #1940: FINERACT-1374: Withdraw-charge-discount

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


   All the runs are failing on the same test ie testApplyAnnualFeeForSavingsJobOutcome()
    
   Can you reproduce the failure locally and see what's going on? As this change is about fees, I'm wondering if it has broken something...


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