Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
Locked
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
 
|
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, <[hidden email]> 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); > > > |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
|
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 < [hidden email]> 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, <[hidden email]> 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); > > > > > > > |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
Administrator
|
I don't think it's 'completely wrong' but it's wrong indeed.
Since I put a finger in it, and Pierre does not seems to see what the problem is, I'll finish it Jacques Le 22/08/2016 à 15:21, Pierre Smits a écrit : > 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 < > [hidden email]> 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, <[hidden email]> 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); >>> >>> |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
|
In reply to this post by Pierre Smits
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 <[hidden email]> 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 < > [hidden email]> 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, <[hidden email]> 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); > > > > > > > > > > > > |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
|
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 <[hidden email]> > 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 < > > [hidden email]> 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, <[hidden email]> 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); > > > > > > > > > > > > > > > > > > |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
|
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 <[hidden email]> 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 <[hidden email]> > > 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 < > > > [hidden email]> 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, <[hidden email]> 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); > > > > > > > > > > > > > > > > > > > > > > > > > |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
Administrator
|
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 <[hidden email]> > 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 <[hidden email]> >>> 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 < >>>> [hidden email]> 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, <[hidden email]> 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); >>>>>> >>>>>> |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
|
Hi Jacopo,
Thank you for reacting. ERP solutions are, by their intent, modular in nature. This is driven by business requirements. And leads - by default - to runtime configuration. Which is especially important in multi-tenancy implementations. Some want solutions like cmssite, ecommerce and webpos. Some don't, Some have other solutions in place Some want solutions like humanres, manufacturing, inventory management and marketing & SFA solutions. Others don't or have something else in place (e.g. SalesForce for SFA/CRM, or Mailchimp for mailing list management). Runtime configuration in ERP solutions are as common as implementation experts.... Runtime configurations can easily be explained, including what the requirements are. And we have to face it, regarding functionalities the applications in the (optional) marketing component - when compared against Mailchimp and - SalesForce - are sub par. It seems we are reverting back to the discussion of what core is. In my opinion, solutions like humanres, manufacturing, inventory management, integrations of 3rd party payment and shipment solutions, and marketing/SFA are not. As components in the specialpurpose stack, and themes are not. It all depends on business requirements. Disentangling these solutions from the application stack into another would certainly increase the appeal for adopters. And would make OFBiz more easier to explain. With respect to OFBiz events and services, I see ambiguity in explanation. Events are web aware, but aren't to be used as services (as in the common understanding of the term). So exposing (or even transforming) these into services would be a good thing (where possible, to be assessed on a case by case scenario). Transforming these TrackingCodeEvents into services could be a good start as the impact is minor. And it would reduce compile time dependencies. Isn't that a goal to strive? Best regards, Pierre Smits ORRTIZ.COM <http://www.orrtiz.com> OFBiz based solutions & services OFBiz Extensions Marketplace http://oem.ofbizci.net/oci-2/ On Tue, Aug 23, 2016 at 10:05 AM, Jacques Le Roux < [hidden email]> wrote: > 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 <[hidden email]> >> 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 <[hidden email]> >>>> 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 < >>>>> [hidden email]> 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, <[hidden email]> 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); >>>>>> >>>>>>> >>>>>>> >>>>>>> > |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
|
Hi Pierre, Jacopo, Jacques,
Sticking specifically to the commit being discussed, the beauty of the tracking code logic is that it effectively does nothing if you aren't using it. Granted TrackingCodeEvents.makeTrackingCodeOrders() could do with being improved so that it exits earlier if no cookies are found, but it still ultimately does nothing functional if you aren't using it. It is true however that there is a build dependency that prevents users from simply removing the marketing component. And I agree that this could be an issue for users who want to strip OFBiz right down to just the components they want to use. Your solution of using a configuration and replacing the build dependency with a conditional runtime dependency does seem good on the surface. But I think it presents issues for a few reasons: 1. I don't think there should be a two step process to enabling/disabling something. Adding a component and then having to switch it on or removing a component and having to switch off its features seems like an unnecessary double step to me. 2. Having the configuration for one component used within another still creates a form of dependency between the two 3. This one kind of reiterates the first two points, but if we're going to deal with this then we should be considering a solution that would work as though the component were being added or removed via hot-deploy. We shouldn't take shortcuts just because we happen to have both components in the same repository. It also presents us with an opportunity to improve the framework for hot-deploy component developers. Rather than saying "we want to remove the build dependency between order and marketing" I think we should instead be saying "we want to allow another component to touch on web order processing without actually touching the order component". For services this is easy using Service ECAs, but unfortunately we don't have anything similar for controller events. I don't have the solution, but I think that's the line of thought we need to pick up in order to proceed effectively with this type of decoupling. Regards Scott On 23 August 2016 at 20:52, Pierre Smits <[hidden email]> wrote: > Hi Jacopo, > > Thank you for reacting. > > ERP solutions are, by their intent, modular in nature. This is driven by > business requirements. And leads - by default - to runtime configuration. > Which is especially important in multi-tenancy implementations. > > Some want solutions like cmssite, ecommerce and webpos. Some don't, Some > have other solutions in place > Some want solutions like humanres, manufacturing, inventory management and > marketing & SFA solutions. Others don't or have something else in place > (e.g. SalesForce for SFA/CRM, or Mailchimp for mailing list management). > > Runtime configuration in ERP solutions are as common as implementation > experts.... > Runtime configurations can easily be explained, including what the > requirements are. > > And we have to face it, regarding functionalities the applications in the > (optional) marketing component - when compared against Mailchimp and - > SalesForce - are sub par. > > It seems we are reverting back to the discussion of what core is. In my > opinion, solutions like humanres, manufacturing, inventory management, > integrations of 3rd party payment and shipment solutions, and marketing/SFA > are not. As components in the specialpurpose stack, and themes are not. It > all depends on business requirements. > Disentangling these solutions from the application stack into another would > certainly increase the appeal for adopters. And would make OFBiz more > easier to explain. > > With respect to OFBiz events and services, I see ambiguity in explanation. > Events are web aware, but aren't to be used as services (as in the common > understanding of the term). So exposing (or even transforming) these into > services would be a good thing (where possible, to be assessed on a case by > case scenario). Transforming these TrackingCodeEvents into services could > be a good start as the impact is minor. And it would reduce compile time > dependencies. Isn't that a goal to strive? > > Best regards, > > > Pierre Smits > > ORRTIZ.COM <http://www.orrtiz.com> > OFBiz based solutions & services > > OFBiz Extensions Marketplace > http://oem.ofbizci.net/oci-2/ > > On Tue, Aug 23, 2016 at 10:05 AM, Jacques Le Roux < > [hidden email]> wrote: > > > 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 <[hidden email]> > >> 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 <[hidden email] > > > >>>> 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 < > >>>>> [hidden email]> 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, <[hidden email]> 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); > >>>>>> > >>>>>>> > >>>>>>> > >>>>>>> > > > |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
|
Hi Scott,
Thank you for the feedback. This is something I can work with. What I see now is the following: 1. compile-time dependency from order on marketing regarding the makeTrackingCodeOrders function 2. run-time dependencies from cmssite, ecommerce and webpos several other functions in TrackingCodeEvents through controller references in those components. I rather see the run-time dependency than the compile-time dependency. With respect to solving the errors generated at run-time (and I think this can happen anywhere in any optional component that is depended upon), I see 3 potential solutions: 1. Have an generic solution - somewhere at a lower level- that first checks the existence of the referred event/service before executing it 2. Put the dependent event/service code in each optional component (in the TrackingCode case: order for make..., cmssite, ecommerce, webpos for the others) 3. Have it the dependency configurable. The first - to me - seems to be very complex. And it adds additional resource consumption. And then we still have to take into consideration that in a multi-tenancy setup some components (and their services/events) are not going to be used by 1 or more tenants. The second also seems undesirable. Best regards, Pierre Smits ORRTIZ.COM <http://www.orrtiz.com> OFBiz based solutions & services OFBiz Extensions Marketplace http://oem.ofbizci.net/oci-2/ On Wed, Aug 24, 2016 at 9:59 AM, Scott Gray <[hidden email]> wrote: > Hi Pierre, Jacopo, Jacques, > > Sticking specifically to the commit being discussed, the beauty of the > tracking code logic is that it effectively does nothing if you aren't using > it. Granted TrackingCodeEvents.makeTrackingCodeOrders() could do with > being improved so that it exits earlier if no cookies are found, but it > still ultimately does nothing functional if you aren't using it. > > It is true however that there is a build dependency that prevents users > from simply removing the marketing component. And I agree that this could > be an issue for users who want to strip OFBiz right down to just the > components they want to use. > > Your solution of using a configuration and replacing the build dependency > with a conditional runtime dependency does seem good on the surface. But I > think it presents issues for a few reasons: > 1. I don't think there should be a two step process to enabling/disabling > something. Adding a component and then having to switch it on or removing > a component and having to switch off its features seems like an unnecessary > double step to me. > 2. Having the configuration for one component used within another still > creates a form of dependency between the two > 3. This one kind of reiterates the first two points, but if we're going to > deal with this then we should be considering a solution that would work as > though the component were being added or removed via hot-deploy. We > shouldn't take shortcuts just because we happen to have both components in > the same repository. It also presents us with an opportunity to improve > the framework for hot-deploy component developers. > > Rather than saying "we want to remove the build dependency between order > and marketing" I think we should instead be saying "we want to allow > another component to touch on web order processing without actually > touching the order component". > > For services this is easy using Service ECAs, but unfortunately we don't > have anything similar for controller events. I don't have the solution, but > I think that's the line of thought we need to pick up in order to proceed > effectively with this type of decoupling. > > Regards > Scott > > > > On 23 August 2016 at 20:52, Pierre Smits <[hidden email]> wrote: > > > Hi Jacopo, > > > > Thank you for reacting. > > > > ERP solutions are, by their intent, modular in nature. This is driven by > > business requirements. And leads - by default - to runtime configuration. > > Which is especially important in multi-tenancy implementations. > > > > Some want solutions like cmssite, ecommerce and webpos. Some don't, Some > > have other solutions in place > > Some want solutions like humanres, manufacturing, inventory management > and > > marketing & SFA solutions. Others don't or have something else in place > > (e.g. SalesForce for SFA/CRM, or Mailchimp for mailing list management). > > > > Runtime configuration in ERP solutions are as common as implementation > > experts.... > > Runtime configurations can easily be explained, including what the > > requirements are. > > > > And we have to face it, regarding functionalities the applications in the > > (optional) marketing component - when compared against Mailchimp and - > > SalesForce - are sub par. > > > > It seems we are reverting back to the discussion of what core is. In my > > opinion, solutions like humanres, manufacturing, inventory management, > > integrations of 3rd party payment and shipment solutions, and > marketing/SFA > > are not. As components in the specialpurpose stack, and themes are not. > It > > all depends on business requirements. > > Disentangling these solutions from the application stack into another > would > > certainly increase the appeal for adopters. And would make OFBiz more > > easier to explain. > > > > With respect to OFBiz events and services, I see ambiguity in > explanation. > > Events are web aware, but aren't to be used as services (as in the common > > understanding of the term). So exposing (or even transforming) these into > > services would be a good thing (where possible, to be assessed on a case > by > > case scenario). Transforming these TrackingCodeEvents into services could > > be a good start as the impact is minor. And it would reduce compile time > > dependencies. Isn't that a goal to strive? > > > > Best regards, > > > > > > Pierre Smits > > > > ORRTIZ.COM <http://www.orrtiz.com> > > OFBiz based solutions & services > > > > OFBiz Extensions Marketplace > > http://oem.ofbizci.net/oci-2/ > > > > On Tue, Aug 23, 2016 at 10:05 AM, Jacques Le Roux < > > [hidden email]> wrote: > > > > > 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 <[hidden email] > > > > >> 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 < > [hidden email] > > > > > >>>> 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 < > > >>>>> [hidden email]> 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, <[hidden email]> 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); > > >>>>>> > > >>>>>>> > > >>>>>>> > > >>>>>>> > > > > > > |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
|
I was thinking that:
1. We need a means for the tracking code logic to be able to access the cookie data in the request without needing to add any logic to the order component 2. We need a means for the order tracking code logic to be able to create TrackingCodeOrder records when the order is created, again without adding any logic to the order component This could be achieved by: 1. Having a generic event that automatically adds all cookies to the session shopping cart as attributes. This could be done on each request or as a chained event running just before the createOrder event 2. Refactoring order creation to not depend so heavily on events (i.e. use a service that takes a shopping cart instead) 3. Add a service ECA (in the marketing component) to the service from #2 that allows the TrackingCodeOrder records to be created from the cart attributes Doing the above would remove the need for the order component to know anything at all about the tracking code logic. It would also allow developers to make use of #1 and #2 for other custom purposes that require reading cookies and/or interacting with the shopping cart while creating orders. Regards Scott On 24 August 2016 at 20:24, Pierre Smits <[hidden email]> wrote: > Hi Scott, > > Thank you for the feedback. This is something I can work with. > > What I see now is the following: > > 1. compile-time dependency from order on marketing regarding the > makeTrackingCodeOrders > function > 2. run-time dependencies from cmssite, ecommerce and webpos several > other functions in TrackingCodeEvents through controller references in > those components. > > I rather see the run-time dependency than the compile-time dependency. > > With respect to solving the errors generated at run-time (and I think this > can happen anywhere in any optional component that is depended upon), I see > 3 potential solutions: > > 1. Have an generic solution - somewhere at a lower level- that first > checks the existence of the referred event/service before executing it > 2. Put the dependent event/service code in each optional component (in > the TrackingCode case: order for make..., cmssite, ecommerce, webpos for > the others) > 3. Have it the dependency configurable. > > The first - to me - seems to be very complex. And it adds additional > resource consumption. And then we still have to take into consideration > that in a multi-tenancy setup some components (and their services/events) > are not going to be used by 1 or more tenants. > > The second also seems undesirable. > > Best regards, > > > Pierre Smits > > ORRTIZ.COM <http://www.orrtiz.com> > OFBiz based solutions & services > > OFBiz Extensions Marketplace > http://oem.ofbizci.net/oci-2/ > > On Wed, Aug 24, 2016 at 9:59 AM, Scott Gray <[hidden email]> > wrote: > > > Hi Pierre, Jacopo, Jacques, > > > > Sticking specifically to the commit being discussed, the beauty of the > > tracking code logic is that it effectively does nothing if you aren't > using > > it. Granted TrackingCodeEvents.makeTrackingCodeOrders() could do with > > being improved so that it exits earlier if no cookies are found, but it > > still ultimately does nothing functional if you aren't using it. > > > > It is true however that there is a build dependency that prevents users > > from simply removing the marketing component. And I agree that this > could > > be an issue for users who want to strip OFBiz right down to just the > > components they want to use. > > > > Your solution of using a configuration and replacing the build dependency > > with a conditional runtime dependency does seem good on the surface. > But I > > think it presents issues for a few reasons: > > 1. I don't think there should be a two step process to enabling/disabling > > something. Adding a component and then having to switch it on or > removing > > a component and having to switch off its features seems like an > unnecessary > > double step to me. > > 2. Having the configuration for one component used within another still > > creates a form of dependency between the two > > 3. This one kind of reiterates the first two points, but if we're going > to > > deal with this then we should be considering a solution that would work > as > > though the component were being added or removed via hot-deploy. We > > shouldn't take shortcuts just because we happen to have both components > in > > the same repository. It also presents us with an opportunity to improve > > the framework for hot-deploy component developers. > > > > Rather than saying "we want to remove the build dependency between order > > and marketing" I think we should instead be saying "we want to allow > > another component to touch on web order processing without actually > > touching the order component". > > > > For services this is easy using Service ECAs, but unfortunately we don't > > have anything similar for controller events. I don't have the solution, > but > > I think that's the line of thought we need to pick up in order to proceed > > effectively with this type of decoupling. > > > > Regards > > Scott > > > > > > > > On 23 August 2016 at 20:52, Pierre Smits <[hidden email]> wrote: > > > > > Hi Jacopo, > > > > > > Thank you for reacting. > > > > > > ERP solutions are, by their intent, modular in nature. This is driven > by > > > business requirements. And leads - by default - to runtime > configuration. > > > Which is especially important in multi-tenancy implementations. > > > > > > Some want solutions like cmssite, ecommerce and webpos. Some don't, > Some > > > have other solutions in place > > > Some want solutions like humanres, manufacturing, inventory management > > and > > > marketing & SFA solutions. Others don't or have something else in place > > > (e.g. SalesForce for SFA/CRM, or Mailchimp for mailing list > management). > > > > > > Runtime configuration in ERP solutions are as common as implementation > > > experts.... > > > Runtime configurations can easily be explained, including what the > > > requirements are. > > > > > > And we have to face it, regarding functionalities the applications in > the > > > (optional) marketing component - when compared against Mailchimp and - > > > SalesForce - are sub par. > > > > > > It seems we are reverting back to the discussion of what core is. In my > > > opinion, solutions like humanres, manufacturing, inventory management, > > > integrations of 3rd party payment and shipment solutions, and > > marketing/SFA > > > are not. As components in the specialpurpose stack, and themes are not. > > It > > > all depends on business requirements. > > > Disentangling these solutions from the application stack into another > > would > > > certainly increase the appeal for adopters. And would make OFBiz more > > > easier to explain. > > > > > > With respect to OFBiz events and services, I see ambiguity in > > explanation. > > > Events are web aware, but aren't to be used as services (as in the > common > > > understanding of the term). So exposing (or even transforming) these > into > > > services would be a good thing (where possible, to be assessed on a > case > > by > > > case scenario). Transforming these TrackingCodeEvents into services > could > > > be a good start as the impact is minor. And it would reduce compile > time > > > dependencies. Isn't that a goal to strive? > > > > > > Best regards, > > > > > > > > > Pierre Smits > > > > > > ORRTIZ.COM <http://www.orrtiz.com> > > > OFBiz based solutions & services > > > > > > OFBiz Extensions Marketplace > > > http://oem.ofbizci.net/oci-2/ > > > > > > On Tue, Aug 23, 2016 at 10:05 AM, Jacques Le Roux < > > > [hidden email]> wrote: > > > > > > > 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 < > [hidden email] > > > > > > >> 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 < > > [hidden email] > > > > > > > >>>> 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 < > > > >>>>> [hidden email]> 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, <[hidden email]> 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); > > > >>>>>> > > > >>>>>>> > > > >>>>>>> > > > >>>>>>> > > > > > > > > > > |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
Administrator
|
HI Scott,
This sounds like an actionable plan, any plan to action it? Jacques Le 25/08/2016 à 07:38, Scott Gray a écrit : > I was thinking that: > 1. We need a means for the tracking code logic to be able to access the > cookie data in the request without needing to add any logic to the order > component > 2. We need a means for the order tracking code logic to be able to create > TrackingCodeOrder records when the order is created, again without adding > any logic to the order component > > This could be achieved by: > 1. Having a generic event that automatically adds all cookies to the > session shopping cart as attributes. This could be done on each request or > as a chained event running just before the createOrder event > 2. Refactoring order creation to not depend so heavily on events (i.e. use > a service that takes a shopping cart instead) > 3. Add a service ECA (in the marketing component) to the service from #2 > that allows the TrackingCodeOrder records to be created from the cart > attributes > > Doing the above would remove the need for the order component to know > anything at all about the tracking code logic. It would also allow > developers to make use of #1 and #2 for other custom purposes that require > reading cookies and/or interacting with the shopping cart while creating > orders. > > Regards > Scott > > > On 24 August 2016 at 20:24, Pierre Smits <[hidden email]> wrote: > >> Hi Scott, >> >> Thank you for the feedback. This is something I can work with. >> >> What I see now is the following: >> >> 1. compile-time dependency from order on marketing regarding the >> makeTrackingCodeOrders >> function >> 2. run-time dependencies from cmssite, ecommerce and webpos several >> other functions in TrackingCodeEvents through controller references in >> those components. >> >> I rather see the run-time dependency than the compile-time dependency. >> >> With respect to solving the errors generated at run-time (and I think this >> can happen anywhere in any optional component that is depended upon), I see >> 3 potential solutions: >> >> 1. Have an generic solution - somewhere at a lower level- that first >> checks the existence of the referred event/service before executing it >> 2. Put the dependent event/service code in each optional component (in >> the TrackingCode case: order for make..., cmssite, ecommerce, webpos for >> the others) >> 3. Have it the dependency configurable. >> >> The first - to me - seems to be very complex. And it adds additional >> resource consumption. And then we still have to take into consideration >> that in a multi-tenancy setup some components (and their services/events) >> are not going to be used by 1 or more tenants. >> >> The second also seems undesirable. >> >> Best regards, >> >> >> Pierre Smits >> >> ORRTIZ.COM <http://www.orrtiz.com> >> OFBiz based solutions & services >> >> OFBiz Extensions Marketplace >> http://oem.ofbizci.net/oci-2/ >> >> On Wed, Aug 24, 2016 at 9:59 AM, Scott Gray <[hidden email]> >> wrote: >> >>> Hi Pierre, Jacopo, Jacques, >>> >>> Sticking specifically to the commit being discussed, the beauty of the >>> tracking code logic is that it effectively does nothing if you aren't >> using >>> it. Granted TrackingCodeEvents.makeTrackingCodeOrders() could do with >>> being improved so that it exits earlier if no cookies are found, but it >>> still ultimately does nothing functional if you aren't using it. >>> >>> It is true however that there is a build dependency that prevents users >>> from simply removing the marketing component. And I agree that this >> could >>> be an issue for users who want to strip OFBiz right down to just the >>> components they want to use. >>> >>> Your solution of using a configuration and replacing the build dependency >>> with a conditional runtime dependency does seem good on the surface. >> But I >>> think it presents issues for a few reasons: >>> 1. I don't think there should be a two step process to enabling/disabling >>> something. Adding a component and then having to switch it on or >> removing >>> a component and having to switch off its features seems like an >> unnecessary >>> double step to me. >>> 2. Having the configuration for one component used within another still >>> creates a form of dependency between the two >>> 3. This one kind of reiterates the first two points, but if we're going >> to >>> deal with this then we should be considering a solution that would work >> as >>> though the component were being added or removed via hot-deploy. We >>> shouldn't take shortcuts just because we happen to have both components >> in >>> the same repository. It also presents us with an opportunity to improve >>> the framework for hot-deploy component developers. >>> >>> Rather than saying "we want to remove the build dependency between order >>> and marketing" I think we should instead be saying "we want to allow >>> another component to touch on web order processing without actually >>> touching the order component". >>> >>> For services this is easy using Service ECAs, but unfortunately we don't >>> have anything similar for controller events. I don't have the solution, >> but >>> I think that's the line of thought we need to pick up in order to proceed >>> effectively with this type of decoupling. >>> >>> Regards >>> Scott >>> >>> >>> >>> On 23 August 2016 at 20:52, Pierre Smits <[hidden email]> wrote: >>> >>>> Hi Jacopo, >>>> >>>> Thank you for reacting. >>>> >>>> ERP solutions are, by their intent, modular in nature. This is driven >> by >>>> business requirements. And leads - by default - to runtime >> configuration. >>>> Which is especially important in multi-tenancy implementations. >>>> >>>> Some want solutions like cmssite, ecommerce and webpos. Some don't, >> Some >>>> have other solutions in place >>>> Some want solutions like humanres, manufacturing, inventory management >>> and >>>> marketing & SFA solutions. Others don't or have something else in place >>>> (e.g. SalesForce for SFA/CRM, or Mailchimp for mailing list >> management). >>>> Runtime configuration in ERP solutions are as common as implementation >>>> experts.... >>>> Runtime configurations can easily be explained, including what the >>>> requirements are. >>>> >>>> And we have to face it, regarding functionalities the applications in >> the >>>> (optional) marketing component - when compared against Mailchimp and - >>>> SalesForce - are sub par. >>>> >>>> It seems we are reverting back to the discussion of what core is. In my >>>> opinion, solutions like humanres, manufacturing, inventory management, >>>> integrations of 3rd party payment and shipment solutions, and >>> marketing/SFA >>>> are not. As components in the specialpurpose stack, and themes are not. >>> It >>>> all depends on business requirements. >>>> Disentangling these solutions from the application stack into another >>> would >>>> certainly increase the appeal for adopters. And would make OFBiz more >>>> easier to explain. >>>> >>>> With respect to OFBiz events and services, I see ambiguity in >>> explanation. >>>> Events are web aware, but aren't to be used as services (as in the >> common >>>> understanding of the term). So exposing (or even transforming) these >> into >>>> services would be a good thing (where possible, to be assessed on a >> case >>> by >>>> case scenario). Transforming these TrackingCodeEvents into services >> could >>>> be a good start as the impact is minor. And it would reduce compile >> time >>>> dependencies. Isn't that a goal to strive? >>>> >>>> Best regards, >>>> >>>> >>>> Pierre Smits >>>> >>>> ORRTIZ.COM <http://www.orrtiz.com> >>>> OFBiz based solutions & services >>>> >>>> OFBiz Extensions Marketplace >>>> http://oem.ofbizci.net/oci-2/ >>>> >>>> On Tue, Aug 23, 2016 at 10:05 AM, Jacques Le Roux < >>>> [hidden email]> wrote: >>>> >>>>> 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 < >> [hidden email] >>>>>> 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 < >>> [hidden email] >>>>>>>> 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 < >>>>>>>>> [hidden email]> 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, <[hidden email]> 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); >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
|
Hi Jacques,
None whatsoever. I was just trying to offer a design that would truly decouple the tracking code functionality from the order component. Has this commit been reverted yet? I think that might be a good first step. Regards Scott On 29 August 2016 at 08:02, Jacques Le Roux <[hidden email]> wrote: > HI Scott, > > This sounds like an actionable plan, any plan to action it? > > Jacques > > > > Le 25/08/2016 à 07:38, Scott Gray a écrit : > >> I was thinking that: >> 1. We need a means for the tracking code logic to be able to access the >> cookie data in the request without needing to add any logic to the order >> component >> 2. We need a means for the order tracking code logic to be able to create >> TrackingCodeOrder records when the order is created, again without adding >> any logic to the order component >> >> This could be achieved by: >> 1. Having a generic event that automatically adds all cookies to the >> session shopping cart as attributes. This could be done on each request >> or >> as a chained event running just before the createOrder event >> 2. Refactoring order creation to not depend so heavily on events (i.e. use >> a service that takes a shopping cart instead) >> 3. Add a service ECA (in the marketing component) to the service from #2 >> that allows the TrackingCodeOrder records to be created from the cart >> attributes >> >> Doing the above would remove the need for the order component to know >> anything at all about the tracking code logic. It would also allow >> developers to make use of #1 and #2 for other custom purposes that require >> reading cookies and/or interacting with the shopping cart while creating >> orders. >> >> Regards >> Scott >> >> >> On 24 August 2016 at 20:24, Pierre Smits <[hidden email]> wrote: >> >> Hi Scott, >>> >>> Thank you for the feedback. This is something I can work with. >>> >>> What I see now is the following: >>> >>> 1. compile-time dependency from order on marketing regarding the >>> makeTrackingCodeOrders >>> function >>> 2. run-time dependencies from cmssite, ecommerce and webpos several >>> other functions in TrackingCodeEvents through controller references >>> in >>> those components. >>> >>> I rather see the run-time dependency than the compile-time dependency. >>> >>> With respect to solving the errors generated at run-time (and I think >>> this >>> can happen anywhere in any optional component that is depended upon), I >>> see >>> 3 potential solutions: >>> >>> 1. Have an generic solution - somewhere at a lower level- that first >>> checks the existence of the referred event/service before executing >>> it >>> 2. Put the dependent event/service code in each optional component >>> (in >>> the TrackingCode case: order for make..., cmssite, ecommerce, webpos >>> for >>> the others) >>> 3. Have it the dependency configurable. >>> >>> The first - to me - seems to be very complex. And it adds additional >>> resource consumption. And then we still have to take into consideration >>> that in a multi-tenancy setup some components (and their services/events) >>> are not going to be used by 1 or more tenants. >>> >>> The second also seems undesirable. >>> >>> Best regards, >>> >>> >>> Pierre Smits >>> >>> ORRTIZ.COM <http://www.orrtiz.com> >>> OFBiz based solutions & services >>> >>> OFBiz Extensions Marketplace >>> http://oem.ofbizci.net/oci-2/ >>> >>> On Wed, Aug 24, 2016 at 9:59 AM, Scott Gray < >>> [hidden email]> >>> wrote: >>> >>> Hi Pierre, Jacopo, Jacques, >>>> >>>> Sticking specifically to the commit being discussed, the beauty of the >>>> tracking code logic is that it effectively does nothing if you aren't >>>> >>> using >>> >>>> it. Granted TrackingCodeEvents.makeTrackingCodeOrders() could do with >>>> being improved so that it exits earlier if no cookies are found, but it >>>> still ultimately does nothing functional if you aren't using it. >>>> >>>> It is true however that there is a build dependency that prevents users >>>> from simply removing the marketing component. And I agree that this >>>> >>> could >>> >>>> be an issue for users who want to strip OFBiz right down to just the >>>> components they want to use. >>>> >>>> Your solution of using a configuration and replacing the build >>>> dependency >>>> with a conditional runtime dependency does seem good on the surface. >>>> >>> But I >>> >>>> think it presents issues for a few reasons: >>>> 1. I don't think there should be a two step process to >>>> enabling/disabling >>>> something. Adding a component and then having to switch it on or >>>> >>> removing >>> >>>> a component and having to switch off its features seems like an >>>> >>> unnecessary >>> >>>> double step to me. >>>> 2. Having the configuration for one component used within another still >>>> creates a form of dependency between the two >>>> 3. This one kind of reiterates the first two points, but if we're going >>>> >>> to >>> >>>> deal with this then we should be considering a solution that would work >>>> >>> as >>> >>>> though the component were being added or removed via hot-deploy. We >>>> shouldn't take shortcuts just because we happen to have both components >>>> >>> in >>> >>>> the same repository. It also presents us with an opportunity to improve >>>> the framework for hot-deploy component developers. >>>> >>>> Rather than saying "we want to remove the build dependency between order >>>> and marketing" I think we should instead be saying "we want to allow >>>> another component to touch on web order processing without actually >>>> touching the order component". >>>> >>>> For services this is easy using Service ECAs, but unfortunately we don't >>>> have anything similar for controller events. I don't have the solution, >>>> >>> but >>> >>>> I think that's the line of thought we need to pick up in order to >>>> proceed >>>> effectively with this type of decoupling. >>>> >>>> Regards >>>> Scott >>>> >>>> >>>> >>>> On 23 August 2016 at 20:52, Pierre Smits <[hidden email]> >>>> wrote: >>>> >>>> Hi Jacopo, >>>>> >>>>> Thank you for reacting. >>>>> >>>>> ERP solutions are, by their intent, modular in nature. This is driven >>>>> >>>> by >>> >>>> business requirements. And leads - by default - to runtime >>>>> >>>> configuration. >>> >>>> Which is especially important in multi-tenancy implementations. >>>>> >>>>> Some want solutions like cmssite, ecommerce and webpos. Some don't, >>>>> >>>> Some >>> >>>> have other solutions in place >>>>> Some want solutions like humanres, manufacturing, inventory management >>>>> >>>> and >>>> >>>>> marketing & SFA solutions. Others don't or have something else in place >>>>> (e.g. SalesForce for SFA/CRM, or Mailchimp for mailing list >>>>> >>>> management). >>> >>>> Runtime configuration in ERP solutions are as common as implementation >>>>> experts.... >>>>> Runtime configurations can easily be explained, including what the >>>>> requirements are. >>>>> >>>>> And we have to face it, regarding functionalities the applications in >>>>> >>>> the >>> >>>> (optional) marketing component - when compared against Mailchimp and - >>>>> SalesForce - are sub par. >>>>> >>>>> It seems we are reverting back to the discussion of what core is. In my >>>>> opinion, solutions like humanres, manufacturing, inventory management, >>>>> integrations of 3rd party payment and shipment solutions, and >>>>> >>>> marketing/SFA >>>> >>>>> are not. As components in the specialpurpose stack, and themes are not. >>>>> >>>> It >>>> >>>>> all depends on business requirements. >>>>> Disentangling these solutions from the application stack into another >>>>> >>>> would >>>> >>>>> certainly increase the appeal for adopters. And would make OFBiz more >>>>> easier to explain. >>>>> >>>>> With respect to OFBiz events and services, I see ambiguity in >>>>> >>>> explanation. >>>> >>>>> Events are web aware, but aren't to be used as services (as in the >>>>> >>>> common >>> >>>> understanding of the term). So exposing (or even transforming) these >>>>> >>>> into >>> >>>> services would be a good thing (where possible, to be assessed on a >>>>> >>>> case >>> >>>> by >>>> >>>>> case scenario). Transforming these TrackingCodeEvents into services >>>>> >>>> could >>> >>>> be a good start as the impact is minor. And it would reduce compile >>>>> >>>> time >>> >>>> dependencies. Isn't that a goal to strive? >>>>> >>>>> Best regards, >>>>> >>>>> >>>>> Pierre Smits >>>>> >>>>> ORRTIZ.COM <http://www.orrtiz.com> >>>>> OFBiz based solutions & services >>>>> >>>>> OFBiz Extensions Marketplace >>>>> http://oem.ofbizci.net/oci-2/ >>>>> >>>>> On Tue, Aug 23, 2016 at 10:05 AM, Jacques Le Roux < >>>>> [hidden email]> wrote: >>>>> >>>>> 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 < >>>>>>> >>>>>> [hidden email] >>> >>>> 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 < >>>>>>>>> >>>>>>>> [hidden email] >>>> >>>>> 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 < >>>>>>>>>> [hidden email]> 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, <[hidden email]> 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); >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> > |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
Administrator
|
Hi Scott,
Yes, this has been reverted one week ago: https://issues.apache.org/jira/browse/OFBIZ-7966?focusedCommentId=15431090 Jacques Le 29/08/2016 à 23:38, Scott Gray a écrit : > Hi Jacques, > > None whatsoever. I was just trying to offer a design that would truly > decouple the tracking code functionality from the order component. > > Has this commit been reverted yet? I think that might be a good first step. > > Regards > Scott > > On 29 August 2016 at 08:02, Jacques Le Roux <[hidden email]> > wrote: > >> HI Scott, >> >> This sounds like an actionable plan, any plan to action it? >> >> Jacques >> >> >> >> Le 25/08/2016 à 07:38, Scott Gray a écrit : >> >>> I was thinking that: >>> 1. We need a means for the tracking code logic to be able to access the >>> cookie data in the request without needing to add any logic to the order >>> component >>> 2. We need a means for the order tracking code logic to be able to create >>> TrackingCodeOrder records when the order is created, again without adding >>> any logic to the order component >>> >>> This could be achieved by: >>> 1. Having a generic event that automatically adds all cookies to the >>> session shopping cart as attributes. This could be done on each request >>> or >>> as a chained event running just before the createOrder event >>> 2. Refactoring order creation to not depend so heavily on events (i.e. use >>> a service that takes a shopping cart instead) >>> 3. Add a service ECA (in the marketing component) to the service from #2 >>> that allows the TrackingCodeOrder records to be created from the cart >>> attributes >>> >>> Doing the above would remove the need for the order component to know >>> anything at all about the tracking code logic. It would also allow >>> developers to make use of #1 and #2 for other custom purposes that require >>> reading cookies and/or interacting with the shopping cart while creating >>> orders. >>> >>> Regards >>> Scott >>> >>> >>> On 24 August 2016 at 20:24, Pierre Smits <[hidden email]> wrote: >>> >>> Hi Scott, >>>> Thank you for the feedback. This is something I can work with. >>>> >>>> What I see now is the following: >>>> >>>> 1. compile-time dependency from order on marketing regarding the >>>> makeTrackingCodeOrders >>>> function >>>> 2. run-time dependencies from cmssite, ecommerce and webpos several >>>> other functions in TrackingCodeEvents through controller references >>>> in >>>> those components. >>>> >>>> I rather see the run-time dependency than the compile-time dependency. >>>> >>>> With respect to solving the errors generated at run-time (and I think >>>> this >>>> can happen anywhere in any optional component that is depended upon), I >>>> see >>>> 3 potential solutions: >>>> >>>> 1. Have an generic solution - somewhere at a lower level- that first >>>> checks the existence of the referred event/service before executing >>>> it >>>> 2. Put the dependent event/service code in each optional component >>>> (in >>>> the TrackingCode case: order for make..., cmssite, ecommerce, webpos >>>> for >>>> the others) >>>> 3. Have it the dependency configurable. >>>> >>>> The first - to me - seems to be very complex. And it adds additional >>>> resource consumption. And then we still have to take into consideration >>>> that in a multi-tenancy setup some components (and their services/events) >>>> are not going to be used by 1 or more tenants. >>>> >>>> The second also seems undesirable. >>>> >>>> Best regards, >>>> >>>> >>>> Pierre Smits >>>> >>>> ORRTIZ.COM <http://www.orrtiz.com> >>>> OFBiz based solutions & services >>>> >>>> OFBiz Extensions Marketplace >>>> http://oem.ofbizci.net/oci-2/ >>>> >>>> On Wed, Aug 24, 2016 at 9:59 AM, Scott Gray < >>>> [hidden email]> >>>> wrote: >>>> >>>> Hi Pierre, Jacopo, Jacques, >>>>> Sticking specifically to the commit being discussed, the beauty of the >>>>> tracking code logic is that it effectively does nothing if you aren't >>>>> >>>> using >>>> >>>>> it. Granted TrackingCodeEvents.makeTrackingCodeOrders() could do with >>>>> being improved so that it exits earlier if no cookies are found, but it >>>>> still ultimately does nothing functional if you aren't using it. >>>>> >>>>> It is true however that there is a build dependency that prevents users >>>>> from simply removing the marketing component. And I agree that this >>>>> >>>> could >>>> >>>>> be an issue for users who want to strip OFBiz right down to just the >>>>> components they want to use. >>>>> >>>>> Your solution of using a configuration and replacing the build >>>>> dependency >>>>> with a conditional runtime dependency does seem good on the surface. >>>>> >>>> But I >>>> >>>>> think it presents issues for a few reasons: >>>>> 1. I don't think there should be a two step process to >>>>> enabling/disabling >>>>> something. Adding a component and then having to switch it on or >>>>> >>>> removing >>>> >>>>> a component and having to switch off its features seems like an >>>>> >>>> unnecessary >>>> >>>>> double step to me. >>>>> 2. Having the configuration for one component used within another still >>>>> creates a form of dependency between the two >>>>> 3. This one kind of reiterates the first two points, but if we're going >>>>> >>>> to >>>> >>>>> deal with this then we should be considering a solution that would work >>>>> >>>> as >>>> >>>>> though the component were being added or removed via hot-deploy. We >>>>> shouldn't take shortcuts just because we happen to have both components >>>>> >>>> in >>>> >>>>> the same repository. It also presents us with an opportunity to improve >>>>> the framework for hot-deploy component developers. >>>>> >>>>> Rather than saying "we want to remove the build dependency between order >>>>> and marketing" I think we should instead be saying "we want to allow >>>>> another component to touch on web order processing without actually >>>>> touching the order component". >>>>> >>>>> For services this is easy using Service ECAs, but unfortunately we don't >>>>> have anything similar for controller events. I don't have the solution, >>>>> >>>> but >>>> >>>>> I think that's the line of thought we need to pick up in order to >>>>> proceed >>>>> effectively with this type of decoupling. >>>>> >>>>> Regards >>>>> Scott >>>>> >>>>> >>>>> >>>>> On 23 August 2016 at 20:52, Pierre Smits <[hidden email]> >>>>> wrote: >>>>> >>>>> Hi Jacopo, >>>>>> Thank you for reacting. >>>>>> >>>>>> ERP solutions are, by their intent, modular in nature. This is driven >>>>>> >>>>> by >>>>> business requirements. And leads - by default - to runtime >>>>> configuration. >>>>> Which is especially important in multi-tenancy implementations. >>>>>> Some want solutions like cmssite, ecommerce and webpos. Some don't, >>>>>> >>>>> Some >>>>> have other solutions in place >>>>>> Some want solutions like humanres, manufacturing, inventory management >>>>>> >>>>> and >>>>> >>>>>> marketing & SFA solutions. Others don't or have something else in place >>>>>> (e.g. SalesForce for SFA/CRM, or Mailchimp for mailing list >>>>>> >>>>> management). >>>>> Runtime configuration in ERP solutions are as common as implementation >>>>>> experts.... >>>>>> Runtime configurations can easily be explained, including what the >>>>>> requirements are. >>>>>> >>>>>> And we have to face it, regarding functionalities the applications in >>>>>> >>>>> the >>>>> (optional) marketing component - when compared against Mailchimp and - >>>>>> SalesForce - are sub par. >>>>>> >>>>>> It seems we are reverting back to the discussion of what core is. In my >>>>>> opinion, solutions like humanres, manufacturing, inventory management, >>>>>> integrations of 3rd party payment and shipment solutions, and >>>>>> >>>>> marketing/SFA >>>>> >>>>>> are not. As components in the specialpurpose stack, and themes are not. >>>>>> >>>>> It >>>>> >>>>>> all depends on business requirements. >>>>>> Disentangling these solutions from the application stack into another >>>>>> >>>>> would >>>>> >>>>>> certainly increase the appeal for adopters. And would make OFBiz more >>>>>> easier to explain. >>>>>> >>>>>> With respect to OFBiz events and services, I see ambiguity in >>>>>> >>>>> explanation. >>>>> >>>>>> Events are web aware, but aren't to be used as services (as in the >>>>>> >>>>> common >>>>> understanding of the term). So exposing (or even transforming) these >>>>> into >>>>> services would be a good thing (where possible, to be assessed on a >>>>> case >>>>> by >>>>> >>>>>> case scenario). Transforming these TrackingCodeEvents into services >>>>>> >>>>> could >>>>> be a good start as the impact is minor. And it would reduce compile >>>>> time >>>>> dependencies. Isn't that a goal to strive? >>>>>> Best regards, >>>>>> >>>>>> >>>>>> Pierre Smits >>>>>> >>>>>> ORRTIZ.COM <http://www.orrtiz.com> >>>>>> OFBiz based solutions & services >>>>>> >>>>>> OFBiz Extensions Marketplace >>>>>> http://oem.ofbizci.net/oci-2/ >>>>>> >>>>>> On Tue, Aug 23, 2016 at 10:05 AM, Jacques Le Roux < >>>>>> [hidden email]> wrote: >>>>>> >>>>>> 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 < >>>>>>>> >>>>>>> [hidden email] >>>>> 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 < >>>>>>>>>> >>>>>>>>> [hidden email] >>>>>> 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 < >>>>>>>>>>> [hidden email]> 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, <[hidden email]> 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); >>>>>>>>>>>> >>>>>>>>>>>>> |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
|
Ok thanks, I guess you had a typo in your revert commit message because I
couldn't find a mention of the original commit number when searching. Regards Scott On 30 August 2016 at 17:58, Jacques Le Roux <[hidden email]> wrote: > Hi Scott, > > Yes, this has been reverted one week ago: https://issues.apache.org/jira > /browse/OFBIZ-7966?focusedCommentId=15431090 > > Jacques > > > Le 29/08/2016 à 23:38, Scott Gray a écrit : > >> Hi Jacques, >> >> None whatsoever. I was just trying to offer a design that would truly >> decouple the tracking code functionality from the order component. >> >> Has this commit been reverted yet? I think that might be a good first >> step. >> >> Regards >> Scott >> >> On 29 August 2016 at 08:02, Jacques Le Roux <[hidden email] >> > >> wrote: >> >> HI Scott, >>> >>> This sounds like an actionable plan, any plan to action it? >>> >>> Jacques >>> >>> >>> >>> Le 25/08/2016 à 07:38, Scott Gray a écrit : >>> >>> I was thinking that: >>>> 1. We need a means for the tracking code logic to be able to access the >>>> cookie data in the request without needing to add any logic to the order >>>> component >>>> 2. We need a means for the order tracking code logic to be able to >>>> create >>>> TrackingCodeOrder records when the order is created, again without >>>> adding >>>> any logic to the order component >>>> >>>> This could be achieved by: >>>> 1. Having a generic event that automatically adds all cookies to the >>>> session shopping cart as attributes. This could be done on each request >>>> or >>>> as a chained event running just before the createOrder event >>>> 2. Refactoring order creation to not depend so heavily on events (i.e. >>>> use >>>> a service that takes a shopping cart instead) >>>> 3. Add a service ECA (in the marketing component) to the service from #2 >>>> that allows the TrackingCodeOrder records to be created from the cart >>>> attributes >>>> >>>> Doing the above would remove the need for the order component to know >>>> anything at all about the tracking code logic. It would also allow >>>> developers to make use of #1 and #2 for other custom purposes that >>>> require >>>> reading cookies and/or interacting with the shopping cart while creating >>>> orders. >>>> >>>> Regards >>>> Scott >>>> >>>> >>>> On 24 August 2016 at 20:24, Pierre Smits <[hidden email]> >>>> wrote: >>>> >>>> Hi Scott, >>>> >>>>> Thank you for the feedback. This is something I can work with. >>>>> >>>>> What I see now is the following: >>>>> >>>>> 1. compile-time dependency from order on marketing regarding the >>>>> makeTrackingCodeOrders >>>>> function >>>>> 2. run-time dependencies from cmssite, ecommerce and webpos >>>>> several >>>>> other functions in TrackingCodeEvents through controller >>>>> references >>>>> in >>>>> those components. >>>>> >>>>> I rather see the run-time dependency than the compile-time dependency. >>>>> >>>>> With respect to solving the errors generated at run-time (and I think >>>>> this >>>>> can happen anywhere in any optional component that is depended upon), I >>>>> see >>>>> 3 potential solutions: >>>>> >>>>> 1. Have an generic solution - somewhere at a lower level- that >>>>> first >>>>> checks the existence of the referred event/service before >>>>> executing >>>>> it >>>>> 2. Put the dependent event/service code in each optional component >>>>> (in >>>>> the TrackingCode case: order for make..., cmssite, ecommerce, >>>>> webpos >>>>> for >>>>> the others) >>>>> 3. Have it the dependency configurable. >>>>> >>>>> The first - to me - seems to be very complex. And it adds additional >>>>> resource consumption. And then we still have to take into consideration >>>>> that in a multi-tenancy setup some components (and their >>>>> services/events) >>>>> are not going to be used by 1 or more tenants. >>>>> >>>>> The second also seems undesirable. >>>>> >>>>> Best regards, >>>>> >>>>> >>>>> Pierre Smits >>>>> >>>>> ORRTIZ.COM <http://www.orrtiz.com> >>>>> OFBiz based solutions & services >>>>> >>>>> OFBiz Extensions Marketplace >>>>> http://oem.ofbizci.net/oci-2/ >>>>> >>>>> On Wed, Aug 24, 2016 at 9:59 AM, Scott Gray < >>>>> [hidden email]> >>>>> wrote: >>>>> >>>>> Hi Pierre, Jacopo, Jacques, >>>>> >>>>>> Sticking specifically to the commit being discussed, the beauty of the >>>>>> tracking code logic is that it effectively does nothing if you aren't >>>>>> >>>>>> using >>>>> >>>>> it. Granted TrackingCodeEvents.makeTrackingCodeOrders() could do with >>>>>> being improved so that it exits earlier if no cookies are found, but >>>>>> it >>>>>> still ultimately does nothing functional if you aren't using it. >>>>>> >>>>>> It is true however that there is a build dependency that prevents >>>>>> users >>>>>> from simply removing the marketing component. And I agree that this >>>>>> >>>>>> could >>>>> >>>>> be an issue for users who want to strip OFBiz right down to just the >>>>>> components they want to use. >>>>>> >>>>>> Your solution of using a configuration and replacing the build >>>>>> dependency >>>>>> with a conditional runtime dependency does seem good on the surface. >>>>>> >>>>>> But I >>>>> >>>>> think it presents issues for a few reasons: >>>>>> 1. I don't think there should be a two step process to >>>>>> enabling/disabling >>>>>> something. Adding a component and then having to switch it on or >>>>>> >>>>>> removing >>>>> >>>>> a component and having to switch off its features seems like an >>>>>> >>>>>> unnecessary >>>>> >>>>> double step to me. >>>>>> 2. Having the configuration for one component used within another >>>>>> still >>>>>> creates a form of dependency between the two >>>>>> 3. This one kind of reiterates the first two points, but if we're >>>>>> going >>>>>> >>>>>> to >>>>> >>>>> deal with this then we should be considering a solution that would work >>>>>> >>>>>> as >>>>> >>>>> though the component were being added or removed via hot-deploy. We >>>>>> shouldn't take shortcuts just because we happen to have both >>>>>> components >>>>>> >>>>>> in >>>>> >>>>> the same repository. It also presents us with an opportunity to >>>>>> improve >>>>>> the framework for hot-deploy component developers. >>>>>> >>>>>> Rather than saying "we want to remove the build dependency between >>>>>> order >>>>>> and marketing" I think we should instead be saying "we want to allow >>>>>> another component to touch on web order processing without actually >>>>>> touching the order component". >>>>>> >>>>>> For services this is easy using Service ECAs, but unfortunately we >>>>>> don't >>>>>> have anything similar for controller events. I don't have the >>>>>> solution, >>>>>> >>>>>> but >>>>> >>>>> I think that's the line of thought we need to pick up in order to >>>>>> proceed >>>>>> effectively with this type of decoupling. >>>>>> >>>>>> Regards >>>>>> Scott >>>>>> >>>>>> >>>>>> >>>>>> On 23 August 2016 at 20:52, Pierre Smits <[hidden email]> >>>>>> wrote: >>>>>> >>>>>> Hi Jacopo, >>>>>> >>>>>>> Thank you for reacting. >>>>>>> >>>>>>> ERP solutions are, by their intent, modular in nature. This is driven >>>>>>> >>>>>>> by >>>>>> business requirements. And leads - by default - to runtime >>>>>> configuration. >>>>>> Which is especially important in multi-tenancy implementations. >>>>>> >>>>>>> Some want solutions like cmssite, ecommerce and webpos. Some don't, >>>>>>> >>>>>>> Some >>>>>> have other solutions in place >>>>>> >>>>>>> Some want solutions like humanres, manufacturing, inventory >>>>>>> management >>>>>>> >>>>>>> and >>>>>> >>>>>> marketing & SFA solutions. Others don't or have something else in >>>>>>> place >>>>>>> (e.g. SalesForce for SFA/CRM, or Mailchimp for mailing list >>>>>>> >>>>>>> management). >>>>>> Runtime configuration in ERP solutions are as common as implementation >>>>>> >>>>>>> experts.... >>>>>>> Runtime configurations can easily be explained, including what the >>>>>>> requirements are. >>>>>>> >>>>>>> And we have to face it, regarding functionalities the applications in >>>>>>> >>>>>>> the >>>>>> (optional) marketing component - when compared against Mailchimp and - >>>>>> >>>>>>> SalesForce - are sub par. >>>>>>> >>>>>>> It seems we are reverting back to the discussion of what core is. In >>>>>>> my >>>>>>> opinion, solutions like humanres, manufacturing, inventory >>>>>>> management, >>>>>>> integrations of 3rd party payment and shipment solutions, and >>>>>>> >>>>>>> marketing/SFA >>>>>> >>>>>> are not. As components in the specialpurpose stack, and themes are >>>>>>> not. >>>>>>> >>>>>>> It >>>>>> >>>>>> all depends on business requirements. >>>>>>> Disentangling these solutions from the application stack into another >>>>>>> >>>>>>> would >>>>>> >>>>>> certainly increase the appeal for adopters. And would make OFBiz more >>>>>>> easier to explain. >>>>>>> >>>>>>> With respect to OFBiz events and services, I see ambiguity in >>>>>>> >>>>>>> explanation. >>>>>> >>>>>> Events are web aware, but aren't to be used as services (as in the >>>>>>> >>>>>>> common >>>>>> understanding of the term). So exposing (or even transforming) these >>>>>> into >>>>>> services would be a good thing (where possible, to be assessed on a >>>>>> case >>>>>> by >>>>>> >>>>>> case scenario). Transforming these TrackingCodeEvents into services >>>>>>> >>>>>>> could >>>>>> be a good start as the impact is minor. And it would reduce compile >>>>>> time >>>>>> dependencies. Isn't that a goal to strive? >>>>>> >>>>>>> Best regards, >>>>>>> >>>>>>> >>>>>>> Pierre Smits >>>>>>> >>>>>>> ORRTIZ.COM <http://www.orrtiz.com> >>>>>>> OFBiz based solutions & services >>>>>>> >>>>>>> OFBiz Extensions Marketplace >>>>>>> http://oem.ofbizci.net/oci-2/ >>>>>>> >>>>>>> On Tue, Aug 23, 2016 at 10:05 AM, Jacques Le Roux < >>>>>>> [hidden email]> wrote: >>>>>>> >>>>>>> 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 < >>>>>>>>> >>>>>>>>> [hidden email] >>>>>>>> >>>>>>> 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 < >>>>>>>>>>> >>>>>>>>>>> [hidden email] >>>>>>>>>> >>>>>>>>> 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 < >>>>>>>>>>>> [hidden email]> 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, <[hidden email]> 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/mark >>>>>>>>>>>>>> eting/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/marke >>>>>>>>>>>>>> ting/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("_QUIC >>>>>>>>>>>>>> K_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(sessio >>>>>>>>>>>>>> n); >>>>>>>>>>>>>> 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); >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>> > |
Re: svn commit: r1757130 - in /ofbiz/trunk/applications: marketing/servicedef/ marketing/src/main/java/org/ap
|
Administrator
|
Right, I just fixed it it was r1757130.
You may have found it by looking in the related Jira Thanks Jacques Le 30/08/2016 à 08:28, Scott Gray a écrit : > Ok thanks, I guess you had a typo in your revert commit message because I > couldn't find a mention of the original commit number when searching. > > Regards > Scott > > > On 30 August 2016 at 17:58, Jacques Le Roux <[hidden email]> > wrote: > >> Hi Scott, >> >> Yes, this has been reverted one week ago: https://issues.apache.org/jira >> /browse/OFBIZ-7966?focusedCommentId=15431090 >> >> Jacques >> >> >> Le 29/08/2016 à 23:38, Scott Gray a écrit : >> >>> Hi Jacques, >>> >>> None whatsoever. I was just trying to offer a design that would truly >>> decouple the tracking code functionality from the order component. >>> >>> Has this commit been reverted yet? I think that might be a good first >>> step. >>> >>> Regards >>> Scott >>> >>> On 29 August 2016 at 08:02, Jacques Le Roux <[hidden email] >>> wrote: >>> >>> HI Scott, >>>> This sounds like an actionable plan, any plan to action it? >>>> >>>> Jacques >>>> >>>> >>>> >>>> Le 25/08/2016 à 07:38, Scott Gray a écrit : >>>> >>>> I was thinking that: >>>>> 1. We need a means for the tracking code logic to be able to access the >>>>> cookie data in the request without needing to add any logic to the order >>>>> component >>>>> 2. We need a means for the order tracking code logic to be able to >>>>> create >>>>> TrackingCodeOrder records when the order is created, again without >>>>> adding >>>>> any logic to the order component >>>>> >>>>> This could be achieved by: >>>>> 1. Having a generic event that automatically adds all cookies to the >>>>> session shopping cart as attributes. This could be done on each request >>>>> or >>>>> as a chained event running just before the createOrder event >>>>> 2. Refactoring order creation to not depend so heavily on events (i.e. >>>>> use >>>>> a service that takes a shopping cart instead) >>>>> 3. Add a service ECA (in the marketing component) to the service from #2 >>>>> that allows the TrackingCodeOrder records to be created from the cart >>>>> attributes >>>>> >>>>> Doing the above would remove the need for the order component to know >>>>> anything at all about the tracking code logic. It would also allow >>>>> developers to make use of #1 and #2 for other custom purposes that >>>>> require >>>>> reading cookies and/or interacting with the shopping cart while creating >>>>> orders. >>>>> >>>>> Regards >>>>> Scott >>>>> >>>>> >>>>> On 24 August 2016 at 20:24, Pierre Smits <[hidden email]> >>>>> wrote: >>>>> >>>>> Hi Scott, >>>>> >>>>>> Thank you for the feedback. This is something I can work with. >>>>>> >>>>>> What I see now is the following: >>>>>> >>>>>> 1. compile-time dependency from order on marketing regarding the >>>>>> makeTrackingCodeOrders >>>>>> function >>>>>> 2. run-time dependencies from cmssite, ecommerce and webpos >>>>>> several >>>>>> other functions in TrackingCodeEvents through controller >>>>>> references >>>>>> in >>>>>> those components. >>>>>> >>>>>> I rather see the run-time dependency than the compile-time dependency. >>>>>> >>>>>> With respect to solving the errors generated at run-time (and I think >>>>>> this >>>>>> can happen anywhere in any optional component that is depended upon), I >>>>>> see >>>>>> 3 potential solutions: >>>>>> >>>>>> 1. Have an generic solution - somewhere at a lower level- that >>>>>> first >>>>>> checks the existence of the referred event/service before >>>>>> executing >>>>>> it >>>>>> 2. Put the dependent event/service code in each optional component >>>>>> (in >>>>>> the TrackingCode case: order for make..., cmssite, ecommerce, >>>>>> webpos >>>>>> for >>>>>> the others) >>>>>> 3. Have it the dependency configurable. >>>>>> >>>>>> The first - to me - seems to be very complex. And it adds additional >>>>>> resource consumption. And then we still have to take into consideration >>>>>> that in a multi-tenancy setup some components (and their >>>>>> services/events) >>>>>> are not going to be used by 1 or more tenants. >>>>>> >>>>>> The second also seems undesirable. >>>>>> >>>>>> Best regards, >>>>>> >>>>>> >>>>>> Pierre Smits >>>>>> >>>>>> ORRTIZ.COM <http://www.orrtiz.com> >>>>>> OFBiz based solutions & services >>>>>> >>>>>> OFBiz Extensions Marketplace >>>>>> http://oem.ofbizci.net/oci-2/ >>>>>> >>>>>> On Wed, Aug 24, 2016 at 9:59 AM, Scott Gray < >>>>>> [hidden email]> >>>>>> wrote: >>>>>> >>>>>> Hi Pierre, Jacopo, Jacques, >>>>>> >>>>>>> Sticking specifically to the commit being discussed, the beauty of the >>>>>>> tracking code logic is that it effectively does nothing if you aren't >>>>>>> >>>>>>> using >>>>>> it. Granted TrackingCodeEvents.makeTrackingCodeOrders() could do with >>>>>>> being improved so that it exits earlier if no cookies are found, but >>>>>>> it >>>>>>> still ultimately does nothing functional if you aren't using it. >>>>>>> >>>>>>> It is true however that there is a build dependency that prevents >>>>>>> users >>>>>>> from simply removing the marketing component. And I agree that this >>>>>>> >>>>>>> could >>>>>> be an issue for users who want to strip OFBiz right down to just the >>>>>>> components they want to use. >>>>>>> >>>>>>> Your solution of using a configuration and replacing the build >>>>>>> dependency >>>>>>> with a conditional runtime dependency does seem good on the surface. >>>>>>> >>>>>>> But I >>>>>> think it presents issues for a few reasons: >>>>>>> 1. I don't think there should be a two step process to >>>>>>> enabling/disabling >>>>>>> something. Adding a component and then having to switch it on or >>>>>>> >>>>>>> removing >>>>>> a component and having to switch off its features seems like an >>>>>>> unnecessary >>>>>> double step to me. >>>>>>> 2. Having the configuration for one component used within another >>>>>>> still >>>>>>> creates a form of dependency between the two >>>>>>> 3. This one kind of reiterates the first two points, but if we're >>>>>>> going >>>>>>> >>>>>>> to >>>>>> deal with this then we should be considering a solution that would work >>>>>>> as >>>>>> though the component were being added or removed via hot-deploy. We >>>>>>> shouldn't take shortcuts just because we happen to have both >>>>>>> components >>>>>>> >>>>>>> in >>>>>> the same repository. It also presents us with an opportunity to >>>>>>> improve >>>>>>> the framework for hot-deploy component developers. >>>>>>> >>>>>>> Rather than saying "we want to remove the build dependency between >>>>>>> order >>>>>>> and marketing" I think we should instead be saying "we want to allow >>>>>>> another component to touch on web order processing without actually >>>>>>> touching the order component". >>>>>>> >>>>>>> For services this is easy using Service ECAs, but unfortunately we >>>>>>> don't >>>>>>> have anything similar for controller events. I don't have the >>>>>>> solution, >>>>>>> >>>>>>> but >>>>>> I think that's the line of thought we need to pick up in order to >>>>>>> proceed >>>>>>> effectively with this type of decoupling. >>>>>>> >>>>>>> Regards >>>>>>> Scott >>>>>>> >>>>>>> >>>>>>> >>>>>>> On 23 August 2016 at 20:52, Pierre Smits <[hidden email]> >>>>>>> wrote: >>>>>>> >>>>>>> Hi Jacopo, >>>>>>> >>>>>>>> Thank you for reacting. >>>>>>>> >>>>>>>> ERP solutions are, by their intent, modular in nature. This is driven >>>>>>>> >>>>>>>> by >>>>>>> business requirements. And leads - by default - to runtime >>>>>>> configuration. >>>>>>> Which is especially important in multi-tenancy implementations. >>>>>>> >>>>>>>> Some want solutions like cmssite, ecommerce and webpos. Some don't, >>>>>>>> >>>>>>>> Some >>>>>>> have other solutions in place >>>>>>> >>>>>>>> Some want solutions like humanres, manufacturing, inventory >>>>>>>> management >>>>>>>> >>>>>>>> and >>>>>>> marketing & SFA solutions. Others don't or have something else in >>>>>>>> place >>>>>>>> (e.g. SalesForce for SFA/CRM, or Mailchimp for mailing list >>>>>>>> >>>>>>>> management). >>>>>>> Runtime configuration in ERP solutions are as common as implementation >>>>>>> >>>>>>>> experts.... >>>>>>>> Runtime configurations can easily be explained, including what the >>>>>>>> requirements are. >>>>>>>> >>>>>>>> And we have to face it, regarding functionalities the applications in >>>>>>>> >>>>>>>> the >>>>>>> (optional) marketing component - when compared against Mailchimp and - >>>>>>> >>>>>>>> SalesForce - are sub par. >>>>>>>> >>>>>>>> It seems we are reverting back to the discussion of what core is. In >>>>>>>> my >>>>>>>> opinion, solutions like humanres, manufacturing, inventory >>>>>>>> management, >>>>>>>> integrations of 3rd party payment and shipment solutions, and >>>>>>>> >>>>>>>> marketing/SFA >>>>>>> are not. As components in the specialpurpose stack, and themes are >>>>>>>> not. >>>>>>>> >>>>>>>> It >>>>>>> all depends on business requirements. >>>>>>>> Disentangling these solutions from the application stack into another >>>>>>>> >>>>>>>> would >>>>>>> certainly increase the appeal for adopters. And would make OFBiz more >>>>>>>> easier to explain. >>>>>>>> >>>>>>>> With respect to OFBiz events and services, I see ambiguity in >>>>>>>> >>>>>>>> explanation. >>>>>>> Events are web aware, but aren't to be used as services (as in the >>>>>>>> common >>>>>>> understanding of the term). So exposing (or even transforming) these >>>>>>> into >>>>>>> services would be a good thing (where possible, to be assessed on a >>>>>>> case >>>>>>> by >>>>>>> >>>>>>> case scenario). Transforming these TrackingCodeEvents into services >>>>>>>> could >>>>>>> be a good start as the impact is minor. And it would reduce compile >>>>>>> time >>>>>>> dependencies. Isn't that a goal to strive? >>>>>>> >>>>>>>> Best regards, >>>>>>>> >>>>>>>> >>>>>>>> Pierre Smits >>>>>>>> >>>>>>>> ORRTIZ.COM <http://www.orrtiz.com> >>>>>>>> OFBiz based solutions & services >>>>>>>> >>>>>>>> OFBiz Extensions Marketplace >>>>>>>> http://oem.ofbizci.net/oci-2/ >>>>>>>> >>>>>>>> On Tue, Aug 23, 2016 at 10:05 AM, Jacques Le Roux < >>>>>>>> [hidden email]> wrote: >>>>>>>> >>>>>>>> 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 < >>>>>>>>>> >>>>>>>>>> [hidden email] >>>>>>>> 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 < >>>>>>>>>>>> >>>>>>>>>>>> [hidden email] >>>>>>>>>> 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 < >>>>>>>>>>>>> [hidden email]> 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, <[hidden email]> 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/mark >>>>>>>>>>>>>>> eting/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/marke >>>>>>>>>>>>>>> ting/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("_QUIC >>>>>>>>>>>>>>> K_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(sessio >>>>>>>>>>>>>>> n); >>>>>>>>>>>>>>> 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); >>>>>>>>>>>>>>> >>>>>>>>>>>>>> |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |