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/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);
> > >
> > >
> > >
> >
>

Reply via email to