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 2020/06/06 18:46:36 UTC

[GitHub] [fineract] percyashu opened a new pull request #987: FINERACT-822 enable OperatorPrecedence error

percyashu opened a new pull request #987:
URL: https://github.com/apache/fineract/pull/987


   FINERACT-822
   


----------------------------------------------------------------
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] percyashu commented on a change in pull request #987: FINERACT-822 enable OperatorPrecedence error

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



##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/GlobalConfigurationProperty.java
##########
@@ -114,8 +114,8 @@ public Date getDateValue(){
 
         final String passwordPropertyName = "force-password-reset-days";
         if (this.name.equalsIgnoreCase(passwordPropertyName)) {
-            if (this.enabled == true && command.hasParameter(valueParamName) && this.value == 0 || this.enabled == true
-                    && !command.hasParameter(valueParamName) && previousValue == 0) { throw new ForcePasswordResetException(); }
+            if (((this.enabled == true) && command.hasParameter(valueParamName) && (this.value == 0)) || ((this.enabled == true)
+                    && !command.hasParameter(valueParamName) && (previousValue == 0))) { throw new ForcePasswordResetException(); }

Review comment:
       yeah will modify this




----------------------------------------------------------------
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 a change in pull request #987: FINERACT-822 enable OperatorPrecedence error

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



##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/GlobalConfigurationProperty.java
##########
@@ -114,8 +114,8 @@ public Date getDateValue(){
 
         final String passwordPropertyName = "force-password-reset-days";
         if (this.name.equalsIgnoreCase(passwordPropertyName)) {
-            if (this.enabled == true && command.hasParameter(valueParamName) && this.value == 0 || this.enabled == true
-                    && !command.hasParameter(valueParamName) && previousValue == 0) { throw new ForcePasswordResetException(); }
+            if (((this.enabled == true) && command.hasParameter(valueParamName) && (this.value == 0)) || ((this.enabled == true)
+                    && !command.hasParameter(valueParamName) && (previousValue == 0))) { throw new ForcePasswordResetException(); }

Review comment:
       Wait, this looks weird to me, shouldn't this just be:
   
   ```suggestion
               if ((this.enabled == true && command.hasParameter(valueParamName) && this.value == 0) || (this.enabled == true
                       && !command.hasParameter(valueParamName) && previousValue == 0)) { throw new ForcePasswordResetException(); }
   ```

##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java
##########
@@ -2384,7 +2384,7 @@ public ChangedTransactionDetail disburse(final AppUser currentUser, final JsonCo
          **/
 
         if (isNoneOrCashOrUpfrontAccrualAccountingEnabledOnLoanProduct()
-                        && (isMultiDisburmentLoan() && getDisbursedLoanDisbursementDetails().size() == 1 || !isMultiDisburmentLoan())) {
+                        && ((isMultiDisburmentLoan() && getDisbursedLoanDisbursementDetails().size() == 1) || !isMultiDisburmentLoan())) {

Review comment:
       Wouldn't this be better & clearer like this, instead:
   
       if ((isNoneOrCashOrUpfrontAccrualAccountingEnabledOnLoanProduct()
           && isMultiDisburmentLoan() && getDisbursedLoanDisbursementDetails().size() == 1) || !isMultiDisburmentLoan()) {
   




----------------------------------------------------------------
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] xurror commented on pull request #987: FINERACT-822 enable OperatorPrecedence error

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


   Good 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] percyashu closed pull request #987: FINERACT-822 enable OperatorPrecedence error

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


   


----------------------------------------------------------------
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] xurror merged pull request #987: FINERACT-822 enable OperatorPrecedence error

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


   


----------------------------------------------------------------
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] percyashu commented on a change in pull request #987: FINERACT-822 enable OperatorPrecedence error

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



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java
##########
@@ -2384,7 +2384,7 @@ public ChangedTransactionDetail disburse(final AppUser currentUser, final JsonCo
          **/
 
         if (isNoneOrCashOrUpfrontAccrualAccountingEnabledOnLoanProduct()
-                        && (isMultiDisburmentLoan() && getDisbursedLoanDisbursementDetails().size() == 1 || !isMultiDisburmentLoan())) {
+                        && ((isMultiDisburmentLoan() && getDisbursedLoanDisbursementDetails().size() == 1) || !isMultiDisburmentLoan())) {

Review comment:
       OK will try this but the parentheses grouping might change the logic of the check




----------------------------------------------------------------
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] xurror commented on pull request #987: FINERACT-822 enable OperatorPrecedence error

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


   Still some merge conflicts and maybe also apply suggested changes.


----------------------------------------------------------------
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] percyashu commented on pull request #987: FINERACT-822 enable OperatorPrecedence error

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


   @xurror done


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