You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ofbiz.apache.org by GitBox <gi...@apache.org> on 2020/05/23 10:52:39 UTC

[GitHub] [ofbiz-framework] vyasdevanshu opened a new pull request #159: Improved: Converted the setPaymentStatus service from XML to groovy DSL. [OFBIZ-11482]

vyasdevanshu opened a new pull request #159:
URL: https://github.com/apache/ofbiz-framework/pull/159


   


----------------------------------------------------------------
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] [ofbiz-framework] nmalin commented on pull request #159: Improved: Converted the setPaymentStatus service from XML to groovy DSL. [OFBIZ-11482]

Posted by GitBox <gi...@apache.org>.
nmalin commented on pull request #159:
URL: https://github.com/apache/ofbiz-framework/pull/159#issuecomment-920028882


   Hello, I merged on trunk this service migration based on your patch @vyasdevanshu  and @dixitdeepak 's remarks


-- 
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: notifications-unsubscribe@ofbiz.apache.org

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



[GitHub] [ofbiz-framework] dixitdeepak commented on a change in pull request #159: Improved: Converted the setPaymentStatus service from XML to groovy DSL. [OFBIZ-11482]

Posted by GitBox <gi...@apache.org>.
dixitdeepak commented on a change in pull request #159:
URL: https://github.com/apache/ofbiz-framework/pull/159#discussion_r454103163



##########
File path: applications/accounting/groovyScripts/payment/PaymentServices.groovy
##########
@@ -94,3 +94,66 @@ def getPaymentRunningTotal(){
     result.paymentRunningTotal = paymentRunningTotal
     return result
 }
