From 4de94c5de36e07d19e5bb6189b86cf437e4a491f Mon Sep 17 00:00:00 2001 From: Mohammad Kathawala Date: Mon, 14 Sep 2026 21:12:17 +0530 Subject: [PATCH] Fixed: Preserve adjustments in quick order returns (OFBIZ-12432) quickReturnFromOrder did not pass orderAdjustmentId when copying order-level adjustments, so createReturnAdjustment produced zero-valued manual adjustments instead of preserving promotions, shipping charges, and taxes. The converted Groovy service also misspelled countNewReturnItems and read availableReturnTotal from the wrong service result, causing return reconciliation to use an incorrect value. ReturnItems.ftl updated returnTotal only inside macro-local scope, so the return page displayed the raw item total without its item-level and return-level adjustments. Pass the source adjustment IDs, use the correct service parameter and result, and accumulate adjustment amounts in the caller scope. Add a completed-order regression covering item and order promotions, shipping, tax, balance-adjustment suppression, and equal 17.00 return and invoice totals. --- .../order/OrderReturnServicesScript.groovy | 15 ++++----- .../order/order/test/OrderReturnTests.groovy | 33 ++++++++++++++++++- .../order/template/return/ReturnItems.ftl | 3 +- .../order/testdef/data/OrderTestData.xml | 11 +++++++ 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/applications/order/src/main/groovy/org/apache/ofbiz/order/order/OrderReturnServicesScript.groovy b/applications/order/src/main/groovy/org/apache/ofbiz/order/order/OrderReturnServicesScript.groovy index b1d2be55762..9757931802e 100644 --- a/applications/order/src/main/groovy/org/apache/ofbiz/order/order/OrderReturnServicesScript.groovy +++ b/applications/order/src/main/groovy/org/apache/ofbiz/order/order/OrderReturnServicesScript.groovy @@ -144,9 +144,9 @@ Map updateReturnHeader() { adjustment: BigDecimal.ZERO] BigDecimal availableReturnTotal = serviceResult.availableReturnTotal BigDecimal returnTotal = serviceResult.returnTotal - BigDecimal orderTotal = serviceResult.returnTotal + BigDecimal orderTotal = serviceResult.orderTotal logInfo("Available amount for return on order # ${returnItem.orderId} is " + - "[${availableReturnTotal}] (orderTotal = [${orderTotal}] - returnTotal = [${returnTotal}]") + "[${availableReturnTotal}] (orderTotal = [${orderTotal}] - returnTotal = [${returnTotal}])") if (availableReturnTotal < -0.01) { return informError('OrderReturnPriceCannotExceedTheOrderTotal') @@ -520,22 +520,21 @@ Map quickReturnFromOrder() { .where(orderId: orderHeader.orderId, orderItemSeqId: '_NA_') .queryList() for (GenericValue orderAdjustment : orderAdjustments) { - Map returnAdjCtx = [:] - returnAdjCtx.returnId = returnId + Map returnAdjCtx = [returnId: returnId, orderAdjustmentId: orderAdjustment.orderAdjustmentId] // filter out orderAdjustment that have been returned if (from('ReturnAdjustment').where(orderAdjustmentId: orderAdjustment.orderAdjustmentId).queryCount() == 0) { logInfo('Create new return adjustment: ' + returnAdjCtx) run service: 'createReturnAdjustment', with: returnAdjCtx } } - // very important: if countNewReturnItemx is not set, + // very important: if countNewReturnItems is not set, // getOrderAvailableReturnedTotal would not count the return items we just created - Map orderAvailableCtx = [orderId: orderHeader.orderId, countNewReturnItemx: true] + Map orderAvailableCtx = [orderId: orderHeader.orderId, countNewReturnItems: true] Map serviceResultART = run service: 'getOrderAvailableReturnedTotal', with: orderAvailableCtx - BigDecimal availableReturnTotal = serviceResult.availableReturnTotal + BigDecimal availableReturnTotal = serviceResultART.availableReturnTotal BigDecimal returnTotal = serviceResultART.returnTotal BigDecimal orderTotal = serviceResultART.orderTotal - logInfo("OrderTotal [${orderTotal}] - ReturnTotal [${returnTotal}] = available Return Total [${}]") + logInfo("OrderTotal [${orderTotal}] - ReturnTotal [${returnTotal}] = available Return Total [${availableReturnTotal}]") // create a manual balance adjustment based on the difference between order total and return total if (availableReturnTotal != (BigDecimal.ZERO)) { diff --git a/applications/order/src/test/groovy/org/apache/ofbiz/order/order/test/OrderReturnTests.groovy b/applications/order/src/test/groovy/org/apache/ofbiz/order/order/test/OrderReturnTests.groovy index bbac0a84c05..01fcd5dd30b 100644 --- a/applications/order/src/test/groovy/org/apache/ofbiz/order/order/test/OrderReturnTests.groovy +++ b/applications/order/src/test/groovy/org/apache/ofbiz/order/order/test/OrderReturnTests.groovy @@ -28,14 +28,45 @@ class OrderReturnTests extends OFBizTestCase { } // Return related test services void testQuickReturnOrder() { + String orderId = 'TEST_RTN12432' Map serviceCtx = [ - orderId: 'TEST_DEMO10090', + orderId: orderId, returnHeaderTypeId: 'CUSTOMER_RETURN', userLogin: userLogin ] Map serviceResult = dispatcher.runSync('quickReturnOrder', serviceCtx) assert ServiceUtil.isSuccess(serviceResult) assert serviceResult.returnId != null + + List returnItems = from('ReturnItem').where(returnId: serviceResult.returnId).queryList() + assert returnItems.size() == 1 + assert returnItems[0].returnQuantity == 2.0G + assert returnItems[0].returnPrice == 10.00G + + List returnAdjustments = from('ReturnAdjustment').where(returnId: serviceResult.returnId).queryList() + assert returnAdjustments*.orderAdjustmentId.toSet() == [ + 'TEST_RTN12432_ITEM', 'TEST_RTN12432_PROMO', 'TEST_RTN12432_SHIP', 'TEST_RTN12432_TAX' + ].toSet() + assert !returnAdjustments.any { it.returnAdjustmentTypeId == 'RET_MAN_ADJ' } + + BigDecimal returnTotal = returnItems.sum(BigDecimal.ZERO) { + it.returnQuantity * it.returnPrice + } + returnAdjustments.sum(BigDecimal.ZERO) { it.amount } + assert returnTotal == 17.00G + + Map invoiceResult = dispatcher.runSync('createInvoiceFromReturn', [ + returnId: serviceResult.returnId, + billItems: returnItems, + userLogin: userLogin + ]) + assert ServiceUtil.isSuccess(invoiceResult) + assert invoiceResult.invoiceId != null + + List invoiceItems = from('InvoiceItem').where(invoiceId: invoiceResult.invoiceId).queryList() + BigDecimal invoiceTotal = invoiceItems.sum(BigDecimal.ZERO) { + (it.quantity ?: BigDecimal.ONE) * it.amount + } + assert invoiceTotal == returnTotal } void testProcessCreditReturn() { Map serviceCtx = [ diff --git a/applications/order/template/return/ReturnItems.ftl b/applications/order/template/return/ReturnItems.ftl index 6bf0836485c..1d2df0af242 100644 --- a/applications/order/template/return/ReturnItems.ftl +++ b/applications/order/template/return/ReturnItems.ftl @@ -64,7 +64,6 @@ under the License. <#local rowCount = rowCount + 1> <#local rowCountForAdjRemove = rowCountForAdjRemove + 1> - <#local returnTotal = returnTotal + returnAdjustment.amount?default(0)> @@ -262,6 +261,7 @@ under the License. <#assign returnItemAdjustments = item.getRelated("ReturnAdjustment", null, null, false)> <#if (returnItemAdjustments?has_content)> <#list returnItemAdjustments as returnItemAdjustment> + <#assign returnTotal = returnTotal + returnItemAdjustment.amount?default(0)> <@displayReturnAdjustment returnAdjustment=returnItemAdjustment adjEditable=false/> <#-- adjustments of return items should never be editable --> @@ -278,6 +278,7 @@ under the License. <#if (returnAdjustments?has_content)> <#list returnAdjustments as returnAdjustment> <#assign adjEditable = !readOnly> <#-- they are editable if the rest of the return items are --> + <#assign returnTotal = returnTotal + returnAdjustment.amount?default(0)> <@displayReturnAdjustment returnAdjustment=returnAdjustment adjEditable=adjEditable/> diff --git a/applications/order/testdef/data/OrderTestData.xml b/applications/order/testdef/data/OrderTestData.xml index 8fe8bb40357..fcc44c1c99c 100644 --- a/applications/order/testdef/data/OrderTestData.xml +++ b/applications/order/testdef/data/OrderTestData.xml @@ -53,6 +53,17 @@ under the License. + + + + + + + + + + +