This is an automated email from the ASF dual-hosted git repository.

mridulpathak pushed a commit to branch release24.09
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/release24.09 by this push:
     new dbb3b93172 Fixed: Six regressions in the minilang-to-Groovy conversion 
of PaymentServices.groovy (OFBIZ-13596)
dbb3b93172 is described below

commit dbb3b93172108fc5719db53ce35131a7daef987d
Author: Mridul Pathak <[email protected]>
AuthorDate: Tue Sep 15 13:24:21 2026 +0530

    Fixed: Six regressions in the minilang-to-Groovy conversion of 
PaymentServices.groovy (OFBIZ-13596)
    
    - createPayment set parameters.paymentId instead of 
parameters.paymentMethodId when defaulting from an OrderPaymentPreference, 
silently dropping the payment method
    - createPaymentAndApplicationForParty used return inside .each{} closures, 
which only exits that iteration rather than the method, letting non-ready 
invoices be silently skipped instead of aborting the whole operation
    - checkAndCreateBatchForValidPayments dropped the original's filterByDate() 
on the already-batched check, so an expired batch membership permanently 
blocked re-batching
    - createPaymentAndPaymentGroupForInvoices dereferenced finAccount.statusId 
without a null guard, raising an NPE when the payment method's finAccountId 
doesn't resolve to a FinAccount
    - setPaymentStatus returned failure() instead of error() for its validation 
guards, so ServiceUtil.isError() missed them and callers relying on default 
break-on-error behavior didn't catch the FAIL response
    - createMatchingPaymentApplication had an inverted existence check that 
created a duplicate application only when one already existed, skipping 
creation in the normal auto-match case; as a SECA on setPaymentStatus and 
setInvoiceStatus, this fired on essentially every payment receipt/send and 
invoice approval
    
    Each fix was verified against the original minilang and covered by a 
temporary RED/GREEN test before removal, and the accounting component's 
payment, invoice, and payment-application test suites pass unchanged.
---
 .../ofbiz/accounting/payment/PaymentServices.groovy     | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git 
a/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/payment/PaymentServices.groovy
 
b/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/payment/PaymentServices.groovy
index 3fadc2a2e0..53e2ed2a9a 100644
--- 
a/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/payment/PaymentServices.groovy
+++ 
b/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/payment/PaymentServices.groovy
@@ -57,7 +57,7 @@ Map createPayment() {
     if (parameters.paymentPreferenceId) {
         GenericValue orderPaymentPreference = from('OrderPaymentPreference')
                 .where('orderPaymentPreferenceId', 
parameters.paymentPreferenceId).queryOne()
-        parameters.paymentId = parameters.paymentId ?: 
orderPaymentPreference.paymentMethodId
+        parameters.paymentMethodId = parameters.paymentMethodId ?: 
orderPaymentPreference.paymentMethodId
         parameters.paymentMethodTypeId = parameters.paymentMethodTypeId ?: 
orderPaymentPreference.paymentMethodTypeId
     }
 
@@ -180,7 +180,7 @@ Map createPaymentAndApplicationForParty() {
     BigDecimal paymentAmount = 0
     List invoiceIds = []
     String paymentId
-    parameters.invoices.each { GenericValue invoice ->
+    for (GenericValue invoice : parameters.invoices) {
         if (invoice.statusId == 'INVOICE_READY') {
             Map serviceResult = run service: 'getInvoicePaymentInfoList', 
with: invoice.getAllFields()
             if (ServiceUtil.isError(serviceResult)) {
@@ -210,7 +210,7 @@ Map createPaymentAndApplicationForParty() {
         }
         paymentId = serviceResult.paymentId
 
-        parameters.invoices.each { GenericValue invoice ->
+        for (GenericValue invoice : parameters.invoices) {
             if (invoice.statusId == 'INVOICE_READY') {
                 serviceResult = run service: 'getInvoicePaymentInfoList', 
with: invoice.getAllFields()
                 if (ServiceUtil.isError(serviceResult)) {
@@ -248,6 +248,7 @@ Map checkAndCreateBatchForValidPayments() {
     }
     List batchPaymentIds = from('PaymentGroupMember')
             .where(EntityCondition.makeCondition('paymentId', 
EntityOperator.IN, parameters.paymentIds))
+            .filterByDate()
             .distinct()
             .getFieldList('paymentId')
     if (batchPaymentIds) {
@@ -571,9 +572,9 @@ Map createPaymentAndPaymentGroupForInvoices() {
 
     if (paymentMethod) {
         GenericValue finAccount = from('FinAccount').where('finAccountId', 
paymentMethod.finAccountId).queryOne()
-        if (finAccount.statusId == 'FNACT_MANFROZEN') {
+        if (finAccount?.statusId == 'FNACT_MANFROZEN') {
             return error(label('AccountingErrorUiLabels', 
'AccountingFinAccountInactiveStatusError'))
-        } else if (finAccount.statusId == 'FNACT_CANCELLED') {
+        } else if (finAccount?.statusId == 'FNACT_CANCELLED') {
             return error(label('AccountingErrorUiLabels', 
'AccountingFinAccountStatusNotValidError'))
         }
     }
@@ -812,13 +813,13 @@ Map setPaymentStatus() {
 
         // payment method is mandatory when set to sent or received
         if (['PMNT_RECEIVED', 'PMNT_SENT'].contains(parameters.statusId) && 
!payment.paymentMethodId) {
-            return failure(label('AccountingUiLabels', 
'AccountingMissingPaymentMethod', [statusItem: statusItem]))
+            return error(label('AccountingUiLabels', 
'AccountingMissingPaymentMethod', [statusItem: statusItem]))
         }
 
         // check if the payment fully applied when set to confirmed
         if (parameters.statusId == 'PMNT_CONFIRMED' &&
                 PaymentWorker.getPaymentNotApplied(payment) != 0) {
-            return failure(label('AccountingUiLabels', 
'AccountingPSNotConfirmedNotFullyApplied'))
+            return error(label('AccountingUiLabels', 
'AccountingPSNotConfirmedNotFullyApplied'))
         }
     }
 
@@ -942,7 +943,7 @@ Map createMatchingPaymentApplication() {
             if (invoiceId) {
                 if (from('PaymentApplication')
                         .where(invoiceId: invoiceId)
-                        .queryCount()) {
+                        .queryCount() == 0) {
                     createPaymentApplicationCtx.paymentId = 
parameters.paymentId
                     createPaymentApplicationCtx.invoiceId = invoiceId
                     createPaymentApplicationCtx.amountApplied = amountApplied

Reply via email to