That makes quite sense, thanks Jacopo!
Jacques
Le 23/08/2016 à 09:50, Jacopo Cappellato a écrit :
Hi Pierre,
cmssite, ecommerce and webpos are specialpurpose components that can rely
on the artifacts (services, entities, labels, screens and *classes*)
defined in the application component (including the "marketing" component
to which TrackingCodeEvents belongs).
With that said, there is a chance that you are misunderstanding the purpose
of Events vs services.
Events should be used for web applications (i.e. they are http aware) while
services should be used to implement re-usable business logic that can be
used in a wider scope (i.e. not limited to web applications, like batch
jobs, integrations with external systems etc...).
Untangling the components' dependencies (or merging mutual dependent parts
into one component, if not possible otherwise) is an important task but it
requires proper (and not so obvious) design and tools; the recent move to
Gradle is a small step in the right direction, in my opinion, but a lot
more design work is required to clear the dependencies in an organized and
consistent way.
And at this stage I don't think that taking shortcuts, like introducing new
settings to disable the calls to the service of another component, as was
proposed in your and Jacques' contribution, is the right way to go because
it will make the system more difficult to configure and manage to our users.
And in general, by converting an event to a service, we indeed remove a
compile time dependency but the runtime dependency is still there: and this
is risky to adopters that may think that there is no dependency and may
disable a component that is instead required by the system.
Kind regards,
Jacopo
On Mon, Aug 22, 2016 at 9:16 PM, Pierre Smits <[email protected]>
wrote:
Hi Jacopo,
I see that most functions in TrackingCodeEvents.java are used in cmssite,
ecommerce and webpos. Should we not convert all to services?
Best regards,
Pierre Smits
ORRTIZ.COM <http://www.orrtiz.com>
OFBiz based solutions & services
OFBiz Extensions Marketplace
http://oem.ofbizci.net/oci-2/
On Mon, Aug 22, 2016 at 4:00 PM, Jacopo Cappellato <jacopo.cappellato@
hotwaxsystems.com> wrote:
Hi Pierre,
since Jacques responded to my mail, acknowledging that there are issues
and
that he is going to fix them, I will let him complete the work before
digging into the code details.
Anyway, they are pretty basic issues and I am concerned that you and
Jacques couldn't identify them with your testing and review process: this
is why I was asking about them.
Kind regards,
Jacopo
On Mon, Aug 22, 2016 at 3:21 PM, Pierre Smits <[email protected]>
wrote:
Hi Jacopo,
Can you explain why it seems 'completely' wrong to you?
Best regards,
Pierre Smits
ORRTIZ.COM <http://www.orrtiz.com>
OFBiz based solutions & services
OFBiz Extensions Marketplace
http://oem.ofbizci.net/oci-2/
On Mon, Aug 22, 2016 at 12:43 PM, Jacopo Cappellato <
[email protected]> wrote:
This contribution seems completely wrong to me. Pierre and Jacques,
have
you performed proper tests and reviews before committing it?
Jacopo
On Mon, Aug 22, 2016 at 11:58 AM, <[email protected]> wrote:
Author: jleroux
Date: Mon Aug 22 09:58:35 2016
New Revision: 1757130
URL: http://svn.apache.org/viewvc?rev=1757130&view=rev
Log:
A modified patch from Pierre Smits for "remove build dependency of
Order
on Marketing" https://issues.apache.org/jira/browse/OFBIZ-7966
Currently there is a build dependency from order -
CheckOutEvents.java
on
marketing - TrackingCodeEvents.java
The createOrder function (in CheckOutEvents.java) calls the
makeTrackingCodeOrders function in TrackingCodeEvents.java
jleroux: I merged parts of the 2 patches, the 2nd was good but
missing
the
makeTrackingCodeOrders service definition. I also fixed the warning
about
trackingCodeOrdersList creation not being generic
Modified:
ofbiz/trunk/applications/marketing/servicedef/services.xml
ofbiz/trunk/applications/marketing/src/main/java/org/
apache/ofbiz/marketing/tracking/TrackingCodeEvents.java
ofbiz/trunk/applications/order/data/
OrderSystemPropertyData.xml
ofbiz/trunk/applications/order/src/main/java/org/
apache/ofbiz/order/
shoppingcart/CheckOutEvents.java
ofbiz/trunk/applications/order/src/main/java/org/
apache/ofbiz/order/
shoppingcart/CheckOutHelper.java
Modified: ofbiz/trunk/applications/marketing/servicedef/services.
xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/
marketing/servicedef/services.xml?rev=1757130&r1=1757129&r2=
1757130&view=diff
============================================================
==================
--- ofbiz/trunk/applications/marketing/servicedef/services.xml
(original)
+++ ofbiz/trunk/applications/marketing/servicedef/services.xml Mon
Aug
22
09:58:35 2016
@@ -419,6 +419,12 @@ under the License.
<attribute type="String" mode="IN" name="returnId"
optional="false"/>
</service>
+ <service name="makeTrackingCodeOrders" engine="java"
location="org.apache.ofbiz.marketing.tracking.TrackingCodeEvents"
invoke="makeTrackingCodeOrders"
auth="true">
+ <description>Makes a list of TrackingCodeOrder entities to
be
attached to the current order</description>
+ <attribute name="request" mode="IN"
type="javax.servlet.http.
HttpServletRequest"/>
+ <attribute name="trackingCodeOrders" type="List"
mode="OUT"
optional="false"/>
+ </service>
+
<!-- marketing permission service -->
<service name="marketingPermissionService" engine="simple"
location="component://common/minilang/permission/
CommonPermissionServices.xml"
invoke="genericBasePermissionCheck">
Modified: ofbiz/trunk/applications/marketing/src/main/java/org/
apache/ofbiz/marketing/tracking/TrackingCodeEvents.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/
marketing/src/main/java/org/apache/ofbiz/marketing/
tracking/TrackingCodeEvents.java?rev=1757130&r1=1757129&
r2=1757130&view=diff
============================================================
==================
--- ofbiz/trunk/applications/marketing/src/main/java/org/
apache/ofbiz/marketing/tracking/TrackingCodeEvents.java (original)
+++ ofbiz/trunk/applications/marketing/src/main/java/org/
apache/ofbiz/marketing/tracking/TrackingCodeEvents.java Mon Aug 22
09:58:35 2016
@@ -31,14 +31,14 @@ import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.UtilDateTime;
import org.apache.ofbiz.base.util.UtilMisc;
import org.apache.ofbiz.base.util.UtilValidate;
-import org.apache.ofbiz.webapp.stats.VisitHandler;
-import org.apache.ofbiz.webapp.website.WebSiteWorker;
import org.apache.ofbiz.entity.Delegator;
import org.apache.ofbiz.entity.GenericEntityException;
import org.apache.ofbiz.entity.GenericValue;
import org.apache.ofbiz.entity.util.EntityQuery;
import org.apache.ofbiz.entity.util.EntityUtilProperties;
import org.apache.ofbiz.product.category.CategoryWorker;
+import org.apache.ofbiz.webapp.stats.VisitHandler;
+import org.apache.ofbiz.webapp.website.WebSiteWorker;
/**
* Events used for maintaining TrackingCode related information
@@ -285,7 +285,7 @@ public class TrackingCodeEvents {
String prodCatalogId = trackingCode.getString("
prodCatalogId");
if (UtilValidate.isNotEmpty(prodCatalogId)) {
session.setAttribute("CURRENT_CATALOG_ID",
prodCatalogId);
- CategoryWorker.setTrail(request, new LinkedList());
+ CategoryWorker.setTrail(request, new
LinkedList<String>());
}
// if forward/redirect is needed, do a
response.sendRedirect
and
return null to tell the control servlet to not do any other
requests/views
Modified: ofbiz/trunk/applications/order/data/
OrderSystemPropertyData.xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/
data/
OrderSystemPropertyData.xml?rev=1757130&r1=1757129&r2=
1757130&view=diff
============================================================
==================
--- ofbiz/trunk/applications/order/data/
OrderSystemPropertyData.xml
(original)
+++ ofbiz/trunk/applications/order/data/
OrderSystemPropertyData.xml
Mon
Aug 22 09:58:35 2016
@@ -50,4 +50,12 @@ order.properties setting is: order.item.
description="Allow comment on Order Item. Choices are: Y or
N."
/>
+<!--
+#
+marketing.tracking.enable=Y
+-->
+ <SystemProperty systemResourceId="order"
systemPropertyId="marketing.tracking.enable"
systemPropertyValue="Y"
+ description="Allow tracking in marketing component (optional).
Choices are: Y or N."
+ />
+
</entity-engine-xml>
\ No newline at end of file
Modified: ofbiz/trunk/applications/order/src/main/java/org/
apache/ofbiz/order/shoppingcart/CheckOutEvents.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/
order/src/main/java/org/apache/ofbiz/order/
shoppingcart/CheckOutEvents.
java?rev=1757130&r1=1757129&r2=1757130&view=diff
============================================================
==================
--- ofbiz/trunk/applications/order/src/main/java/org/
apache/ofbiz/order/
shoppingcart/CheckOutEvents.java (original)
+++ ofbiz/trunk/applications/order/src/main/java/org/
apache/ofbiz/order/
shoppingcart/CheckOutEvents.java Mon Aug 22 09:58:35 2016
@@ -42,7 +42,7 @@ import org.apache.ofbiz.entity.Delegator
import org.apache.ofbiz.entity.GenericEntityException;
import org.apache.ofbiz.entity.GenericValue;
import org.apache.ofbiz.entity.util.EntityQuery;
-import org.apache.ofbiz.marketing.tracking.TrackingCodeEvents;
+import org.apache.ofbiz.entity.util.EntityUtilProperties;
import org.apache.ofbiz.order.order.OrderReadHelper;
import org.apache.ofbiz.party.party.PartyWorker;
import org.apache.ofbiz.product.store.ProductStoreWorker;
@@ -53,6 +53,7 @@ import org.apache.ofbiz.service.ServiceU
import org.apache.ofbiz.webapp.stats.VisitHandler;
import org.apache.ofbiz.webapp.website.WebSiteWorker;
+
/**
* Events used for processing checkout and orders.
*/
@@ -438,15 +439,28 @@ public class CheckOutEvents {
session.removeAttribute("_QUICK_REORDER_PRODUCTS_");
boolean areOrderItemsExploded =
explodeOrderItems(delegator,
cart);
-
- //get the TrackingCodeOrder List
- List<GenericValue> trackingCodeOrders =
TrackingCodeEvents.
makeTrackingCodeOrders(request);
+
+ //get the TrackingCodeOrder List
+ String trackingEnabled = EntityUtilProperties.
getPropertyValue("order","marketing.tracking.enable", delegator);
+ Map<String, Object> trackingCodeOrders = new
HashMap<String,
Object>();
+ if (trackingEnabled != null && trackingEnabled == "Y") {
+ //get the TrackingCodeOrder List
+ Map<String, Object> inMap = new HashMap<String,
Object>();
+ inMap.put("request", request);
+ try {
+ trackingCodeOrders = dispatcher.runSync("
makeTrackingCodeOrder",inMap);
+ } catch (GenericServiceException e) {
+ Debug.logError(e, module);
+ }
+ }
+
String distributorId = (String) session.getAttribute("_
DISTRIBUTOR_ID_");
String affiliateId = (String) session.getAttribute("_
AFFILIATE_ID_");
String visitId = VisitHandler.getVisitId(session);
String webSiteId = WebSiteWorker.getWebSiteId(request);
+ List<GenericValue> trackingCodeOrdersList =
UtilGenerics.checkList(new ArrayList<GenericValue>());
- callResult = checkOutHelper.createOrder(userLogin,
distributorId, affiliateId, trackingCodeOrders,
areOrderItemsExploded,
visitId, webSiteId);
+ callResult = checkOutHelper.createOrder(userLogin,
distributorId, affiliateId, trackingCodeOrdersList,
areOrderItemsExploded,
visitId, webSiteId);
if (callResult != null) {
ServiceUtil.getMessages(request, callResult, null);
if (ServiceUtil.isError(callResult)) {
Modified: ofbiz/trunk/applications/order/src/main/java/org/
apache/ofbiz/order/shoppingcart/CheckOutHelper.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/
order/src/main/java/org/apache/ofbiz/order/
shoppingcart/CheckOutHelper.
java?rev=1757130&r1=1757129&r2=1757130&view=diff
============================================================
==================
--- ofbiz/trunk/applications/order/src/main/java/org/
apache/ofbiz/order/
shoppingcart/CheckOutHelper.java (original)
+++ ofbiz/trunk/applications/order/src/main/java/org/
apache/ofbiz/order/
shoppingcart/CheckOutHelper.java Mon Aug 22 09:58:35 2016
@@ -559,7 +559,7 @@ public class CheckOutHelper {
// Create order event - uses createOrder service for
processing
public Map<String, Object> createOrder(GenericValue userLogin,
String
distributorId, String affiliateId,
- List<GenericValue> trackingCodeOrders, boolean
areOrderItemsExploded, String visitId, String webSiteId) {
+ List<GenericValue> trackingCodeOrdersList, boolean
areOrderItemsExploded, String visitId, String webSiteId) {
if (this.cart == null) {
return null;
}
@@ -575,7 +575,7 @@ public class CheckOutHelper {
Map<String, Object> context = this.cart.makeCartMap(this.
dispatcher,
areOrderItemsExploded);
//get the TrackingCodeOrder List
- context.put("trackingCodeOrders", trackingCodeOrders);
+ context.put("trackingCodeOrders",
trackingCodeOrdersList);
if (distributorId != null) context.put("distributorId",
distributorId);
if (affiliateId != null) context.put("affiliateId",
affiliateId);