+
+def setPaymentStatus() {
+    if (parameters.paymentId) {
+        Map resultMap = new HashMap()
+        payment = from("Payment").where("paymentId", parameters.paymentId).queryOne()
+        if (payment) {
+            oldStatusId = payment.statusId
+            statusItem = from("StatusItem").where("statusId", parameters.statusId).queryOne()

Review comment:
       We should use cache here.
   

##########
File path: applications/accounting/groovyScripts/payment/PaymentServices.groovy
##########
@@ -94,3 +94,66 @@ def getPaymentRunningTotal(){
     result.paymentRunningTotal = paymentRunningTotal
     return result
 }
+
+def setPaymentStatus() {
+    if (parameters.paymentId) {
+        Map resultMap = new HashMap()
+        payment = from("Payment").where("paymentId", parameters.paymentId).queryOne()
+        if (payment) {
+            oldStatusId = payment.statusId
+            statusItem = from("StatusItem").where("statusId", parameters.statusId).queryOne()
+            if (statusItem) {
+                if (!oldStatusId.equals(parameters.statusId)) {
+                    statusChange = from("StatusValidChange").where("statusId", oldStatusId, "statusIdTo", parameters.statusId).queryOne()

Review comment:
       Use cache. 

##########
File path: applications/accounting/groovyScripts/payment/PaymentServices.groovy
##########
@@ -94,3 +94,66 @@ def getPaymentRunningTotal(){
     result.paymentRunningTotal = paymentRunningTotal
     return result
 }
+
+def setPaymentStatus() {
+    if (parameters.paymentId) {
+        Map resultMap = new HashMap()
+        payment = from("Payment").where("paymentId", parameters.paymentId).queryOne()
+        if (payment) {
+            oldStatusId = payment.statusId
+            statusItem = from("StatusItem").where("statusId", parameters.statusId).queryOne()
+            if (statusItem) {
+                if (!oldStatusId.equals(parameters.statusId)) {
+                    statusChange = from("StatusValidChange").where("statusId", oldStatusId, "statusIdTo", parameters.statusId).queryOne()
+                    if (statusChange) {
+                        // payment method is mandatory when set to sent or received
+                        if (("PMNT_RECEIVED".equals(parameters.paymentId) || "PMNT_SENT".equals(parameters.paymentId)) && !payment.paymentMethodId) {
+                            resultMap.errorMessage = UtilProperties.getMessage("AccountingUiLabels","AccountingMissingPaymentMethod", [statusItem.description], locale)
+                            logError("Cannot set status to " + parameters.statusId + " on payment " + payment.paymentId + ": payment method is missing")
+                            return failure(resultMap.errorMessage)
+                        }
+                        // check if the payment fully applied when set to confirmed
+                        if ("PMNT_CONFIRMED".equals(parameters.statusId)) {
+                            notYetApplied = org.apache.ofbiz.accounting.payment.PaymentWorker.getPaymentNotApplied(payment)
+                            if (BigDecimal.ZERO.compareTo(notYetApplied)) {
+                                resultMap.errorMessage = UtilProperties.getMessage("AccountingUiLabels","AccountingPSNotConfirmedNotFullyApplied", locale)
+                                logError("Cannot change from " + payment.statusId + " to " + parameters.statusId + ", payment not fully applied:" + notYetapplied)
+                                return failure(resultMap.errorMessage)
+                            }
+                        }
+                        // if new status is cancelled delete existing payment applications
+                        if ("PMNT_CANCELLED".equals(parameters.statusId)) {
+                            paymentApplications = payment.getRelated("PaymentApplication", null, null, false);
+                            if (paymentApplications) {
+                                paymentApplications.each { paymentApplication ->
+                                    removePaymentApplicationMap.paymentApplicationId = paymentApplication.paymentApplicationId
+                                    paymentAppResult = runService('removePaymentApplication', removePaymentApplicationMap)
+                                }
+                            }
+                            // if new status is cancelled and the payment is associated to an OrderPaymentPreference, update the status of that record too
+                            orderPaymentPreference = payment.getRelatedOne("OrderPaymentPreference", false)
+                            if (orderPaymentPreference) {
+                                updateOrderPaymentPreferenceMap.orderPaymentPreferenceId = orderPaymentPreference.orderPaymentPreferenceId
+                                updateOrderPaymentPreferenceMap.statusId = "PAYMENT_CANCELLED"
+                                runService('updateOrderPaymentPreference', updateOrderPaymentPreferenceMap)
+                            }
+                        }
+
+                        // everything ok, so now change the status field
+                        payment.statusId = parameters.statusId
+                        payment.store()
+                    } else {
+                        resultMap.errorMessage = "Cannot change from " + oldStatusId + " to " + parameters.statusId

Review comment:
       I think we should use CommonErrorNoStatusValidChange uiLabel here. 

##########
File path: applications/accounting/groovyScripts/payment/PaymentServices.groovy
##########
@@ -94,3 +94,66 @@ def getPaymentRunningTotal(){
     result.paymentRunningTotal = paymentRunningTotal
     return result
 }
+
+def setPaymentStatus() {
+    if (parameters.paymentId) {
+        Map resultMap = new HashMap()
+        payment = from("Payment").where("paymentId", parameters.paymentId).queryOne()
+        if (payment) {
+            oldStatusId = payment.statusId
+            statusItem = from("StatusItem").where("statusId", parameters.statusId).queryOne()
+            if (statusItem) {
+                if (!oldStatusId.equals(parameters.statusId)) {
+                    statusChange = from("StatusValidChange").where("statusId", oldStatusId, "statusIdTo", parameters.statusId).queryOne()
+                    if (statusChange) {
+                        // payment method is mandatory when set to sent or received
+                        if (("PMNT_RECEIVED".equals(parameters.paymentId) || "PMNT_SENT".equals(parameters.paymentId)) && !payment.paymentMethodId) {
+                            resultMap.errorMessage = UtilProperties.getMessage("AccountingUiLabels","AccountingMissingPaymentMethod", [statusItem.description], locale)
+                            logError("Cannot set status to " + parameters.statusId + " on payment " + payment.paymentId + ": payment method is missing")
+                            return failure(resultMap.errorMessage)
+                        }
+                        // check if the payment fully applied when set to confirmed
+                        if ("PMNT_CONFIRMED".equals(parameters.statusId)) {
+                            notYetApplied = org.apache.ofbiz.accounting.payment.PaymentWorker.getPaymentNotApplied(payment)
+                            if (BigDecimal.ZERO.compareTo(notYetApplied)) {
+                                resultMap.errorMessage = UtilProperties.getMessage("AccountingUiLabels","AccountingPSNotConfirmedNotFullyApplied", locale)

Review comment:
       I don't think we need to assign this to map, resultMap is unused 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.

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



[GitHub] [ofbiz-framework] sonarcloud[bot] commented on pull request #159: Improved: Converted the setPaymentStatus service from XML to groovy DSL. [OFBIZ-11482]

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #159:
URL: https://github.com/apache/ofbiz-framework/pull/159#issuecomment-633026359


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=159&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=159&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=159&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=159&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=159&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=159&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) to review)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=159&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=159&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=159&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_ofbiz-framework&pullRequest=159) No Coverage information  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_ofbiz-framework&pullRequest=159&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_ofbiz-framework&pullRequest=159&metric=new_duplicated_lines_density&view=list)
   
   


----------------------------------------------------------------
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] [ofbiz-framework] nmalin closed pull request #159: Improved: Converted the setPaymentStatus service from XML to groovy DSL. [OFBIZ-11482]

Posted by GitBox <gi...@apache.org>.
nmalin closed pull request #159:
URL: https://github.com/apache/ofbiz-framework/pull/159


   


-- 
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: notifications-unsubscribe@ofbiz.apache.org

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