This is an automated email from the ASF dual-hosted git repository.
ashishvijaywargiya pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new 7aff21fa2a Groovy DSL cleanup: WorkEffort helper, double-negation
fixes, update() adoption (#2039)
7aff21fa2a is described below
commit 7aff21fa2a71aecb47128473146b61ec9d3383bf
Author: Ashish Vijaywargiya <[email protected]>
AuthorDate: Thu Sep 17 17:20:42 2026 +0530
Groovy DSL cleanup: WorkEffort helper, double-negation fixes, update()
adoption (#2039)
I continued the Groovy DSL cleanup work on top of the earlier
require()/fail() rollout, fixing a double-negation footgun across applications
and framework and extracting the duplicated WorkEffort delete-permission check
into one shared helper.
It also picks up the update() DSL adoption from where the last pass left
off, converting 7 more call sites that were previously skipped in my last pass.
I re-checked every other converted site for the same issue and none of them
are affected, so that risk is isolated to the one site I backed out.
Since the update() DSL had never been caught by a script-level try/catch
before, I added negative tests for all seven conversions plus a couple of
missing happy-path tests, and all four affected test suites pass clean.
Nothing here changes behavior on purpose, every conversion preserves the
exact original error message a caller would have seen before.
---
.../accounting/config/AccountingUiLabels.xml | 3 +
.../invoice/InvoiceServicesScript.groovy | 15 +--
.../accounting/payment/PaymentServices.groovy | 42 +++----
.../ofbiz/accounting/rate/RateServices.groovy | 15 ++-
.../tax/TaxAuthorityServicesScript.groovy | 6 +-
.../accounting/PaymentContentTests.groovy | 125 +++++++++++++++++++++
.../ofbiz/accounting/accounting/RateTests.groovy | 22 ++++
applications/accounting/testdef/paymenttests.xml | 5 +-
.../ofbiz/order/order/CheckoutServices.groovy | 6 +-
.../ofbiz/order/quote/QuoteServicesScript.groovy | 57 +++++-----
.../requirement/RequirementServicesScript.groovy | 58 +++++-----
.../shoppinglist/ShoppingListServicesScript.groovy | 10 +-
.../ofbiz/order/order/test/QuoteTests.groovy | 60 ++++++++++
.../party/contact/ContactMechServicesScript.groovy | 6 +-
.../party/party/PartyInvitationServices.groovy | 15 +--
.../ofbiz/party/party/PartyServicesScript.groovy | 6 +-
.../ofbiz/party/party/test/PartyMiscTests.groovy | 19 ++++
.../product/category/CategoryServicesScript.groovy | 3 +-
.../inventory/InventoryServicesScript.groovy | 4 +-
.../product/product/ProductServicesScript.groovy | 3 +-
.../product/promo/PromoServicesScript.groovy | 19 ++--
.../product/store/ProductStoreServices.groovy | 6 +-
.../shipment/ShipmentReceiptServices.groovy | 4 +-
.../ofbiz/product/shipment/ShipmentServices.groovy | 15 +--
.../product/test/ProductPromoCondTests.groovy | 61 ++++++++++
.../workeffort/WorkEffortServicesScript.groovy | 29 ++---
.../apache/ofbiz/common/PortalPageServices.groovy | 2 +-
27 files changed, 464 insertions(+), 152 deletions(-)
diff --git a/applications/accounting/config/AccountingUiLabels.xml
b/applications/accounting/config/AccountingUiLabels.xml
index a5e1245873..9fcd5ecfbc 100644
--- a/applications/accounting/config/AccountingUiLabels.xml
+++ b/applications/accounting/config/AccountingUiLabels.xml
@@ -13838,6 +13838,9 @@
<value xml:lang="zh">财务状态</value>
<value xml:lang="zh-TW">財務狀態</value>
</property>
+ <property key="AccountingStatusItemNotFound">
+ <value xml:lang="en">Status [${statusId}] not found</value>
+ </property>
<property key="AccountingSubsidiaryLedger">
<value xml:lang="ar">دفتر الشركة التابعة</value>
<value xml:lang="de">Nebenbuch</value>
diff --git
a/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/invoice/InvoiceServicesScript.groovy
b/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/invoice/InvoiceServicesScript.groovy
index 14e28955bd..05c45d6389 100644
---
a/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/invoice/InvoiceServicesScript.groovy
+++
b/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/invoice/InvoiceServicesScript.groovy
@@ -70,7 +70,8 @@ Map getNextInvoiceId() {
if (invoiceIdTemp) {
//check the provided ID
String errorMsg = UtilValidate.checkValidDatabaseId(invoiceIdTemp)
- require(!(errorMsg != null), "In getNextInvoiceId ${errorMsg}")
+ boolean isInvoiceIdValid = errorMsg == null
+ require(isInvoiceIdValid, "In getNextInvoiceId ${errorMsg}")
} else {
invoiceIdTemp = delegator.getNextSeqId('Invoice', 1)
}
@@ -169,7 +170,7 @@ Map getInvoice() {
Map updateInvoice() {
GenericValue invoice = from('Invoice').where(parameters).queryOne()
require(invoice as boolean, label('AccountingUiLabels',
'AccountingInvoiceNotFound', parameters))
- require(!(invoice.statusId != 'INVOICE_IN_PROCESS'),
+ require(invoice.statusId == 'INVOICE_IN_PROCESS',
label('AccountingUiLabels',
'AccountingInvoiceUpdateOnlyWithInProcessStatus', [statusId: invoice.statusId]))
// only save if something has changed, do not update status here
@@ -240,14 +241,14 @@ Map setInvoiceStatus() {
return success(returnResult)
}
- require(!(from('StatusValidChange')
+ require(from('StatusValidChange')
.where(statusId: oldStatusId, statusIdTo: parameters.statusId)
- .queryCount() == 0), label('AccountingUiLabels',
'AccountingPSInvalidStatusChange'))
+ .queryCount() > 0, label('AccountingUiLabels',
'AccountingPSInvalidStatusChange'))
// if new status is paid check if the complete invoice is applied
if (parameters.statusId == 'INVOICE_PAID') {
BigDecimal notApplied = InvoiceWorker.getInvoiceNotApplied(invoice)
- require(!(notApplied != 0), label('AccountingUiLabels',
'AccountingInvoiceCannotChangeStatusToPaid'))
+ require(notApplied == 0, label('AccountingUiLabels',
'AccountingInvoiceCannotChangeStatusToPaid'))
// if it's OK to mark invoice paid, use parameters for paidDate
invoice.paidDate = parameters.paidDate ?: UtilDateTime.nowTimestamp()
}
@@ -362,7 +363,7 @@ Map createInvoiceItem() {
}
}
// accept 0
- require(!(invoiceItem.amount == null), label('AccountingUiLabels',
'AccountingInvoiceAmountIsMandatory'))
+ require(invoiceItem.amount != null, label('AccountingUiLabels',
'AccountingInvoiceAmountIsMandatory'))
invoiceItem.create()
return success([invoiceId: invoiceItem.invoiceId,
invoiceItemSeqId: invoiceItem.invoiceItemSeqId])
@@ -385,7 +386,7 @@ Map updateInvoiceItem() {
Map serviceResult = run service: 'calculateProductPrice', with:
[product: product]
invoiceItem.amount = serviceResult.price
}
- require(!(invoiceItem.amount == null), label('AccountingUiLabels',
'AccountingInvoiceAmountIsMandatory'))
+ require(invoiceItem.amount != null, label('AccountingUiLabels',
'AccountingInvoiceAmountIsMandatory'))
if (lookedInvoiceItem != invoiceItem) {
invoiceItem.store()
}
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 ab4c66d7f0..0d8b75600f 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
@@ -32,6 +32,7 @@ import
org.apache.ofbiz.entity.condition.EntityConditionBuilder
import org.apache.ofbiz.entity.condition.EntityOperator
import org.apache.ofbiz.entity.util.EntityTypeUtil
import org.apache.ofbiz.entity.util.EntityUtilProperties
+import org.apache.ofbiz.service.ServiceErrorException
import org.apache.ofbiz.service.ServiceUtil
Map createPayment() {
@@ -144,7 +145,7 @@ Map updatePayment() {
oldPayment.comments = newPayment.comments
oldPayment.paymentRefNum = newPayment.paymentRefNum ?: null
oldPayment.finAccountTransId = newPayment.finAccountTransId ?: null
- require(!(oldPayment != newPayment), label('AccountingUiLabels',
'AccountingPSUpdateNotAllowedBecauseOfStatus'))
+ require(oldPayment == newPayment, label('AccountingUiLabels',
'AccountingPSUpdateNotAllowedBecauseOfStatus'))
}
String statusIdSave = payment.statusId // do not allow status change here
payment.setNonPKFields(parameters)
@@ -287,17 +288,16 @@ Map createPaymentContent() {
//TODO: This can be converted into entity-auto with a seca rule for
updateContent
Map updatePaymentContent() {
- GenericValue lookedUpValue =
from('PaymentContent').where(parameters).queryOne()
- if (lookedUpValue) {
- lookedUpValue.setNonPKFields(parameters)
- lookedUpValue.store()
- Map result = run service: 'updateContent', with: parameters
- if (ServiceUtil.isError(result)) {
- return result
- }
- return success()
+ try {
+ update('PaymentContent').where(parameters).set(parameters)
+ } catch (ServiceErrorException e) {
+ return error('Error getting Payment Content')
}
- return error('Error getting Payment Content')
+ Map result = run service: 'updateContent', with: parameters
+ if (ServiceUtil.isError(result)) {
+ return result
+ }
+ return success()
}
Map massChangePaymentStatus() {
@@ -463,7 +463,7 @@ Map cancelPaymentBatch() {
.queryList()
if (paymentGroupMemberAndTransList) {
- require(!(paymentGroupMemberAndTransList[0].finAccountTransStatusId ==
'FINACT_TRNS_APPROVED'),
+ require(paymentGroupMemberAndTransList[0].finAccountTransStatusId !=
'FINACT_TRNS_APPROVED',
label('AccountingErrorUiLabels',
'AccountingTransactionIsAlreadyReconciled'))
for (GenericValue paymentGroupMember : paymentGroupMemberAndTransList)
{
@@ -723,7 +723,7 @@ Map createPaymentFromOrder() {
Map createPaymentApplication() {
// Create a Payment Application
- require(!(!parameters.invoiceId && !parameters.billingAccountId &&
!parameters.taxAuthGeoId && !parameters.toPaymentId),
+ require(parameters.invoiceId || parameters.billingAccountId ||
parameters.taxAuthGeoId || parameters.toPaymentId,
label('AccountingUiLabels',
'AccountingPaymentApplicationParameterMissing'))
GenericValue paymentAppl = makeValue('PaymentApplication', parameters)
@@ -736,7 +736,7 @@ Map createPaymentApplication() {
// get the invoice and do some further validation against it
GenericValue invoice = from('Invoice').where('invoiceId',
parameters.invoiceId).queryOne()
// check the currencies if they are compatible
- require(!(invoice.currencyUomId != payment.currencyUomId &&
invoice.currencyUomId != payment.actualCurrencyUomId),
+ require(invoice.currencyUomId == payment.currencyUomId ||
invoice.currencyUomId == payment.actualCurrencyUomId,
label('AccountingUiLabels',
'AccountingCurrenciesOfInvoiceAndPaymentNotCompatible'))
if (invoice.currencyUomId != payment.currencyUomId &&
invoice.currencyUomId == payment.actualCurrencyUomId) {
// if required get the payment amount in foreign currency (local
we already have)
@@ -778,22 +778,22 @@ Map createPaymentApplication() {
Map setPaymentStatus() {
GenericValue payment = from('Payment').where('paymentId',
parameters.paymentId).queryOne()
- require(payment as boolean, "No payment found with ID
${parameters.paymentId}")
+ require(payment as boolean, label('AccountingUiLabels',
'AccountingPaymentRecordNotFound', parameters))
String oldStatusId = payment.statusId
GenericValue statusItem = from('StatusItem').where('statusId',
parameters.statusId).cache().queryOne()
- require(statusItem as boolean, "No status found with status ID
${parameters.statusId}")
+ require(statusItem as boolean, label('AccountingUiLabels',
'AccountingStatusItemNotFound', parameters))
if (oldStatusId != parameters.statusId) {
GenericValue statusChange =
from('StatusValidChange').where('statusId', oldStatusId, 'statusIdTo',
parameters.statusId).cache().queryOne()
require(statusChange as boolean, label('CommonUiLabels',
'CommonErrorNoStatusValidChange'))
// payment method is mandatory when set to sent or received
- require(!(['PMNT_RECEIVED', 'PMNT_SENT'].contains(parameters.statusId)
&& !payment.paymentMethodId),
+ require(!['PMNT_RECEIVED', 'PMNT_SENT'].contains(parameters.statusId)
|| payment.paymentMethodId,
label('AccountingUiLabels', 'AccountingMissingPaymentMethod',
[statusItem: statusItem]))
// check if the payment fully applied when set to confirmed
- require(!(parameters.statusId == 'PMNT_CONFIRMED' &&
- PaymentWorker.getPaymentNotApplied(payment) != 0),
label('AccountingUiLabels', 'AccountingPSNotConfirmedNotFullyApplied'))
+ require(parameters.statusId != 'PMNT_CONFIRMED' ||
+ PaymentWorker.getPaymentNotApplied(payment) == 0,
label('AccountingUiLabels', 'AccountingPSNotConfirmedNotFullyApplied'))
}
// if new status is cancelled delete existing payment applications
@@ -952,7 +952,7 @@ Map removePaymentApplication() {
// check payment
if (paymentApplication.paymentId) {
GenericValue payment = from('Payment').where(paymentId:
paymentApplication.paymentId).queryOne()
- require(!(payment.statusId == 'PMNT_CONFIRMED'),
label('AccountingUiLabels',
'AccountingPaymentApplicationCannotRemovedWithConfirmedStatus'))
+ require(payment.statusId != 'PMNT_CONFIRMED',
label('AccountingUiLabels',
'AccountingPaymentApplicationCannotRemovedWithConfirmedStatus'))
}
// check invoice
@@ -974,7 +974,7 @@ Map removePaymentApplication() {
// check toPayment
if (paymentApplication.toPaymentId) {
GenericValue toPayment = from('Payment').where(paymentId:
paymentApplication.toPaymentId).queryOne()
- require(!(toPayment.statusId == 'PMNT_CONFIRMED'),
+ require(toPayment.statusId != 'PMNT_CONFIRMED',
label('AccountingUiLabels',
'AccountingPaymentApplicationCannotRemovedWithConfirmedStatus'))
toMessage = label('AccountingUiLabels',
'AccountingPaymentApplToPayment', paymentApplicationFields)
}
diff --git
a/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/rate/RateServices.groovy
b/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/rate/RateServices.groovy
index 7fcc4a393d..dba46e33ab 100644
---
a/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/rate/RateServices.groovy
+++
b/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/rate/RateServices.groovy
@@ -22,6 +22,7 @@ import org.apache.ofbiz.base.util.UtilDateTime
import org.apache.ofbiz.base.util.UtilProperties
import org.apache.ofbiz.entity.GenericValue
import org.apache.ofbiz.entity.util.EntityUtil
+import org.apache.ofbiz.service.ServiceErrorException
import org.apache.ofbiz.service.ServiceUtil
import java.sql.Timestamp
@@ -62,15 +63,13 @@ Map updateRateAmount() {
* Service to expire a rate amount value
*/
Map expireRateAmount() {
- GenericValue lookedUpValue = delegator.makeValidValue('RateAmount',
parameters)
- lookedUpValue.rateCurrencyUomId = lookedUpValue.rateCurrencyUomId ?:
UtilProperties.getPropertyValue('general.properties',
+ GenericValue lookupValue = delegator.makeValidValue('RateAmount',
parameters)
+ lookupValue.rateCurrencyUomId = lookupValue.rateCurrencyUomId ?:
UtilProperties.getPropertyValue('general.properties',
'currency.uom.id.default')
- lookedUpValue = from('RateAmount').where(lookedUpValue).queryOne()
- if (lookedUpValue) {
- Timestamp previousDay =
UtilDateTime.adjustTimestamp(UtilDateTime.nowTimestamp(), 5, -1)
- lookedUpValue.thruDate = UtilDateTime.getDayEnd(previousDay)
- lookedUpValue.store()
- } else {
+ Timestamp previousDay =
UtilDateTime.adjustTimestamp(UtilDateTime.nowTimestamp(), 5, -1)
+ try {
+ update('RateAmount').where(lookupValue).set([thruDate:
UtilDateTime.getDayEnd(previousDay)])
+ } catch (ServiceErrorException e) {
return error('AccountingErrorUiLabels', 'AccountingDeleteRateAmount')
}
return success()
diff --git
a/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/tax/TaxAuthorityServicesScript.groovy
b/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/tax/TaxAuthorityServicesScript.groovy
index 706d7ed297..a961b711b4 100644
---
a/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/tax/TaxAuthorityServicesScript.groovy
+++
b/applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/tax/TaxAuthorityServicesScript.groovy
@@ -31,7 +31,8 @@ Map createPartyTaxAuthInfo() {
GenericValue taxAuthority =
from('TaxAuthority').where(parameters).queryOne()
require(taxAuthority as boolean, label('PartyUiLabels',
'PartyTaxAuthPartyAndGeoNotAvailable'))
String errorMesg = validatePartyTaxIdInline()
- require(!(errorMesg), errorMesg)
+ boolean isTaxIdValid = !errorMesg
+ require(isTaxIdValid, errorMesg)
GenericValue partyAuthInfo = makeValue('PartyTaxAuthInfo', parameters)
partyAuthInfo.fromDate = partyAuthInfo.fromDate ?:
UtilDateTime.nowTimestamp()
partyAuthInfo.create()
@@ -43,7 +44,8 @@ Map createPartyTaxAuthInfo() {
*/
Map updatePartyTaxAuthInfo() {
String errorMesg = validatePartyTaxIdInline()
- require(!(errorMesg), errorMesg)
+ boolean isTaxIdValid = !errorMesg
+ require(isTaxIdValid, errorMesg)
GenericValue partyAuthInfo =
from('PartyTaxAuthInfo').where(parameters).queryOne()
require(partyAuthInfo as boolean, 'PartyTaxAuthInfo not found for the
given parameters')
partyAuthInfo.setNonPKFields(parameters, false)
diff --git
a/applications/accounting/src/test/groovy/org/apache/ofbiz/accounting/accounting/PaymentContentTests.groovy
b/applications/accounting/src/test/groovy/org/apache/ofbiz/accounting/accounting/PaymentContentTests.groovy
new file mode 100644
index 0000000000..c969b254c5
--- /dev/null
+++
b/applications/accounting/src/test/groovy/org/apache/ofbiz/accounting/accounting/PaymentContentTests.groovy
@@ -0,0 +1,125 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.ofbiz.accounting.accounting
+
+import java.sql.Timestamp
+import org.apache.ofbiz.base.util.UtilDateTime
+import org.apache.ofbiz.entity.GenericValue
+import org.apache.ofbiz.service.ServiceUtil
+import org.apache.ofbiz.testtools.JunitJupiterTest
+import org.apache.ofbiz.testtools.JupiterTestHelper
+import org.junit.jupiter.api.Order
+import org.junit.jupiter.api.Test
+
+// updatePaymentContent() has no other caller anywhere in the codebase (only
reachable by direct
+// service invocation), so unlike the other update() DSL conversions it had no
existing test file
+// to add coverage to - this class is its dedicated home.
+@JunitJupiterTest
+class PaymentContentTests implements JupiterTestHelper {
+
+ // Regression coverage for the update() DSL / ServiceErrorException catch
site added in
+ // updatePaymentContent(): a PK with no matching PaymentContent record
must come back as a
+ // service error carrying the original plain-string "Error getting Payment
Content" message,
+ // not the generic EntityUpdateBuilder message and not a
silently-successful result.
+ @Test
+ @Order(1)
+ void testUpdatePaymentContentNotFound() {
+ String paymentId = testParams.paymentId ?: 'TEST_NONEXISTENT_PAYMENT'
+ String paymentContentTypeId = testParams.paymentContentTypeId ?:
'COMMENTS'
+ String contentId = testParams.contentId ?: 'TEST_NONEXISTENT_CONTENT'
+ Timestamp fromDate = UtilDateTime.toTimestamp('01/01/2099 00:00:00')
+ Map serviceCtx = [
+ paymentId: paymentId,
+ paymentContentTypeId: paymentContentTypeId,
+ contentId: contentId,
+ fromDate: fromDate,
+ userLogin: userLogin
+ ]
+ Map serviceResult = dispatcher.runSync('updatePaymentContent',
serviceCtx)
+ assert ServiceUtil.isError(serviceResult)
+ assert ServiceUtil.getErrorMessage(serviceResult) == 'Error getting
Payment Content'
+ }
+
+ // updatePaymentContent() had no coverage at all before this - not even a
happy-path test.
+ // Builds its own Payment/PaymentContentType/Content/PaymentContent
fixture chain directly
+ // (rather than depending on another testdef suite's data-load - each
<test-suite> in
+ // accounting's ofbiz-component.xml runs in its own isolated, rolled-back
transaction, so a
+ // fixture loaded by one suite's entity-xml is not visible to a different
suite), then calls
+ // the service and verifies both the PaymentContent update and the
follow-on updateContent
+ // call persisted.
+ @Test
+ @Order(2)
+ void testUpdatePaymentContent() {
+ String paymentContentTypeId = testParams.paymentContentTypeId ?:
'TEST_PMT_CNT_TYPE'
+ delegator.createOrStore(delegator.makeValue('PaymentContentType',
[paymentContentTypeId: paymentContentTypeId]))
+
+ String paymentId = testParams.paymentId ?: 'TEST_PMT_CONTENT'
+ GenericValue payment = delegator.makeValue('Payment', [
+ paymentId: paymentId,
+ paymentTypeId: 'CUSTOMER_PAYMENT',
+ partyIdFrom: 'DemoCustomer',
+ partyIdTo: 'Company',
+ statusId: 'PMNT_NOT_PAID',
+ effectiveDate: UtilDateTime.nowTimestamp(),
+ amount: BigDecimal.valueOf(20),
+ currencyUomId: 'USD'
+ ])
+ delegator.createOrStore(payment)
+
+ Map createContentResult = dispatcher.runSync('createContent',
+ [contentName: 'Original Payment Content', userLogin:
userLogin])
+ assert ServiceUtil.isSuccess(createContentResult)
+ String contentId = createContentResult.contentId
+ assert contentId
+
+ Timestamp fromDate = UtilDateTime.toTimestamp('01/01/2020 00:00:00')
+ GenericValue paymentContent = delegator.makeValue('PaymentContent', [
+ paymentId: paymentId,
+ paymentContentTypeId: paymentContentTypeId,
+ contentId: contentId,
+ fromDate: fromDate
+ ])
+ paymentContent.create()
+
+ Timestamp thruDate = UtilDateTime.toTimestamp('01/01/2030 00:00:00')
+ String newContentName = 'Updated Payment Content'
+ Map serviceCtx = [
+ paymentId: paymentId,
+ paymentContentTypeId: paymentContentTypeId,
+ contentId: contentId,
+ fromDate: fromDate,
+ thruDate: thruDate,
+ contentName: newContentName,
+ userLogin: userLogin
+ ]
+ Map serviceResult = dispatcher.runSync('updatePaymentContent',
serviceCtx)
+ assert ServiceUtil.isSuccess(serviceResult)
+
+ GenericValue updatedPaymentContent = from('PaymentContent')
+ .where('paymentId', paymentId, 'paymentContentTypeId',
paymentContentTypeId,
+ 'contentId', contentId, 'fromDate',
fromDate).queryOne()
+ assert updatedPaymentContent
+ assert updatedPaymentContent.thruDate == thruDate
+
+ GenericValue updatedContent = from('Content').where('contentId',
contentId).queryOne()
+ assert updatedContent
+ assert updatedContent.contentName == newContentName
+ }
+
+}
diff --git
a/applications/accounting/src/test/groovy/org/apache/ofbiz/accounting/accounting/RateTests.groovy
b/applications/accounting/src/test/groovy/org/apache/ofbiz/accounting/accounting/RateTests.groovy
index d30ede3e15..d7dea0018b 100644
---
a/applications/accounting/src/test/groovy/org/apache/ofbiz/accounting/accounting/RateTests.groovy
+++
b/applications/accounting/src/test/groovy/org/apache/ofbiz/accounting/accounting/RateTests.groovy
@@ -23,6 +23,7 @@ import org.apache.ofbiz.testtools.JupiterTestHelper
import org.apache.ofbiz.service.ServiceUtil
import org.apache.ofbiz.entity.GenericValue
import org.apache.ofbiz.base.util.UtilDateTime
+import org.apache.ofbiz.base.util.UtilProperties
import java.sql.Timestamp
import org.junit.jupiter.api.Order
@@ -225,4 +226,25 @@ class RateTests implements JupiterTestHelper {
assert rateAmount.thruDate
}
+ // Regression coverage for the update() DSL / ServiceErrorException catch
site added in
+ // expireRateAmount(): a rateTypeId with no matching RateAmount record
must come back as a
+ // service error carrying the original localized
AccountingDeleteRateAmount message, not the
+ // generic EntityUpdateBuilder message and not a silently-successful
result.
+ @Test
+ @Order(10)
+ void testExpireRateAmountNotFound() {
+ Timestamp fromDate = UtilDateTime.toTimestamp('01/01/2099 00:00:00')
+ String rateTypeId = testParams.rateTypeId ?:
'TEST_NONEXISTENT_RATE_TYPE'
+ Map serviceCtx = [
+ rateTypeId: rateTypeId,
+ fromDate: fromDate,
+ userLogin: userLogin
+
+ ]
+ Map serviceResult = dispatcher.runSync('expireRateAmount', serviceCtx)
+ assert ServiceUtil.isError(serviceResult)
+ assert ServiceUtil.getErrorMessage(serviceResult) ==
+ UtilProperties.getMessage('AccountingErrorUiLabels',
'AccountingDeleteRateAmount', Locale.US)
+ }
+
}
diff --git a/applications/accounting/testdef/paymenttests.xml
b/applications/accounting/testdef/paymenttests.xml
index 7b93729e90..245a042a25 100644
--- a/applications/accounting/testdef/paymenttests.xml
+++ b/applications/accounting/testdef/paymenttests.xml
@@ -25,6 +25,9 @@
<test-case case-name="auto-payment-tests">
<jupiter-test-suite
class-name="org.apache.ofbiz.accounting.accounting.AutoPaymentTests"/>
</test-case>
-
+
+ <test-case case-name="payment-content-tests">
+ <jupiter-test-suite
class-name="org.apache.ofbiz.accounting.accounting.PaymentContentTests"/>
+ </test-case>
</test-suite>
diff --git
a/applications/order/src/main/groovy/org/apache/ofbiz/order/order/CheckoutServices.groovy
b/applications/order/src/main/groovy/org/apache/ofbiz/order/order/CheckoutServices.groovy
index 4ae132d07d..d058cb207c 100644
---
a/applications/order/src/main/groovy/org/apache/ofbiz/order/order/CheckoutServices.groovy
+++
b/applications/order/src/main/groovy/org/apache/ofbiz/order/order/CheckoutServices.groovy
@@ -41,7 +41,8 @@ Map createUpdateCustomerAndShippingAddress() {
SimpleMapProcessor.runSimpleMapProcessor('component://party/minilang/contact/PartyContactMechMapProcs.xml',
'emailAddress', parameters, emailAddressCtx, messages,
context.locale)
// Check errors
- require(!(messages), StringUtil.join(messages, ','))
+ boolean hasNoValidationErrors = !messages
+ require(hasNoValidationErrors, StringUtil.join(messages, ','))
ShoppingCart shoppingCart = parameters.shoppingCart
String partyId = parameters.partyId
@@ -117,7 +118,8 @@ Map createUpdateBillingAddressAndPaymentMethod() {
SimpleMapProcessor.runSimpleMapProcessor('component://order/minilang/customer/CheckoutMapProcs.xml',
'billToPhone', parameters, billToPhoneContext, messages,
context.locale)
// Check Errors
- require(!(messages), StringUtil.join(messages, ','))
+ boolean hasNoValidationErrors = !messages
+ require(hasNoValidationErrors, StringUtil.join(messages, ','))
ShoppingCart shoppingCart = parameters.shoppingCart
GenericValue userLogin = shoppingCart.getUserLogin()
diff --git
a/applications/order/src/main/groovy/org/apache/ofbiz/order/quote/QuoteServicesScript.groovy
b/applications/order/src/main/groovy/org/apache/ofbiz/order/quote/QuoteServicesScript.groovy
index 33faacedc3..10a86cef60 100644
---
a/applications/order/src/main/groovy/org/apache/ofbiz/order/quote/QuoteServicesScript.groovy
+++
b/applications/order/src/main/groovy/org/apache/ofbiz/order/quote/QuoteServicesScript.groovy
@@ -30,6 +30,7 @@ import org.apache.ofbiz.product.config.ProductConfigWorker
import org.apache.ofbiz.product.config.ProductConfigWrapper
import org.apache.ofbiz.service.ExecutionServiceException
import org.apache.ofbiz.service.ModelService
+import org.apache.ofbiz.service.ServiceErrorException
import org.apache.ofbiz.service.ServiceUtil
/**
@@ -38,10 +39,11 @@ import org.apache.ofbiz.service.ServiceUtil
Map checkUpdateQuoteStatus() {
require(security.hasEntityPermission('ORDERMGR', '_UPDATE', userLogin) as
boolean,
'OrderErrorUiLabels',
'OrderSecurityErrorToRunCheckUpdateQuoteStatus')
- GenericValue quote = from('Quote').where(parameters).queryOne()
- require(quote as boolean, 'OrderErrorUiLabels', 'OrderQuoteDoesNotExists')
- quote.statusId = 'QUO_ORDERED'
- quote.store()
+ try {
+ update('Quote').where(parameters).set([statusId: 'QUO_ORDERED'])
+ } catch (ServiceErrorException e) {
+ fail('OrderErrorUiLabels', 'OrderQuoteDoesNotExists')
+ }
return success()
}
@@ -81,7 +83,8 @@ Map getNextQuoteId() {
require(!(quote), 'OrderErrorUiLabels',
'OrderQuoteIdAlreadyExists', [quoteId: quoteId])
// Check the provided ID
String errorMessage = UtilValidate.checkValidDatabaseId(quoteId)
- require(!(errorMessage), label('OrderErrorUiLabels',
'OrderQuoteGetNextIdError') + errorMessage)
+ boolean isQuoteIdValid = !errorMessage
+ require(isQuoteIdValid, label('OrderErrorUiLabels',
'OrderQuoteGetNextIdError') + errorMessage)
} else {
quoteId = delegator.getNextSeqId('Quote')
}
@@ -111,9 +114,9 @@ Map quoteSequenceEnforced() {
* Create a new Quote.
*/
Map createQuote() {
- require(!(parameters.partyId
- && parameters.partyId != userLogin.partyId
- && !security.hasEntityPermission('ORDERMGR', '_CREATE',
userLogin)),
+ require(!parameters.partyId
+ || parameters.partyId == userLogin.partyId
+ || security.hasEntityPermission('ORDERMGR', '_CREATE', userLogin),
'OrderErrorUiLabels', 'OrderSecurityErrorToRunCreateQuote')
// Create new entity and create all the fields.
@@ -316,9 +319,9 @@ Map ensureWorkEffortAndCreateQuoteWorkEffort() {
Map createQuoteItem() {
GenericValue quote = from('Quote').where(parameters).queryOne()
require(quote as boolean, 'OrderErrorUiLabels', 'OrderQuoteDoesNotExists')
- require(!(quote.partyId
- && quote.partyId != userLogin.partyId
- && !security.hasEntityPermission('ORDERMGR', '_CREATE',
userLogin)),
+ require(!quote.partyId
+ || quote.partyId == userLogin.partyId
+ || security.hasEntityPermission('ORDERMGR', '_CREATE', userLogin),
'OrderErrorUiLabels', 'OrderSecurityErrorToRunCreateQuoteItem')
GenericValue quoteItem = delegator.makeValidValue('QuoteItem', parameters)
if (!quoteItem.quoteItemSeqId) {
@@ -327,7 +330,7 @@ Map createQuoteItem() {
if (!parameters.quoteUnitPrice && parameters.productId) {
GenericValue product = from('Product').where('productId',
parameters.productId).cache().queryOne()
- require(!(product?.isVirtual == 'Y'), 'OrderErrorUiLabels',
'OrderCannotAddVirtualProductToQuote')
+ require(product?.isVirtual != 'Y', 'OrderErrorUiLabels',
'OrderCannotAddVirtualProductToQuote')
if (product?.productTypeId?.startsWith('AGGREGATED')
&& parameters.configId) {
ProductConfigWrapper configWrapper =
ProductConfigWorker.loadProductConfigWrapper(delegator, dispatcher,
parameters.configId,
@@ -354,10 +357,11 @@ Map updateQuoteItem() {
'OrderErrorUiLabels', 'OrderSecurityErrorToRunUpdateQuoteItem')
Map pksQuoteItem = [quoteId: parameters.quoteId, quoteItemSeqId:
parameters.quoteItemSeqId]
- GenericValue quoteItem = from('QuoteItem').where(pksQuoteItem).queryOne()
- require(quoteItem as boolean, 'OrderErrorUiLabels',
'OrderQuoteItemDoesNotExists')
- quoteItem.setNonPKFields(parameters)
- quoteItem.store()
+ try {
+ update('QuoteItem').where(pksQuoteItem).set(parameters)
+ } catch (ServiceErrorException e) {
+ fail('OrderErrorUiLabels', 'OrderQuoteItemDoesNotExists')
+ }
return success()
}
@@ -463,9 +467,9 @@ Map createQuoteFromCart() {
Map createQuoteInMap = parameters
createQuoteInMap.partyId = cart.getPartyId()
- require(!(createQuoteInMap.partyId
- && createQuoteInMap.partyId != userLogin.partyId
- && !security.hasEntityPermission('ORDERMGR', '_CREATE',
userLogin)),
+ require(!createQuoteInMap.partyId
+ || createQuoteInMap.partyId == userLogin.partyId
+ || security.hasEntityPermission('ORDERMGR', '_CREATE', userLogin),
'OrderErrorUiLabels', 'OrderSecurityErrorToRunCreateQuoteFromCart')
createQuoteInMap.currencyUomId = cart.getCurrency()
@@ -540,14 +544,17 @@ Map createQuoteFromShoppingList() {
Map autoUpdateQuotePrice() {
require(security.hasEntityPermission('ORDERMGR', '_UPDATE', userLogin) as
boolean,
'OrderErrorUiLabels',
'OrderSecurityErrorToRunAutoUpdateQuotePrice')
- GenericValue quoteItem = from('QuoteItem').where(parameters).queryOne()
- require(quoteItem as boolean, 'OrderErrorUiLabels',
'OrderQuoteItemDoesNotExists')
+ Map fieldsToSet = [:]
if (parameters.manualQuoteUnitPrice) {
- quoteItem.quoteUnitPrice = parameters.manualQuoteUnitPrice
+ fieldsToSet.quoteUnitPrice = parameters.manualQuoteUnitPrice
} else if (parameters.defaultQuoteUnitPrice) {
- quoteItem.quoteUnitPrice = parameters.defaultQuoteUnitPrice
+ fieldsToSet.quoteUnitPrice = parameters.defaultQuoteUnitPrice
+ }
+ try {
+ update('QuoteItem').where(parameters).set(fieldsToSet)
+ } catch (ServiceErrorException e) {
+ fail('OrderErrorUiLabels', 'OrderQuoteItemDoesNotExists')
}
- quoteItem.store()
return success()
}
@@ -563,7 +570,7 @@ Map createQuoteFromCustRequest() {
[custRequestId: parameters.custRequestId])
// Error if request type not equals to RF_QUOTE or RF_PUR_QUOTE
- require(!(custRequest.custRequestTypeId != 'RF_QUOTE' &&
custRequest.custRequestTypeId != 'RF_PUR_QUOTE'),
+ require(custRequest.custRequestTypeId == 'RF_QUOTE' ||
custRequest.custRequestTypeId == 'RF_PUR_QUOTE',
'OrderErrorUiLabels', 'OrderQuoteNotARequest')
Map createQuoteInMap = [
diff --git
a/applications/order/src/main/groovy/org/apache/ofbiz/order/requirement/RequirementServicesScript.groovy
b/applications/order/src/main/groovy/org/apache/ofbiz/order/requirement/RequirementServicesScript.groovy
index 303c2212e7..7e11e89519 100644
---
a/applications/order/src/main/groovy/org/apache/ofbiz/order/requirement/RequirementServicesScript.groovy
+++
b/applications/order/src/main/groovy/org/apache/ofbiz/order/requirement/RequirementServicesScript.groovy
@@ -31,15 +31,13 @@ import java.sql.Timestamp
*/
Map deleteRequirementAndRelated() {
GenericValue requirement = from('Requirement').where(parameters).queryOne()
- if (requirement) {
- requirement.removeRelated('RequirementAttribute')
- requirement.removeRelated('RequirementRole')
- requirement.removeRelated('RequirementStatus')
- requirement.removeRelated('RequirementCustRequest')
- requirement.remove()
- return success()
- }
- return error('Entity value not found with name: requirement Method =
deleteRequirementAndRelated')
+ require(requirement as boolean, 'Entity value not found with name:
requirement Method = deleteRequirementAndRelated')
+ requirement.removeRelated('RequirementAttribute')
+ requirement.removeRelated('RequirementRole')
+ requirement.removeRelated('RequirementStatus')
+ requirement.removeRelated('RequirementCustRequest')
+ requirement.remove()
+ return success()
}
/**
@@ -47,30 +45,28 @@ Map deleteRequirementAndRelated() {
*/
Map autoAssignRequirementToSupplier() {
GenericValue requirement = from('Requirement').where(parameters).queryOne()
- if (requirement) {
- if (requirement.requirementTypeId == 'PRODUCT_REQUIREMENT'
- && requirement.productId
- && requirement.quantity) {
- EntityCondition condition = new EntityConditionBuilder().AND {
- EQUALS(productId: requirement.productId)
- LESS_THAN_EQUAL_TO(minimumOrderQuantity: requirement.quantity)
- }
- EntityQuery supplierProductsQuery =
from('SupplierProduct').where(condition).orderBy('lastPrice',
'supplierPrefOrderId')
- if (requirement.requiredByDate) {
- supplierProductsQuery.filterByDate((Timestamp)
requirement.requiredByDate, 'availableFromDate', 'availableThruDate')
- }
- GenericValue supplierProduct = supplierProductsQuery.queryFirst()
- if (supplierProduct?.partyId) {
- GenericValue requirementRole =
delegator.makeValue('RequirementRole', [requirementId:
requirement.requirementId,
- partyId:
supplierProduct.partyId,
- roleTypeId:
'SUPPLIER',
- fromDate:
UtilDateTime.nowTimestamp()])
- delegator.createOrStore(requirementRole)
- }
+ require(requirement as boolean, 'Entity value not found with name:
requirement Method = autoAssignRequirementToSupplier')
+ if (requirement.requirementTypeId == 'PRODUCT_REQUIREMENT'
+ && requirement.productId
+ && requirement.quantity) {
+ EntityCondition condition = new EntityConditionBuilder().AND {
+ EQUALS(productId: requirement.productId)
+ LESS_THAN_EQUAL_TO(minimumOrderQuantity: requirement.quantity)
+ }
+ EntityQuery supplierProductsQuery =
from('SupplierProduct').where(condition).orderBy('lastPrice',
'supplierPrefOrderId')
+ if (requirement.requiredByDate) {
+ supplierProductsQuery.filterByDate((Timestamp)
requirement.requiredByDate, 'availableFromDate', 'availableThruDate')
+ }
+ GenericValue supplierProduct = supplierProductsQuery.queryFirst()
+ if (supplierProduct?.partyId) {
+ GenericValue requirementRole =
delegator.makeValue('RequirementRole', [requirementId:
requirement.requirementId,
+ partyId:
supplierProduct.partyId,
+ roleTypeId: 'SUPPLIER',
+ fromDate:
UtilDateTime.nowTimestamp()])
+ delegator.createOrStore(requirementRole)
}
- return success()
}
- return error('Entity value not found with name: requirement Method =
autoAssignRequirementToSupplier')
+ return success()
}
/**
diff --git
a/applications/order/src/main/groovy/org/apache/ofbiz/order/shoppinglist/ShoppingListServicesScript.groovy
b/applications/order/src/main/groovy/org/apache/ofbiz/order/shoppinglist/ShoppingListServicesScript.groovy
index 74f51ca49c..8384fe26ca 100644
---
a/applications/order/src/main/groovy/org/apache/ofbiz/order/shoppinglist/ShoppingListServicesScript.groovy
+++
b/applications/order/src/main/groovy/org/apache/ofbiz/order/shoppinglist/ShoppingListServicesScript.groovy
@@ -213,9 +213,9 @@ Map calculateShoppingListDeepTotalPrice() {
* Checks security on a ShoppingList
*/
Map checkShoppingListSecurity() {
- require(!(userLogin && (userLogin.userLoginId != 'anonymous') &&
- parameters.partyId && (userLogin.partyId != parameters.partyId)
- && !security.hasEntityPermission('PARTYMGR',
"_${parameters.permissionAction}", parameters.userLogin)),
+ require(!userLogin || (userLogin.userLoginId == 'anonymous') ||
+ !parameters.partyId || (userLogin.partyId == parameters.partyId)
+ || security.hasEntityPermission('PARTYMGR',
"_${parameters.permissionAction}", parameters.userLogin),
'OrderErrorUiLabels', 'OrderSecurityErrorToRunForAnotherParty')
Map result = success()
@@ -228,8 +228,8 @@ Map checkShoppingListSecurity() {
*/
Map checkShoppingListItemSecurity() {
GenericValue shoppingList =
from('ShoppingList').where(parameters).queryOne()
- require(!(shoppingList?.partyId && userLogin.partyId !=
shoppingList.partyId &&
- !security.hasEntityPermission('PARTYMGR',
"_${parameters.permissionAction}", parameters.userLogin)),
+ require(!shoppingList?.partyId || userLogin.partyId ==
shoppingList.partyId ||
+ security.hasEntityPermission('PARTYMGR',
"_${parameters.permissionAction}", parameters.userLogin),
'OrderErrorUiLabels',
'OrderSecurityErrorToRunForAnotherParty',
[parentMethodName: parameters.parentMethodName,
diff --git
a/applications/order/src/test/groovy/org/apache/ofbiz/order/order/test/QuoteTests.groovy
b/applications/order/src/test/groovy/org/apache/ofbiz/order/order/test/QuoteTests.groovy
index e05505070f..40222512a1 100644
---
a/applications/order/src/test/groovy/org/apache/ofbiz/order/order/test/QuoteTests.groovy
+++
b/applications/order/src/test/groovy/org/apache/ofbiz/order/order/test/QuoteTests.groovy
@@ -18,6 +18,7 @@
*/
package org.apache.ofbiz.order.order.test
+import org.apache.ofbiz.base.util.UtilProperties
import org.apache.ofbiz.entity.GenericValue
import org.apache.ofbiz.order.shoppingcart.ShoppingCart
import org.apache.ofbiz.service.ServiceUtil
@@ -550,4 +551,63 @@ class QuoteTests implements JupiterTestHelper {
assert ServiceUtil.isSuccess(serviceResult)
}
+ // Regression coverage for the update() DSL / ServiceErrorException catch
site added in
+ // checkUpdateQuoteStatus(): a quoteId with no matching Quote record must
come back as a
+ // service error carrying the original localized OrderQuoteDoesNotExists
message, not the
+ // generic EntityUpdateBuilder message and not a silently-successful
result.
+ @Test
+ @Order(26)
+ void testCheckUpdateQuoteStatusNotFound() {
+ String quoteId = testParams.quoteId ?: 'TEST_NONEXISTENT_QUOTE'
+ Map serviceCtx = [
+ userLogin: userLogin,
+ quoteId: quoteId,
+ ]
+ Map serviceResult = dispatcher.runSync('checkUpdateQuoteStatus',
serviceCtx)
+ assert ServiceUtil.isError(serviceResult)
+ assert ServiceUtil.getErrorMessage(serviceResult) ==
+ UtilProperties.getMessage('OrderErrorUiLabels',
'OrderQuoteDoesNotExists', Locale.US)
+ }
+
+ // Regression coverage for the update() DSL / ServiceErrorException catch
site added in
+ // updateQuoteItem(): a quoteId/quoteItemSeqId with no matching QuoteItem
record must come back
+ // as a service error carrying the original localized
OrderQuoteItemDoesNotExists message, not
+ // the generic EntityUpdateBuilder message and not a silently-successful
result.
+ @Test
+ @Order(27)
+ void testUpdateQuoteItemNotFound() {
+ String quoteId = testParams.quoteId ?: 'TEST_NONEXISTENT_QUOTE'
+ String quoteItemSeqId = testParams.quoteItemSeqId ?: '00001'
+ Map serviceCtx = [
+ userLogin: userLogin,
+ quoteId: quoteId,
+ quoteItemSeqId: quoteItemSeqId,
+ ]
+ Map serviceResult = dispatcher.runSync('updateQuoteItem', serviceCtx)
+ assert ServiceUtil.isError(serviceResult)
+ assert ServiceUtil.getErrorMessage(serviceResult) ==
+ UtilProperties.getMessage('OrderErrorUiLabels',
'OrderQuoteItemDoesNotExists', Locale.US)
+ }
+
+ // Regression coverage for the update() DSL / ServiceErrorException catch
site added in
+ // autoUpdateQuotePrice(): a quoteId/quoteItemSeqId with no matching
QuoteItem record must come
+ // back as a service error carrying the original localized
OrderQuoteItemDoesNotExists message,
+ // not the generic EntityUpdateBuilder message and not a
silently-successful result.
+ @Test
+ @Order(28)
+ void testAutoUpdateQuotePriceNotFound() {
+ String quoteId = testParams.quoteId ?: 'TEST_NONEXISTENT_QUOTE'
+ String quoteItemSeqId = testParams.quoteItemSeqId ?: '00001'
+ Map serviceCtx = [
+ userLogin: userLogin,
+ quoteId: quoteId,
+ quoteItemSeqId: quoteItemSeqId,
+ defaultQuoteUnitPrice: BigDecimal.valueOf(12)
+ ]
+ Map serviceResult = dispatcher.runSync('autoUpdateQuotePrice',
serviceCtx)
+ assert ServiceUtil.isError(serviceResult)
+ assert ServiceUtil.getErrorMessage(serviceResult) ==
+ UtilProperties.getMessage('OrderErrorUiLabels',
'OrderQuoteItemDoesNotExists', Locale.US)
+ }
+
}
diff --git
a/applications/party/src/main/groovy/org/apache/ofbiz/party/contact/ContactMechServicesScript.groovy
b/applications/party/src/main/groovy/org/apache/ofbiz/party/contact/ContactMechServicesScript.groovy
index af681b16bd..8cd054624e 100644
---
a/applications/party/src/main/groovy/org/apache/ofbiz/party/contact/ContactMechServicesScript.groovy
+++
b/applications/party/src/main/groovy/org/apache/ofbiz/party/contact/ContactMechServicesScript.groovy
@@ -75,7 +75,8 @@ String hasValidStateProvince(String countryGeoId, String
stateProvinceGeoId) {
*/
Map createPostalAddress() {
String errorMessage = hasValidStateProvince(parameters.countryGeoId,
parameters.stateProvinceGeoId)
- require(!(errorMessage), 'PartyUiLabels', errorMessage)
+ boolean isStateProvinceValid = !errorMessage
+ require(isStateProvinceValid, 'PartyUiLabels', errorMessage)
GenericValue newValue = makeValue('PostalAddress', parameters)
Map createContactMechMap = [contactMechTypeId: 'POSTAL_ADDRESS',
contactMechId: parameters.contactMechId]
Map serviceResult = run service: 'createContactMech', with:
createContactMechMap
@@ -91,7 +92,8 @@ Map createPostalAddress() {
*/
Map updatePostalAddress() {
String errorMessage = hasValidStateProvince(parameters.countryGeoId,
parameters.stateProvinceGeoId)
- require(!(errorMessage), 'PartyUiLabels', errorMessage)
+ boolean isStateProvinceValid = !errorMessage
+ require(isStateProvinceValid, 'PartyUiLabels', errorMessage)
GenericValue lookedValue = from('PostalAddress').where('contactMechId',
parameters.contactMechId).queryOne()
require(lookedValue as boolean, 'ServiceErrorUiLabels',
'ServiceValueNotFound')
GenericValue newValue = (GenericValue) lookedValue.clone()
diff --git
a/applications/party/src/main/groovy/org/apache/ofbiz/party/party/PartyInvitationServices.groovy
b/applications/party/src/main/groovy/org/apache/ofbiz/party/party/PartyInvitationServices.groovy
index ba9ca1e667..148dfdc435 100644
---
a/applications/party/src/main/groovy/org/apache/ofbiz/party/party/PartyInvitationServices.groovy
+++
b/applications/party/src/main/groovy/org/apache/ofbiz/party/party/PartyInvitationServices.groovy
@@ -20,6 +20,7 @@ package org.apache.ofbiz.party.party
import org.apache.ofbiz.base.util.UtilDateTime
import org.apache.ofbiz.entity.GenericValue
+import org.apache.ofbiz.service.ServiceErrorException
// Party Invitation Services
Map createPartyInvitation() {
@@ -36,15 +37,15 @@ Map createPartyInvitation() {
}
Map updatePartyInvitation() {
- GenericValue lookedUpValue =
from('PartyInvitation').where(parameters).queryOne()
- if (!lookedUpValue) {
- return error('PartyUiLabels', 'PartyInvitationNotValidError')
- }
+ Map fieldsToSet = new HashMap(parameters)
if (! parameters.toName && parameters.partyId) {
- parameters.toName = PartyHelper.getPartyName(delegator,
parameters.partyId, false)
+ fieldsToSet.toName = PartyHelper.getPartyName(delegator,
parameters.partyId, false)
+ }
+ try {
+ update('PartyInvitation').where(parameters).set(fieldsToSet)
+ } catch (ServiceErrorException e) {
+ return error('PartyUiLabels', 'PartyInvitationNotValidError')
}
- lookedUpValue.setNonPKFields(parameters)
- lookedUpValue.store()
return success()
}
diff --git
a/applications/party/src/main/groovy/org/apache/ofbiz/party/party/PartyServicesScript.groovy
b/applications/party/src/main/groovy/org/apache/ofbiz/party/party/PartyServicesScript.groovy
index c364777e1a..0786627a75 100644
---
a/applications/party/src/main/groovy/org/apache/ofbiz/party/party/PartyServicesScript.groovy
+++
b/applications/party/src/main/groovy/org/apache/ofbiz/party/party/PartyServicesScript.groovy
@@ -684,7 +684,8 @@ Map createUpdatePerson() {
'person', parameters, personContext, messages, context.locale)
// Check errors
- require(!(messages), StringUtil.join(messages, ','))
+ boolean hasNoValidationErrors = !messages
+ require(hasNoValidationErrors, StringUtil.join(messages, ','))
GenericValue party = from('Party')
.where(partyId: partyId)
@@ -714,7 +715,8 @@ Map quickCreateCustomer() {
'emailAddress', parameters, emailContext, messages, context.locale)
// Check errors
- require(!(messages), StringUtil.join(messages, ','))
+ boolean hasNoValidationErrors = !messages
+ require(hasNoValidationErrors, StringUtil.join(messages, ','))
// Create person
Map serviceResult = run service: 'createPerson', with: personContext
diff --git
a/applications/party/src/test/groovy/org/apache/ofbiz/party/party/test/PartyMiscTests.groovy
b/applications/party/src/test/groovy/org/apache/ofbiz/party/party/test/PartyMiscTests.groovy
index c47c3ccdf8..ec826322ff 100644
---
a/applications/party/src/test/groovy/org/apache/ofbiz/party/party/test/PartyMiscTests.groovy
+++
b/applications/party/src/test/groovy/org/apache/ofbiz/party/party/test/PartyMiscTests.groovy
@@ -18,6 +18,7 @@
*/
package org.apache.ofbiz.party.party.test
+import org.apache.ofbiz.base.util.UtilProperties
import org.apache.ofbiz.entity.GenericValue
import org.apache.ofbiz.service.ServiceUtil
import org.apache.ofbiz.testtools.JunitJupiterTest
@@ -321,4 +322,22 @@ class PartyMiscTests implements JupiterTestHelper {
assert partyInvitation.emailAddress == emailAddress
}
+ // Regression coverage for the update() DSL / ServiceErrorException catch
site added in
+ // updatePartyInvitation(): a partyInvitationId with no matching
PartyInvitation record must
+ // come back as a service error carrying the original localized
PartyInvitationNotValidError
+ // message, not the generic EntityUpdateBuilder message and not a
silently-successful result.
+ @Test
+ @Order(16)
+ void testUpdatePartyInvitationNotFound() {
+ String partyInvitationId = testParams.partyInvitationId ?:
'TEST_NONEXISTENT_INVITE'
+ Map serviceCtx = [
+ partyInvitationId: partyInvitationId,
+ userLogin: userLogin
+ ]
+ Map serviceResult = dispatcher.runSync('updatePartyInvitation',
serviceCtx)
+ assert ServiceUtil.isError(serviceResult)
+ assert ServiceUtil.getErrorMessage(serviceResult) ==
+ UtilProperties.getMessage('PartyUiLabels',
'PartyInvitationNotValidError', Locale.US)
+ }
+
}
diff --git
a/applications/product/src/main/groovy/org/apache/ofbiz/product/product/category/CategoryServicesScript.groovy
b/applications/product/src/main/groovy/org/apache/ofbiz/product/product/category/CategoryServicesScript.groovy
index 2568d387af..b7720f8fee 100644
---
a/applications/product/src/main/groovy/org/apache/ofbiz/product/product/category/CategoryServicesScript.groovy
+++
b/applications/product/src/main/groovy/org/apache/ofbiz/product/product/category/CategoryServicesScript.groovy
@@ -56,7 +56,8 @@ Map createProductCategory() {
if (parameters.productCategoryId) {
newEntity.productCategoryId = parameters.productCategoryId
String errorMessage =
UtilValidate.checkValidDatabaseId(newEntity.productCategoryId)
- require(!(errorMessage != null), errorMessage)
+ boolean isCategoryIdValid = errorMessage == null
+ require(isCategoryIdValid, errorMessage)
} else {
newEntity.productCategoryId = delegator.getNextSeqId('ProductCategory')
}
diff --git
a/applications/product/src/main/groovy/org/apache/ofbiz/product/product/inventory/InventoryServicesScript.groovy
b/applications/product/src/main/groovy/org/apache/ofbiz/product/product/inventory/InventoryServicesScript.groovy
index 4c18d8e611..35d7b1558e 100644
---
a/applications/product/src/main/groovy/org/apache/ofbiz/product/product/inventory/InventoryServicesScript.groovy
+++
b/applications/product/src/main/groovy/org/apache/ofbiz/product/product/inventory/InventoryServicesScript.groovy
@@ -152,12 +152,12 @@ Map createInventoryItem() {
// if inventoryItem's unitCost is still empty, or negative return an error
message
// TODO/WARNING: getProductCost returns 0 even if no std costs are found
- require(!(!inventoryItem.unitCost && inventoryItem.unitCost !=
(BigDecimal) 0),
+ require(inventoryItem.unitCost || inventoryItem.unitCost == (BigDecimal) 0,
label('ProductUiLabels', 'FacilityInventoryItemsMissingUnitCost'))
// if you don't want inventory item with unitCost = 0, change the operator
// attribute from "less" to "less-equals".
- require(!(inventoryItem.unitCost < (BigDecimal) 0),
label('ProductUiLabels', 'FacilityInventoryItemsNegativeUnitCost'))
+ require(inventoryItem.unitCost >= (BigDecimal) 0, label('ProductUiLabels',
'FacilityInventoryItemsNegativeUnitCost'))
inventoryItem.inventoryItemId = delegator.getNextSeqId('InventoryItem')
inventoryItem.create()
diff --git
a/applications/product/src/main/groovy/org/apache/ofbiz/product/product/product/ProductServicesScript.groovy
b/applications/product/src/main/groovy/org/apache/ofbiz/product/product/product/ProductServicesScript.groovy
index 23199d6256..d9f6702719 100644
---
a/applications/product/src/main/groovy/org/apache/ofbiz/product/product/product/ProductServicesScript.groovy
+++
b/applications/product/src/main/groovy/org/apache/ofbiz/product/product/product/ProductServicesScript.groovy
@@ -42,7 +42,8 @@ Map createProduct() {
GenericValue newEntity = makeValue('Product', parameters)
if (newEntity.productId) {
String errorMessage =
UtilValidate.checkValidDatabaseId(newEntity.productId)
- require(!(errorMessage), errorMessage)
+ boolean isProductIdValid = !errorMessage
+ require(isProductIdValid, errorMessage)
GenericValue dummyProduct =
from('Product').where(parameters).queryOne()
require(!(dummyProduct), 'CommonErrorUiLabels',
'CommonErrorDuplicateKey')
} else {
diff --git
a/applications/product/src/main/groovy/org/apache/ofbiz/product/product/promo/PromoServicesScript.groovy
b/applications/product/src/main/groovy/org/apache/ofbiz/product/product/promo/PromoServicesScript.groovy
index 7cbc2a68b3..706f64a8f4 100644
---
a/applications/product/src/main/groovy/org/apache/ofbiz/product/product/promo/PromoServicesScript.groovy
+++
b/applications/product/src/main/groovy/org/apache/ofbiz/product/product/promo/PromoServicesScript.groovy
@@ -19,6 +19,7 @@
package org.apache.ofbiz.product.product.promo
import org.apache.ofbiz.entity.GenericValue
+import org.apache.ofbiz.service.ServiceErrorException
Map createProductPromoCond() {
if (parameters.carrierShipmentMethod) {
@@ -31,14 +32,14 @@ Map createProductPromoCond() {
}
Map updateProductPromoCond() {
- GenericValue lookedUpValue =
from('ProductPromoCond').where(parameters).queryOne()
- if (lookedUpValue) {
- if (parameters.carrierShipmentMethod) {
- parameters.otherValue = parameters.carrierShipmentMethod
- }
- lookedUpValue.setNonPKFields(parameters)
- lookedUpValue.store()
- return success()
+ Map fieldsToSet = new HashMap(parameters)
+ if (parameters.carrierShipmentMethod) {
+ fieldsToSet.otherValue = parameters.carrierShipmentMethod
+ }
+ try {
+ update('ProductPromoCond').where(parameters).set(fieldsToSet)
+ } catch (ServiceErrorException e) {
+ fail(label('ServiceErrorUiLabels', 'ServiceValueNotFound'))
}
- fail(label('ServiceErrorUiLabels', 'ServiceValueNotFound'))
+ return success()
}
diff --git
a/applications/product/src/main/groovy/org/apache/ofbiz/product/product/store/ProductStoreServices.groovy
b/applications/product/src/main/groovy/org/apache/ofbiz/product/product/store/ProductStoreServices.groovy
index 3ea288f9e6..9c5ae0b553 100644
---
a/applications/product/src/main/groovy/org/apache/ofbiz/product/product/store/ProductStoreServices.groovy
+++
b/applications/product/src/main/groovy/org/apache/ofbiz/product/product/store/ProductStoreServices.groovy
@@ -37,8 +37,8 @@ Map createProductStore() {
Map result = success()
require(security.hasEntityPermission('CATALOG', '_CREATE',
parameters.userLogin) as boolean,
'ProductUiLabels', 'ProductCatalogCreatePermissionError')
- require(!(parameters.oneInventoryFacility == 'Y'
- && !parameters.inventoryFacilityId), 'ProductUiLabels',
'InventoryFacilityIdRequired')
+ require(parameters.oneInventoryFacility != 'Y'
+ || parameters.inventoryFacilityId, 'ProductUiLabels',
'InventoryFacilityIdRequired')
if (parameters.showPricesWithVatTax == 'Y') {
require(parameters.vatTaxAuthGeoId as boolean, 'ProductUiLabels',
'ProductVatTaxAuthGeoNotSet')
require(parameters.vatTaxAuthPartyId as boolean, 'ProductUiLabels',
'ProductVatTaxAuthPartyNotSet')
@@ -67,7 +67,7 @@ Map createProductStore() {
Map updateProductStore() {
require(security.hasEntityPermission('CATALOG', '_UPDATE',
parameters.userLogin) as boolean,
'ProductUiLabels', 'ProductCatalogUpdatePermissionError')
- require(!(parameters.oneInventoryFacility == 'Y' &&
!parameters.inventoryFacilityId),
+ require(parameters.oneInventoryFacility != 'Y' ||
parameters.inventoryFacilityId,
'ProductUiLabels', 'InventoryFacilityIdRequired')
GenericValue store = from('ProductStore').where(productStoreId:
parameters.productStoreId).queryOne()
String oldFacilityId = store.inventoryFacilityId
diff --git
a/applications/product/src/main/groovy/org/apache/ofbiz/product/shipment/ShipmentReceiptServices.groovy
b/applications/product/src/main/groovy/org/apache/ofbiz/product/shipment/ShipmentReceiptServices.groovy
index 33897d5763..cafb44cb7c 100644
---
a/applications/product/src/main/groovy/org/apache/ofbiz/product/shipment/ShipmentReceiptServices.groovy
+++
b/applications/product/src/main/groovy/org/apache/ofbiz/product/shipment/ShipmentReceiptServices.groovy
@@ -74,8 +74,8 @@ Map receiveInventoryProduct () {
// Return an error if both quantityAccepted and quantityRejected are zero
or less than zero
BigDecimal quantityRejected = parameters.quantityRejected ?:
BigDecimal.ZERO
- require(!((quantityRejected == BigDecimal.ZERO &&
parameters.quantityAccepted == BigDecimal.ZERO)
- || (quantityRejected < BigDecimal.ZERO || parameters.quantityAccepted
< BigDecimal.ZERO)),
+ require((quantityRejected != BigDecimal.ZERO ||
parameters.quantityAccepted != BigDecimal.ZERO)
+ && (quantityRejected >= BigDecimal.ZERO && parameters.quantityAccepted
>= BigDecimal.ZERO),
'ProductUiLabels', 'ProductNoItemsToAcceptOrReject')
Map result = success()
diff --git
a/applications/product/src/main/groovy/org/apache/ofbiz/product/shipment/ShipmentServices.groovy
b/applications/product/src/main/groovy/org/apache/ofbiz/product/shipment/ShipmentServices.groovy
index 00b6646369..2b2d30f2fc 100644
---
a/applications/product/src/main/groovy/org/apache/ofbiz/product/shipment/ShipmentServices.groovy
+++
b/applications/product/src/main/groovy/org/apache/ofbiz/product/shipment/ShipmentServices.groovy
@@ -60,7 +60,8 @@ Map updateShipment() {
}
}
// now finally check for errors
- require(!(errorList), errorList.toString())
+ boolean hasNoValidationErrors = !errorList
+ require(hasNoValidationErrors, errorList.toString())
Map serviceResult = run service: 'checkAndUpdateWorkEffort', with:
parameters
require(ServiceUtil.isSuccess(serviceResult) as boolean,
serviceResult.errorMessage)
@@ -756,15 +757,15 @@ Map quickShipEntireOrder() {
List shipmentShipGroupFacilityList
// first get the order header; make sure we have a product store
GenericValue orderHeader = from('OrderHeader').where(parameters).queryOne()
- require(!(!orderHeader || !orderHeader.productStoreId),
+ require(orderHeader && orderHeader.productStoreId,
'ProductUiLabels', 'FacilityShipmentMissingProductStore')
// get the product store entity
GenericValue productStore = from('ProductStore').where(productStoreId:
orderHeader.productStoreId).queryOne()
// no reservations; no shipment; cannot use quick ship
- require(!('Y' != productStore?.reserveInventory), 'ProductUiLabels',
+ require(productStore?.reserveInventory == 'Y', 'ProductUiLabels',
'FacilityShipmentNotCreatedForNotReserveInventory', [productStore:
productStore])
// can't insert duplicate rows in shipmentPackageContent
- require(!(productStore.explodeOrderItems == 'Y'), 'ProductUiLabels',
+ require(productStore.explodeOrderItems != 'Y', 'ProductUiLabels',
'FacilityShipmentNotCreatedForExplodesOrderItems', [productStore:
productStore])
// locate shipping facilities associated with order item rez's
List orderItemShipGrpInvResFacilityIds =
from('OrderItemAndShipGrpInvResAndItem')
@@ -1313,12 +1314,12 @@ Map addOrderShipmentToShipment() {
// get orderItem
GenericValue orderItem = from('OrderItem').where(parameters).queryOne()
// make sure the orderItem is not already present in this shipment
- require(!(from('OrderShipment')
+ require(from('OrderShipment')
.where(orderId: parameters.orderId,
orderItemSeqId: parameters.orderItemSeqId,
shipGroupSeqId: parameters.shipGroupSeqId,
shipmentId: parameters.shipmentId)
- .queryCount() != 0),
+ .queryCount() == 0,
"Not adding Order Item to plan for shipment
[${parameters.shipmentId}] because" +
" the order item is already in the shipment (order
[${parameters.orderId}]," +
" order item [${parameters.orderItemSeqId}])")
@@ -1329,7 +1330,7 @@ Map addOrderShipmentToShipment() {
}
BigDecimal remainingQuantity = serviceResult.remainingQuantity
- require(!(parameters.quantity > remainingQuantity),
+ require(parameters.quantity <= remainingQuantity,
"Not adding Order Item to plan for shipment
[${parameters.shipmentId}] because" +
' the quantity is greater than the remaining quantity' +
" (order [${parameters.orderId}], order item
[${parameters.orderItemSeqId}])")
diff --git
a/applications/product/src/test/groovy/org/apache/ofbiz/product/product/test/ProductPromoCondTests.groovy
b/applications/product/src/test/groovy/org/apache/ofbiz/product/product/test/ProductPromoCondTests.groovy
index 6ecb5e5f08..8f17a06be8 100644
---
a/applications/product/src/test/groovy/org/apache/ofbiz/product/product/test/ProductPromoCondTests.groovy
+++
b/applications/product/src/test/groovy/org/apache/ofbiz/product/product/test/ProductPromoCondTests.groovy
@@ -20,6 +20,7 @@ package org.apache.ofbiz.product.product.test
import java.sql.Timestamp
import org.apache.ofbiz.base.util.UtilDateTime
+import org.apache.ofbiz.base.util.UtilProperties
import org.apache.ofbiz.entity.GenericValue
import org.apache.ofbiz.order.shoppingcart.ShoppingCart
import org.apache.ofbiz.testtools.JunitJupiterTest
@@ -393,6 +394,66 @@ class ProductPromoCondTests implements JupiterTestHelper {
assert serviceResult.compareBase < 0
}
+ // Regression coverage for the update() DSL / ServiceErrorException catch
site added in
+ // updateProductPromoCond(): a PK with no matching ProductPromoCond record
must come back as a
+ // service error carrying the original localized ServiceValueNotFound
message, not the generic
+ // EntityUpdateBuilder message and not a silently-successful result.
+ @Test
+ @Order(12)
+ void testUpdateProductPromoCondNotFound() {
+ String productPromoId = testParams.productPromoId ?:
'TEST_NONEXISTENT_PROMO'
+ String productPromoRuleId = testParams.productPromoRuleId ?: '01'
+ String productPromoCondSeqId = testParams.productPromoCondSeqId ?: '01'
+ Map serviceCtx = [
+ productPromoId: productPromoId,
+ productPromoRuleId: productPromoRuleId,
+ productPromoCondSeqId: productPromoCondSeqId,
+ userLogin: userLogin
+ ]
+ Map serviceResult = dispatcher.runSync('updateProductPromoCond',
serviceCtx)
+ assert ServiceUtil.isError(serviceResult)
+ assert ServiceUtil.getErrorMessage(serviceResult) ==
+ UtilProperties.getMessage('ServiceErrorUiLabels',
'ServiceValueNotFound', Locale.US)
+ }
+
+ // updateProductPromoCond() had no coverage at all before this - not even
a happy-path test.
+ // Creates a fresh ProductPromoCond fixture directly, then calls the
service and verifies the
+ // change actually persisted (this also exercises the update() DSL's
success path, which the
+ // not-found test above cannot).
+ @Test
+ @Order(13)
+ void testUpdateProductPromoCond() {
+ String productPromoId = testParams.productPromoId ?: 'TEST_PROMO_UPD'
+ String productPromoRuleId = testParams.productPromoRuleId ?: '01'
+ String productPromoCondSeqId = testParams.productPromoCondSeqId ?: '01'
+ GenericValue productPromo = delegator.makeValue('ProductPromo',
[productPromoId: productPromoId])
+ delegator.createOrStore(productPromo)
+ GenericValue productPromoRule = delegator.makeValue('ProductPromoRule',
+ [productPromoId: productPromoId, productPromoRuleId:
productPromoRuleId])
+ delegator.createOrStore(productPromoRule)
+ GenericValue productPromoCond = delegator.makeValue('ProductPromoCond',
+ [productPromoId: productPromoId, productPromoRuleId:
productPromoRuleId,
+ productPromoCondSeqId: productPromoCondSeqId, condValue:
'OLD_VALUE'])
+ delegator.createOrStore(productPromoCond)
+
+ String newCondValue = testParams.condValue ?: 'NEW_VALUE'
+ Map serviceCtx = [
+ productPromoId: productPromoId,
+ productPromoRuleId: productPromoRuleId,
+ productPromoCondSeqId: productPromoCondSeqId,
+ condValue: newCondValue,
+ userLogin: userLogin
+ ]
+ Map serviceResult = dispatcher.runSync('updateProductPromoCond',
serviceCtx)
+ assert ServiceUtil.isSuccess(serviceResult)
+
+ GenericValue updated = from('ProductPromoCond')
+ .where('productPromoId', productPromoId, 'productPromoRuleId',
productPromoRuleId,
+ 'productPromoCondSeqId',
productPromoCondSeqId).queryOne()
+ assert updated
+ assert updated.condValue == newCondValue
+ }
+
private Map prepareConditionMap(ShoppingCart cart, String condValue) {
return prepareConditionMap(cart, condValue, false)
}
diff --git
a/applications/workeffort/src/main/groovy/org/apache/ofbiz/workeffort/workeffort/workeffort/WorkEffortServicesScript.groovy
b/applications/workeffort/src/main/groovy/org/apache/ofbiz/workeffort/workeffort/workeffort/WorkEffortServicesScript.groovy
index 9325d79dee..efed762390 100644
---
a/applications/workeffort/src/main/groovy/org/apache/ofbiz/workeffort/workeffort/workeffort/WorkEffortServicesScript.groovy
+++
b/applications/workeffort/src/main/groovy/org/apache/ofbiz/workeffort/workeffort/workeffort/WorkEffortServicesScript.groovy
@@ -198,7 +198,8 @@ Map createWorkEffort() {
GenericValue workEffort = makeValue('WorkEffort', parameters)
workEffort.workEffortId = workEffort.workEffortId ?:
delegator.getNextSeqId('WorkEffort')
String errMsg = UtilValidate.checkValidDatabaseId(workEffort.workEffortId)
- require(!(errMsg), errMsg)
+ boolean isWorkEffortIdValid = !errMsg
+ require(isWorkEffortIdValid, errMsg)
Timestamp now = UtilDateTime.nowTimestamp()
workEffort.setFields([lastStatusUpdate: now,
@@ -230,7 +231,7 @@ Map updateWorkEffort() {
if (parameters.currentStatusId && workEffort.currentStatusId &&
parameters.currentStatusId != workEffort.currentStatusId) {
Map statusValidChange = [statusId: workEffort.currentStatusId,
statusIdTo: parameters.currentStatusId]
-
require(!(from('StatusValidChange').where(statusValidChange).queryCount() == 0),
+
require(from('StatusValidChange').where(statusValidChange).queryCount() > 0,
label('WorkEffortUiLabels', 'WorkEffortStatusChangeNotValid',
statusValidChange))
run service: 'createWorkEffortStatus', with: [*: parameters,
statusId:
parameters.currentStatusId,
@@ -256,12 +257,10 @@ Map updateWorkEffort() {
*/
Map deleteWorkEffort() {
// check permissions before moving on: if update or delete logged in user
must be associated OR have corresponding UPDATE or DELETE permissions
- require(!(from('WorkEffortPartyAssignment')
- .where(workEffortId: parameters.workEffortId,
- partyId: userLogin.partyId)
- .queryCount() == 0 &&
- !security.hasEntityPermission('WORKEFFORTMGR', '_DELETE',
userLogin)),
- label('WorkEffortUiLabels', 'WorkEffortDeletePermissionError'))
+ boolean isAssignedToWorkEffort = from('WorkEffortPartyAssignment')
+ .where(workEffortId: parameters.workEffortId, partyId:
userLogin.partyId)
+ .queryCount() > 0
+ requireWorkEffortDeletePermission(isAssignedToWorkEffort)
GenericValue workEffort = from('WorkEffort').where(parameters).queryOne()
@@ -324,12 +323,11 @@ Map copyWorkEffort() {
* @return Success response containing the workEffortId, error response
otherwise.
*/
Map duplicateWorkEffort() {
- require(!((parameters.removeWorkEffortAssocs == 'Y' ||
+ boolean removesWorkEffortData = parameters.removeWorkEffortAssocs == 'Y' ||
parameters.removeWorkEffortContents == 'Y' ||
parameters.removeWorkEffortNotes == 'Y' ||
- parameters.removeWorkEffortAssignmentRates == 'Y') &&
- !security.hasEntityPermission('WORKEFFORTMGR', '_DELETE',
userLogin)),
- label('WorkEffortUiLabels', 'WorkEffortDeletePermissionError'))
+ parameters.removeWorkEffortAssignmentRates == 'Y'
+ requireWorkEffortDeletePermission(!removesWorkEffortData)
String workEffortId = parameters.workEffortId ?:
delegator.getNextSeqId('WorkEffort')
GenericValue oldWorkEffort = from('WorkEffort').where(workEffortId:
parameters.oldWorkEffortId).queryOne()
GenericValue duplicateWorkEffort = oldWorkEffort.clone()
@@ -407,7 +405,7 @@ void duplicateWorkEffortAssoc(String relationEntityName,
String oldWorkEffortId,
Map assocAcceptedCustRequestToWorkEffort() {
// check status of customer request if valid
GenericValue custRequet =
from('CustRequest').where(parameters).cache().queryOne()
- require(!(custRequet.statusId != 'CRQ_ACCEPTED'), label('CommonUiLabels',
'CommonErrorStatusNotValid'))
+ require(custRequet.statusId == 'CRQ_ACCEPTED', label('CommonUiLabels',
'CommonErrorStatusNotValid'))
// create customer request / work effort relation
run service: 'createWorkEffortRequest', with: parameters
@@ -547,3 +545,8 @@ Map updateWorkEffortContactMech() {
}
return success([contactMechId: newContactMechId, oldContactMechId:
workEffortContactMech.contactMechId])
}
+
+private void requireWorkEffortDeletePermission(boolean permissionBypassed) {
+ require(permissionBypassed ||
security.hasEntityPermission('WORKEFFORTMGR', '_DELETE', userLogin),
+ label('WorkEffortUiLabels', 'WorkEffortDeletePermissionError'))
+}
diff --git
a/framework/common/src/main/groovy/org/apache/ofbiz/common/PortalPageServices.groovy
b/framework/common/src/main/groovy/org/apache/ofbiz/common/PortalPageServices.groovy
index 456ed3f3aa..626f2b37cf 100644
---
a/framework/common/src/main/groovy/org/apache/ofbiz/common/PortalPageServices.groovy
+++
b/framework/common/src/main/groovy/org/apache/ofbiz/common/PortalPageServices.groovy
@@ -127,7 +127,7 @@ Map deletePortalPagePortlet() {
* @return Success response with all attributes
*/
Map getPortletAttributes() {
- require(!(!parameters.ownerUserLoginId && !parameters.portalPageId),
+ require(parameters.ownerUserLoginId || parameters.portalPageId,
'Service getPortletAttributes did not receive either ownerUserLoginId
OR portalPageId')
if (parameters.ownerUserLoginId) {
GenericValue portalPagePortlet = from('PortalPageAndPortlet')
