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 <jacques.le.r...@les7arts.com>
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 <jacques.le.r...@les7arts.com
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 <pierre.sm...@gmail.com>
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 <
scott.g...@hotwaxsystems.com>
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 <pierre.sm...@gmail.com>
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 <
jacques.le.r...@les7arts.com> 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 <

pierre.sm...@gmail.com
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 <

pierre.sm...@gmail.com
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 <
jacopo.cappell...@hotwaxsystems.com> 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, <jler...@apache.org> 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);



Reply via email to