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/07/14 05:14:49 UTC

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

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